Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2] net: stmmac: add XDP multi-buff support for TX side
@ 2026-09-24 12:34 Lorenzo Bianconi
  2026-09-25 13:27 ` netdev-bot+sashiko
  0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2026-09-24 12:34 UTC (permalink / raw)
  To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Stanislav Fomichev
  Cc: netdev, linux-stm32, linux-arm-kernel, bpf, Lorenzo Bianconi

Extend stmmac_xdp_xmit_xdpf() to transmit XDP frames with fragments
(multi-buff). Each buffer, i.e. the frame head and every frag, is mapped
and programmed into a dedicated TX descriptor.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
Changes in v2:
- Fix Tx coalescing logic in stmmac_xdp_xmit_xdpf()
- Consider the max between STMMAC_TX_THRESH() and num_frames as
  threshold in stmmac_xdp_xmit_xdpf().
- Link to v1: https://lore.kernel.org/r/20260923-b4-stmmac-xmit-mb-v1-1-a71a1c522142@oss.qualcomm.com
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 126 ++++++++++++++--------
 1 file changed, 83 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bf9e7e4cb1c3..089281ef9e61 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5239,77 +5239,117 @@ static unsigned int stmmac_rx_buf2_len(struct stmmac_priv *priv,
 static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 				struct xdp_frame *xdpf, bool dma_map)
 {
-	struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[queue];
+	struct skb_shared_info *sinfo = xdp_get_shared_info_from_frame(xdpf);
 	struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
 	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
+	unsigned int txq_thr = STMMAC_TX_THRESH(priv);
+	unsigned int first_entry = tx_q->cur_tx;
 	unsigned int entry = tx_q->cur_tx;
-	enum stmmac_txbuf_type buf_type;
-	struct dma_desc *tx_desc;
-	dma_addr_t dma_addr;
-	bool set_ic;
+	unsigned int num_frames = 1;
+	struct dma_desc *desc;
+	int i = 0;
 
-	if (stmmac_tx_avail(priv, queue) < STMMAC_TX_THRESH(priv))
+	if (unlikely(xdp_frame_has_frags(xdpf)))
+		num_frames += sinfo->nr_frags;
+
+	if (stmmac_tx_avail(priv, queue) < max(num_frames, txq_thr))
 		return STMMAC_XDP_CONSUMED;
 
 	if (priv->est && priv->est->enable &&
 	    priv->est->max_sdu[queue] &&
-	    xdpf->len > priv->est->max_sdu[queue]) {
+	    xdp_get_frame_len(xdpf) > priv->est->max_sdu[queue]) {
 		priv->xstats.max_sdu_txq_drop[queue]++;
 		return STMMAC_XDP_CONSUMED;
 	}
 
-	tx_desc = stmmac_get_tx_desc(priv, tx_q, entry);
-	if (dma_map) {
-		dma_addr = dma_map_single(priv->device, xdpf->data,
-					  xdpf->len, DMA_TO_DEVICE);
-		if (dma_mapping_error(priv->device, dma_addr))
-			return STMMAC_XDP_CONSUMED;
-
-		buf_type = STMMAC_TXBUF_T_XDP_NDO;
-	} else {
-		struct page *page = virt_to_page(xdpf->data);
-
-		dma_addr = page_pool_get_dma_addr(page) + sizeof(*xdpf) +
-			   xdpf->headroom;
-		dma_sync_single_for_device(priv->device, dma_addr,
-					   xdpf->len, DMA_BIDIRECTIONAL);
+	while (true) {
+		skb_frag_t *frag = i ? &sinfo->frags[i - 1] : NULL;
+		int len = frag ? skb_frag_size(frag) : xdpf->len;
+		bool last_frame = i == num_frames - 1;
+		enum stmmac_txbuf_type buf_type;
+		dma_addr_t dma_addr;
 
-		buf_type = STMMAC_TXBUF_T_XDP_TX;
-	}
+		desc = stmmac_get_tx_desc(priv, tx_q, entry);
+		if (dma_map) {
+			if (frag)
+				dma_addr = skb_frag_dma_map(priv->device,
+							    frag, 0, len,
+							    DMA_TO_DEVICE);
+			else
+				dma_addr = dma_map_single(priv->device,
+							  xdpf->data, len,
+							  DMA_TO_DEVICE);
+			if (dma_mapping_error(priv->device, dma_addr))
+				goto error_dma_unmap;
 
-	stmmac_set_tx_dma_entry(tx_q, entry, buf_type, dma_addr, xdpf->len,
-				false);
-	stmmac_set_tx_dma_last_segment(tx_q, entry);
+			buf_type = STMMAC_TXBUF_T_XDP_NDO;
+		} else {
+			struct page *page;
 
-	tx_q->xdpf[entry] = xdpf;
+			page = frag ? skb_frag_page(frag)
+				    : virt_to_page(xdpf->data);
+			dma_addr = page_pool_get_dma_addr(page);
+			if (frag)
+				dma_addr += skb_frag_off(frag);
+			else
+				dma_addr += sizeof(*xdpf) + xdpf->headroom;
+			dma_sync_single_for_device(priv->device, dma_addr,
+						   len, DMA_BIDIRECTIONAL);
+			buf_type = STMMAC_TXBUF_T_XDP_TX;
+		}
 
-	stmmac_set_desc_addr(priv, tx_desc, dma_addr);
+		stmmac_set_tx_dma_entry(tx_q, entry, buf_type, dma_addr, len,
+					dma_map && frag);
+		stmmac_set_desc_addr(priv, desc, dma_addr);
+		stmmac_prepare_tx_desc(priv, desc, !i, len, csum,
+				       priv->descriptor_mode, !!i, last_frame,
+				       xdp_get_frame_len(xdpf));
+		tx_q->xdpf[entry] = last_frame ? xdpf : NULL;
+		if (last_frame) {
+			stmmac_set_tx_dma_last_segment(tx_q, entry);
+			break;
+		}
 
-	stmmac_prepare_tx_desc(priv, tx_desc, 1, xdpf->len,
-			       csum, priv->descriptor_mode, true, true,
-			       xdpf->len);
+		entry = STMMAC_NEXT_ENTRY(entry, priv->dma_conf.dma_tx_size);
+		i++;
+	}
+	tx_q->tx_count_frames += num_frames;
 
-	tx_q->tx_count_frames++;
+	if (priv->tx_coal_frames[queue] &&
+	    (tx_q->tx_count_frames % priv->tx_coal_frames[queue]) < num_frames) {
+		struct stmmac_txq_stats *txq_stats;
 
-	if (tx_q->tx_count_frames % priv->tx_coal_frames[queue] == 0)
-		set_ic = true;
-	else
-		set_ic = false;
-
-	if (set_ic) {
+		desc = stmmac_get_tx_desc(priv, tx_q, entry);
+		stmmac_set_tx_ic(priv, desc);
 		tx_q->tx_count_frames = 0;
-		stmmac_set_tx_ic(priv, tx_desc);
+
+		txq_stats = &priv->xstats.txq_stats[queue];
 		u64_stats_update_begin(&txq_stats->q_syncp);
 		u64_stats_inc(&txq_stats->q.tx_set_ic_bit);
 		u64_stats_update_end(&txq_stats->q_syncp);
 	}
 
+	/* Set the OWN bit on the first descriptor now that all descriptors
+	 * for this skb are populated.
+	 */
+	desc = stmmac_get_tx_desc(priv, tx_q, first_entry);
+	stmmac_set_tx_owner(priv, desc);
+	tx_q->cur_tx = STMMAC_NEXT_ENTRY(entry, priv->dma_conf.dma_tx_size);
 	stmmac_enable_dma_transmission(priv, priv->ioaddr, queue);
 
-	entry = STMMAC_NEXT_ENTRY(entry, priv->dma_conf.dma_tx_size);
-	tx_q->cur_tx = entry;
-
 	return STMMAC_XDP_TX;
+
+error_dma_unmap:
+	while (first_entry != entry) {
+		desc = stmmac_get_tx_desc(priv, tx_q, first_entry);
+		stmmac_release_tx_desc(priv, desc, priv->descriptor_mode);
+		stmmac_free_tx_buffer(priv, &priv->dma_conf, queue,
+				      first_entry);
+		first_entry = STMMAC_NEXT_ENTRY(first_entry,
+						priv->dma_conf.dma_tx_size);
+	}
+
+	return STMMAC_XDP_CONSUMED;
 }
 
 static int stmmac_xdp_get_tx_queue(struct stmmac_priv *priv,

---
base-commit: 1c85ea5df86bed136589a2866c22f394020d3fdf
change-id: 20260922-b4-stmmac-xmit-mb-bb78e0684cf4

Best regards,
-- 
Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v2] net: stmmac: add XDP multi-buff support for TX side
  2026-09-24 12:34 [PATCH net-next v2] net: stmmac: add XDP multi-buff support for TX side Lorenzo Bianconi
@ 2026-09-25 13:27 ` netdev-bot+sashiko
  2026-09-25 15:55   ` Lorenzo Bianconi
  0 siblings, 1 reply; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-09-25 13:27 UTC (permalink / raw)
  To: lorenzo.bianconi
  Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32, alexandre.torgue, ast, daniel, hawk,
	john.fastabend, sdf, netdev, linux-stm32, linux-arm-kernel, bpf

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch adds multi-buffer (frag) handling to
  stmmac_xdp_xmit_xdpf() in…
- [Medium] In stmmac_xdp_xmit_xdpf()
  (drivers/net/ethernet/stmicro/stmmac/stmmac_main.c), the patch removes…
- [Low] A new comment in stmmac_xdp_xmit_xdpf()
  (drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5332-5334) reads…

--

net: stmmac: add XDP multi-buff support for TX side

This commit extends stmmac_xdp_xmit_xdpf() so it can send XDP frames that
carry fragments. The frame head and each frag are mapped and placed in
their own TX descriptor. The OWN bit on the first descriptor is set only
after all the descriptors are filled in.

Link: https://lore.kernel.org/r/20260923-b4-stmmac-xmit-mb-v1-1-a71a1c522142@oss.qualcomm.com

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index bf9e7e4cb1c31..089281ef9e612 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5239,77 +5239,117 @@ static unsigned int stmmac_rx_buf2_len(struct stmmac_priv *priv,
>  static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
>  				struct xdp_frame *xdpf, bool dma_map)
>  {

[ ... ]

> -	if (stmmac_tx_avail(priv, queue) < STMMAC_TX_THRESH(priv))
> +	if (unlikely(xdp_frame_has_frags(xdpf)))
> +		num_frames += sinfo->nr_frags;

[Severity: Medium]
Can a frame with frags reach this function from either caller?

On the ndo_xdp_xmit path, stmmac_xdp_set_prog() still registers the
redirect target without SG support:

drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c:stmmac_xdp_set_prog() {
    ...
	if (prog)
		xdp_features_set_redirect_target(dev, false);
    ...
}

The static mask in stmmac_dvr_probe() is only NETDEV_XDP_ACT_BASIC,
NETDEV_XDP_ACT_REDIRECT and NETDEV_XDP_ACT_XSK_ZEROCOPY. Because
NETDEV_XDP_ACT_NDO_XMIT_SG is never set, devmap drops fragmented frames
before they get to stmmac_xdp_xmit():

kernel/bpf/devmap.c:__xdp_enqueue() {
    ...
	if (unlikely(!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT_SG) &&
		     xdp_frame_has_frags(xdpf)))
		return -EOPNOTSUPP;
    ...
}

is_valid_dst() does the same check on the broadcast path.

On the XDP_TX path, stmmac_xdp_xmit_back() only gets frames built by the
stmmac RX path, and that path never builds a multi-buffer xdp_buff. The
driver has no xdp_buff_add_frag() or xdp_buff_set_frags() calls.
stmmac_xdp_set_prog() also rejects XDP when mtu > ETH_DATA_LEN,
stmmac_change_mtu() blocks jumbo MTU while XDP is loaded, and SPH is turned
off for XDP.

If so, the frag loop, the skb_frag_dma_map() and skb_frag_off() handling,
and the multi-descriptor error_dma_unmap unwind can't be run or tested. The
subject "add XDP multi-buff support for TX side" also doesn't match what
the driver advertises.

Should this patch advertise NDO_XMIT_SG as well, for example by calling
xdp_features_set_redirect_target(dev, true)?

[ ... ]

> +		stmmac_set_tx_dma_entry(tx_q, entry, buf_type, dma_addr, len,
> +					dma_map && frag);
> +		stmmac_set_desc_addr(priv, desc, dma_addr);
> +		stmmac_prepare_tx_desc(priv, desc, !i, len, csum,
> +				       priv->descriptor_mode, !!i, last_frame,
> +				       xdp_get_frame_len(xdpf));
> +		tx_q->xdpf[entry] = last_frame ? xdpf : NULL;
> +		if (last_frame) {
> +			stmmac_set_tx_dma_last_segment(tx_q, entry);
> +			break;
> +		}
>  
> -	stmmac_prepare_tx_desc(priv, tx_desc, 1, xdpf->len,
> -			       csum, priv->descriptor_mode, true, true,
> -			       xdpf->len);

[Severity: Medium]
Does this drop the dma_wmb() that used to come before the OWN bit on the
normal single-buffer XDP_TX and ndo_xdp_xmit path?

The old call passed is_fs=1 and tx_own=true. That hit the barrier in
enh_desc_prepare_tx_desc(), and the dwmac4 and dwxgmac2 versions have the
same one:

drivers/net/ethernet/stmicro/stmmac/enh_desc.c:enh_desc_prepare_tx_desc() {
    ...
	if (is_fs && tx_own)
		/* When the own bit, for the first frame, has to be set, all
		 * descriptors for the same frame has to be set before, to
		 * avoid race condition.
		 */
		dma_wmb();

	p->des0 = cpu_to_le32(tdes0);
}

Now the first descriptor is prepared with is_fs=1 and tx_own=0, and the
frag descriptors with is_fs=0. Neither combination reaches the barrier.
OWN on first_entry is set later by stmmac_set_tx_owner(). Every
implementation of that is a plain read-modify-write with no barrier:

static void enh_desc_set_tx_owner(struct dma_desc *p)
{
	p->des0 |= cpu_to_le32(ETDES0_OWN);
}

dwmac100 and dwmac1000 cores have no tail pointer.
dwmac_enable_dma_transmission() only writes DMA_CHAN_XMT_POLL_DEMAND, and a
DMA that is already running keeps fetching owned descriptors. On a weakly
ordered CPU, can the DMA see OWN=1 before the new des1/des2 buffer length
and address? It could then send a stale address left over from the slot's
earlier use.

stmmac_xmit() has the same pattern, but that is not new. Would adding a
dma_wmb() just before stmmac_set_tx_owner() on first_entry restore the old
ordering?

[ ... ]

> +	/* Set the OWN bit on the first descriptor now that all descriptors
> +	 * for this skb are populated.
> +	 */

[Severity: Low]
This isn't a bug, but this comment appears to come from stmmac_xmit(). This
function transmits an xdp_frame and does not use an skb.

> +	desc = stmmac_get_tx_desc(priv, tx_q, first_entry);
> +	stmmac_set_tx_owner(priv, desc);
> +	tx_q->cur_tx = STMMAC_NEXT_ENTRY(entry, priv->dma_conf.dma_tx_size);
>  	stmmac_enable_dma_transmission(priv, priv->ioaddr, queue);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924-b4-stmmac-xmit-mb-v2-1-003347b7bc25%40oss.qualcomm.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v2] net: stmmac: add XDP multi-buff support for TX side
  2026-09-25 13:27 ` netdev-bot+sashiko
