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 88DE543E32; Wed, 10 Apr 2024 11:33:59 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0F3FB402C5; Wed, 10 Apr 2024 11:33:59 +0200 (CEST) Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by mails.dpdk.org (Postfix) with ESMTP id 95E594029E for ; Wed, 10 Apr 2024 11:33:56 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.19.88.163]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4VDyHn60MGz2NW5v for ; Wed, 10 Apr 2024 17:31:01 +0800 (CST) Received: from dggpeml500024.china.huawei.com (unknown [7.185.36.10]) by mail.maildlp.com (Postfix) with ESMTPS id 8051318001A for ; Wed, 10 Apr 2024 17:33:53 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Wed, 10 Apr 2024 17:33:53 +0800 From: fengchengwen Subject: Strict aliasing problem with rte_eth_linkstatus_set() To: "dev@dpdk.org" , Dengdui Huang Message-ID: <8175c905-e661-b910-7f20-59b6ab605c38@huawei.com> Date: Wed, 10 Apr 2024 17:33:53 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpeml500024.china.huawei.com (7.185.36.10) 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 All, We have a problem introduced by a compiler upgrade (from gcc10 to gcc12.3), we've done some research but haven't been able to figure out why. We'd like the community's help. Environment: 1. Source: DPDK 23.11 2. GCC: 12.3.1 [1] 3. Compiled with target kunpeng SoC (ARM64) 4. Run on kunpeng SoC Problem & Debug: 1. We found the hns3 driver fails to update the link status. The corresponding function is hns3_update_linkstatus_and_event [2], and we found the rte_eth_linkstatus_set [3] always return zero. 2. After disassembly the hns3_update_linkstatus_and_event, and found rte_eth_linkstatus_set's rte_atomic_exchange_explicit return to xzr register (which is zero register): 1239fec: 3900f3e0 strb w0, [sp, #60] 1239ff0: 9101a041 add x1, x2, #0x68 ---x2 seem not the variable new_link 1239ff4: f8ff8022 swpal xzr, x2, [x1] ---this instr corresponding rte_atomic_exchange_explicit, it will place the resut in xzr which always zero, and this will lead to rte_eth_linkstatus_set return 0. 1239ff8: 3940f3e0 ldrb w0, [sp, #60] 1239ffc: d3609c41 ubfx x1, x2, #32, #8 123a000: 4a010000 eor w0, w0, w1 123a004: 36100080 tbz w0, #2, 123a014 3. We checked other "ret = rte_eth_linkstatus_set" calls, and can't find similar problem. 4. After reading a lot of documents, we preliminarily think that the problem is caused by -fstrict-aliasing (which was enabled default with O2 or O3), if compiled with -fno-strict-aliasing, then this problem don't exist. We guest this maybe strict-aliasing's bug which only happened in our function. 5. We also try to use union to avoid such aliasing in rte_eth_linkstatus_set, we changed the struct rte_eth_link define, and it works: -__extension__ -struct __rte_aligned(8) rte_eth_link { /**< aligned for atomic64 read/write */ - uint32_t link_speed; /**< RTE_ETH_SPEED_NUM_ */ - uint16_t link_duplex : 1; /**< RTE_ETH_LINK_[HALF/FULL]_DUPLEX */ - uint16_t link_autoneg : 1; /**< RTE_ETH_LINK_[AUTONEG/FIXED] */ - uint16_t link_status : 1; /**< RTE_ETH_LINK_[DOWN/UP] */ +struct rte_eth_link { /**< aligned for atomic64 read/write */ + union { + uint64_t val64; + struct { + uint32_t link_speed; /**< RTE_ETH_SPEED_NUM_ */ + uint16_t link_duplex : 1; /**< RTE_ETH_LINK_[HALF/FULL]_DUPLEX */ + uint16_t link_autoneg : 1; /**< RTE_ETH_LINK_[AUTONEG/FIXED] */ + uint16_t link_status : 1; /**< RTE_ETH_LINK_[DOWN/UP] */ + }; + }; }; the corresponding rte_eth_linkstatus_set: @@ -1674,18 +1674,13 @@ static inline int rte_eth_linkstatus_set(struct rte_eth_dev *dev, const struct rte_eth_link *new_link) { - RTE_ATOMIC(uint64_t) *dev_link = (uint64_t __rte_atomic *)&(dev->data->dev_link); - union { - uint64_t val64; - struct rte_eth_link link; - } orig; - - RTE_BUILD_BUG_ON(sizeof(*new_link) != sizeof(uint64_t)); + struct rte_eth_link old_link; - orig.val64 = rte_atomic_exchange_explicit(dev_link, *(const uint64_t *)new_link, + old_link.val64 = rte_atomic_exchange_explicit(&dev->data->dev_link.val64, + new_link->val64, rte_memory_order_seq_cst); - return (orig.link.link_status == new_link->link_status) ? -1 : 0; + return (old_link.link_status == new_link->link_status) ? -1 : 0; } 6. BTW: the linux kernel enabled "-fno-strict-aliasing" default, please see [4] for more. Last： We think there are two ways to solve this problem. 1. Add the compilation option '-fno-strict-aliasing' for hold DPDK project. 2. Use union to avoid such aliasing in rte_eth_linkstatus_set (please see above). PS: We prefer first way. Hope for more discuess. Thanks [1] https://developer.arm.com/downloads/-/arm-gnu-toolchain-downloads/12-3-rel1 [2] void hns3_update_linkstatus_and_event(struct hns3_hw *hw, bool query) { struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id]; struct rte_eth_link new_link; int ret; if (query) hns3_update_port_link_info(dev); memset(&new_link, 0, sizeof(new_link)); hns3_setup_linkstatus(dev, &new_link); ret = rte_eth_linkstatus_set(dev, &new_link); if (ret == 0 && dev->data->dev_conf.intr_conf.lsc != 0) hns3_start_report_lse(dev); } [3] static inline int rte_eth_linkstatus_set(struct rte_eth_dev *dev, const struct rte_eth_link *new_link) { RTE_ATOMIC(uint64_t) *dev_link = (uint64_t __rte_atomic *)&(dev->data->dev_link); union { uint64_t val64; struct rte_eth_link link; } orig; RTE_BUILD_BUG_ON(sizeof(*new_link) != sizeof(uint64_t)); orig.val64 = rte_atomic_exchange_explicit(dev_link, *(const uint64_t *)new_link, rte_memory_order_seq_cst); return (orig.link.link_status == new_link->link_status) ? -1 : 0; } [4] https://lore.kernel.org/all/b3itcd$2bi$1@penguin.transmeta.com/