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 DFDC542A68; Fri, 5 May 2023 04:42:40 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7AAEC410EA; Fri, 5 May 2023 04:42:40 +0200 (CEST) Received: from loongson.cn (mail.loongson.cn [114.242.206.163]) by mails.dpdk.org (Postfix) with ESMTP id 3866840ED7 for ; Fri, 5 May 2023 04:42:38 +0200 (CEST) Received: from loongson.cn (unknown [10.20.42.90]) by gateway (Coremail) with SMTP id _____8DxZPAbbVRk0vkEAA--.8256S3; Fri, 05 May 2023 10:42:35 +0800 (CST) Received: from [10.20.42.90] (unknown [10.20.42.90]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Axmr0abVRksBRLAA--.6732S3; Fri, 05 May 2023 10:42:34 +0800 (CST) Subject: Re: [PATCH v2] net/ixgbe: add proper memory barriers for some Rx functions To: "Zhang, Qi Z" , =?UTF-8?Q?Morten_Br=c3=b8rup?= , Konstantin Ananyev Cc: "dev@dpdk.org" , "maobibo@loongson.cn" , "Yang, Qiming" , "Wu, Wenjun1" , "ruifeng.wang@arm.com" , "drc@linux.vnet.ibm.com" , Tyler Retzlaff References: <20230424090532.367194-1-zhoumin@loongson.cn> <20671e5e-8e86-4bc3-2d95-4cec014b0539@yandex.ru> <43fb4e62-63b5-7076-327e-dc30cd561fff@loongson.cn> <98CBD80474FA8B44BF855DF32C47DC35D878E4@smartserver.smartshare.dk> From: zhoumin Message-ID: <35cb2f3a-6590-ca35-12c7-cbc61bbfb506@loongson.cn> Date: Fri, 5 May 2023 10:42:34 +0800 User-Agent: Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-CM-TRANSID: AQAAf8Axmr0abVRksBRLAA--.6732S3 X-CM-SenderInfo: 52kr3ztlq6z05rqj20fqof0/ X-Coremail-Antispam: 1Uk129KBjvJXoW3JF47Ar4xCFyxWFyrCFW3Wrg_yoW7Ar1DpF WkJ3WjkrZ8Gr48Jw12qF18Gry3trWrJ3WUXry8J3W0kw4qqw1YgF42qr1j9FyDJr48J3Wj vr1UJ3sxuFs8AaDanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUj1kv1TuYvTs0mT0YCTnIWj qI5I8CrVACY4xI64kE6c02F40Ex7xfYxn0WfASr-VFAUDa7-sFnT9fnUUIcSsGvfJTRUUU b-xYFVCjjxCrM7AC8VAFwI0_Jr0_Gr1l1xkIjI8I6I8E6xAIw20EY4v20xvaj40_Wr0E3s 1l1IIY67AEw4v_JrI_Jryl8cAvFVAK0II2c7xJM28CjxkF64kEwVA0rcxSw2x7M28EF7xv wVC0I7IYx2IY67AKxVWDJVCq3wA2z4x0Y4vE2Ix0cI8IcVCY1x0267AKxVW8Jr0_Cr1UM2 8EF7xvwVC2z280aVAFwI0_Gr1j6F4UJwA2z4x0Y4vEx4A2jsIEc7CjxVAFwI0_Gr1j6F4U JwAS0I0E0xvYzxvE52x082IY62kv0487Mc804VCY07AIYIkI8VC2zVCFFI0UMc02F40EFc xC0VAKzVAqx4xG6I80ewAv7VC0I7IYx2IY67AKxVWUJVWUGwAv7VC2z280aVAFwI0_Gr0_ Cr1lOx8S6xCaFVCjc4AY6r1j6r4UM4x0Y48IcVAKI48JMxk0xIA0c2IEe2xFo4CEbIxvr2 1l42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1l4IxYO2xFxVAFwI0_Jw0_ GFylx2IqxVAqx4xG67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17 CE14v26r1q6r43MIIYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0 I7IYx2IY6xkF7I0E14v26r1j6r4UMIIF0xvE42xK8VAvwI8IcIk0rVWUJVWUCwCI42IY6I 8E87Iv67AKxVW8JVWxJwCI42IY6I8E87Iv6xkF7I0E14v26r4j6r4UJbIYCTnIWIevJa73 UjIFyTuYvjxUcyxRUUUUU 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 Hi Qi, On Thur, May 4, 2023 at 9:33PM, Zhang, Qi Z wrote: > >> -----Original Message----- >> From: Morten Brørup >> Sent: Thursday, May 4, 2023 9:22 PM >> To: zhoumin ; Konstantin Ananyev >> >> Cc: dev@dpdk.org; maobibo@loongson.cn; Yang, Qiming >> ; Wu, Wenjun1 ; >> ruifeng.wang@arm.com; drc@linux.vnet.ibm.com; Tyler Retzlaff >> >> Subject: RE: [PATCH v2] net/ixgbe: add proper memory barriers for some Rx >> functions >> >>> From: zhoumin [mailto:zhoumin@loongson.cn] >>> Sent: Thursday, 4 May 2023 15.17 >>> >>> Hi Konstantin, >>> >>> Thanks for your  comments. >>> >>> On 2023/5/1 下午9:29, Konstantin Ananyev wrote: >>>>> Segmentation fault has been observed while running the >>>>> ixgbe_recv_pkts_lro() function to receive packets on the Loongson >>>>> 3C5000 processor which has 64 cores and 4 NUMA nodes. >>>>> >>>>> From the ixgbe_recv_pkts_lro() function, we found that as long as >>>>> the first packet has the EOP bit set, and the length of this packet >>>>> is less than or equal to rxq->crc_len, the segmentation fault will >>>>> definitely happen even though on the other platforms, such as X86. > Sorry to interrupt, but I am curious why this issue still exists on x86 architecture. Can volatile be used to instruct the compiler to generate read instructions in a specific order, and does x86 guarantee not to reorder load operations? Actually, I did not see the segmentation fault on X86. I just made the first packet which had the EOP bit set had a zero length, then the segmentation fault would happen on X86. So, I thought that the out-of-order access to the descriptor might be possible to make the ready packet zero length, and this case was more likely to cause the segmentation fault. >>>>> Because when processd the first packet the first_seg->next will be >>>>> NULL, if at the same time this packet has the EOP bit set and its >>>>> length is less than or equal to rxq->crc_len, the following loop >>>>> will be excecuted: >>>>> >>>>>     for (lp = first_seg; lp->next != rxm; lp = lp->next) >>>>>         ; >>>>> >>>>> We know that the first_seg->next will be NULL under this condition. >>>>> So the >>>>> expression of lp->next->next will cause the segmentation fault. >>>>> >>>>> Normally, the length of the first packet with EOP bit set will be >>>>> greater than rxq->crc_len. However, the out-of-order execution of >>>>> CPU may make the read ordering of the status and the rest of the >>>>> descriptor fields in this function not be correct. The related >>>>> codes are as following: >>>>> >>>>>         rxdp = &rx_ring[rx_id]; >>>>>  #1     staterr = rte_le_to_cpu_32(rxdp->wb.upper.status_error); >>>>> >>>>>         if (!(staterr & IXGBE_RXDADV_STAT_DD)) >>>>>             break; >>>>> >>>>>  #2     rxd = *rxdp; >>>>> >>>>> The sentence #2 may be executed before sentence #1. This action is >>>>> likely to make the ready packet zero length. If the packet is the >>>>> first packet and has the EOP bit set, the above segmentation fault >>>>> will happen. >>>>> >>>>> So, we should add rte_rmb() to ensure the read ordering be correct. >>>>> We also >>>>> did the same thing in the ixgbe_recv_pkts() function to make the >>>>> rxd data be valid even thougth we did not find segmentation fault >>>>> in this function. >>>>> >>>>> Signed-off-by: Min Zhou >>>>> --- >>>>> v2: >>>>> - Make the calling of rte_rmb() for all platforms >>>>> --- >>>>>  drivers/net/ixgbe/ixgbe_rxtx.c | 3 +++ >>>>>  1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c >>>>> b/drivers/net/ixgbe/ixgbe_rxtx.c index c9d6ca9efe..302a5ab7ff >>>>> 100644 >>>>> --- a/drivers/net/ixgbe/ixgbe_rxtx.c >>>>> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c >>>>> @@ -1823,6 +1823,8 @@ ixgbe_recv_pkts(void *rx_queue, struct >>>>> rte_mbuf **rx_pkts, >>>>>          staterr = rxdp->wb.upper.status_error; >>>>>          if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD))) >>>>>              break; >>>>> + >>>>> +        rte_rmb(); >>>>>          rxd = *rxdp; >>>> >>>> >>>> Indeed, looks like a problem to me on systems with relaxed MO. >>>> Strange that it was never hit on arm or ppc - cc-ing ARM/PPC maintainers. >>> The LoongArch architecture uses the Weak Consistency model which can >>> cause the problem, especially in scenario with many cores, such as >>> Loongson 3C5000 with four NUMA node, which has 64 cores. I cannot >>> reproduce it on Loongson 3C5000 with one NUMA node, which just has 16 >> cores. >>>> About a fix - looks right, but a bit excessive to me - as I >>>> understand all we need here is to prevent re-ordering by CPU itself. >>> Yes, thanks for cc-ing. >>>> So rte_smp_rmb() seems enough here. >>>> Or might be just: >>>> staterr = __atomic_load_n(&rxdp->wb.upper.status_error, >>>> __ATOMIC_ACQUIRE); >>>> >>> Does __atomic_load_n() work on Windows if we use it to solve this >> problem ? >> >> Yes, __atomic_load_n() works on Windows too. >> Best regards, Min