[PATCH net-next v2] net: stmmac: add XDP multi-buff support for TX side
netdev-bot+sashiko at kernel.org
netdev-bot+sashiko at kernel.org
Fri Sep 25 06:27:01 PDT 2026
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
More information about the linux-arm-kernel
mailing list