Re: [PATCH net-next 08/15] net/mlx5e: SHAMPO, Skipping on duplicate flush of the same SHAMPO SKB

[Simon Horman <horms@kernel.org>]

On Wed, Jun 05, 2024 at 05:55:24PM +0000, Dragos Tatulea wrote:
> On Wed, 2024-06-05 at 14:48 +0100, Simon Horman wrote:
> > On Tue, May 28, 2024 at 05:28:00PM +0300, Tariq Toukan wrote:
> > > From: Yoray Zack <yorayz@nvidia.com>
> > > 
> > > SHAMPO SKB can be flushed in mlx5e_shampo_complete_rx_cqe().
> > > If the SKB was flushed, rq->hw_gro_data->skb was also set to NULL.
> > > 
> > > We can skip on flushing the SKB in mlx5e_shampo_flush_skb
> > > if rq->hw_gro_data->skb == NULL.
> > > 
> > > Signed-off-by: Yoray Zack <yorayz@nvidia.com>
> > > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > > ---
> > >  drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > > index 1e3a5b2afeae..3f76c33aada0 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > > @@ -2334,7 +2334,7 @@ static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cq
> > >  	}
> > >  
> > >  	mlx5e_shampo_complete_rx_cqe(rq, cqe, cqe_bcnt, *skb);
> > > -	if (flush)
> > > +	if (flush && rq->hw_gro_data->skb)
> > >  		mlx5e_shampo_flush_skb(rq, cqe, match);
> > 
> > nit: It seems awkward to reach inside rq like this
> >      when mlx5e_shampo_flush_skb already deals with the skb in question.
> > 
> We don't need to reach inside the rq, we could use *skb instead (skb is &rq-
> >hw_gro_data->skb). *skb is used often in this function.

So it is, thanks for pointing that out.

Clearly this is a pretty minor thing,
so no need to respin just because of it.

> 
> >      Would it make esnse for the NULL skb check to
> >      be moved inside mlx5e_shampo_flush_skb() ?
> > 
> 
> Thanks,
> Dragos