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 048D342A5D; Thu, 4 May 2023 14:42:59 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CB6D241144; Thu, 4 May 2023 14:42:58 +0200 (CEST) Received: from loongson.cn (mail.loongson.cn [114.242.206.163]) by mails.dpdk.org (Postfix) with ESMTP id CCA64410DC for ; Thu, 4 May 2023 14:42:55 +0200 (CEST) Received: from loongson.cn (unknown [10.20.42.90]) by gateway (Coremail) with SMTP id _____8Bxa+pMqFNkrKMEAA--.7457S3; Thu, 04 May 2023 20:42:53 +0800 (CST) Received: from [10.20.42.90] (unknown [10.20.42.90]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Bxab1KqFNk+fJJAA--.5184S3; Thu, 04 May 2023 20:42:50 +0800 (CST) Subject: Re: [PATCH v2] net/ixgbe: add proper memory barriers for some Rx functions To: "Zhang, Qi Z" , "Yang, Qiming" , "Wu, Wenjun1" Cc: "dev@dpdk.org" , "maobibo@loongson.cn" References: <20230424090532.367194-1-zhoumin@loongson.cn> From: zhoumin Message-ID: <88b24978-d541-b26f-1c3d-14b2c76a67f9@loongson.cn> Date: Thu, 4 May 2023 20:42:50 +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: 7bit Content-Language: en-US X-CM-TRANSID: AQAAf8Bxab1KqFNk+fJJAA--.5184S3 X-CM-SenderInfo: 52kr3ztlq6z05rqj20fqof0/ X-Coremail-Antispam: 1Uk129KBjvJXoWxZr4Dtw1fXF15uryxJw18Zrb_yoW5KF4Dpa yfJ3W2kryDXr47K34SqayrGF9ay3yrWr18WrWvywnYkw1qqwn0vF1jgryj9FyDJr4jy3W0 vF4UGws8KF4DZa7anT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUj1kv1TuYvTs0mT0YCTnIWj qI5I8CrVACY4xI64kE6c02F40Ex7xfYxn0WfASr-VFAUDa7-sFnT9fnUUIcSsGvfJTRUUU bxkYFVCjjxCrM7AC8VAFwI0_Jr0_Gr1l1xkIjI8I6I8E6xAIw20EY4v20xvaj40_Wr0E3s 1l1IIY67AEw4v_JrI_Jryl8cAvFVAK0II2c7xJM28CjxkF64kEwVA0rcxSw2x7M28EF7xv wVC0I7IYx2IY67AKxVWDJVCq3wA2z4x0Y4vE2Ix0cI8IcVCY1x0267AKxVW8Jr0_Cr1UM2 8EF7xvwVC2z280aVAFwI0_GcCE3s1l84ACjcxK6I8E87Iv6xkF7I0E14v26rxl6s0DM2AI xVAIcxkEcVAq07x20xvEncxIr21l57IF6xkI12xvs2x26I8E6xACxx1l5I8CrVACY4xI64 kE6c02F40Ex7xfMcIj6xIIjxv20xvE14v26r106r15McIj6I8E87Iv67AKxVWUJVW8JwAm 72CE4IkC6x0Yz7v_Jr0_Gr1lF7xvr2IY64vIr41lc7I2V7IY0VAS07AlzVAYIcxG8wCF04 k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s026c02F40E14v26r1j6r18 MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_JF0_Jw1lIxkGc2Ij64vIr4 1lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7CjxVAFwI0_Jr0_Gr1l IxAIcVCF04k26cxKx2IYs7xG6r1j6r1xMIIF0xvEx4A2jsIE14v26r1j6r4UMIIF0xvEx4 A2jsIEc7CjxVAFwI0_Jr0_GrUvcSsGvfC2KfnxnUUI43ZEXa7IU1EksDUUUUU== 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, Thanks for your review. On Fri, Apr 28, 2023 at 11:43AM, Zhang, Qi Z wrote: > >> -----Original Message----- >> From: Min Zhou >> Sent: Monday, April 24, 2023 5:06 PM >> To: Yang, Qiming ; Wu, Wenjun1 >> ; zhoumin@loongson.cn >> Cc: dev@dpdk.org; maobibo@loongson.cn >> Subject: [PATCH v2] net/ixgbe: add proper memory barriers for some Rx >> functions >> >> 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. >> >> 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(); > So "volatile" does not prevent re-order with Loongson compiler? The memory consistency model of the LoongArch [1] uses the Weak Consistency model in which memory operations can be reordered. [1] https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN#overview-of-memory-consistency >> rxd = *rxdp; >> >> /* >> @@ -2122,6 +2124,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct >> rte_mbuf **rx_pkts, uint16_t nb_pkts, >> if (!(staterr & IXGBE_RXDADV_STAT_DD)) >> break; >> >> + rte_rmb(); >> rxd = *rxdp; >> >> PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u " >> -- >> 2.31.1 Best regards, Min