From: Nicolai Buchwitz <nb@tipi-net.de>
To: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@kernel.org>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Jesper Dangaard Brouer <hawk@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
Stanislav Fomichev <sdf@fomichev.me>,
Jose Abreu <Jose.Abreu@synopsys.com>,
Ong Boon Leong <boon.leong.ong@intel.com>,
netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
bpf@vger.kernel.org
Subject: Re: [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode
Date: Mon, 12 Oct 2026 23:04:11 +0200 [thread overview]
Message-ID: <b11af7d5d430dcddcc681484b50ae7e2@tipi-net.de> (raw)
In-Reply-To: <20261012-stmmac-rx-mb-v6-3-2225050915c1@oss.qualcomm.com>
Hi Lorenzo
On 12.10.2026 18:44, Lorenzo Bianconi wrote:
> stmmac_rx() runs the XDP program once per descriptor buffer and uses
> skb == NULL as the only marker for the start of a frame. A frame larger
> than one buffer is split by the hardware across several descriptors;
> GMAC4 enables jumbo frames unconditionally (GMAC_CONFIG_JE in
> GMAC_CORE_INIT), so an on-link peer can deliver such a frame even when
> the interface MTU is left at 1500.
>
> When an XDP verdict ends a non-last descriptor, skb is cleared and the
> next descriptor of the same hardware frame is parsed as a new frame.
> For XDP_CONSUMED this happens via read_again with len preserved, so
> stmmac_rx_buf1_len() then returns min(dma_buf_sz, plen - len) for the
> last descriptor, which can be as small as 0-3 bytes. buf1_len -=
> ETH_FCS_LEN underflows and xdp_prepare_buff() puts data_end far past
> data; on XDP_PASS this produces an skb whose length is close to 4G that
> reaches the network stack (skb->tail and skb->len are 32-bit and wrap
> below skb->end, so skb_over_panic() does not fire). A 4+ byte tail is
> instead offered to the XDP program and delivered as a standalone
> mid-payload Ethernet frame on XDP_PASS. XDP_TX/XDP_REDIRECT instead
> falls through to the next iteration, resetting the running length, so
> the tail is re-parsed as a full-size new frame.
>
> XDP does not support scatter-gather frames until the multi-buffer
> rework, so drop the whole frame instead: mark it as errored as soon as
> a non-last descriptor is seen while an XDP program is attached, drain
> and release every descriptor - including the last - and account the
> frame once in rx_dropped. Removing the re-parse lets both XDP verdict
> branches simply complete the packet via next.
>
> Note: the subsequent "net: stmmac: rework stmmac_rx to support XDP rx
> multi-buff" accumulates the whole frame into a single xdp_buff and
> supersedes this patch; the fix is kept so the net portion of the series
> stands on its own.
>
> Fixes: 5fabb01207a2 ("net: stmmac: Add initial XDP support")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 28
> ++++++++++-------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 426bb68c9bbc..8c08d44fdbd3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5893,18 +5893,24 @@ static int stmmac_rx(struct stmmac_priv *priv,
> int limit, u32 queue)
>
> if (priv->extend_desc)
> stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx +
> entry);
> - if (unlikely(status == discard_frame)) {
> - page_pool_put_page(rx_q->page_pool, buf->page, 0, true);
> - buf->page = NULL;
> +
> + if (unlikely(!error && status == discard_frame)) {
> error = 1;
> if (!priv->hwts_rx_en)
> rx_errors++;
> }
>
> - if (unlikely(error && (status & rx_not_ls)))
> - goto read_again;
> + /* We do not support SG in XDP mode */
> + if (unlikely(!error && (status & rx_not_ls) &&
> + stmmac_xdp_is_enabled(priv))) {
> + rx_dropped++;
> + error = 1;
> + }
AFAIU GMAC4 and XGMAC always use the second buffer of a descriptor. If
so, at MTU 1500 a frame of up to 3072 bytes can fit in one descriptor,
and this check would not catch it?
The XDP program would then only see the first buffer. With XDP_TX the
frame is cut and sent too short. With XDP_PASS the second buffer is
added
after the program already decided.
Patch 4 fixes this. But if this patch should work on its own, should it
also drop the frame when buf2_len is not zero? Or am I missing
something?
> [...]
Thanks,
Nicolai
next prev parent reply other threads:[~2026-10-12 21:04 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-12 16:44 [PATCH net-next v6 0/4] net: stmmac: introduce XDP rx multi-buff support Lorenzo Bianconi
2026-10-12 16:44 ` [PATCH net-next v6 1/4] net: stmmac: take ownership of saved RX state at poll entry Lorenzo Bianconi
2026-10-12 20:58 ` Nicolai Buchwitz
2026-10-12 16:44 ` [PATCH net-next v6 2/4] net: stmmac: drop split frames on AF_XDP zero-copy RX Lorenzo Bianconi
2026-10-12 20:59 ` Nicolai Buchwitz
2026-10-13 16:45 ` sashiko-bot
2026-10-13 17:33 ` netdev-bot+sashiko
2026-10-12 16:44 ` [PATCH net-next v6 3/4] net: stmmac: drop multi-buffer frames in XDP mode Lorenzo Bianconi
2026-10-12 21:04 ` Nicolai Buchwitz [this message]
2026-10-13 8:17 ` Lorenzo Bianconi
2026-10-13 8:24 ` Nicolai Buchwitz
2026-10-13 8:32 ` Lorenzo Bianconi
2026-10-13 16:45 ` sashiko-bot
2026-10-13 17:33 ` netdev-bot+sashiko
2026-10-12 16:44 ` [PATCH net-next v6 4/4] net: stmmac: rework stmmac_rx to support XDP rx multi-buff Lorenzo Bianconi
2026-10-12 21:10 ` Nicolai Buchwitz
2026-10-13 9:12 ` Lorenzo Bianconi
2026-10-13 10:16 ` Lorenzo Bianconi
2026-10-13 17:33 ` netdev-bot+sashiko
2026-10-12 16:50 ` [PATCH net-next v6 0/4] net: stmmac: introduce XDP rx multi-buff support netdev-bot+sinfo
2026-10-12 20:40 ` Lorenzo Bianconi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b11af7d5d430dcddcc681484b50ae7e2@tipi-net.de \
--to=nb@tipi-net.de \
--cc=Jose.Abreu@synopsys.com \
--cc=alexandre.torgue@foss.st.com \
--cc=andrew+netdev@lunn.ch \
--cc=ast@kernel.org \
--cc=boon.leong.ong@intel.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@kernel.org \
--cc=hawk@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=lorenzo.bianconi@oss.qualcomm.com \
--cc=maxime.chevallier@bootlin.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sdf@fomichev.me \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.