Re: [PATCH net 7/7] net/mlx5e: shampo: Do not merge PSP packets

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] net/mlx5e: redundant #ifdef for PSP in mlx5e_hw_gro_psp_match()

--

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -2224,6 +2224,16 @@ static bool mlx5e_hw_gro_skb_has_enough_space(struct sk_buff *skb,
>  		return page_size * nr_frags + data_bcnt <= GRO_LEGACY_MAX_SIZE;
>  }
>  
> +static bool mlx5e_hw_gro_psp_match(struct sk_buff *skb, struct mlx5_cqe64 *cqe)
> +{
> +#ifdef CONFIG_MLX5_EN_PSP
> +	/* PSP packets cannot be merged. */
> +	return !mlx5e_psp_is_rx_flow(cqe);
> +#else
> +	return true;
> +#endif
> +}
> +
>  static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
>  {
>  	u16 data_bcnt		= mpwrq_get_cqe_byte_cnt(cqe) - cqe->shampo.header_size;

[Severity: Low]
The #ifdef CONFIG_MLX5_EN_PSP in mlx5e_hw_gro_psp_match() is
redundant. en_accel/psp_rxtx.h already provides a stub
mlx5e_psp_is_rx_flow() that returns false when CONFIG_MLX5_EN_PSP is
not set, so !mlx5e_psp_is_rx_flow(cqe) already evaluates to true in
that configuration, which is exactly what the #else branch returns.

Please drop the conditional and just return
!mlx5e_psp_is_rx_flow(cqe) unconditionally. That keeps both
configurations compiling the same code path, avoids the risk of the
two branches drifting apart when the PSP helper changes, and matches
the rest of the driver, which relies on the header stubs rather than
sprinkling #ifdefs through en_rx.c. It also means the stub keeps
getting type-checked when PSP is disabled.

While you are there: the skb argument of mlx5e_hw_gro_psp_match() is
never used. Is it there for a follow-up that will compare the
in-progress skb against the new CQE, or can it be dropped?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917175433.4090878-1-tariqt%40nvidia.com