> From: fengchengwen [mailto:fengcheng...@huawei.com]
> Sent: Friday, 12 April 2024 05.28
[...]
> >> @@ -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);
> >
> > It is not safe to assume that the link pointer points to local memory
> on the caller's stack.
> > The link pointer might point to a shared memory area, used by multiple
> threads/processes, so it needs to be stored atomically using
> rte_atomic_store_explicit(&link->val64, ..., rte_memory_order_seq_cst).
>
> I checked every call of rte_eth_linkstatus_get in DPDK, and all the link
> parameters are local variables.
> The dev->data->dev_link is placed in shared memory which could access
> from different threads/processes, it seems no need maintain another link
> struct which act the same role.
>
> So I think we should keep current impl, and not using
> rte_atomic_store_explicit(&link->val64,...
The application may pass a pointer to shared memory to the public
rte_eth_link_get() function, which passes the pointer on to
rte_eth_linkstatus_get():
https://elixir.bootlin.com/dpdk/v24.03/source/lib/ethdev/rte_ethdev.c#L2986
