Strict aliasing problem with rte_eth_linkstatus_set()

[fengchengwen <fengchengwen@huawei.com>]

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 <hns3_update_linkstatus_and_event+0xd4>
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/