diff mbox series

net/mlx5: fix wrong segmented packet in Rx

Message ID 1613384114-17855-1-git-send-email-17826875952@163.com (mailing list archive)
State New
Delegated to: Raslan Darawsheh
Headers show
Series net/mlx5: fix wrong segmented packet in Rx | expand

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot success github build: passed
ci/travis-robot fail travis build: failed
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Jiawei Zhu Feb. 15, 2021, 10:15 a.m. UTC
Fixed issue could occur when Mbuf starvation happens
in a middle of reception of a segmented packet.
In such a situation, after release the segments of that
packet, it does not align consumer index to the next stride.
This would cause receive a wrong segmented packet.

Fixes: 15a756b63734 ("net/mlx5: fix possible NULL dereference in Rx path")
Cc: stable@dpdk.org

Signed-off-by: Jiawei Zhu <17826875952@163.com>
---
 drivers/net/mlx5/mlx5_rxtx.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Slava Ovsiienko Feb. 24, 2021, 1:20 p.m. UTC | #1
Hi, Jiawei

Thank you for the patch, but It seems I need some clarifications.
As far I understand the issue:

- we are in the midst of receiving the multi-segment packet
- we have some mbufs allocated and packet chain is partially built
- we fail on allocation replenishing mbuf for the segment
- we free all the mbuf of the built chain
- exit from the rx_burtst loop
- rq_ci is expected to be kept pointing to the beginning of the current
  stride - it is supposed on next rx_burst() invocation we'll continue
  Rx queue handling from the stride where we failed
- on loop exit we see the code:
   if (unlikely((i == 0) && ((rq_ci >> sges_n) == rxq->rq_ci)))
          return 0;
   /* Update the consumer index. */
   rxq->rq_ci = rq_ci >> sges_n;
hence, rq_ci is always shifted by sges_n, all increments happened
during failed packet processing are just discarded, it seems no fix is needed.

Did I miss something? 

With best regards,
Slava

> -----Original Message-----
> From: Jiawei Zhu <17826875952@163.com>
> Sent: Monday, February 15, 2021 12:15
> To: dev@dpdk.org
> Cc: zhujiawei12@huawei.com; Matan Azrad <matan@nvidia.com>; Shahaf
> Shuler <shahafs@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> Jiawei Zhu <17826875952@163.com>; stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix wrong segmented packet in Rx
> 
> Fixed issue could occur when Mbuf starvation happens in a middle of
> reception of a segmented packet.
> In such a situation, after release the segments of that packet, it does not align
> consumer index to the next stride.
> This would cause receive a wrong segmented packet.
> 
> Fixes: 15a756b63734 ("net/mlx5: fix possible NULL dereference in Rx path")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jiawei Zhu <17826875952@163.com>
> ---
>  drivers/net/mlx5/mlx5_rxtx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index
> 2e4b87c..e3ce9fd 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -1480,6 +1480,9 @@ enum mlx5_txcmp_code {
>  				rte_mbuf_raw_free(pkt);
>  				pkt = rep;
>  			}
> +			rq_ci >>= sges_n;
> +			++rq_ci;
> +			rq_ci <<= sges_n;
>  			break;
>  		}
>  		if (!pkt) {
> --
> 1.8.3.1
>
diff mbox series

Patch

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 2e4b87c..e3ce9fd 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1480,6 +1480,9 @@  enum mlx5_txcmp_code {
 				rte_mbuf_raw_free(pkt);
 				pkt = rep;
 			}
+			rq_ci >>= sges_n;
+			++rq_ci;
+			rq_ci <<= sges_n;
 			break;
 		}
 		if (!pkt) {