patch 'ethdev: fix strict aliasing in link up' has been queued to stable release 23.11.2
Xueming Li
xuemingl at nvidia.com
Fri Jul 12 12:44:04 CEST 2024
Hi,
FYI, your patch has been queued to stable release 23.11.2
Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 07/14/24. So please
shout if anyone has objections.
Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.
Queued patches are on a temporary branch at:
https://git.dpdk.org/dpdk-stable/log/?h=23.11-staging
This queued commit can be viewed at:
https://git.dpdk.org/dpdk-stable/commit/?h=23.11-staging&id=598269fd16d996336544f2bd309c7f7ce941b99d
Thanks.
Xueming Li <xuemingl at nvidia.com>
---
>From 598269fd16d996336544f2bd309c7f7ce941b99d Mon Sep 17 00:00:00 2001
From: Chengwen Feng <fengchengwen at huawei.com>
Date: Mon, 22 Apr 2024 06:38:13 +0000
Subject: [PATCH] ethdev: fix strict aliasing in link up
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Cc: Xueming Li <xuemingl at nvidia.com>
[ upstream commit b9a87346b05c562dd6005ee025eca67a1a80bea8 ]
Fix a problem introduced by a compiler upgrade (from gcc10 to gcc12.3),
which will lead the hns3 NIC can't link up. The root cause is strict
aliasing violation in rte_eth_linkstatus_set() with hns3 driver, see
[1] for more details.
This commit use union to avoid such aliasing violation. Also the
impacted components (cxgbe and qos_sched) have been adapted to the
struct change.
[1] https://inbox.dpdk.org/dev/8175c905-e661-b910-7f20-59b6ab605c38@huawei.com/
Signed-off-by: Chengwen Feng <fengchengwen at huawei.com>
Signed-off-by: Dengdui Huang <huangdengdui at huawei.com>
Acked-by: Morten Brørup <mb at smartsharesystems.com>
Acked-by: Ferruh Yigit <ferruh.yigit at amd.com>
---
drivers/net/cxgbe/cxgbe_ethdev.c | 3 ++-
examples/qos_sched/init.c | 3 ++-
lib/ethdev/ethdev_driver.h | 24 +++++++++---------------
lib/ethdev/rte_ethdev.h | 17 +++++++++++------
4 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 8cc3d9f257..781f48cfac 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -211,9 +211,9 @@ int cxgbe_dev_link_update(struct rte_eth_dev *eth_dev,
unsigned int i, work_done, budget = 32;
struct link_config *lc = &pi->link_cfg;
struct adapter *adapter = pi->adapter;
- struct rte_eth_link new_link = { 0 };
u8 old_link = pi->link_cfg.link_ok;
struct sge *s = &adapter->sge;
+ struct rte_eth_link new_link;
for (i = 0; i < CXGBE_LINK_STATUS_POLL_CNT; i++) {
if (!s->fw_evtq.desc)
@@ -232,6 +232,7 @@ int cxgbe_dev_link_update(struct rte_eth_dev *eth_dev,
rte_delay_ms(CXGBE_LINK_STATUS_POLL_MS);
}
+ memset(&new_link, 0, sizeof(new_link));
new_link.link_status = cxgbe_force_linkup(adapter) ?
RTE_ETH_LINK_UP : pi->link_cfg.link_ok;
new_link.link_autoneg = (lc->link_caps & FW_PORT_CAP32_ANEG) ? 1 : 0;
diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
index d8abae635a..32964fd57e 100644
--- a/examples/qos_sched/init.c
+++ b/examples/qos_sched/init.c
@@ -335,7 +335,7 @@ int app_init(void)
for(i = 0; i < nb_pfc; i++) {
uint32_t socket = rte_lcore_to_socket_id(qos_conf[i].rx_core);
struct rte_ring *ring;
- struct rte_eth_link link = {0};
+ struct rte_eth_link link;
int retry_count = 100, retry_delay = 100; /* try every 100ms for 10 sec */
snprintf(ring_name, MAX_NAME_LEN, "ring-%u-%u", i, qos_conf[i].rx_core);
@@ -367,6 +367,7 @@ int app_init(void)
app_init_port(qos_conf[i].rx_port, qos_conf[i].mbuf_pool);
app_init_port(qos_conf[i].tx_port, qos_conf[i].mbuf_pool);
+ memset(&link, 0, sizeof(link));
rte_eth_link_get(qos_conf[i].tx_port, &link);
if (link.link_status == 0)
printf("Waiting for link on port %u\n", qos_conf[i].tx_port);
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index b482cd12bb..ec56925882 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1655,18 +1655,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,
- rte_memory_order_seq_cst);
+ 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;
}
/**
@@ -1682,12 +1677,11 @@ static inline void
rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
struct rte_eth_link *link)
{
- RTE_ATOMIC(uint64_t) *src = (uint64_t __rte_atomic *)&(dev->data->dev_link);
- uint64_t *dst = (uint64_t *)link;
-
- RTE_BUILD_BUG_ON(sizeof(*link) != sizeof(uint64_t));
+ struct rte_eth_link curr_link;
- *dst = rte_atomic_load_explicit(src, rte_memory_order_seq_cst);
+ curr_link.val64 = rte_atomic_load_explicit(&dev->data->dev_link.val64,
+ rte_memory_order_seq_cst);
+ rte_atomic_store_explicit(&link->val64, curr_link.val64, rte_memory_order_seq_cst);
}
/**
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 77331ce652..545799c341 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -331,13 +331,18 @@ struct rte_eth_stats {
/**
* A structure used to retrieve link-level information of an Ethernet port.
*/
-__extension__
struct rte_eth_link {
- 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] */
-} __rte_aligned(8); /**< aligned for atomic64 read/write */
+ union {
+ RTE_ATOMIC(uint64_t) val64; /**< used for atomic64 read/write */
+ __extension__
+ 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] */
+ };
+ };
+};
/**@{@name Link negotiation
* Constants used in link management.
--
2.34.1
---
Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- - 2024-07-12 18:40:15.117580306 +0800
+++ 0018-ethdev-fix-strict-aliasing-in-link-up.patch 2024-07-12 18:40:13.956594245 +0800
@@ -1 +1 @@
-From b9a87346b05c562dd6005ee025eca67a1a80bea8 Mon Sep 17 00:00:00 2001
+From 598269fd16d996336544f2bd309c7f7ce941b99d Mon Sep 17 00:00:00 2001
@@ -7,0 +8,3 @@
+Cc: Xueming Li <xuemingl at nvidia.com>
+
+[ upstream commit b9a87346b05c562dd6005ee025eca67a1a80bea8 ]
@@ -20,2 +22,0 @@
-Cc: stable at dpdk.org
-
@@ -34 +35 @@
-index a27b9b266e..9b6a3651f9 100644
+index 8cc3d9f257..781f48cfac 100644
@@ -78 +79 @@
-index 0dbf2dd6a2..883e59a927 100644
+index b482cd12bb..ec56925882 100644
@@ -81 +82 @@
-@@ -1674,18 +1674,13 @@ static inline int
+@@ -1655,18 +1655,13 @@ static inline int
@@ -105 +106 @@
-@@ -1701,12 +1696,11 @@ static inline void
+@@ -1682,12 +1677,11 @@ static inline void
@@ -123 +124 @@
-index 147257d6a2..548fada1c7 100644
+index 77331ce652..545799c341 100644
@@ -126 +127 @@
-@@ -332,12 +332,17 @@ struct rte_eth_stats {
+@@ -331,13 +331,18 @@ struct rte_eth_stats {
@@ -131 +132 @@
--struct __rte_aligned(8) rte_eth_link { /**< aligned for atomic64 read/write */
+ struct rte_eth_link {
@@ -136 +137 @@
-+struct rte_eth_link {
+-} __rte_aligned(8); /**< aligned for atomic64 read/write */
@@ -147 +148 @@
- };
++};
@@ -149,0 +151 @@
+ * Constants used in link management.
More information about the stable
mailing list