Re: [PATCH v4 3/4] net/macb: add NEON vectorized Rx/Tx

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed,  2 Apr 2025 06:59:12 +0000
liwencheng <liwencheng@phytium.com.cn> wrote:

> +static inline uint8x8_t macb_mbuf_initializer(struct macb_rx_queue *rxq)
> +{
> +	volatile struct rte_mbuf mbuf = {.buf_addr = 0}; /* zeroed mbuf */

There is no way an on stack variable could be volatile. What it looks like
your are doing is trying to workaround misunderstanding of atomic.

> +	uint64x1_t mbuf_initializer;
> +	uint8x8_t rearm_data_vec;
> +
> +	mbuf.data_off = RTE_PKTMBUF_HEADROOM + MACB_RX_DATA_OFFSET;
> +	mbuf.nb_segs = 1;
> +	mbuf.port = rxq->port_id;

Why not just put these into the initializer?

> +	rte_mbuf_refcnt_set((struct rte_mbuf *)&mbuf, 1);
> +
> +	/* prevent compiler reordering: rearm_data covers previous fields */
> +	rte_compiler_barrier();

You would be better off doing an atomic store with the right type of memory
order, rather than weak + barrier.  But would have to handle case where 
refcounts are compiled out.

> +	mbuf_initializer =
> +		vset_lane_u64(*(uint64_t *)(&mbuf.rearm_data), mbuf_initializer, 0);

Take the volatile of mbuf and you won't have compiler warning.


> +	rearm_data_vec = vld1_u8((uint8_t *)&mbuf_initializer);
> +	return rearm_data_vec;
> +}