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

[PATCH net-next v4 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 9K 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,
sizes the RX rings from it and keeps the MAC max-rx limits in sync with
it.  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 bucket programming (the largest the field
encodes is 2048), 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.

Once any MAC's MTU needs more than ~3.5K, every RX buffer becomes a
16 KiB compound page; with the 9K ring size that pins around 64 MiB
while DMA is up and makes the refill a high-order GFP_ATOMIC
allocation.  That cost is inherent to the downstream design and is
called out in the commit message; skb->truesize now reports the
allocation that was actually made.

Changes in v4 (both v3 reviews, in full):

- XDP admission (High): the attach check now uses eth->rx_buf_len, the
  length the live rings were actually built with.  It is recomputed
  from the current MTUs whenever the rings are not live, or when the
  attach itself tears them down, so a program can neither attach while
  the rings have no page_pool nor be rejected on a closed device over a
  stale ring length; the dead dev->mtu clause in the same check is
  removed.
- the MAC max-rx rewrite in mtk_mac_config() and the new value in
  mtk_hw_init() are gated on MTK_NETSYS_RX_9K, so the other SoCs keep
  their per-device MCR buckets exactly as before this series.
- the PDMA RX SDL write is described as the new register write it is
  (the message no longer claims it replaces OR-ing).  The field layout
  (bits 16-30, frame length offset by MTK_PDMA_LRO_SDL) is the MediaTek
  downstream layout, validated on MT7988 hardware; the write is
  read-modify-write so every other RX_CFG bit keeps its value, and it
  now happens only in mtk_dma_init(), which runs at every DMA start.
- GMAC1 (the internal switch port) keeps its MCR bucket programming
  (largest encodable bucket is 2048); XMAC_RX_CFG2 is not used for it,
  matching the downstream reference.  The write in mtk_mac_config() now
  runs after the XGMII setup, whose MAC_MCR_FORCE_LINK_DOWN write is a
  full-register write that previously reset GMAC1's max-rx field back
  to the 1518 bucket on every mac_config().  Whether the internal path
  enforces the MCR field for switch-side 9K frames is a hardware
  property and is documented in the commit message: the downstream
  reference programs no limit for GMAC1 and jumbo frames pass the
  switch on the reference hardware, but if the field is enforced,
  switch-side frames above 2048 bytes are dropped and no driver write
  can lift that.
- XMAC_RX_CFG2 is always programmed from the shared buffer length
  (2K minimum), which includes 802.1Q headroom at every MTU, so the
  limit is the same value no matter which path ran last.
- skb->truesize reports the size of the compound page that was actually
  allocated, not the smaller frag_size.
- the -EBUSY rule's consequence for DSA (a user port MTU can only be
  raised with every MAC down) is now spelled out in the commit message.
- the stale comments above the pool gate and in mtk_dma_init() are
  rewritten to describe the code as it stands.
- patch 1/3 fixes the pre-existing 36-bit DMA descriptor corruption in
  mtk_poll_rx() found by the review of patch 1; it is small, sits in
  the file this series touches, and the introducing commit was
  identified upstream, so the Fixes: tag is included.
- patch 2/3 is retitled to say when the pool is skipped and its message
  no longer implies a frag_size gate existed before it.

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-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: reset RX DMA address each poll iteration
  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 | 284 ++++++++++++++++----
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  12 +-
 2 files changed, 238 insertions(+), 58 deletions(-)

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.53.0

[PATCH net-next v4 1/3] net: ethernet: mtk_eth_soc: reset RX DMA

[Sandeep Haemoon]

mtk_poll_rx() keeps a single dma_addr_t across the whole 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) then hand the descriptor back with the high address bits of the
buffer mapped for the previous descriptor, while the low bits in rxd1
still belong to the buffer the descriptor actually holds.

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.

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 high
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 ad0fdc413..a46cf5441 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 v4 1/3] net: ethernet: mtk_eth_soc: reset RX DMA

