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

[Cosmin Ratiu]

On Mon, 2026-09-21 at 18:49 +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> 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?
> 

Will address.