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 7DFE842C98 for ; Mon, 12 Jun 2023 13:59:02 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 790FB41611; Mon, 12 Jun 2023 13:59:02 +0200 (CEST) Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by mails.dpdk.org (Postfix) with ESMTP id B38824014F; Mon, 12 Jun 2023 13:58:59 +0200 (CEST) Received: from loongson.cn (unknown [10.20.42.90]) by gateway (Coremail) with SMTP id _____8Bxb+t+CIdk7b0DAA--.8112S3; Mon, 12 Jun 2023 19:58:54 +0800 (CST) Received: from [10.20.42.90] (unknown [10.20.42.90]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Axfcp9CIdk6f4VAA--.54562S3; Mon, 12 Jun 2023 19:58:54 +0800 (CST) Subject: Re: [PATCH v3] net/ixgbe: add proper memory barriers for some Rx functions To: Thomas Monjalon , Ruifeng Wang , dev@dpdk.org, "Zhang, Qi Z" Cc: "mb@smartsharesystems.com" , "konstantin.v.ananyev@yandex.ru" , "Yang, Qiming" , "Wu, Wenjun1" , "drc@linux.vnet.ibm.com" , "roretzla@linux.microsoft.com" , "stable@dpdk.org" , "maobibo@loongson.cn" , nd , david.marchand@redhat.com References: <20230424090532.367194-1-zhoumin@loongson.cn> <3202143.AJdgDx1Vlc@thomas> From: zhoumin Message-ID: Date: Mon, 12 Jun 2023 19:58:53 +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: <3202143.AJdgDx1Vlc@thomas> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-CM-TRANSID: AQAAf8Axfcp9CIdk6f4VAA--.54562S3 X-CM-SenderInfo: 52kr3ztlq6z05rqj20fqof0/ X-Coremail-Antispam: 1Uk129KBj93XoWxAFWxAF1furWrJF1kZw4xuFX_yoW5ZrW8pa y3JF1Ykr4kWryUKwnIqFn5Kr1Sk3yrJF48X34kAw4vkws0qw1rtF43Kr4UuFy8Ar4UAw10 vF1UG39xXa1UZFgCm3ZEXasCq-sJn29KB7ZKAUJUUUUU529EdanIXcx71UUUUU7KY7ZEXa sCq-sGcSsGvfJ3Ic02F40EFcxC0VAKzVAqx4xG6I80ebIjqfuFe4nvWSU5nxnvy29KBjDU 0xBIdaVrnRJUUUvYb4IE77IF4wAFF20E14v26r1j6r4UM7CY07I20VC2zVCF04k26cxKx2 IYs7xG6rWj6s0DM7CIcVAFz4kK6r106r15M28lY4IEw2IIxxk0rwA2F7IY1VAKz4vEj48v e4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_Jr0_JF4l84ACjcxK6xIIjxv20xvEc7CjxVAFwI 0_Jr0_Gr1l84ACjcxK6I8E87Iv67AKxVWxJr0_GcWl84ACjcxK6I8E87Iv6xkF7I0E14v2 6F4UJVW0owAS0I0E0xvYzxvE52x082IY62kv0487Mc804VCY07AIYIkI8VC2zVCFFI0UMc 02F40EFcxC0VAKzVAqx4xG6I80ewAv7VC0I7IYx2IY67AKxVWUJVWUGwAv7VC2z280aVAF wI0_Jr0_Gr1lOx8S6xCaFVCjc4AY6r1j6r4UM4x0Y48IcVAKI48JMxk0xIA0c2IEe2xFo4 CEbIxvr21l42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2IqxVAqx4xG 67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v26r1q6r43MI IYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IYx2IY6xkF7I0E 14v26r1j6r4UMIIF0xvE42xK8VAvwI8IcIk0rVWUJVWUCwCI42IY6I8E87Iv67AKxVWUJV W8JwCI42IY6I8E87Iv6xkF7I0E14v26r1j6r4UYxBIdaVFxhVjvjDU0xZFpf9x07j8yCJU UUUU= X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Hi Thomas, On Mon, June 12, 2023 at 6:26PM, Thomas Monjalon wrote: > 15/05/2023 04:10, Zhang, Qi Z: >> From: Ruifeng Wang >>> From: Min Zhou >>>> 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. For example, if >>>> we made the first packet which had the EOP bit set had a zero length by >>> force, the segmentation fault would happen on X86. >>>> 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 >>> executed: >>>> 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 a proper memory barrier 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 though we did not find >>> segmentation fault in this function. >>>> Fixes: 8eecb3295ae ("ixgbe: add LRO support") >>>> Cc: stable@dpdk.org >>>> >>>> Signed-off-by: Min Zhou >>>> --- >>>> v3: >>>> - Use rte_smp_rmb() as the proper memory barrier instead of rte_rmb() >>>> --- >>>> v2: >>>> - Make the calling of rte_rmb() for all platforms >>>> --- > [...] >>> Reviewed-by: Ruifeng Wang >> Applied to dpdk-next-net-intel. >> >> Thanks >> Qi >> > Why ignoring checkpatch? > It is saying: > " > Warning in drivers/net/ixgbe/ixgbe_rxtx.c: > Using rte_smp_[r/w]mb > " I'm sorry. Should we never use rte_smp_[r/w]mb in the driver's code? > Ruifeng proposed "rte_atomic_thread_fence(__ATOMIC_ACQUIRE)" > in a comment on the v2. Thanks, I see. I think I also can use rte_atomic_thread_fence() to solve this problem. I will send the V4 patch. > > I will drop this patch from the pull of next-net-intel branch. >