[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 does not accurately describe the change.

--

net: ethernet: mtk_eth_soc: reset RX DMA

This patch moves the DMA_MAPPING_ERROR initialisation of dma_addr in
mtk_poll_rx() to the top of the RX loop, so it is reset on every
iteration. On MTK_36BIT_DMA SoCs, a descriptor that is released through
release_desc without a new mapping then keeps its own high address bits
in rxd2.

> net: ethernet: mtk_eth_soc: reset RX DMA
>
> [ ... ]
>
> 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.

[Severity: Low]
This isn't a bug, but could the subject line be more specific?

The patch only resets the local dma_addr variable at the top of each
iteration of the loop in mtk_poll_rx(). No RX DMA engine is reset.

In this driver, "reset" usually refers to the hardware/SER reset code
(MTK_RESETTING, mtk_hw_reset(), reset.monitor_work). Someone reading the
shortlog, or a stable backporter, could take this for a change to the
reset path.

Should the body also say that this completes commit 6e0490fc36cd ("net:
ethernet: mtk_eth_soc: fix SER panic with 4GB+ RAM")?

That commit added this check at release_desc in mtk_poll_rx():

		if (MTK_HAS_CAPS(eth->soc->caps, MTK_36BIT_DMA)) {
			if (unlikely(dma_addr == DMA_MAPPING_ERROR))
				addr64 = FIELD_GET(RX_DMA_ADDR64_MASK,
						   rxd->rxd2);
			else
				addr64 = RX_DMA_PREP_ADDR64(dma_addr);
		}

The check only works if dma_addr is DMA_MAPPING_ERROR on every release
path. Since commit e10a35abb3da ("net: ethernet: mtk_eth_soc: fix
uninitialized variable"), dma_addr is set only once, before the loop. So
the MTK_RESETTING path that 6e0490fc36cd was fixing can still pick up the
high bits of a buffer mapped for an earlier descriptor in the same poll.

mtk_rx_clean() then unmaps an address built from mismatched high and low
bits:

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

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

This is the SER panic described in 6e0490fc36cd. The body covers the
misdirected DMA and the stale data passed up the stack, but not this
unmap during SER.

Would it make sense to mention it and reference 6e0490fc36cd in the
commit message?

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

[PATCH net-next v4 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 a46cf5441..6768ab8eb 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2825,7 +2825,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

[PATCH net-next v4 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 9K 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;
- derive the shared RX buffer length from the largest MTU of any MAC,
  with a 2K minimum, and size the RX rings from it;
- program the PDMA RX SDL max-frame field from that length on netsys
  v3+.  The field stores the frame length offset by MTK_PDMA_LRO_SDL,
  the layout of the MediaTek downstream driver, and the DMA engine
  accepts frames only up to it, so without this new write frames above
  the small reset default (~1518 bytes) are dropped on the wire;
- 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 bucket programming instead: the XMAC register is not used for it,
  and the MCR field only encodes up to 2048, so that is the largest
  limit the driver can program for it.  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 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, but if it
  is, switch-side frames above 2048 bytes are dropped and no driver
  write can lift that;
- 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 buckets are 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.  Both the XDP attach and the MTU
change are 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.

Once any MAC's MTU needs more than ~3.5K, every RX buffer is a 16 KiB
compound page: with the 9K ring size that pins around 64 MiB while DMA
is up and turns the refill into a high-order GFP_ATOMIC allocation.
skb->truesize then reports the size actually allocated for those
buffers, not the smaller frag_size.

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 | 268 ++++++++++++++++----
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  12 +-
 2 files changed, 224 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6768ab8eb..527fdaeec 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -171,6 +171,7 @@ static const struct mtk_reg_map mt7988_reg_map = {
 		.glo_cfg	= 0x6a04,
 		.rst_idx	= 0x6a08,
 		.delay_irq	= 0x6a0c,
+		.rx_cfg		= 0x6a10,
 		.irq_status	= 0x6a20,
 		.irq_mask	= 0x6a28,
 		.adma_rx_dbg0	= 0x6a38,
@@ -554,6 +555,47 @@ 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 and its
+ * MCR field only encodes up to 2048, so it keeps the MCR bucket
+ * programming the other SoCs use.
+ */
+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);
+
+		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 +720,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 +1296,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(struct mtk_eth *eth, 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 shared rx buffer length */
+	if (mtu + MTK_RX_ETH_HLEN < eth->rx_buf_len)
+		mtu = eth->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(struct mtk_eth *eth, 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 < eth->rx_buf_len);
 
 	return buf_size;
 }
@@ -1280,9 +1334,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,
@@ -2313,10 +2375,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 +2406,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);
@@ -2818,8 +2885,8 @@ 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);
+	ring->frag_size = mtk_max_frag_size(eth, rx_data_len);
+	ring->buf_size = mtk_max_buf_size(eth, ring->frag_size);
 	ring->data = kcalloc(rx_dma_size, sizeof(*ring->data),
 			     GFP_KERNEL);
 	if (!ring->data)
@@ -2864,7 +2931,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;
@@ -3260,6 +3328,56 @@ 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;
+
+	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 stores the frame
+ * length offset by MTK_PDMA_LRO_SDL, the layout used by the MediaTek
+ * downstream driver; the DMA engine accepts frames only up to it, so
+ * without this write jumbo frames are dropped.
+ */
+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;
@@ -3268,6 +3386,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
@@ -3311,6 +3437,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;
 }
 
@@ -3601,17 +3729,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);
@@ -3781,6 +3898,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) {
@@ -3788,12 +3906,26 @@ 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 operation tears them down,
+	 * the next open re-derives it from the current MTUs, so check
+	 * that value instead.
+	 */
+	rx_buf_len = eth->rx_buf_len;
+	if (!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 (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);
 
@@ -3928,30 +4060,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;
@@ -4219,8 +4327,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
@@ -4361,21 +4472,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;
 }
 
@@ -5021,8 +5170,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;
 	else
 		eth->netdev[id]->max_mtu = MTK_MAX_RX_LENGTH_2K - MTK_RX_ETH_HLEN;
 
@@ -5163,6 +5321,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 88a9b3b23..6525fcc5c 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -29,6 +29,7 @@
 #define MTK_QDMA_PAGE_SIZE	2048
 #define MTK_MAX_RX_LENGTH	1536
 #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 +199,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)
@@ -530,6 +532,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 +989,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 +1031,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 +1133,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;
@@ -1151,6 +1157,7 @@ struct mtk_reg_map {
 		u32	glo_cfg;	/* global configuration */
 		u32	rst_idx;	/* reset index */
 		u32	delay_irq;	/* delay interrupt */
+		u32	rx_cfg;		/* rx dma configuration */
 		u32	irq_status;	/* interrupt status */
 		u32	irq_mask;	/* interrupt mask */
 		u32	adma_rx_dbg0;
@@ -1314,6 +1321,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 v4 3/3] net: ethernet: mtk_eth_soc: add 9K jumbo

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 (1) · Medium: 6 · 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 shared RX length leaves no room for an 802.1Q tag at 1 KiB
  boundaries, including the advertised max_mtu.
- [Medium] GMAC1 (the built-in switch port and DSA conduit on MT7988)
  advertises max_mtu = 9198, but the only receive limit the driver can…
- [Medium] The unpolled QDMA RX ring is now filled with jumbo-sized
  buffers.
- [Medium] Possible kernel memory exposure (speculative).
- [Medium] The PDMA RX max-frame (SDL) field may now be set 12 KiB above
  the posted buffer size (speculative).
- [Medium] net: mtk_eth_soc: mtk_xdp_setup() rejects XDP detach when MTU
  is jumbo
- [Low] The commit message hides a behaviour change on other SoCs and
  gets the buffer sizes wrong.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but on the first
  open, a failure while filling an RX ring leaks the buffers and frees…

--

net: ethernet: mtk_eth_soc: add 9K jumbo

This adds an MTK_NETSYS_RX_9K capability for MT7988. With it, the XGMII
MACs advertise a 9K max_mtu and the shared RX rings are sized from the
largest MTU of any MAC. The PDMA RX SDL field and the XMAC_RX_CFG2 / MCR
max-rx limits are programmed from that shared length, and larger-buffer MTU
changes are rejected while DMA is running.

Link: https://github.com/frank-w/BPI-Router-Linux/commit/b4c6cb1d5532
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 6768ab8ebe4aa..527fdaeec50f8 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c

[ ... ]

> @@ -2313,10 +2375,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;
> +

[Severity: Low]
This also changes the HWLRO rings on SoCs that don't have MTK_NETSYS_RX_9K.
On mt2701, mt7622, mt7623 and mt7629 the LRO rings use rx_data_len =
MTK_MAX_LRO_RX_LENGTH (12288). Their frag_size is therefore above PAGE_SIZE,
and alloc_size becomes 16384 instead of roughly 12.7 KiB.

Those LRO skbs get a larger truesize, and skb_shared_info moves to the end
of the order-2 page. The commit message says the other SoCs are left as
before and links the truesize change only to large MTUs. Should this change
be mentioned there?

The buffer sizes in the commit message also don't seem to match:

  Once any MAC's MTU needs more than ~3.5K, every RX buffer is a 16 KiB
  compound page

On arm64, NET_SKB_PAD is 64 and NET_IP_ALIGN is 0. With those values,
frag_size first goes above PAGE_SIZE at rx_buf_len = 4096, which is any MTU
above 3054.

For rx_buf_len 4096 to 7168 (MTU 3055-7150), get_order() gives order-1
(8 KiB) buffers. Only rx_buf_len 8192 and up (MTU 7151 and up) gives 16 KiB
buffers.

>  			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 +2406,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);

