DPDK patches and discussions
 help / color / mirror / Atom feed
From: Slava Ovsiienko <viacheslavo@nvidia.com>
To: Jiawei Zhu <17826875952@163.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "zhujiawei12@huawei.com" <zhujiawei12@huawei.com>,
	Matan Azrad <matan@nvidia.com>,
	Shahaf Shuler <shahafs@nvidia.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix wrong segmented packet in Rx
Date: Mon, 1 Mar 2021 09:13:08 +0000	[thread overview]
Message-ID: <DM6PR12MB37536615D5794160C3DEB018DF9A9@DM6PR12MB3753.namprd12.prod.outlook.com> (raw)
In-Reply-To: <2623ef20-39e9-fc8b-ca70-c7c450d95cfb@163.com>

Hi, Jiawei

Thank you for the clarification. I missed the point that we have updated elts array 
with new allocated mbufs and are not able to retry packet building anymore.
Very good catch, thank you!  Could you, please, add this extra explanation
to the  commit message and send the v2 ?

With best regards, 
Slava

> -----Original Message-----
> From: Jiawei Zhu <17826875952@163.com>
> Sent: Friday, February 26, 2021 18:11
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org
> Cc: zhujiawei12@huawei.com; Matan Azrad <matan@nvidia.com>; Shahaf
> Shuler <shahafs@nvidia.com>; stable@dpdk.org
> Subject: Re: [PATCH] net/mlx5: fix wrong segmented packet in Rx
> 
> Hi, Slava
> 
> Thanks for reading my patch, my issue may not be clear.
> Here I give a possible error.
> - we assume segs_n is 4 and we are receiving 4 segments multi-segment
> packet.
> - we fail to alloc mbuf when receive the 3th segment,so it will free the
> mbufs which packet chain we have built. Here are the 1st and 2nd segment.
> - Rx queue in this stride, the 1st and the 2nd segment are fill the new mbuf
> and there data will be rand, but the 3th and 4th segment are still fill the last
> data. So next if still begin on this stride, it will reveice wrong multi-segment
> packet.
> 
> - So we should discarded this packets and pass this stride. After exit the loop,
> we should align the next consumer index.
> 
> What Do you thinking?
> 
> With best regards
> Jiawei
> 
> On 2021/2/24 9:20 PM, Slava Ovsiienko wrote:
> > 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
> >>
> >


  reply	other threads:[~2021-03-01  9:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-15 10:15 Jiawei Zhu
2021-02-24 13:20 ` Slava Ovsiienko
2021-02-26 16:11   ` Jiawei Zhu
2021-03-01  9:13     ` Slava Ovsiienko [this message]
2021-03-01 17:01       ` Jiawei Zhu
2021-03-02  8:10         ` Slava Ovsiienko
2021-03-02 16:44           ` Jiawei Zhu
2021-03-01 16:58 ` [dpdk-dev] [PATCH v2] " Jiawei Zhu
2021-03-01 17:19   ` [dpdk-dev] [PATCH v3] " Jiawei Zhu
2021-03-02 17:18     ` Slava Ovsiienko
2021-03-04  9:00     ` Raslan Darawsheh
2021-03-13 19:45       ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR12MB37536615D5794160C3DEB018DF9A9@DM6PR12MB3753.namprd12.prod.outlook.com \
    --to=viacheslavo@nvidia.com \
    --cc=17826875952@163.com \
    --cc=dev@dpdk.org \
    --cc=matan@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=stable@dpdk.org \
    --cc=zhujiawei12@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).