| Message ID | 20240528142807.903965-9-tariqt@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/mlx5e: SHAMPO, Enable HW GRO once more | expand |
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. Would it make esnse for the NULL skb check to be moved inside mlx5e_shampo_flush_skb() ? > free_hd_entry: > if (likely(head_size)) > -- > 2.31.1 > >
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. > Would it make esnse for the NULL skb check to > be moved inside mlx5e_shampo_flush_skb() ? > Thanks, Dragos
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
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); free_hd_entry: if (likely(head_size))