[Severity: Medium]
Could this let the skb include memory the DMA engine never wrote?

Before this change, the tailroom left after skb_reserve(NET_SKB_PAD +
NET_IP_ALIGN) was exactly ring->buf_size, which is the DMA-mapped region.
For a 9K ring (buf_size 9216 in a 16 KiB allocation) it is now about 16000
bytes.

A little further down, pktlen comes from the descriptor and goes straight
into skb_put():

	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
	skb_put(skb, pktlen);

Nothing checks pktlen against ring->buf_size. If the hardware ever reports
a PLEN0 larger than the posted buffer, the old code hit skb_over_panic().
The new code would instead pass several KB of stale memory from
__get_free_pages() (no __GFP_ZERO) into the stack.

The new PDMA SDL write makes this harder to rule out; see the comment on
mtk_pdma_set_sdl() below.

Would it make sense to drop descriptors with pktlen > ring->buf_size before
skb_put()? Another option is to keep build_skb() at frag_size and account
for truesize separately.

[ ... ]

> @@ -2818,8 +2885,8 @@ 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);
> +	ring->frag_size = mtk_max_frag_size(eth, rx_data_len);
> +	ring->buf_size = mtk_max_buf_size(eth, ring->frag_size);

[Severity: Medium]
This sizing also applies to eth->rx_ring_qdma. On MT7988 (MTK_QDMA),
mtk_dma_init() allocates that ring with soc->rx.dma_size = 2048 entries.

