[PATCH net-next v6 0/3] net: ethernet: mtk_eth_soc: 9K jumbo frame support

[PATCH net-next v6 0/3] net: ethernet: mtk_eth_soc: 9K jumbo frame support

[Sandeep Haemoon]

This series makes the MediaTek SoC ethernet driver support 9K jumbo
frames on the 10G XGMII MACs (MT7988).  The driver advertises a
2030-byte max_mtu and programs the MAC max-rx field for at most 2048
bytes, so MTU > 2K is rejected and 9K frames are not usable.

Patch 1 fixes a pre-existing 36-bit DMA bug in mtk_poll_rx() that the
review of v3 found: a descriptor released without a new mapping is
handed back with the previous buffer's high address bits.  It is
independent of the rest of the series and carries a Fixes: tag for the
commit that introduced the bug, so the maintainers can route it to
net directly if they prefer.

Patch 2 gates the page_pool on the ring's descriptor length: the pool
hands out order-0 pages and maps at most MTK_PP_MAX_BUF_SIZE, so a
ring whose descriptor length is larger has to use the
netdev_alloc_frag()/mtk_max_buf_alloc() path.

Patch 3 adds the MTK_NETSYS_RX_9K capability, advertises a 9194-byte
max_mtu on the XGMII MACs (from the DT phy_mode at probe, so DSA can
raise the conduit MTU before the netdev is opened), programs
XMAC_RX_CFG2 and the PDMA RX SDL, derives the shared RX buffer length
from the largest MTU (802.1Q headroom included) and sizes the RX rings
from it, keeping the never-polled QDMA ring at its 2K buffers.  The
MAC max-rx limits stay in sync with that shared length, including
GMAC1's MCR jumbo field.  A runtime MTU change that needs larger
buffers is rejected with -EBUSY while DMA is running; shrinking is
allowed.  GMAC1, the built-in switch port, keeps its MCR programming
(bucket plus the jumbo field in 1024-byte units), written after the
MAC_MCR_FORCE_LINK_DOWN reset so it survives every mac_config();
whether the internal switch path enforces that field at all is a
hardware property the commit message documents.

This work is a forward-port of the MediaTek downstream jumbo work:
Mason Chang's "change default rx buffer length" and "add 9k jumbo
frame support" (MediaTek OpenWrt feed 999-eth-30/31, carried in
frank-w/BPI-Router-Linux), plus Frank Wunderlich's "try to fix crash
the right way".  The runtime reallocation of Mason's "dynamic rx
buffer adjustment" (feed 999-eth-33) is not included and will be sent
separately.  Attribution is prose plus Link: tags: Co-developed-by
requires the original authors' own Signed-off-by lines, which we do
not have, and the port diverges from the downstream implementation
(2K buffer floor, per-ring pool gate, the -EBUSY rule, the XDP checks
and the truesize fix are not in the downstream patches).

The equivalent change was tested on a BPI-R4 Pro 8X (MT7988, 10G
lan6 / MxL86252) on the 6.18.60 backport: with both ends at MTU 9000,
8K ICMP passes and iperf3 with MSS 8900 reaches ~9.87 Gbit/s (vs
~2.57 Gbit/s at MSS 1460); the DSA conduit is raised to 9004 at boot.
This v7.3 series is compile-tested with CONFIG_NET_MEDIATEK_SOC=y and
COMPILE_TEST=y; no hardware run of this exact tree yet.

With the 1 KiB rounding the RX buffer length is order-1 (8 KiB) for
rx_buf_len 4096..7168 and order-2 (16 KiB) from rx_buf_len 8192 up;
at the 9K ring size the 16 KiB buffers pin around 64 MiB while DMA is
up and make the refill a high-order GFP_ATOMIC allocation.  That cost
is inherent to the downstream design; skb->truesize now reports the
allocation that was actually made.

Changes in v6:
- rebase onto current net-next (a76e8bf4be) so the series applies
  again: net-next converted mtk_rx_alloc() to kzalloc_objs(), which
  the hunk context depended on.  No functional change.

Changes in v5 (the v4 review, in full):

- 802.1Q headroom (Medium): the shared RX buffer length now includes
  VLAN_HLEN before the 1 KiB round-up, and the advertised max_mtu is
  lowered by the same four bytes to 9194, so a tagged maximum-size
  frame (9194 + 18 + 4) still fits in rx_buf_len 9216 and the XMAC
  9216 cap.  XMAC_RX_CFG2 and the ring buffers therefore cover a
  tagged frame at every MTU, including the maximum.
- GMAC1 MCR jumbo field (Medium): the vendor driver has a second MCR
  max-rx field we had not ported.  mtk_set_mcr_max_rx() now clears
  and programs the jumbo field (bits 31:28, 1024-byte units,
  MAC_MCR_MAX_RX_JUMBO) for GMAC1, so the driver can program the same
  shared length for the switch port as for the XGMII MACs.  The new
  field handling is limited to the MTK_NETSYS_RX_9K SoCs; every other
  SoC keeps its previous MCR programming bit for bit.
- QDMA ring sizing (Medium): the helpers now take the buffer length
  instead of struct mtk_eth, and mtk_rx_alloc() sizes the
  never-polled QDMA RX ring at the fixed 2K length instead of the
  jumbo length, so it no longer pins ~32 MiB of high-order pages.
  HW-LRO rings keep their 12288-byte sizing.
- pktlen bound (Medium): mtk_poll_rx() drops descriptors whose PLEN0
  exceeds ring->buf_size, with rx_length_errors, before the buffer
  can be passed up, so the larger build_skb() size can no longer hand
  stale memory to the stack if the engine over-reports the length.
- PDMA SDL encoding (Medium): kept as in v4; the comment and commit
  message now say explicitly that the GENMASK(30,16) encoding (frame
  length offset by MTK_PDMA_LRO_SDL) is the vendor's and not publicly
  documented, and that a datasheet confirmation would be welcome.
- XDP detach (Medium): the pool-size check now runs only when a
  program is attached, so a detach always succeeds.  The shared
  eth->prog weakness the review also points out is pre-existing and
  is left to a separate fix.
- 3/3 message (Low): the buffer-size math is corrected (order-1 for
  rx_buf_len 4096..7168, order-2 from 8192) and the message now says
  that the build_skb()/truesize change also covers the HW-LRO rings
  on mt2701, mt7622, mt7623 and mt7629.
- 1/3 subject and message (Low): retitled to describe the fix ("fix
  the RX descriptor address on the release path") and the message now
  cites 6e0490fc36cd and e10a35abb3da, which moved the
  DMA_MAPPING_ERROR initialisation out of the loop and made the first
  commit's protection ineffective after the first iteration.

The pre-existing High finding of the review (a failure while filling
an RX ring leaks the partially filled buffers and DMA mappings
because ring->dma_size is still 0) is not part of this series: it is
in the untouched v7.3 code and will be sent as a separate fix that
sets ring->dma_size before the fill loop.

Simon Horman's Reviewed-by is kept on patch 2 only: patches 1 and 3
are new or changed materially since his review of v1.

Previous versions:
https://lore.kernel.org/all/patch-f1e62a87eb0f914e1ea52a395f465998@loljews.com/ (v5)
https://lore.kernel.org/all/patch-fc08a7ef40667bf3017366e452e6d3c0@loljews.com/ (v4)
https://lore.kernel.org/all/patch-aad4cfa8d9522d1f7dbc9893ea12e813@loljews.com/ (v3)
https://lore.kernel.org/all/patch-2793186c653124c4c0b3d463bde2ea3a@loljews.com/ (v2)
https://lore.kernel.org/all/patch-4f863a941889f0bceca1c082f2d79624@loljews.com/ (v1)

Sandeep Haemoon (3):
  net: ethernet: mtk_eth_soc: fix the RX descriptor address on the
    release path
  net: ethernet: mtk_eth_soc: skip page_pool for RX rings whose buffers
    do not fit
  net: ethernet: mtk_eth_soc: add 9K jumbo frame support

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 324 ++++++++++++++++----
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  16 +-
 2 files changed, 282 insertions(+), 58 deletions(-)

base-commit: a76e8bf4be552eed20801186188e5606365870a0
-- 
2.53.0

[PATCH net-next v6 1/3] net: ethernet: mtk_eth_soc: fix the RX

[Sandeep Haemoon]

mtk_poll_rx() keeps one dma_addr_t across the whole poll loop and only
refreshes it when a replacement buffer was mapped for the descriptor
being processed.  The paths that jump straight to release_desc (no
netdev for the source port, MTK_RESETTING set, or a refill allocation
failure) re-arm the descriptor with the buffer it still holds.  The
address written back must therefore come from the descriptor's own
rxd2, and the release path relies on dma_addr being DMA_MAPPING_ERROR
to select those bits.

That protection was added by commit 6e0490fc36cd ("net: ethernet:
mtk_eth_soc: fix SER panic with 4GB+ RAM") for the MTK_RESETTING path,
but commit e10a35abb3da ("net: ethernet: mtk_eth_soc: fix
uninitialized variable") moved the DMA_MAPPING_ERROR initialisation
out of the loop, so it only holds for the first iteration.  From the
second iteration on, the previous descriptor's high address bits are
combined with the current descriptor's low bits in rxd1.  With
MTK_36BIT_DMA and DRAM above 4 GiB the hardware then DMAs the next
frame to an address that is neither buffer, and the driver passes the
stale contents of the untouched buffer up the stack.  mtk_rx_clean()
unmaps the same mismatched address during a SER, which is the panic
commit 6e0490fc36cd ("net: ethernet: mtk_eth_soc: fix SER panic with
4GB+ RAM") was fixing.

Reset dma_addr to DMA_MAPPING_ERROR at the start of every iteration,
so a descriptor that is released without a new mapping keeps its own
address bits.

Fixes: 2d75891ebc09 ("net: ethernet: mtk_eth_soc: support 36-bit DMA addressing on MT7988")
Signed-off-by: Sandeep Haemoon <sandeeph@loljews.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 21abeccfe0..6c8b039fad 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2210,7 +2210,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 	u8 *data, *new_data;
 	struct mtk_rx_dma_v2 *rxd, trxd;
 	int done = 0, bytes = 0;
-	dma_addr_t dma_addr = DMA_MAPPING_ERROR;
+	dma_addr_t dma_addr;
 	int ppe_idx = 0;
 
 	while (done < budget) {
@@ -2219,6 +2219,12 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 		u32 hash, reason;
 		int mac = 0;
 
+		/* The release paths below hand the descriptor back with the
+		 * buffer it still holds, so only overwrite its address when
+		 * a new buffer was actually mapped for it.
+		 */
+		dma_addr = DMA_MAPPING_ERROR;
+
 		ring = mtk_get_rx_ring(eth);
 		if (unlikely(!ring))
 			goto rx_done;
-- 
2.53.0

Re: [PATCH net-next v6 1/3] net: ethernet: mtk_eth_soc: fix the RX

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account.

*IMPORTANT*: If you do repost (for other reasons) - please include
the relevant info in the cover letter or commit messages.
-- 
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next v6 1/3] net: ethernet: mtk_eth_soc: fix the RX descriptor address on the release path

[Sandeep Haemoon]

On the automated "looks like a fix" inquiry for this series:

The change in 1/3 was found by an automated review, not in production. The
reviewer read the release path of mtk_poll_rx() and noticed that the guard
added by 6e0490fc36cd ("net: ethernet: mtk_eth_soc: fix SER panic with 4GB+
RAM") relies on dma_addr being DMA_MAPPING_ERROR on every release, while
e10a35abb3da ("net: ethernet: mtk_eth_soc: fix uninitialized variable") had
moved that initialisation out of the loop and left it valid across
iterations.

It has not been triggered at runtime by us. It needs MTK_36BIT_DMA, DRAM
above 4 GiB, and one of the early release paths (no netdev for the source
port, MTK_RESETTING set, or a refill allocation failure) so that descriptor
N+1 is handed back with the high address bits of the buffer that was mapped
for descriptor N. The visible symptom would be the engine writing the next
frame to an address that is neither buffer, and stale contents of the
untouched buffer going up the stack; there is no stack trace to report.

Hardware: the series as a whole was tested on a BPI-R4 Pro 8X (MT7988, 10G
port) on a 6.18.60 backport - 8K ICMP passes and iperf3 with MSS 8900 at
MTU 9000 reaches about 9.87 Gbit/s - but the 36-bit path fix itself was not
exercised there because the condition was never hit. The v7.3 series is
compile-tested with CONFIG_NET_MEDIATEK_SOC=y and COMPILE_TEST=y.

As suggested, this is a reply rather than a repost; please apply it to the
series.

Re: [PATCH net-next v6 1/3] net: ethernet: mtk_eth_soc: fix the RX

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message for "net: ethernet: mtk_eth_soc: fix the RX"
  gets the history and cause backwards.
- [Low] The subject line "net: ethernet: mtk_eth_soc: fix the RX" does
  not say what is being fixed.

--

net: ethernet: mtk_eth_soc: fix the RX

This patch resets dma_addr to DMA_MAPPING_ERROR at the start of every
loop iteration in mtk_poll_rx(). Without it, a descriptor that goes to
release_desc without a new mapping can pick up high address bits left
over from an earlier iteration. That matters on MTK_36BIT_DMA SoCs with
DRAM above 4 GiB.

> That protection was added by commit 6e0490fc36cd ("net: ethernet:
> mtk_eth_soc: fix SER panic with 4GB+ RAM") for the MTK_RESETTING path,
> but commit e10a35abb3da ("net: ethernet: mtk_eth_soc: fix
> uninitialized variable") moved the DMA_MAPPING_ERROR initialisation
> out of the loop, so it only holds for the first iteration.

[Severity: Low]
Is this history accurate? e10a35abb3da is dated 2023-09-10 and
6e0490fc36cd is dated 2025-04-27, so e10a35abb3da came first.

e10a35abb3da also does not appear to move an initialiser out of the
loop. Before it, mtk_poll_rx() declared dma_addr inside the while loop
with no initialiser:

    dma_addr_t dma_addr;

e10a35abb3da moved the declaration to function scope:

    dma_addr_t dma_addr = DMA_MAPPING_ERROR;

It also added the first dma_addr != DMA_MAPPING_ERROR check at
release_desc. Later, 6e0490fc36cd replaced that check with the FIELD_GET()
branch:

    if (unlikely(dma_addr == DMA_MAPPING_ERROR))
        addr64 = FIELD_GET(RX_DMA_ADDR64_MASK,
                           rxd->rxd2);
    else
        addr64 = RX_DMA_PREP_ADDR64(dma_addr);

So that branch only ever worked for the first loop iteration. Nothing
broke it later. Could the message give the order that way?

The message also does not say that this fix depends on the FIELD_GET()
branch from 6e0490fc36cd. Both hunks touch only the declaration and the
top of the loop, which have looked the same since e10a35abb3da. So the
patch would apply cleanly to a tree that has e10a35abb3da but not
6e0490fc36cd.

In that tree, every descriptor reaching release_desc would hit the
dma_addr == DMA_MAPPING_ERROR case and skip RX_DMA_PREP_ADDR64(). Its
high address bits would then be cleared. That is the original SER
failure, now on every iteration instead of only the first.

6e0490fc36cd carries Fixes: 2d75891ebc09, so most stable trees probably
have it already. Would it still help to name 6e0490fc36cd as a
prerequisite?

> net: ethernet: mtk_eth_soc: fix the RX

[Severity: Low]
This isn't a bug, but the subject does not say what is being fixed.
Something like "reset dma_addr per RX descriptor" or "keep descriptor
high address bits on release" would be more specific.

> With MTK_36BIT_DMA and DRAM above 4 GiB the hardware then DMAs the next
> frame to an address that is neither buffer, and the driver passes the
> stale contents of the untouched buffer up the stack.

Could the consequence be stated more directly? Before this patch, a
stale dma_addr from an earlier iteration reaches release_desc in
mtk_poll_rx() and goes through:

    addr64 = RX_DMA_PREP_ADDR64(dma_addr);
    ...
    rxd->rxd2 = RX_DMA_PREP_PLEN0(ring->buf_size) | addr64;

RX_DMA_PREP_ADDR64() fills RX_DMA_ADDR64_MASK, which holds physical
address bits 35:32. These bits are combined with the current buffer's
low bits in rxd1. The device then writes received frames to the same
offset in a different 4 GiB window. That is silent DMA corruption of
memory the driver does not own.

The mismatched address is also passed to dma_unmap_single() later. One
caller is mtk_poll_rx() itself, on the non-page_pool path:

    if (MTK_HAS_CAPS(eth->soc->caps, MTK_36BIT_DMA))
        addr64 = RX_DMA_GET_ADDR64(trxd.rxd2);

    dma_unmap_single(eth->dma_dev, ((u64)trxd.rxd1 | addr64),
                     ring->buf_size, DMA_FROM_DEVICE);

The other is mtk_rx_clean() during SER.

Would it be worth calling this memory corruption in the message, so
stable and security triage can judge how urgent it is?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/patch-4b4c5003bcf028fafd094e719c5e910b%40loljews.com

[PATCH net-next v6 2/3] net: ethernet: mtk_eth_soc: skip page_pool

[Sandeep Haemoon]

The page_pool is created with order-0 pages and MTK_PP_MAX_BUF_SIZE as
the maximum DMA length, so it can only back a ring whose descriptor
length fits in that.

Gate the pool on ring->buf_size <= MTK_PP_MAX_BUF_SIZE and let larger
rings use the netdev_alloc_frag()/mtk_max_buf_alloc() path that already
exists.  ring->frag_size cannot serve as the limit: it follows the
netdev_alloc_frag() layout, whose headroom and skb_shared_info footprint
differ from the pool's, so a ring can have frag_size <= PAGE_SIZE while
its descriptor length is already larger than the pool can map.

With the current MTU limits every ring fits and the gate always passes;
it becomes active with the 9K jumbo support that follows, when the
shared rings grow past the limit.

Link: https://github.com/mediatek/mtk-openwrt-feeds
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Sandeep Haemoon <sandeeph@loljews.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6c8b039fad..16c3e41570 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2824,7 +2824,13 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
 	if (!ring->data)
 		return -ENOMEM;
 
-	if (mtk_page_pool_enabled(eth)) {
+	/* A page_pool hands out order-0 pages and is created with
+	 * MTK_PP_MAX_BUF_SIZE, so it can only back a ring whose descriptor
+	 * length fits.  Larger rings use the netdev_alloc_frag()/
+	 * mtk_max_buf_alloc() path below instead.
+	 */
+	if (mtk_page_pool_enabled(eth) &&
+	    ring->buf_size <= MTK_PP_MAX_BUF_SIZE) {
 		struct page_pool *pp;
 
 		pp = mtk_create_page_pool(eth, &ring->xdp_q, ring_no,
-- 
2.53.0

Re: [PATCH net-next v6 2/3] net: ethernet: mtk_eth_soc: skip page_pool

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 (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] mtk_rx_alloc() (drivers/net/ethernet/mediatek/mtk_eth_soc.c)
  now creates the page_pool only when `ring->buf_size <=…
- [Low] The new comment in mtk_rx_alloc() and this commit's message both
  name `mtk_max_buf_alloc()` as the fallback allocator.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  mtk_rx_alloc() leaks ring->data on two error paths.
- [Medium] This problem wasn't introduced by this patch, but
  mtk_rx_clean() calls `dma_unmap_single(eth->dma_dev, rxd1|addr64,…

--

net: ethernet: mtk_eth_soc: skip page_pool

This patch makes mtk_rx_alloc() create a page_pool only when the ring's
descriptor length (ring->buf_size) fits within MTK_PP_MAX_BUF_SIZE. Larger
rings fall back to the netdev_alloc_frag() path. With the current MTU
limits the gate always passes. It is meant to take effect with the 9K
jumbo support that follows.

Link: https://github.com/mediatek/mtk-openwrt-feeds

> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 6c8b039fad415..16c3e4157085b 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2824,7 +2824,13 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
>  	if (!ring->data)
>  		return -ENOMEM;
>  
> -	if (mtk_page_pool_enabled(eth)) {
> +	/* A page_pool hands out order-0 pages and is created with
> +	 * MTK_PP_MAX_BUF_SIZE, so it can only back a ring whose descriptor
> +	 * length fits.  Larger rings use the netdev_alloc_frag()/
> +	 * mtk_max_buf_alloc() path below instead.
> +	 */

[Severity: Low]
This isn't a bug, but mtk_max_buf_alloc() doesn't exist yet at this commit.
The fallback below still calls the LRO-sized allocator:

			if (ring->frag_size <= PAGE_SIZE)
				data = netdev_alloc_frag(ring->frag_size);
			else
				data = mtk_max_lro_buf_alloc(GFP_KERNEL);

The commit message has the same forward reference:

    Gate the pool on ring->buf_size <= MTK_PP_MAX_BUF_SIZE and let larger
    rings use the netdev_alloc_frag()/mtk_max_buf_alloc() path that already
    exists.

mtk_max_buf_alloc() comes from the next patch, "net: ethernet:
mtk_eth_soc: add 9K jumbo". That patch renames mtk_max_lro_buf_alloc() and
sizes the buffer from ring->frag_size.

Could this patch name mtk_max_lro_buf_alloc() instead, or leave the
reference to the later patch?

> +	if (mtk_page_pool_enabled(eth) &&
> +	    ring->buf_size <= MTK_PP_MAX_BUF_SIZE) {

[Severity: Medium]
When this condition is false on a netsys v2+ SoC, ring->page_pool stays
NULL and ring->xdp_q is never registered. XDP only runs inside the
page_pool branch of mtk_poll_rx():

mtk_poll_rx() {
    ...
		if (ring->page_pool) {
			...
			ret = mtk_xdp_run(eth, ring, &xdp, netdev);
			...
		} else {
			...
}

At this commit, the XDP admission checks still compare only the per-device
MTU against MTK_PP_MAX_BUF_SIZE:

mtk_xdp_setup() {
    ...
	if (dev->mtu > MTK_PP_MAX_BUF_SIZE) {
    ...
}

mtk_change_mtu() {
	int length = new_mtu + MTK_RX_ETH_HLEN;
    ...
	if (rcu_access_pointer(eth->prog) &&
	    length > MTK_PP_MAX_BUF_SIZE) {
    ...
}

Once the shared rings can grow past MTK_PP_MAX_BUF_SIZE, could an XDP
program attach successfully and then never run? Every packet would then be
handled as if the program returned XDP_PASS.

This can't trigger at this commit. Non-LRO rings have a buf_size of about
2110, and HW-LRO rings only exist on netsys v1, where the page_pool is
disabled.

The next patch, "net: ethernet: mtk_eth_soc: add 9K jumbo", changes
mtk_xdp_setup() and mtk_change_mtu() to check the shared rx_buf_len. That
appears to make the admission checks match this gate.

Would it be cleaner to make the admission checks use the same length as
this gate in this patch, so each step of the series stays consistent?

>  		struct page_pool *pp;
>  
>  		pp = mtk_create_page_pool(eth, &ring->xdp_q, ring_no,

[Severity: Medium]
This isn't a bug introduced by this patch, but does mtk_rx_alloc() leak
ring->data on the error paths right after this?

		if (IS_ERR(pp))
			return PTR_ERR(pp);
	...
	ring->dma = mtk_dma_ring_alloc(eth,
				       rx_dma_size * eth->soc->rx.desc_size,
				       &ring->phys,
				       rx_flag == MTK_RX_FLAGS_NORMAL);
	if (!ring->dma)
		return -ENOMEM;

Both paths return with ring->data allocated and ring->dma still NULL. The
cleanup through mtk_start_dma()->mtk_dma_free()->mtk_rx_clean() only frees
the array when both pointers are set:

mtk_rx_clean() {
	...
	if (ring->data && ring->dma) {
		...
		kfree(ring->data);
		ring->data = NULL;
	}
	...
}

The next mtk_rx_alloc() then overwrites ring->data. So each time ndo_open
fails at either step, the old array of rx_dma_size pointers is lost.

[Severity: Medium]
This is a pre-existing issue, but now that the page_pool decision is made
per ring, should mtk_rx_clean() follow it too? It unmaps every live RX
buffer unconditionally:

mtk_rx_clean() {
	...
			dma_unmap_single(eth->dma_dev, ((u64)rxd->rxd1 | addr64),
					 ring->buf_size, DMA_FROM_DEVICE);
			mtk_rx_put_buff(ring, ring->data[i], false);
	...
}

For page_pool rings, the pool owns the mapping (PP_FLAG_DMA_MAP), and
rxd1 holds page_pool_get_dma_addr(page) + MTK_PP_HEADROOM from
mtk_page_pool_get_buff().

Doesn't this unmap an offset address with ring->buf_size, and then have
the pool unmap the same page again with PAGE_SIZE when it is destroyed?
With CONFIG_DMA_API_DEBUG this would be reported. With an IOMMU or swiotlb
bouncing, it could corrupt the mapping state.

mtk_rx_put_buff() already checks ring->page_pool to decide how to free the
buffer. Should dma_unmap_single() be skipped under the same condition?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/patch-4b4c5003bcf028fafd094e719c5e910b%40loljews.com

[PATCH net-next v6 3/3] net: ethernet: mtk_eth_soc: add 9K jumbo

[Sandeep Haemoon]

The driver caps max_mtu at 2030 bytes and programs the MAC max-rx
field for at most 2048-byte frames, so MTU > 2K is rejected and 9K
jumbo frames are not usable on the 10G XGMII MACs.

Add a MTK_NETSYS_RX_9K capability for MT7988 and:
- advertise a 9194-byte max_mtu on the XGMII MACs, from the DT
  phy_mode at probe time, so DSA can raise the conduit MTU before the
  netdev is opened.  The four bytes below the 9216-byte MAC and ring
  cap keep room for the 802.1Q tag of a maximum-size frame;
- derive the shared RX buffer length from the largest MTU of any MAC,
  with the 802.1Q headroom added before the 1 KiB round-up and a 2K
  minimum, and size the RX rings from it.  The QDMA RX ring is never
  polled or refilled by the CPU, so it keeps its 2K buffers;
- program the PDMA RX SDL max-frame field from that length on netsys
  v3+.  The field is written the way the MediaTek downstream driver
  does, frame length offset by MTK_PDMA_LRO_SDL in GENMASK(30, 16);
  the encoding is not publicly documented, and without this new write
  the DMA engine keeps its small reset default (~1518 bytes) and
  drops larger frames on the wire, so a datasheet confirmation of the
  layout would be welcome;
- program XMAC_RX_CFG2 from the same length on the XGMII MACs, keeping
  the bits outside the length field, so 802.1Q-tagged frames fit at
  any MTU.  GMAC1 (the built-in switch port, phy-mode internal) keeps
  its MCR programming instead: the XMAC register is not used for it,
  and the MCR max-rx field gets the jumbo field (bits 31:28, in
  1024-byte units, the encoding of the MediaTek downstream driver),
  so the same shared length can be programmed for GMAC1 as for the
  XGMII MACs.  The write runs after the XGMII setup of
  mtk_mac_config(), whose MAC_MCR_FORCE_LINK_DOWN write is a
  full-register write that would otherwise reset the field to the
  1518 bucket.  Whether the internal switch path enforces the MCR
  field for switch-side frames at all is a hardware property: the
  downstream reference programs no limit for GMAC1 and jumbo frames
  pass the switch on the reference hardware, which suggests the field
  is not enforced there;
- rewrite the MAC limits on every successful mtk_mac_config() and on
  every mtk_hw_init() call on the SoCs with the capability, so no MAC
  accepts a frame the rings cannot hold, not even after a reopen with
  a smaller MTU.  The other SoCs keep their per-device limits: they
  never grow their rings, and their MCR programming is left exactly
  as before this patch;
- reject a runtime MTU change that would need larger buffers while
  DMA is running (-EBUSY).  Shrinking is allowed: the rings keep the
  buffers they already have.  The rings are shared by every MAC, so
  raising a DSA user port MTU on a live system fails while any MAC is
  up; the runtime reallocation that lifts this is a separate series.

XDP runs only on rings that have a page_pool, and the pool cannot back
buffers larger than MTK_PP_MAX_BUF_SIZE, so an MTU that needs a larger
buffer cannot be combined with XDP; attaching a program is rejected in
that case.  The check uses eth->rx_buf_len, the length the live rings
were actually built with; when the rings are not live, or the attach
itself tears them down, the next open re-derives the length from the
current MTUs and that value is checked instead.  A program can
therefore never attach and silently not run, and an attach on a
closed device is never rejected over a stale ring length.  Detaching
is always allowed: it does not need a page_pool.

With the 1 KiB rounding, the buffer length is order-1 (8 KiB) for
rx_buf_len 4096..7168 (MTU about 3051..7146) and order-2 (16 KiB)
from rx_buf_len 8192 (MTU 7147) up: once any MAC's MTU reaches 7147,
every RX buffer is a 16 KiB compound page, which pins around 64 MiB
at the 9K ring size while DMA is up and turns the refill into a
high-order GFP_ATOMIC allocation.  skb->truesize reports the size
that was actually allocated for those buffers, not the smaller
frag_size.  That correction also covers the HW-LRO rings on mt2701,
mt7622, mt7623 and mt7629, whose ~12.4 KiB buffers now report 16 KiB.

This is a forward-port of the MediaTek downstream jumbo work: Mason
Chang's "change default rx buffer length" and "add 9k jumbo frame
support" (MediaTek OpenWrt feed 999-eth-30/31, carried in
frank-w/BPI-Router-Linux), plus Frank Wunderlich's "try to fix crash
the right way".  The runtime reallocation of Mason's "dynamic rx
buffer adjustment" (feed 999-eth-33) is not included; it will be sent
separately.

Link: https://github.com/frank-w/BPI-Router-Linux/commit/b4c6cb1d5532
Link: https://github.com/mediatek/mtk-openwrt-feeds
Signed-off-by: Sandeep Haemoon <sandeeph@loljews.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 308 ++++++++++++++++----
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  16 +-
 2 files changed, 268 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 16c3e41570..618f9a5e8f 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -170,6 +170,7 @@ static const struct mtk_reg_map mt7988_reg_map = {
 		.pcrx_ptr	= 0x6908,
 		.glo_cfg	= 0x6a04,
 		.rst_idx	= 0x6a08,
+		.rx_cfg		= 0x6a10,
 		.delay_irq	= 0x6a0c,
 		.irq_status	= 0x6a20,
 		.irq_mask	= 0x6a28,
@@ -554,6 +555,62 @@ static int mtk_mac_prepare(struct phylink_config *config, unsigned int mode,
 	return 0;
 }
 
+/* The MACs are fed from the shared RX rings, so the maximum frame they may
+ * accept is a property of the ring buffer length.  GMAC1 on netsys v3+ is
+ * the built-in switch port: it has no XMAC_RX_CFG2 limit register, so it
+ * keeps the MCR programming the other SoCs use, extended with the MCR
+ * jumbo field so its limit can grow with the rings like the XGMII MACs'.
+ */
+static void mtk_set_mcr_max_rx(struct mtk_mac *mac, u32 val)
+{
+	struct mtk_eth *eth = mac->hw;
+	u32 mcr_cur, mcr_new;
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
+		return;
+
+	if (!mtk_interface_mode_is_xgmii(eth, mac->interface) ||
+	    mac->id == MTK_GMAC1_ID) {
+		mcr_cur = mtk_r32(mac->hw, MTK_MAC_MCR(mac->id));
+		mcr_new = mcr_cur & ~MAC_MCR_MAX_RX_MASK;
+
+		if (val <= 1518)
+			mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_1518);
+		else if (val <= 1536)
+			mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_1536);
+		else if (val <= 1552)
+			mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_1552);
+		else
+			mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_2048);
+
+		/* The bucket encodes at most 2048; the SoCs with the 9K
+		 * capability need the jumbo field, in 1024-byte units, for
+		 * anything larger.  Clearing it together with the bucket
+		 * keeps a later MTU shrink from leaving a stale larger
+		 * limit behind.
+		 */
+		if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_RX_9K)) {
+			u32 jumbo = DIV_ROUND_UP(val, MTK_MAX_RX_LENGTH_UNIT);
+
+			mcr_new &= ~MAC_MCR_MAX_RX_JUMBO_MASK;
+
+			if (val > 1552)
+				mcr_new |= MAC_MCR_MAX_RX_JUMBO(jumbo);
+		}
+
+		if (mcr_new != mcr_cur)
+			mtk_w32(mac->hw, mcr_new, MTK_MAC_MCR(mac->id));
+	} else {
+		mcr_cur = mtk_r32(mac->hw, MTK_XMAC_RX_CFG2(mac->id));
+		mcr_new = mcr_cur & ~MTK_XMAC_MAX_RX_MASK;
+		mcr_new |= FIELD_PREP(MTK_XMAC_MAX_RX_MASK,
+				      min_t(u32, val, MTK_MAX_RX_LENGTH_9K));
+
+		if (mcr_new != mcr_cur)
+			mtk_w32(mac->hw, mcr_new, MTK_XMAC_RX_CFG2(mac->id));
+	}
+}
+
 static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
 			   const struct phylink_link_state *state)
 {
@@ -678,6 +735,18 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
 			mtk_setup_bridge_switch(eth);
 	}
 
+	/* On the SoCs with the 9K capability the rings grow with the MTU, so
+	 * rewrite the MAC limit on every successful mac_config, not only
+	 * when the interface mode changes: a reopen after an MTU change
+	 * must narrow the MACs that still carry the previous, larger
+	 * limit.  This must run after the XGMII setup above: its
+	 * MAC_MCR_FORCE_LINK_DOWN write is a full-register write that
+	 * would otherwise reset GMAC1's max-rx field (its limit lives in
+	 * the MCR register, not in XMAC_RX_CFG2) back to the 1518 bucket.
+	 */
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_RX_9K))
+		mtk_set_mcr_max_rx(mac, eth->rx_buf_len);
+
 	return;
 
 err_phy:
@@ -1242,22 +1311,22 @@ static void mtk_get_stats64(struct net_device *dev,
 	storage->tx_dropped = dev->stats.tx_dropped;
 }
 
-static inline int mtk_max_frag_size(int mtu)
+static inline int mtk_max_frag_size(u32 rx_buf_len, int mtu)
 {
-	/* make sure buf_size will be at least MTK_MAX_RX_LENGTH */
-	if (mtu + MTK_RX_ETH_HLEN < MTK_MAX_RX_LENGTH_2K)
-		mtu = MTK_MAX_RX_LENGTH_2K - MTK_RX_ETH_HLEN;
+	/* make sure buf_size will be at least the rx buffer length */
+	if (mtu + MTK_RX_ETH_HLEN < rx_buf_len)
+		mtu = rx_buf_len - MTK_RX_ETH_HLEN;
 
 	return SKB_DATA_ALIGN(MTK_RX_HLEN + mtu) +
 		SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 }
 
-static inline int mtk_max_buf_size(int frag_size)
+static inline int mtk_max_buf_size(u32 rx_buf_len, int frag_size)
 {
 	int buf_size = frag_size - NET_SKB_PAD - NET_IP_ALIGN -
 		       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 
-	WARN_ON(buf_size < MTK_MAX_RX_LENGTH_2K);
+	WARN_ON(buf_size < rx_buf_len);
 
 	return buf_size;
 }
@@ -1280,9 +1349,17 @@ static bool mtk_rx_get_desc(struct mtk_eth *eth, struct mtk_rx_dma_v2 *rxd,
 	return true;
 }
 
-static void *mtk_max_lro_buf_alloc(gfp_t gfp_mask)
+/* mtk_max_buf_alloc() rounds the buffer up to a whole page order, so
+ * report the size it actually allocated, not the smaller frag_size, in
+ * skb->truesize.
+ */
+static unsigned int mtk_buf_alloc_size(unsigned int size)
+{
+	return PAGE_SIZE << get_order(size);
+}
+
+static void *mtk_max_buf_alloc(unsigned int size, gfp_t gfp_mask)
 {
-	unsigned int size = mtk_max_frag_size(MTK_MAX_LRO_RX_LENGTH);
 	unsigned long data;
 
 	data = __get_free_pages(gfp_mask | __GFP_COMP | __GFP_NOWARN,
@@ -2268,6 +2345,15 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 
 		pktlen = RX_DMA_GET_PLEN0(trxd.rxd2);
 
+		/* The engine can report a length larger than the posted
+		 * buffer; never pass the unwritten remainder of the buffer
+		 * up the stack.
+		 */
+		if (unlikely(pktlen > ring->buf_size)) {
+			netdev->stats.rx_length_errors++;
+			goto release_desc;
+		}
+
 		/* alloc new buffer */
 		if (ring->page_pool) {
 			struct page *page = virt_to_head_page(data);
@@ -2313,10 +2399,15 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 				skb_metadata_set(skb, metasize);
 			skb_mark_for_recycle(skb);
 		} else {
+			unsigned int alloc_size = ring->frag_size > PAGE_SIZE ?
+				mtk_buf_alloc_size(ring->frag_size) :
+				ring->frag_size;
+
 			if (ring->frag_size <= PAGE_SIZE)
 				new_data = napi_alloc_frag(ring->frag_size);
 			else
-				new_data = mtk_max_lro_buf_alloc(GFP_ATOMIC);
+				new_data = mtk_max_buf_alloc(ring->frag_size,
+							     GFP_ATOMIC);
 
 			if (unlikely(!new_data)) {
 				netdev->stats.rx_dropped++;
@@ -2339,7 +2430,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 			dma_unmap_single(eth->dma_dev, ((u64)trxd.rxd1 | addr64),
 					 ring->buf_size, DMA_FROM_DEVICE);
 
-			skb = build_skb(data, ring->frag_size);
+			skb = build_skb(data, alloc_size);
 			if (unlikely(!skb)) {
 				netdev->stats.rx_dropped++;
 				skb_free_frag(data);
@@ -2800,6 +2891,7 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
 	const struct mtk_soc_data *soc = eth->soc;
 	struct mtk_rx_ring *ring;
 	int rx_data_len, rx_dma_size;
+	u32 buf_len;
 	int i;
 
 	if (rx_flag == MTK_RX_FLAGS_QDMA) {
@@ -2818,8 +2910,19 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
 		rx_dma_size = soc->rx.dma_size;
 	}
 
-	ring->frag_size = mtk_max_frag_size(rx_data_len);
-	ring->buf_size = mtk_max_buf_size(ring->frag_size);
+	/* The QDMA RX ring is only ever scratch memory for the engine: the
+	 * CPU neither polls nor refills it, so it keeps its 2K buffers
+	 * even when the polled rings grow.
+	 */
+	if (rx_flag == MTK_RX_FLAGS_QDMA)
+		buf_len = MTK_MAX_RX_LENGTH_2K;
+	else if (rx_flag == MTK_RX_FLAGS_HWLRO)
+		buf_len = max_t(u32, eth->rx_buf_len, MTK_MAX_LRO_RX_LENGTH);
+	else
+		buf_len = eth->rx_buf_len;
+
+	ring->frag_size = mtk_max_frag_size(buf_len, rx_data_len);
+	ring->buf_size  = mtk_max_buf_size(buf_len, ring->frag_size);
 	ring->data = kzalloc_objs(*ring->data, rx_dma_size);
 	if (!ring->data)
 		return -ENOMEM;
@@ -2863,7 +2966,8 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
 			if (ring->frag_size <= PAGE_SIZE)
 				data = netdev_alloc_frag(ring->frag_size);
 			else
-				data = mtk_max_lro_buf_alloc(GFP_KERNEL);
+				data = mtk_max_buf_alloc(ring->frag_size,
+							 GFP_KERNEL);
 
 			if (!data)
 				return -ENOMEM;
@@ -3259,6 +3363,58 @@ static int mtk_dma_busy_wait(struct mtk_eth *eth)
 	return ret;
 }
 
+static int mtk_max_gmac_mtu(struct mtk_eth *eth)
+{
+	int i, max_mtu = ETH_DATA_LEN;
+
+	for (i = 0; i < ARRAY_SIZE(eth->netdev); i++)
+		if (eth->netdev[i] && eth->netdev[i]->mtu > max_mtu)
+			max_mtu = eth->netdev[i]->mtu;
+
+	return max_mtu;
+}
+
+static u32 mtk_rx_buf_len_from_mtu(int max_mtu)
+{
+	int length = max_mtu + MTK_RX_ETH_HLEN + VLAN_HLEN;
+
+	if (length <= MTK_MAX_RX_LENGTH_2K)
+		return MTK_MAX_RX_LENGTH_2K;
+
+	return DIV_ROUND_UP(length, 1024) * 1024;
+}
+
+/* The rx rings are shared by every MAC, so their buffers are sized for
+ * the largest MTU configured on any of them.
+ */
+static u32 mtk_rx_buf_len(struct mtk_eth *eth)
+{
+	return mtk_rx_buf_len_from_mtu(mtk_max_gmac_mtu(eth));
+}
+
+/* Program the PDMA RX max-frame (SDL) field from the shared RX buffer
+ * length.  The field only exists on netsys v3+ and is programmed the way
+ * the MediaTek downstream driver does: frame length offset by
+ * MTK_PDMA_LRO_SDL in GENMASK(30, 16).  The encoding is the vendor's and
+ * not publicly documented; without the write the DMA engine keeps its
+ * small reset default (~1518 bytes) and drops larger frames on the wire,
+ * so a datasheet confirmation of the layout would be welcome.
+ */
+static void mtk_pdma_set_sdl(struct mtk_eth *eth)
+{
+	const struct mtk_reg_map *reg_map = eth->soc->reg_map;
+	u32 val;
+
+	if (!mtk_is_netsys_v3_or_greater(eth))
+		return;
+
+	val = mtk_r32(eth, reg_map->pdma.rx_cfg);
+	val &= ~MTK_RX_CFG_SDL_MASK;
+	val |= FIELD_PREP(MTK_RX_CFG_SDL_MASK,
+			  MTK_PDMA_LRO_SDL + eth->rx_buf_len);
+	mtk_w32(eth, val, reg_map->pdma.rx_cfg);
+}
+
 static int mtk_dma_init(struct mtk_eth *eth)
 {
 	int err;
@@ -3267,6 +3423,14 @@ static int mtk_dma_init(struct mtk_eth *eth)
 	if (mtk_dma_busy_wait(eth))
 		return -EBUSY;
 
+	/* Derived here, at ring allocation, so eth->rx_buf_len always
+	 * describes the buffers the rings actually hold; mtk_change_mtu()
+	 * updates it while DMA is down so it describes the rings-to-be,
+	 * and mtk_mac_config() then never lets a MAC accept a frame the
+	 * rings cannot take.
+	 */
+	eth->rx_buf_len = mtk_rx_buf_len(eth);
+
 	if (MTK_HAS_CAPS(eth->soc->caps, MTK_QDMA)) {
 		/* QDMA needs scratch memory for internal reordering of the
 		 * descriptors
@@ -3310,6 +3474,8 @@ static int mtk_dma_init(struct mtk_eth *eth)
 		mtk_w32(eth, 0x0, eth->soc->reg_map->qdma.hred);
 	}
 
+	mtk_pdma_set_sdl(eth);
+
 	return 0;
 }
 
@@ -3600,17 +3766,6 @@ static int mtk_device_event(struct notifier_block *n, unsigned long event, void
 	return NOTIFY_DONE;
 }
 
-static int mtk_max_gmac_mtu(struct mtk_eth *eth)
-{
-	int i, max_mtu = ETH_DATA_LEN;
-
-	for (i = 0; i < ARRAY_SIZE(eth->netdev); i++)
-		if (eth->netdev[i] && eth->netdev[i]->mtu > max_mtu)
-			max_mtu = eth->netdev[i]->mtu;
-
-	return max_mtu;
-}
-
 static int mtk_open(struct net_device *dev)
 {
 	struct mtk_mac *mac = netdev_priv(dev);
@@ -3780,6 +3935,7 @@ static int mtk_xdp_setup(struct net_device *dev, struct bpf_prog *prog,
 	struct mtk_mac *mac = netdev_priv(dev);
 	struct mtk_eth *eth = mac->hw;
 	struct bpf_prog *old_prog;
+	u32 rx_buf_len;
 	bool need_update;
 
 	if (eth->hwlro) {
@@ -3787,12 +3943,28 @@ static int mtk_xdp_setup(struct net_device *dev, struct bpf_prog *prog,
 		return -EOPNOTSUPP;
 	}
 
-	if (dev->mtu > MTK_PP_MAX_BUF_SIZE) {
+	need_update = !!eth->prog != !!prog;
+
+	/* XDP only runs on rings that have a page_pool, and the pool is
+	 * only created when the shared RX buffer length fits
+	 * MTK_PP_MAX_BUF_SIZE.  eth->rx_buf_len describes the live rings;
+	 * when the rings are not live, or this attach tears them down,
+	 * the next open re-derives it from the current MTUs, so check
+	 * that value instead.  A detach never needs a pool, so it is not
+	 * checked at all.
+	 */
+	rx_buf_len = eth->rx_buf_len;
+	if (prog &&
+	    (!refcount_read(&eth->dma_refcnt) ||
+	     (netif_running(dev) && need_update &&
+	      refcount_read(&eth->dma_refcnt) == 1)))
+		rx_buf_len = mtk_rx_buf_len(eth);
+
+	if (prog && rx_buf_len > MTK_PP_MAX_BUF_SIZE) {
 		NL_SET_ERR_MSG_MOD(extack, "MTU too large for XDP");
 		return -EOPNOTSUPP;
 	}
 
-	need_update = !!eth->prog != !!prog;
 	if (netif_running(dev) && need_update)
 		mtk_stop(dev);
 
@@ -3927,30 +4099,6 @@ static void mtk_dim_tx(struct work_struct *work)
 	dim->state = DIM_START_MEASURE;
 }
 
-static void mtk_set_mcr_max_rx(struct mtk_mac *mac, u32 val)
-{
-	struct mtk_eth *eth = mac->hw;
-	u32 mcr_cur, mcr_new;
-
-	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
-		return;
-
-	mcr_cur = mtk_r32(mac->hw, MTK_MAC_MCR(mac->id));
-	mcr_new = mcr_cur & ~MAC_MCR_MAX_RX_MASK;
-
-	if (val <= 1518)
-		mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_1518);
-	else if (val <= 1536)
-		mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_1536);
-	else if (val <= 1552)
-		mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_1552);
-	else
-		mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_2048);
-
-	if (mcr_new != mcr_cur)
-		mtk_w32(mac->hw, mcr_new, MTK_MAC_MCR(mac->id));
-}
-
 static void mtk_hw_reset(struct mtk_eth *eth)
 {
 	u32 val;
@@ -4218,8 +4366,11 @@ static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 			continue;
 
 		mtk_w32(eth, MAC_MCR_FORCE_LINK_DOWN, MTK_MAC_MCR(i));
-		mtk_set_mcr_max_rx(netdev_priv(dev),
-				   dev->mtu + MTK_RX_ETH_HLEN);
+		if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_RX_9K))
+			mtk_set_mcr_max_rx(netdev_priv(dev), eth->rx_buf_len);
+		else
+			mtk_set_mcr_max_rx(netdev_priv(dev),
+					   dev->mtu + MTK_RX_ETH_HLEN);
 	}
 
 	/* Indicates CDM to parse the MTK special tag from CPU
@@ -4360,21 +4511,59 @@ static int mtk_change_mtu(struct net_device *dev, int new_mtu)
 	int length = new_mtu + MTK_RX_ETH_HLEN;
 	struct mtk_mac *mac = netdev_priv(dev);
 	struct mtk_eth *eth = mac->hw;
+	u32 rx_buf_len;
+	bool running;
 	int max_mtu, i;
 
+	/* The RX rings are shared by every MAC, so the buffer length
+	 * follows the largest MTU of all of them, the new one included.
+	 */
+	max_mtu = new_mtu;
+	for (i = 0; i < MTK_MAX_DEVS; i++) {
+		if (eth->netdev[i] && eth->netdev[i] != dev &&
+		    eth->netdev[i]->mtu > max_mtu)
+			max_mtu = eth->netdev[i]->mtu;
+	}
+	rx_buf_len = mtk_rx_buf_len_from_mtu(max_mtu);
+
 	if (rcu_access_pointer(eth->prog) &&
-	    length > MTK_PP_MAX_BUF_SIZE) {
+	    rx_buf_len > MTK_PP_MAX_BUF_SIZE) {
 		netdev_err(dev, "Invalid MTU for XDP mode\n");
 		return -EINVAL;
 	}
 
-	mtk_set_mcr_max_rx(mac, length);
+	/* The rings cannot be resized in place while DMA is running: a
+	 * larger buffer can only come from the reallocation at the next
+	 * open, so reject it.  A smaller one is fine, the buffers the
+	 * rings already hold are big enough for it.
+	 */
+	running = refcount_read(&eth->dma_refcnt);
+	if (running && rx_buf_len > eth->rx_buf_len)
+		return -EBUSY;
+
+	/* On the SoCs whose rings grow with the MTU, keep the MAC limit in
+	 * step with the shared buffers; the other SoCs keep the per-device
+	 * limit they always had.
+	 */
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_RX_9K))
+		mtk_set_mcr_max_rx(mac, rx_buf_len);
+	else
+		mtk_set_mcr_max_rx(mac, length);
+
 	WRITE_ONCE(dev->mtu, new_mtu);
 
 	max_mtu = mtk_max_gmac_mtu(eth);
 	for (i = 0; i < ARRAY_SIZE(eth->ppe); i++)
 		mtk_ppe_update_mtu(eth->ppe[i], max_mtu);
 
+	/* While DMA is up the rings keep the buffers they were built
+	 * with; update eth->rx_buf_len only when DMA is down so it keeps
+	 * describing the buffers the rings hold, or will hold at the next
+	 * open.
+	 */
+	if (!running)
+		eth->rx_buf_len = rx_buf_len;
+
 	return 0;
 }
 
@@ -5018,8 +5207,17 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	eth->netdev[id]->irq = eth->irq[MTK_FE_IRQ_SHARED];
 	eth->netdev[id]->dev.of_node = np;
 
+	/* DSA sets the conduit MTU (user MTU + tag) before the netdev is
+	 * opened, so max_mtu must already be final here.  Deriving it only
+	 * in mtk_open() leaves the conduit at its 1504 default and blocks
+	 * 9K frames on the DSA user ports.
+	 */
 	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
 		eth->netdev[id]->max_mtu = MTK_MAX_RX_LENGTH - MTK_RX_ETH_HLEN;
+	else if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_RX_9K) &&
+		 mtk_interface_mode_is_xgmii(eth, phy_mode))
+		eth->netdev[id]->max_mtu =
+			MTK_MAX_RX_LENGTH_9K - MTK_RX_ETH_HLEN - VLAN_HLEN;
 	else
 		eth->netdev[id]->max_mtu = MTK_MAX_RX_LENGTH_2K - MTK_RX_ETH_HLEN;
 
@@ -5160,6 +5358,8 @@ static int mtk_probe(struct platform_device *pdev)
 	spin_lock_init(&eth->rx_irq_lock);
 	spin_lock_init(&eth->dim_lock);
 
+	eth->rx_buf_len = MTK_MAX_RX_LENGTH_2K;
+
 	eth->rx_dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
 	INIT_WORK(&eth->rx_dim.work, mtk_dim_rx);
 	INIT_DELAYED_WORK(&eth->reset.monitor_work, mtk_hw_reset_monitor_work);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 88a9b3b23b..4b4a544612 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -28,7 +28,9 @@
 #define MTK_QDMA_NUM_QUEUES	16
 #define MTK_QDMA_PAGE_SIZE	2048
 #define MTK_MAX_RX_LENGTH	1536
+#define MTK_MAX_RX_LENGTH_UNIT	1024
 #define MTK_MAX_RX_LENGTH_2K	2048
+#define MTK_MAX_RX_LENGTH_9K	9216
 #define MTK_TX_DMA_BUF_LEN	0x3fff
 #define MTK_TX_DMA_BUF_LEN_V2	0xffff
 #define MTK_QDMA_RING_SIZE	2048
@@ -198,9 +200,10 @@
 #define MTK_MULTI_EN		BIT(10)
 #define MTK_PDMA_SIZE_8DWORDS	(1 << 4)
 
-/* PDMA Global Configuration Register */
+/* PDMA RX Configuration Register */
 #define MTK_PDMA_LRO_SDL	0x3000
 #define MTK_RX_CFG_SDL_OFFSET	16
+#define MTK_RX_CFG_SDL_MASK	GENMASK(30, MTK_RX_CFG_SDL_OFFSET)
 
 /* PDMA Reset Index Register */
 #define MTK_PST_DRX_IDX0	BIT(16)
@@ -451,6 +454,9 @@
 
 /* Mac control registers */
 #define MTK_MAC_MCR(x)		(0x10100 + (x * 0x100))
+#define MAC_MCR_MAX_RX_JUMBO_MASK	GENMASK(31, 28)
+#define MAC_MCR_MAX_RX_JUMBO(x)		\
+	FIELD_PREP(MAC_MCR_MAX_RX_JUMBO_MASK, (x))
 #define MAC_MCR_MAX_RX_MASK	GENMASK(25, 24)
 #define MAC_MCR_MAX_RX(_x)	(MAC_MCR_MAX_RX_MASK & ((_x) << 24))
 #define MAC_MCR_MAX_RX_1518	0x0
@@ -530,6 +536,8 @@
 /* XFI Mac control registers */
 #define MTK_XMAC_BASE(x)	(0x12000 + (((x) - 1) * 0x1000))
 #define MTK_XMAC_MCR(x)		(MTK_XMAC_BASE(x))
+#define MTK_XMAC_RX_CFG2(x)	(MTK_XMAC_MCR(x) + 0xd0)
+#define MTK_XMAC_MAX_RX_MASK	GENMASK(13, 0)
 #define XMAC_MCR_TRX_DISABLE	0xf
 #define XMAC_MCR_FORCE_TX_FC	BIT(5)
 #define XMAC_MCR_FORCE_RX_FC	BIT(4)
@@ -985,6 +993,7 @@ enum mkt_eth_capabilities {
 	MTK_INFRA_BIT,
 	MTK_SHARED_SGMII_BIT,
 	MTK_HWLRO_BIT,
+	MTK_NETSYS_RX_9K_BIT,
 	MTK_SHARED_INT_BIT,
 	MTK_TRGMII_MT7621_CLK_BIT,
 	MTK_QDMA_BIT,
@@ -1026,6 +1035,7 @@ enum mkt_eth_capabilities {
 #define MTK_INFRA		BIT_ULL(MTK_INFRA_BIT)
 #define MTK_SHARED_SGMII	BIT_ULL(MTK_SHARED_SGMII_BIT)
 #define MTK_HWLRO		BIT_ULL(MTK_HWLRO_BIT)
+#define MTK_NETSYS_RX_9K	BIT_ULL(MTK_NETSYS_RX_9K_BIT)
 #define MTK_SHARED_INT		BIT_ULL(MTK_SHARED_INT_BIT)
 #define MTK_TRGMII_MT7621_CLK	BIT_ULL(MTK_TRGMII_MT7621_CLK_BIT)
 #define MTK_QDMA		BIT_ULL(MTK_QDMA_BIT)
@@ -1127,7 +1137,7 @@ enum mkt_eth_capabilities {
 
 #define MT7988_CAPS  (MTK_36BIT_DMA | MTK_GDM1_ESW | MTK_GMAC2_2P5GPHY | \
 		      MTK_MUX_GMAC2_TO_2P5GPHY | MTK_QDMA | MTK_RSTCTRL_PPE1 | \
-		      MTK_RSTCTRL_PPE2 | MTK_SRAM)
+		      MTK_RSTCTRL_PPE2 | MTK_SRAM | MTK_NETSYS_RX_9K)
 
 struct mtk_tx_dma_desc_info {
 	dma_addr_t	addr;
@@ -1150,6 +1160,7 @@ struct mtk_reg_map {
 		u32	pcrx_ptr;	/* rx cpu pointer */
 		u32	glo_cfg;	/* global configuration */
 		u32	rst_idx;	/* reset index */
+		u32	rx_cfg;		/* rx dma configuration */
 		u32	delay_irq;	/* delay interrupt */
 		u32	irq_status;	/* interrupt status */
 		u32	irq_mask;	/* interrupt mask */
@@ -1314,6 +1325,7 @@ struct mtk_eth {
 	struct mtk_tx_ring		tx_ring;
 	struct mtk_rx_ring		rx_ring[MTK_MAX_RX_RING_NUM];
 	struct mtk_rx_ring		rx_ring_qdma;
+	u32				rx_buf_len;
 	struct napi_struct		tx_napi;
 	struct napi_struct		rx_napi;
 	void				*scratch_ring;
-- 
2.53.0

Re: [PATCH net-next v6 3/3] net: ethernet: mtk_eth_soc: add 9K jumbo

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 4 · 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] SoCs without MTK_NETSYS_RX_9K regress, and the code
  contradicts the commit message ("The other SoCs keep their per-device…
- [Medium] The patch adds a new drop path in mtk_poll_rx() for every SoC
  and every ring: `if (unlikely(pktlen > ring->buf_size)) {…
- [Medium] On every netsys v3+ DMA init, including the default MTU with
  2K rings, mtk_pdma_set_sdl() writes FIELD_PREP(GENMASK(30,16),…
- [Medium] net: mediatek: mismatched XDP buffer size check in
  mtk_xdp_setup()
- [Low] The commit message overstates the memory cost of 9K buffers.

--

net: ethernet: mtk_eth_soc: add 9K jumbo

This adds a MTK_NETSYS_RX_9K capability for MT7988. It raises max_mtu on
the XGMII MACs to 9194 and sizes the shared RX rings from the largest MTU
of any MAC. It also programs the PDMA RX SDL field, XMAC_RX_CFG2 and the
MCR jumbo field from the shared buffer length, and rejects MTU changes
that need larger buffers while DMA is running.

Link: https://github.com/frank-w/BPI-Router-Linux/commit/b4c6cb1d5532
Link: https://github.com/mediatek/mtk-openwrt-feeds

> With the 1 KiB rounding, the buffer length is order-1 (8 KiB) for
> rx_buf_len 4096..7168 (MTU about 3051..7146) and order-2 (16 KiB)
> from rx_buf_len 8192 (MTU 7147) up: once any MAC's MTU reaches 7147,
> every RX buffer is a 16 KiB compound page, which pins around 64 MiB
> at the 9K ring size while DMA is up and turns the refill into a
> high-order GFP_ATOMIC allocation.

[Severity: Low]
Is the 64 MiB figure right for MT7988?

mt7988_data has rx.dma_size = MTK_DMA_SIZE(2K), which is 2048
descriptors. MT7988_CAPS does not include MTK_HWLRO, so eth->hwlro is
false. That means mtk_dma_init() allocates only rx_ring[0] as a
CPU-polled ring, and rx_ring_qdma keeps its 2K buffers.

At MTU 9194 this works out to 2048 x 16 KiB = 32 MiB, not 64 MiB.

> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 16c3e4157085b..618f9a5e8fcec 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c

[ ... ]

> @@ -2268,6 +2345,15 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
>  
>  		pktlen = RX_DMA_GET_PLEN0(trxd.rxd2);
>  
> +		/* The engine can report a length larger than the posted
> +		 * buffer; never pass the unwritten remainder of the buffer
> +		 * up the stack.
> +		 */
> +		if (unlikely(pktlen > ring->buf_size)) {
> +			netdev->stats.rx_length_errors++;
> +			goto release_desc;
> +		}

[Severity: Medium]
Will these drops show up in any exported counter? mtk_get_stats64()
implements ndo_get_stats64 and fills rx_length_errors only from the
hardware MIB counters:

drivers/net/ethernet/mediatek/mtk_eth_soc.c:mtk_get_stats64() {
    ...
		storage->rx_length_errors = hw_stats->rx_short_errors +
			hw_stats->rx_long_errors;
    ...
	storage->tx_errors = dev->stats.tx_errors;
	storage->rx_dropped = dev->stats.rx_dropped;
	storage->tx_dropped = dev->stats.tx_dropped;
}

dev->stats.rx_length_errors is never merged in. dev_get_stats() also
does not fold dev->stats in when ndo_get_stats64 is provided. So every
frame dropped here looks like it disappears from all exported
statistics.

Could this count into rx_dropped instead, or could mtk_get_stats64()
merge dev->stats.rx_length_errors?

This check also applies to every SoC and every ring, and the commit
message doesn't mention it. Before this patch, an oversized pktlen would
have gone to skb_put() past the build_skb() tailroom and hit
skb_over_panic(). Now the frame is dropped silently.

The comment says the engine can report such lengths. Would this be
better as a separate fix with a Fixes: tag?

[ ... ]

> @@ -3259,6 +3363,58 @@ static int mtk_dma_busy_wait(struct mtk_eth *eth)

[ ... ]

> +static u32 mtk_rx_buf_len_from_mtu(int max_mtu)
> +{
> +	int length = max_mtu + MTK_RX_ETH_HLEN + VLAN_HLEN;
> +
> +	if (length <= MTK_MAX_RX_LENGTH_2K)
> +		return MTK_MAX_RX_LENGTH_2K;
> +
> +	return DIV_ROUND_UP(length, 1024) * 1024;
> +}

[ ... ]

> +static void mtk_pdma_set_sdl(struct mtk_eth *eth)
> +{
> +	const struct mtk_reg_map *reg_map = eth->soc->reg_map;
> +	u32 val;
> +
> +	if (!mtk_is_netsys_v3_or_greater(eth))
> +		return;
> +
> +	val = mtk_r32(eth, reg_map->pdma.rx_cfg);
> +	val &= ~MTK_RX_CFG_SDL_MASK;
> +	val |= FIELD_PREP(MTK_RX_CFG_SDL_MASK,
> +			  MTK_PDMA_LRO_SDL + eth->rx_buf_len);
> +	mtk_w32(eth, val, reg_map->pdma.rx_cfg);
> +}

[Severity: Medium]
Does this set the PDMA limit well above the RX buffer length?
MTK_PDMA_LRO_SDL is 0x3000. With the default 2K rings this writes
12288 + 2048 = 14336, and with 9K rings it writes 12288 + 9216 = 21504.
Both values are larger than ring->buf_size and larger than
MTK_MAX_RX_LENGTH_9K.

mtk_dma_init() calls this on every netsys v3+ open, including at the
default MTU. It therefore replaces the ~1518-byte reset default that,
per the commit message, used to drop larger frames at the DMA engine.

After that, two bounds are left: the MAC limits and the descriptor
PLEN0. The commit message says GMAC1's MCR field is probably not
enforced for switch-side frames.

If the engine doesn't strictly honour PLEN0, could a frame be DMA'd
past the end of an order-0 napi_alloc_frag() or netdev_alloc_frag()
buffer and into a neighbouring fragment?

If the engine instead chains oversized frames across descriptors,
mtk_poll_rx() doesn't check LS0 on non-MT7628 descriptors, so each
piece would be delivered as a separate packet. The new
pktlen > ring->buf_size check in mtk_poll_rx() only runs after the DMA
has completed.

The GENMASK(30, 16) width of MTK_RX_CFG_SDL_MASK in mtk_eth_soc.h is
also undocumented. If the real field is narrower, values of 0x4000 and
above would set bits outside it.

Would it be safer to program the SDL from the actual descriptor buffer
length (ring->buf_size), and to confirm the encoding against the
datasheet?

[ ... ]

> @@ -4360,21 +4511,59 @@ static int mtk_change_mtu(struct net_device *dev, int new_mtu)

[ ... ]

> +	rx_buf_len = mtk_rx_buf_len_from_mtu(max_mtu);

[ ... ]

> +	running = refcount_read(&eth->dma_refcnt);
> +	if (running && rx_buf_len > eth->rx_buf_len)
> +		return -EBUSY;

[Severity: Medium]
Can this regress the SoCs without MTK_NETSYS_RX_9K? The commit message
says "The other SoCs keep their per-device limits: they never grow
their rings". However, only the MAC register writes are gated on the
capability. Both the rx_buf_len derivation in mtk_rx_buf_len_from_mtu()
and this -EBUSY check run on every SoC.

mtk_add_mac() leaves those MACs at
max_mtu = MTK_MAX_RX_LENGTH_2K - MTK_RX_ETH_HLEN = 2030.
mtk_rx_buf_len_from_mtu() adds VLAN_HLEN before rounding up. Any MTU
from 2027 to 2030 therefore gives 2027 + 18 + 4 = 2049 or more, which
rounds up to 3072.

With DMA up at the default MTU, eth->rx_buf_len is 2048. Running
"ip link set ethX mtu 2030", the advertised maximum, now returns -EBUSY
on MT7621, MT7622, MT7623, MT7629, MT7981, MT7986, MT2701 and on the
non-XGMII MACs of MT7988. Before this patch it just reprogrammed the
MCR bucket.

With an MT7530/MT7531 DSA switch, a user port MTU of 2023 to 2026
raises the conduit to 2027 to 2030, and that fails the same way.

If the MTU is set while DMA is down instead, mtk_dma_init() builds
3072-byte normal rings through mtk_rx_alloc(). On these SoCs the MCR
bucket tops out at 2048, so the extra 1 KiB per buffer can never be
used.

Should the rx_buf_len growth and the -EBUSY check be gated on
MTK_NETSYS_RX_9K? Alternatively, should VLAN_HLEN be kept from pushing
the length past the 2K bucket on those SoCs?

[ ... ]

> @@ -3787,12 +3943,28 @@ static int mtk_xdp_setup(struct net_device *dev, struct bpf_prog *prog,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	if (dev->mtu > MTK_PP_MAX_BUF_SIZE) {
> +	need_update = !!eth->prog != !!prog;
> +
> +	/* XDP only runs on rings that have a page_pool, and the pool is
> +	 * only created when the shared RX buffer length fits
> +	 * MTK_PP_MAX_BUF_SIZE.  eth->rx_buf_len describes the live rings;
> +	 * when the rings are not live, or this attach tears them down,
> +	 * the next open re-derives it from the current MTUs, so check
> +	 * that value instead.  A detach never needs a pool, so it is not
> +	 * checked at all.
> +	 */
> +	rx_buf_len = eth->rx_buf_len;
> +	if (prog &&
> +	    (!refcount_read(&eth->dma_refcnt) ||
> +	     (netif_running(dev) && need_update &&
> +	      refcount_read(&eth->dma_refcnt) == 1)))
> +		rx_buf_len = mtk_rx_buf_len(eth);
> +
> +	if (prog && rx_buf_len > MTK_PP_MAX_BUF_SIZE) {
>  		NL_SET_ERR_MSG_MOD(extack, "MTU too large for XDP");
>  		return -EOPNOTSUPP;
>  	}
>  
> -	need_update = !!eth->prog != !!prog;
>  	if (netif_running(dev) && need_update)
>  		mtk_stop(dev);
>  

[Severity: Medium]
Can the XDP checks here and in mtk_change_mtu() disagree with the
page_pool gate in mtk_rx_alloc()?

Both checks compare the derived rx_buf_len against
MTK_PP_MAX_BUF_SIZE. mtk_rx_alloc(), however, only creates
ring->page_pool when ring->buf_size <= MTK_PP_MAX_BUF_SIZE, and
ring->buf_size is not the same value as rx_buf_len.

mtk_max_frag_size() aligns NET_SKB_PAD + NET_IP_ALIGN + buf_len up to
SMP_CACHE_BYTES. mtk_max_buf_size() then subtracts NET_SKB_PAD and
NET_IP_ALIGN again. On arm64 with 64-byte cachelines this leaves
ring->buf_size = buf_len + 62, so a buf_len of 3072 becomes a
buf_size of 3134.

MTK_PP_MAX_BUF_SIZE depends on sizeof(struct skb_shared_info), which
grows with CONFIG_MAX_SKB_FRAGS. Take 4 KiB pages and a
CONFIG_MAX_SKB_FRAGS between 41 and 44. SKB_DATA_ALIGN() of the shared
info is then 768, so MTK_PP_MAX_BUF_SIZE is 4096 - 256 - 768 = 3072.

For any MTU from 2027 to 3050, mtk_rx_buf_len_from_mtu() returns 3072.
That passes the rx_buf_len > MTK_PP_MAX_BUF_SIZE check here and in
mtk_change_mtu(). The ring is then built with buf_size 3134, so
mtk_rx_alloc() skips the page_pool and mtk_poll_rx() takes the
non-pool path. The program attaches successfully but never sees a
packet.

The default MAX_SKB_FRAGS of 17 doesn't hit this. But the commit
message says "A program can therefore never attach and silently not
run". That guarantee currently depends on the cacheline and
skb_shared_info rounding happening to line up.

Would it be more robust to check the buf_size the ring will actually
be built with? For example, the mtk_max_frag_size() and
mtk_max_buf_size() computation could be factored into one helper used
by mtk_xdp_setup(), mtk_change_mtu() and mtk_rx_alloc(), so the three
cannot drift apart.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/patch-4b4c5003bcf028fafd094e719c5e910b%40loljews.com