ethdev: fix strict aliasing lead to link cannot be up
Checks
Commit Message
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.
[1] Strict aliasing problem with rte_eth_linkstatus_set()
https://marc.info/?l=dpdk-dev&m=171274148514777&w=3
Cc: stable@dpdk.org
Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
lib/ethdev/ethdev_driver.h | 23 +++++++----------------
lib/ethdev/rte_ethdev.h | 16 ++++++++++------
2 files changed, 17 insertions(+), 22 deletions(-)
Comments
> From: Chengwen Feng [mailto:fengchengwen@huawei.com]
> Sent: Thursday, 11 April 2024 05.08
>
> 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.
>
> [1] Strict aliasing problem with rte_eth_linkstatus_set()
> https://marc.info/?l=dpdk-dev&m=171274148514777&w=3
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> ---
The patch mixes atomic and non-atomic access.
This is not new for DPDK, which used to rely on compiler built-in atomics.
I'm not sure if it needs to be changed, but my suggestion is inline below.
> lib/ethdev/ethdev_driver.h | 23 +++++++----------------
> lib/ethdev/rte_ethdev.h | 16 ++++++++++------
> 2 files changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 0dbf2dd6a2..9d831d5c84 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -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,
> - rte_memory_order_seq_cst);
> + old_link.val64 = rte_atomic_exchange_explicit(&dev->data-
> >dev_link.val64,
old_link.val64 should be written using:
rte_atomic_store_explicit(&old_link.val64, rte_atomic_exchange_explicit(dev_link, ...), rte_memory_order_seq_cst)
> + new_link->val64,
new_link->val64 should be read using:
rte_atomic_load_explicit(&new_link->val64, rte_memory_order_seq_cst)
> + 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;
> }
>
> /**
> @@ -1701,12 +1696,8 @@ 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));
> -
> - *dst = rte_atomic_load_explicit(src, rte_memory_order_seq_cst);
> + link->val64 = rte_atomic_load_explicit(&dev->data->dev_link.val64,
link->val64 should be written using:
rte_atomic_store_explicit(&link->val64, rte_atomic_load_explicit(&dev->data->dev_link.val64, ...), rte_memory_order_seq_cst)
> + rte_memory_order_seq_cst);
> }
>
> /**
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 147257d6a2..0b5d3d2318 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -332,12 +332,16 @@ struct rte_eth_stats {
> /**
> * A structure used to retrieve link-level information of an Ethernet
> port.
> */
> -__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 {
> + union {
> + uint64_t val64; /**< used for atomic64 read/write */
> + 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
> --
> 2.17.1
> From: Chengwen Feng [mailto:fengchengwen@huawei.com]
> Sent: Thursday, 11 April 2024 05.08
>
> 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.
>
> [1] Strict aliasing problem with rte_eth_linkstatus_set()
> https://marc.info/?l=dpdk-dev&m=171274148514777&w=3
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> ---
The patch mixes atomic and non-atomic access.
This is not new for DPDK, which used to rely on compiler built-in atomics.
I'm not sure it needs to be changed, but my suggestion is inline below.
I don't think it makes any practical different for 64 bit arch, but it might for 32 bit arch.
> lib/ethdev/ethdev_driver.h | 23 +++++++----------------
> lib/ethdev/rte_ethdev.h | 16 ++++++++++------
> 2 files changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 0dbf2dd6a2..9d831d5c84 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -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,
> - rte_memory_order_seq_cst);
> + old_link.val64 = rte_atomic_exchange_explicit(&dev->data-
> >dev_link.val64,
old_link.val64 should be written using:
rte_atomic_store_explicit(&old_link.val64, ..., rte_memory_order_seq_cst)
> + new_link->val64,
new_link->val64 should be read using:
rte_atomic_load_explicit(&new_link->val64, rte_memory_order_seq_cst)
> + 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;
> }
>
> /**
> @@ -1701,12 +1696,8 @@ 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));
> -
> - *dst = rte_atomic_load_explicit(src, rte_memory_order_seq_cst);
> + link->val64 = rte_atomic_load_explicit(&dev->data->dev_link.val64,
link->val64 should be written using:
rte_atomic_store_explicit(&link->val64, ..., rte_memory_order_seq_cst)
> + rte_memory_order_seq_cst);
> }
>
> /**
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 147257d6a2..0b5d3d2318 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -332,12 +332,16 @@ struct rte_eth_stats {
> /**
> * A structure used to retrieve link-level information of an Ethernet
> port.
> */
> -__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 {
> + union {
> + uint64_t val64; /**< used for atomic64 read/write */
The type of val64 should be:
RTE_ATOMIC(uint64_t)
> + 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
> --
> 2.17.1
Hi Morten,
On 2024/4/11 14:58, Morten Brørup wrote:
>> From: Chengwen Feng [mailto:fengchengwen@huawei.com]
>> Sent: Thursday, 11 April 2024 05.08
>>
>> 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.
>>
>> [1] Strict aliasing problem with rte_eth_linkstatus_set()
>> https://marc.info/?l=dpdk-dev&m=171274148514777&w=3
>>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>
> The patch mixes atomic and non-atomic access.
> This is not new for DPDK, which used to rely on compiler built-in atomics.
>
> I'm not sure it needs to be changed, but my suggestion is inline below.
> I don't think it makes any practical different for 64 bit arch, but it might for 32 bit arch.
>
>> lib/ethdev/ethdev_driver.h | 23 +++++++----------------
>> lib/ethdev/rte_ethdev.h | 16 ++++++++++------
>> 2 files changed, 17 insertions(+), 22 deletions(-)
>>
>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>> index 0dbf2dd6a2..9d831d5c84 100644
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -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,
>> - rte_memory_order_seq_cst);
>> + old_link.val64 = rte_atomic_exchange_explicit(&dev->data-
>>> dev_link.val64,
>
> old_link.val64 should be written using:
> rte_atomic_store_explicit(&old_link.val64, ..., rte_memory_order_seq_cst)
I'm afraid I don't agree this, the &dev->data->dev_link.val64 should use atomic not the stack variable old_link.
>
>> + new_link->val64,
>
> new_link->val64 should be read using:
> rte_atomic_load_explicit(&new_link->val64, rte_memory_order_seq_cst)
The same reason with above.
>
>> + 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;
>> }
>>
>> /**
>> @@ -1701,12 +1696,8 @@ 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));
>> -
>> - *dst = rte_atomic_load_explicit(src, rte_memory_order_seq_cst);
>> + link->val64 = rte_atomic_load_explicit(&dev->data->dev_link.val64,
>
> link->val64 should be written using:
> rte_atomic_store_explicit(&link->val64, ..., rte_memory_order_seq_cst)
The same reason with above, the &dev->data->dev_link.val64 should use atomic not the stack variable link.
>
>> + rte_memory_order_seq_cst);
>> }
>>
>> /**
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 147257d6a2..0b5d3d2318 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -332,12 +332,16 @@ struct rte_eth_stats {
>> /**
>> * A structure used to retrieve link-level information of an Ethernet
>> port.
>> */
>> -__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 {
>> + union {
>> + uint64_t val64; /**< used for atomic64 read/write */
>
> The type of val64 should be:
> RTE_ATOMIC(uint64_t)
ack
Plus: yes, this patch mixes atomic and non-atomic access, but the main reason
is that we want to simplify the implementation. If we want to separate it clearly,
maybe we should defined as this:
struct rte_eth_link {
union {
RTE_ATOMIC(uint64_t) atomic64; /**< used for atomic64 read/write */
struct {
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] */
};
};
};
Thanks
>
>> + 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
>> --
>> 2.17.1
>
> .
>
> From: fengchengwen [mailto:fengchengwen@huawei.com]
> Sent: Thursday, 11 April 2024 13.58
[...]
> Plus: yes, this patch mixes atomic and non-atomic access, but the main
> reason is that we want to simplify the implementation.
Yes, your design in patch v3 follows the current standard design pattern for atomics in DPDK.
I agree with you on this.
Thank you for describing the alternative, though.
> If we want to separate it clearly,
> maybe we should defined as this:
> struct rte_eth_link {
> union {
> RTE_ATOMIC(uint64_t) atomic64; /**< used for atomic64
> read/write */
> struct {
> 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]
> */
> };
> };
> };
PS: More review comments provided in reply to the v3 patch.
On Thu, 11 Apr 2024 03:07:49 +0000
Chengwen Feng <fengchengwen@huawei.com> wrote:
> 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.
>
> [1] Strict aliasing problem with rte_eth_linkstatus_set()
> https://marc.info/?l=dpdk-dev&m=171274148514777&w=3
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> ---
The patch to use union is correct.
Examining the link status fuller raises a couple of pre-existing issues.
1. Why is this an inline function, there is no way this is in the
fast path of any driver or application?
2. Why is it marked sequential consistent and not relaxed? How could there be a visible
relationship between link status and other variables. Drivers would
not be using the link status state as internal variable.
Hi Stephen,
On 2024/4/11 23:05, Stephen Hemminger wrote:
> On Thu, 11 Apr 2024 03:07:49 +0000
> Chengwen Feng <fengchengwen@huawei.com> wrote:
>
>> 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.
>>
>> [1] Strict aliasing problem with rte_eth_linkstatus_set()
>> https://marc.info/?l=dpdk-dev&m=171274148514777&w=3
>>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>
> The patch to use union is correct.
> Examining the link status fuller raises a couple of pre-existing issues.
>
> 1. Why is this an inline function, there is no way this is in the
> fast path of any driver or application?
Nice catch.
The rte_eth_linkstatus_set/get was first introduced by commit "b77d21cc23 ethdev: add link status get/set helper functions".
which were inline function.
But I check this patchset [1], and found in v1~v3 is was impl without static inline, and the v4 just with static inline. and
I can't get the change reason. @Stephen could you recall something about this?
Below is what I try to find the reason from source code:
1, Why rte_eth_linkstatus_get is inline, I think maybe due:
a, memif driver invoke rte_eth_link_get() in Rx/Tx path (eth_memif_rx/eth_memif_rx_zc/eth_memif_tx/eth_memif_tx_zc)
b, before the above commit, rte_eth_link_get() invoke rte_eth_dev_atomic_read_link_status() which was static inline.
from above, I think it mainly due to performance reason, so keep the original impl.
2, Why rte_eth_linkstatus_set is inline, I check the following patch:
5042dde07d net/enic: use link status helper functions
2b4ab4223d net/octeontx: use link status helper functions
cc92eb9a97 net/szedata2: use link status helper functions
8e14dc285a net/thunderx: use link status helper functions
e324523c6c net/liquidio: use link status helper functions
e66b0fd123 net/i40e: use link status helper functions
4abe903e50 net/sfc: use link status helper functions
faadebad81 net/ixgbe: use link status helper functions
80ba61115e net/e1000: use link status helper functions
aab28ea2bc net/nfp: use link status helper functions
7e2eb5f0d2 net/dpaa2: use link status helper functions
13086a8f50 net/vmxnet3: use link status helper functions
717b2e8eae net/virtio: use link status helper functions
almost all pmd's set link function is static inline, I also check, and found only sfc driver will invoke it in
Tx path: sfc_efx_recv_pkts->sfc_ev_qpoll->efx_ev_qpoll->eevo_qpoll->siena_ef10_ev_qpoll->rhead_ev_mcdi->ef10_ev_mcdi->sfc_ev_link_change->rte_eth_linkstatus_set
[1] https://inbox.dpdk.org/dev/20180108174514.14688-1-stephen@networkplumber.org/
https://inbox.dpdk.org/dev/20180116182558.6254-1-stephen@networkplumber.org/
>
> 2. Why is it marked sequential consistent and not relaxed? How could there be a visible
> relationship between link status and other variables. Drivers would
> not be using the link status state as internal variable.
The original was impl by rte_atomic64_cmpset():
1, In powerpc platform, it was impl as:
static inline int
rte_atomic64_cmpset(volatile uint64_t *dst, uint64_t exp, uint64_t src)
{
return rte_atomic_compare_exchange_strong_explicit(dst, &exp, src, rte_memory_order_acquire,
rte_memory_order_acquire) ? 1 : 0;
}
2, In ARM64 platform, it was impl as:
static inline int
rte_atomic64_cmpset(volatile uint64_t *dst, uint64_t exp, uint64_t src)
{
return __sync_bool_compare_and_swap(dst, exp, src);
}
The __sync_bool_compare_and_swap will compiled with a dmb.
So there's no problem from the point of view of replacement (just with the memory barrier).
I also think there are no need with sequential consistent, because this atomic mainly to solve
the concurrent problem of rte_eth_link_get and driver lsc interrupt setting.
BTW: the above two problem is OK, but I think we could tackle with another commit (NOT this).
Thanks
>
> .
>
@@ -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,
- 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;
}
/**
@@ -1701,12 +1696,8 @@ 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));
-
- *dst = rte_atomic_load_explicit(src, rte_memory_order_seq_cst);
+ link->val64 = rte_atomic_load_explicit(&dev->data->dev_link.val64,
+ rte_memory_order_seq_cst);
}
/**
@@ -332,12 +332,16 @@ struct rte_eth_stats {
/**
* A structure used to retrieve link-level information of an Ethernet port.
*/
-__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 {
+ union {
+ uint64_t val64; /**< used for atomic64 read/write */
+ 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