As far as I can tell, that ring is only used in mtk_rx_alloc() and
mtk_rx_clean(). mtk_poll_rx() only takes rings from mtk_get_rx_ring(), and
that returns eth->rx_ring[].

Once any MAC MTU is above about 7150, is each of those 2048 entries an
order-2 16 KiB page from mtk_max_buf_alloc(..., GFP_KERNEL)? That would pin
about 32 MiB for a ring the CPU never reads or refills, compared with about
5 MiB of page frags before. It also doubles the number of high-order
allocations at open.

Could the QDMA RX ring stay at a fixed 2K buffer length, with the WARN_ON in
mtk_max_buf_size() relaxed for it?

>  	ring->data = kcalloc(rx_dma_size, sizeof(*ring->data),
>  			     GFP_KERNEL);
>  	if (!ring->data)
> @@ -2864,7 +2931,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;

[Severity: High]
This is a pre-existing issue and was not introduced by this patch. What
happens to the entries that were already filled when this return is taken on
the first open?

eth comes from devm_kzalloc(), and mtk_rx_alloc() only sets ring->dma_size
after the fill loop:

	ring->dma_size = rx_dma_size;

On a partial fill (an allocation failure or a dma_mapping_error()), the
cleanup runs with ring->dma_size == 0:

mtk_start_dma()
  mtk_dma_free()
    mtk_rx_clean()