@ 2026-09-25 15:55   ` Lorenzo Bianconi
  0 siblings, 0 replies; 3+ messages in thread
From: Lorenzo Bianconi @ 2026-09-25 15:55 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32, alexandre.torgue, ast, daniel, hawk,
	john.fastabend, sdf, netdev, linux-stm32, linux-arm-kernel, bpf

[-- Attachment #1: Type: text/plain, Size: 6710 bytes --]

> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 2 · Low: 1
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The patch adds multi-buffer (frag) handling to
>   stmmac_xdp_xmit_xdpf() in…
> - [Medium] In stmmac_xdp_xmit_xdpf()
>   (drivers/net/ethernet/stmicro/stmmac/stmmac_main.c), the patch removes…
> - [Low] A new comment in stmmac_xdp_xmit_xdpf()
>   (drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5332-5334) reads…

I will fix sashiko's reported issues in v3.

Regard,
Lorenzo

> 
> --
> 
> net: stmmac: add XDP multi-buff support for TX side
> 
> This commit extends stmmac_xdp_xmit_xdpf() so it can send XDP frames that
> carry fragments. The frame head and each frag are mapped and placed in
> their own TX descriptor. The OWN bit on the first descriptor is set only
> after all the descriptors are filled in.
> 
> Link: https://lore.kernel.org/r/20260923-b4-stmmac-xmit-mb-v1-1-a71a1c522142@oss.qualcomm.com
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index bf9e7e4cb1c31..089281ef9e612 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -5239,77 +5239,117 @@ static unsigned int stmmac_rx_buf2_len(struct stmmac_priv *priv,
> >  static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
> >  				struct xdp_frame *xdpf, bool dma_map)
> >  {
> 
> [ ... ]
> 
> > -	if (stmmac_tx_avail(priv, queue) < STMMAC_TX_THRESH(priv))
> > +	if (unlikely(xdp_frame_has_frags(xdpf)))
> > +		num_frames += sinfo->nr_frags;
> 
> [Severity: Medium]
> Can a frame with frags reach this function from either caller?
> 
> On the ndo_xdp_xmit path, stmmac_xdp_set_prog() still registers the
> redirect target without SG support:
> 
> drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c:stmmac_xdp_set_prog() {
>     ...
> 	if (prog)
> 		xdp_features_set_redirect_target(dev, false);
>     ...
> }
> 
> The static mask in stmmac_dvr_probe() is only NETDEV_XDP_ACT_BASIC,
> NETDEV_XDP_ACT_REDIRECT and NETDEV_XDP_ACT_XSK_ZEROCOPY. Because
> NETDEV_XDP_ACT_NDO_XMIT_SG is never set, devmap drops fragmented frames
> before they get to stmmac_xdp_xmit():
> 
> kernel/bpf/devmap.c:__xdp_enqueue() {
>     ...
> 	if (unlikely(!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT_SG) &&
> 		     xdp_frame_has_frags(xdpf)))
> 		return -EOPNOTSUPP;
>     ...
> }
> 
> is_valid_dst() does the same check on the broadcast path.
> 
> On the XDP_TX path, stmmac_xdp_xmit_back() only gets frames built by the
> stmmac RX path, and that path never builds a multi-buffer xdp_buff. The
> driver has no xdp_buff_add_frag() or xdp_buff_set_frags() calls.
> stmmac_xdp_set_prog() also rejects XDP when mtu > ETH_DATA_LEN,
> stmmac_change_mtu() blocks jumbo MTU while XDP is loaded, and SPH is turned
> off for XDP.
> 
> If so, the frag loop, the skb_frag_dma_map() and skb_frag_off() handling,
> and the multi-descriptor error_dma_unmap unwind can't be run or tested. The
> subject "add XDP multi-buff support for TX side" also doesn't match what
> the driver advertises.
> 
> Should this patch advertise NDO_XMIT_SG as well, for example by calling
> xdp_features_set_redirect_target(dev, true)?
> 
> [ ... ]
> 
> > +		stmmac_set_tx_dma_entry(tx_q, entry, buf_type, dma_addr, len,
> > +					dma_map && frag);
> > +		stmmac_set_desc_addr(priv, desc, dma_addr);
> > +		stmmac_prepare_tx_desc(priv, desc, !i, len, csum,
> > +				       priv->descriptor_mode, !!i, last_frame,
> > +				       xdp_get_frame_len(xdpf));
> > +		tx_q->xdpf[entry] = last_frame ? xdpf : NULL;
> > +		if (last_frame) {
> > +			stmmac_set_tx_dma_last_segment(tx_q, entry);
> > +			break;
> > +		}
> >  
> > -	stmmac_prepare_tx_desc(priv, tx_desc, 1, xdpf->len,
> > -			       csum, priv->descriptor_mode, true, true,
> > -			       xdpf->len);
> 
> [Severity: Medium]
> Does this drop the dma_wmb() that used to come before the OWN bit on the
> normal single-buffer XDP_TX and ndo_xdp_xmit path?
> 
> The old call passed is_fs=1 and tx_own=true. That hit the barrier in
> enh_desc_prepare_tx_desc(), and the dwmac4 and dwxgmac2 versions have the
> same one:
> 
> drivers/net/ethernet/stmicro/stmmac/enh_desc.c:enh_desc_prepare_tx_desc() {
>     ...
> 	if (is_fs && tx_own)
> 		/* When the own bit, for the first frame, has to be set, all
> 		 * descriptors for the same frame has to be set before, to
> 		 * avoid race condition.
> 		 */
> 		dma_wmb();
> 
> 	p->des0 = cpu_to_le32(tdes0);
> }
> 
> Now the first descriptor is prepared with is_fs=1 and tx_own=0, and the
> frag descriptors with is_fs=0. Neither combination reaches the barrier.
> OWN on first_entry is set later by stmmac_set_tx_owner(). Every
> implementation of that is a plain read-modify-write with no barrier:
> 
> static void enh_desc_set_tx_owner(struct dma_desc *p)
> {
> 	p->des0 |= cpu_to_le32(ETDES0_OWN);
> }
> 
> dwmac100 and dwmac1000 cores have no tail pointer.
> dwmac_enable_dma_transmission() only writes DMA_CHAN_XMT_POLL_DEMAND, and a
> DMA that is already running keeps fetching owned descriptors. On a weakly
> ordered CPU, can the DMA see OWN=1 before the new des1/des2 buffer length
> and address? It could then send a stale address left over from the slot's
> earlier use.
> 
> stmmac_xmit() has the same pattern, but that is not new. Would adding a
> dma_wmb() just before stmmac_set_tx_owner() on first_entry restore the old
> ordering?
> 
> [ ... ]
> 
> > +	/* Set the OWN bit on the first descriptor now that all descriptors
> > +	 * for this skb are populated.
> > +	 */
> 
> [Severity: Low]
> This isn't a bug, but this comment appears to come from stmmac_xmit(). This
> function transmits an xdp_frame and does not use an skb.
> 
> > +	desc = stmmac_get_tx_desc(priv, tx_q, first_entry);
> > +	stmmac_set_tx_owner(priv, desc);
> > +	tx_q->cur_tx = STMMAC_NEXT_ENTRY(entry, priv->dma_conf.dma_tx_size);
> >  	stmmac_enable_dma_transmission(priv, priv->ioaddr, queue);
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924-b4-stmmac-xmit-mb-v2-1-003347b7bc25%40oss.qualcomm.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-25 15:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 12:34 [PATCH net-next v2] net: stmmac: add XDP multi-buff support for TX side Lorenzo Bianconi
2026-09-25 13:27 ` netdev-bot+sashiko
2026-09-25 15:55   ` Lorenzo Bianconi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox