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 <17826875...@163.com>
> Sent: Monday, February 15, 2021 12:15
> To: dev@dpdk.org
> Cc: zhujiawe...@huawei.com; Matan Azrad <ma...@nvidia.com>; Shahaf
> Shuler <shah...@nvidia.com>; Slava Ovsiienko <viachesl...@nvidia.com>;
> Jiawei Zhu <17826875...@163.com>; sta...@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: sta...@dpdk.org
>
> Signed-off-by: Jiawei Zhu <17826875...@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
>