The buffer loop in mtk_rx_clean() visits nothing, so the filled buffers and
their DMA mappings leak. It then calls:

	mtk_dma_ring_free(eth, ring->dma_size * eth->soc->rx.desc_size,
			  ring->dma, ring->phys, in_sram);

That frees the descriptor ring with size 0. For a non-SRAM ring the path is:

dma_free_coherent(dev, 0, ...)
  dma_direct_free()
    dma_free_contiguous()
      __free_pages(page, get_order(0))

Here the page is outside CMA. At runtime, get_order(0) is 52 on 64-bit with
4K pages. Can that corrupt the buddy allocator?

For rx_ring[0] in SRAM the call becomes gen_pool_free(pool, addr, 0), which
looks like it leaks the SRAM block permanently.

This patch adds GFP_KERNEL order-1 and order-2 allocations to this path for
the MT7988 jumbo rings, so the path stays reachable. Would setting
ring->dma_size (or a fill count) before the loop, or unwinding inside
mtk_rx_alloc(), avoid this?

[ ... ]

> @@ -3260,6 +3328,56 @@ static int mtk_dma_busy_wait(struct mtk_eth *eth)
>  	return ret;
>  }
>  

[ ... ]

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

[Severity: Medium]
Is there room for an 802.1Q tag here? MTK_RX_ETH_HLEN is ETH_HLEN +
ETH_FCS_LEN (18 bytes) and does not include VLAN_HLEN. This one value sets
the ring buf_size, the PDMA SDL field and XMAC_RX_CFG2.

On MT7988 (arm64, NET_IP_ALIGN 0, NET_SKB_PAD 64), mtk_max_buf_size()
returns exactly rx_buf_len, so the descriptor has no slack either.

A full-size tagged frame is mtu + 22 bytes. That exceeds every limit when
(mtu + 18) mod 1024 is 0, 1021, 1022 or 1023. Examples are MTU 3051-3054,
4075-4078, ..., 9195-9198.

That range includes the max_mtu now advertised in mtk_add_mac():

	eth->netdev[id]->max_mtu =
		MTK_MAX_RX_LENGTH_9K - MTK_RX_ETH_HLEN;

MTU 9198 gives rx_buf_len 9216, but a tagged frame at that MTU is 9220
bytes.

The commit message says XMAC_RX_CFG2 is programmed "so 802.1Q-tagged frames
fit at any MTU". That doesn't seem to hold at these MTUs.
mtk_ppe_update_mtu() already adds VLAN_HLEN per tag for the same reason.

Should the length be derived from mtu + VLAN_ETH_HLEN + ETH_FCS_LEN, or
VLAN_HLEN added before rounding, with max_mtu lowered to match?

[ ... ]

> +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]
Has the offset encoding of this field been confirmed? MTK_PDMA_LRO_SDL was
unused before this patch. Its value, 0x3000 (12288), equals
MTK_MAX_LRO_RX_LENGTH, which suggests it is the LRO segment length in bytes
rather than an offset.

If the SDL field is a plain byte length, a stock MTU 1500 system gets
0x3800 (14336) while its ring buffers are 2048 bytes, and a 9K setup gets
0x5400. The DMA engine would then no longer limit frames to ring->buf_size.

