DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jiawei Zhu <17826875952@163.com>
To: Slava Ovsiienko <viacheslavo@nvidia.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: Wed, 3 Mar 2021 00:44:53 +0800
Message-ID: <94838c4e-af71-bef5-9546-793f541db643@163.com> (raw)
In-Reply-To: <DM6PR12MB375346FF2BC944BB91E55086DF999@DM6PR12MB3753.namprd12.prod.outlook.com>

Hi, Slava

Thanks a lot for your correction and advice. I'm not very good at 
grammar 😁.
Here is new patch and used your words.😃

https://patches.dpdk.org/project/dpdk/patch/1614619190-3846-1-git-send-email-17826875952@163.com/

With best regards,
Jiawei

On 2021/3/2 4:10 PM, Slava Ovsiienko wrote:
> Hi, Jiawei
> 
> Thanks a lot for the update.
> There are some common points for the commit messages of fixing patches:
> - the bug/error/issue should be described in PAST tense (what we HAD before the fix)
> - what fix is doing should be described in PRESENT tense (what we HAVE right now, after fix apply)
> 
> Also, can we fix some typos in the message and reword it a little bit?
> What do you think about something like this:
> 
> The issue occurred if mbuf starvation happened
> in the middle of segmented packet reception.
> In such a situation, after release the segments of
> packet being received, code did not advance the
> consumer index to the next stride. This caused
> the receiving of the wrong segmented packet data.
> 
> The possible error scenario:
> - we assume segs_n is 4 and we are receiving 4
>    segments of multi-segment packet.
> - we fail to allocate mbuf while receiving the 3rd segment,
>    and this frees the mbufs of the packet chain we have built.
>    There are the 1st and 2nd segments in the chain.
> - the 1st and the 2nd segments of this stride of Rx queue
>    are filled up (in elts array) with the new allocated
>    mbufs and their data are random (the 3rd and 4th
>    segments still contain the valid data of the packet though).
> - on the next iteration of stride processing we get
>    the wrong two segments of the multi-segment packet.
> 
> Hence, we should skip these mbufs in the stride and
> we should advance the consumer index on loop exit.
> 
> With best regards,
> Slava
> 
>> -----Original Message-----
>> From: Jiawei Zhu <17826875952@163.com>
>> Sent: Monday, March 1, 2021 19:02
>> 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
>> Thank you for your agreement. Here is the v2 patch:
>>
>> https://patches.dpdk.org/project/dpdk/patch/1614617885-2650-1-git-send-
>> email-17826875952@163.com/
>>
>> With best regards,
>> Jiawei
>>
>> On 2021/3/1 5:13 PM, Slava Ovsiienko wrote:
>>> 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-02 16:45 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
2021-03-01 17:01       ` Jiawei Zhu
2021-03-02  8:10         ` Slava Ovsiienko
2021-03-02 16:44           ` Jiawei Zhu [this message]
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=94838c4e-af71-bef5-9546-793f541db643@163.com \
    --to=17826875952@163.com \
    --cc=dev@dpdk.org \
    --cc=matan@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=stable@dpdk.org \
    --cc=viacheslavo@nvidia.com \
    --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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git