From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 75A95A054F; Tue, 2 Mar 2021 17:45:01 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4008822A2E4; Tue, 2 Mar 2021 17:44:59 +0100 (CET) Received: from m12-12.163.com (m12-12.163.com [220.181.12.12]) by mails.dpdk.org (Postfix) with ESMTP id 0BAE422A2CA; Tue, 2 Mar 2021 17:44:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Subject:From:Message-ID:Date:MIME-Version; bh=6vlA7 r9KHvvaPjEgvjIgijxwJ6Noi70b8a9rKFJaYS8=; b=nxEgfAmPoZmUGuAkaptQd kpNyiawKYcVpLOPAoH8SOHKuA6iVFwt/IO2jvpQuJR4UEksmW83TGG/5lhEN2Yaj FXYmZf7CkPJE6JpKYjXbDW/B/DoiDVjMMftoSF9MzWtI0p1/uJcwOaD1ASRQU1/c iNV3jJWbWgiILcDGp3/qAU= Received: from appledeMacBook-Pro.local (unknown [112.10.64.142]) by smtp8 (Coremail) with SMTP id DMCowABX19aFaz5ghc+nSw--.45315S3; Wed, 03 Mar 2021 00:44:54 +0800 (CST) To: Slava Ovsiienko , "dev@dpdk.org" Cc: "zhujiawei12@huawei.com" , Matan Azrad , Shahaf Shuler , "stable@dpdk.org" References: <1613384114-17855-1-git-send-email-17826875952@163.com> <2623ef20-39e9-fc8b-ca70-c7c450d95cfb@163.com> From: Jiawei Zhu <17826875952@163.com> Message-ID: <94838c4e-af71-bef5-9546-793f541db643@163.com> Date: Wed, 3 Mar 2021 00:44:53 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-CM-TRANSID: DMCowABX19aFaz5ghc+nSw--.45315S3 X-Coremail-Antispam: 1Uf129KBjvJXoW3WFWrGrykJw1xKFWkGw4fAFb_yoWxCF1Upr 48tFy7AFWkA340vwnFq3WYg34Fvw4rJw4UWwn8Gw4fZ3s09r1FqFW2qw4UuFy8Cr48K3W2 qr1DXr1xGry5ZrDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07b0mRUUUUUU= X-Originating-IP: [112.10.64.142] X-CM-SenderInfo: bprxmjywyxkmivs6il2tof0z/1tbiRQZJ9ll91Wv75AAAs5 Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix wrong segmented packet in Rx X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 ; dev@dpdk.org >> Cc: zhujiawei12@huawei.com; Matan Azrad ; Shahaf >> Shuler ; 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 ; dev@dpdk.org >>>> Cc: zhujiawei12@huawei.com; Matan Azrad ; Shahaf >>>> Shuler ; 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 ; >> Shahaf >>>>>> Shuler ; Slava Ovsiienko >>>>>> ; 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 >>>>>> >>>>> >>> >