mtk_poll_rx() doesn't handle frames that span several descriptors, and it
doesn't check pktlen against buf_size before skb_put(). The commit message
also notes that the GMAC1 MAC limit may not be enforced.

The "~1518" reset default in the commit message would be a register value of
0x35EE under the offset reading and 0x05EE under a plain-length reading.

There is also the field width. If the field is narrower than the 15 bits
that GENMASK(30, 16) assumes, 0x5400 sets register bit 30.

Hardware testing alone wouldn't catch a wrong encoding, because any value of
9216 or more lets 9K frames through. Can the register definition be
confirmed? Would programming rx_buf_len or ring->buf_size directly be more
appropriate?

[ ... ]

> @@ -5021,8 +5170,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;
>  

[ ... ]

>  	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;

[Severity: Medium]
Does this also give GMAC1 a max_mtu of 9198? On netsys v3,
mtk_interface_mode_is_xgmii() returns true for PHY_INTERFACE_MODE_INTERNAL.
That is the only interface GMAC1 supports on MT7988.

mtk_set_mcr_max_rx() always sends GMAC1 down the MCR bucket path:

	if (!mtk_interface_mode_is_xgmii(eth, mac->interface) ||
	    mac->id == MTK_GMAC1_ID) {
		...
		else
			mcr_new |= MAC_MCR_MAX_RX(MAC_MCR_MAX_RX_2048);

MAC_MCR_MAX_RX_MASK is GENMASK(25, 24), so 2048 is the largest limit it can
program.

If the internal switch path enforces this field, frames above 2048 bytes
sent to the CPU through the DSA conduit are dropped, and 9K on the DSA user
ports doesn't work. If it doesn't enforce it, the GMAC1 MCR programming in
mtk_mac_config(), mtk_hw_init() and mtk_change_mtu() has no effect.

Should GMAC1's max_mtu be capped to what the driver can program, or can
enforcement be confirmed on hardware before this is merged?

>  	else
>  		eth->netdev[id]->max_mtu = MTK_MAX_RX_LENGTH_2K - MTK_RX_ETH_HLEN;
>  

[ ... ]

> @@ -3788,12 +3906,26 @@ 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 operation tears them down,
> +	 * the next open re-derives it from the current MTUs, so check
> +	 * that value instead.
> +	 */
> +	rx_buf_len = eth->rx_buf_len;
> +	if (!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 (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]
Should this check be skipped when prog is NULL? As written it also
runs on a detach, and a detach does not need a page_pool at all.

eth->prog is a single pointer shared by every MAC on the mtk_eth.
The core, by contrast, tracks the attached program per net_device in
dev->xdp_state.

Take an MT7988 with programs attached to both eth0 and eth1.
Detaching from eth0 clears eth->prog, but the core still records
eth1's program or bpf_xdp_link. With eth->prog NULL, the XDP check in
mtk_change_mtu() no longer fires. While DMA is down, eth1's MTU can
then be raised to 9000, and mtk_rx_buf_len() returns 9216.

A later detach on eth1 reaches this check with prog == NULL. It picks
up rx_buf_len = 9216, either from mtk_rx_buf_len() with the rings
down or from eth->rx_buf_len once they were rebuilt at the jumbo
size. The detach then fails with -EOPNOTSUPP.

>From netlink, that leaves the administrator unable to remove the
program. The other callers are worse. dev_xdp_uninstall() runs at
unregister, and dev_xdp_detach_link() runs when the link is closed.
Both wrap dev_xdp_install() in WARN_ON(), so the failure warns and
leaves the core state out of sync with the driver.

Before this patch max_mtu was at most 2030, so the old dev->mtu check
could not fail. The 9K max_mtu is what makes this reachable.

Would it be enough to apply the size check only when prog is
non-NULL? The check in mtk_change_mtu() looks only at eth->prog, not
at whether any netdev on this mtk_eth still has a program attached.
Should that be tightened here, or left to a separate fix of the
shared eth->prog handling?

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