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

[PATCH net-next v2 0/2] 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 gates page_pool on the ring buffer size.  The pool hands out
order-0 pages and is created with MTK_PP_MAX_BUF_SIZE, so rings that need
larger buffers - the HW-LRO ring already, and the rings used for jumbo
after patch 2 - cannot be backed by it.

Patch 2 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, derives
the shared RX buffer length from the largest MTU, sizes the RX rings and
programs the PDMA RX SDL max frame from it.  A runtime MTU change that
would need a different ring buffer size is rejected with -EBUSY while DMA
is running; the rings are sized at the next open.

This work is a forward-port of the MediaTek downstream jumbo series:
Mason Chang's "change default rx buffer length", "add 9k jumbo frame
support" and "add dynamic rx buffer adjustment support" (MediaTek
OpenWrt feed 999-eth-30/31/33, carried in
frank-w/BPI-Router-Linux), plus Frank Wunderlich's "try to fix crash
the right way".  The per-ring page_pool rule is added here to make the
9K path safe without disabling page_pool for every ring.

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.

Changes in v2:
- put the page_pool change first, so the series builds up to jumbo
  instead of adding it and then fixing it, as suggested by Simon Horman;
- drop the forward declarations by moving mtk_set_mcr_max_rx() above
  mtk_mac_config() and mtk_max_gmac_mtu() above mtk_dma_init(), as
  requested by Jakub Kicinski;
- wrap three lines that exceeded 80 columns;
- send patch 2 as a reply to the cover letter instead of chaining it off
  patch 1;
- Cc the maintainers listed by get_maintainer.pl.

v1: https://lore.kernel.org/all/patch-4f863a941889f0bceca1c082f2d79624@loljews.com/

The runtime-reallocation patch is not included: it reallocates the shared
RX rings when the MTU changes while an interface is up, removing the
-EBUSY restriction. It is a larger change and will be sent separately.

Sandeep Haemoon (2):
  net: ethernet: mtk_eth_soc: skip page_pool for large RX buffers
  net: ethernet: mtk_eth_soc: add 9K jumbo frame support

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 185 ++++++++++++++------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |   9 +-
 2 files changed, 144 insertions(+), 50 deletions(-)

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.53.0

[PATCH net-next v2 1/2] net: ethernet: mtk_eth_soc: skip page_pool

[Sandeep Haemoon]

On NETSYS v2 and later every RX ring gets a page_pool.  The pool is
created with order-0 pages and MTK_PP_MAX_BUF_SIZE (PAGE_SIZE minus the
headroom), so it can only serve rings whose buffers fit in one page.
The HW-LRO ring already needs 12K buffers, and 9K jumbo adds more such
rings; for those the descriptor length is larger than a page, so the
pool cannot back them.

Only create the pool when ring->frag_size fits in a page.  The other
rings fall back to the netdev_alloc_frag()/mtk_max_lro_buf_alloc() path
that is already there.

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 | 7 ++++++-
 1 file changed, 6 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..29370376f 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2819,7 +2819,12 @@ 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)) {
+	/* page_pool hands out order-0 pages and the pool is created with
+	 * MTK_PP_MAX_BUF_SIZE, so only rings whose buffers fit in a page can
+	 * use it.  The larger rings (HW-LRO, and 9K jumbo once enabled) take
+	 * the netdev_alloc_frag()/mtk_max_lro_buf_alloc() path below.
+	 */
+	if (mtk_page_pool_enabled(eth) && ring->frag_size <= PAGE_SIZE) {
 		struct page_pool *pp;
 
 		pp = mtk_create_page_pool(eth, &ring->xdp_q, ring_no,
-- 
2.53.0

Re: [PATCH net-next v2 1/2] 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 · 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 page_pool gate added in mtk_rx_alloc()
  (drivers/net/ethernet/mediatek/mtk_eth_soc.c:2827) checks…
- [Low] The commit message gives the reason for the gate as "On NETSYS v2
  and later every RX ring gets a page_pool ... The HW-LRO ring already…

--

net: ethernet: mtk_eth_soc: skip page_pool

With this change, mtk_rx_alloc() creates a page_pool for an RX ring only
when ring->frag_size fits in PAGE_SIZE. Rings with larger buffers fall
back to the existing netdev_alloc_frag()/mtk_max_lro_buf_alloc() path.

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 ad0fdc4133fb6..29370376f0322 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2819,7 +2819,12 @@ 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)) {
> +	/* page_pool hands out order-0 pages and the pool is created with
> +	 * MTK_PP_MAX_BUF_SIZE, so only rings whose buffers fit in a page can
> +	 * use it.  The larger rings (HW-LRO, and 9K jumbo once enabled) take
> +	 * the netdev_alloc_frag()/mtk_max_lro_buf_alloc() path below.
> +	 */

[Severity: Low]
Is the HW-LRO part of this comment and of the commit message accurate?
The commit message says:

    On NETSYS v2 and later every RX ring gets a page_pool.
    ...
    The HW-LRO ring already needs 12K buffers, and 9K jumbo adds more such
    rings; for those the descriptor length is larger than a page, so the
    pool cannot back them.

mtk_page_pool_enabled() returns mtk_is_netsys_v2_or_greater(eth). MTK_HWLRO
is set only in mt2701_data, mt7622_data, mt7623_data and mt7629_data, and
all of those are .version = 1. MT7981_CAPS, MT7986_CAPS and MT7988_CAPS
don't include it. So a HW-LRO ring is never allocated on a SoC that has
page_pool enabled.

At this commit, every ring that reaches the new check uses ETH_DATA_LEN,
and mtk_add_mac() limits max_mtu:

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

As a result, ring->frag_size <= PAGE_SIZE is always true here. The patch
doesn't change behavior until the following "add 9K jumbo" patch.

Could the commit message describe this as preparation for jumbo frames?
As written, it could be read as a fix for an existing HW-LRO overrun on
v2+, for example by stable backporters.

The next patch in the series renames mtk_max_lro_buf_alloc() to
mtk_max_buf_alloc() and enables 9K jumbo. After that, this comment still
says mtk_max_lro_buf_alloc() and "9K jumbo once enabled". Should that
patch also update the comment?

> +	if (mtk_page_pool_enabled(eth) && ring->frag_size <= PAGE_SIZE) {

[Severity: Medium]
Does this check match what the pool can actually hold? The comment refers
to MTK_PP_MAX_BUF_SIZE, but the condition compares frag_size with
PAGE_SIZE.

The pool uses the page_pool layout:

mtk_create_page_pool() {
    ...
		.offset = MTK_PP_HEADROOM,
		.max_len = MTK_PP_MAX_BUF_SIZE,
    ...
}

#define MTK_PP_MAX_BUF_SIZE	(PAGE_SIZE - MTK_PP_PAD)

MTK_PP_PAD is XDP_PACKET_HEADROOM (256) plus the aligned skb_shared_info.

The descriptor length is calculated for the netdev_alloc_frag() layout
instead:

mtk_max_buf_size() {
	int buf_size = frag_size - NET_SKB_PAD - NET_IP_ALIGN -
		       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
    ...
}

mtk_rx_alloc() then programs that value for page_pool buffers too:

	rxd->rxd2 = RX_DMA_PREP_PLEN0(ring->buf_size);

buf_size is larger than MTK_PP_MAX_BUF_SIZE once frag_size goes above
about PAGE_SIZE - 256 + NET_SKB_PAD + NET_IP_ALIGN (around 3906 with 4K
pages). Those rings still pass the frag_size <= PAGE_SIZE check.

For example, with the "add 9K jumbo" patch applied, consider an MT7988
with 4K pages, CONFIG_MAX_SKB_FRAGS between 42 and 45, and an MTU set
before ifup so that rx_buf_len is 3072:

    frag_size = 3200 + 768 = 3968   (passes this check)
    buf_size = 3134
    MTK_PP_MAX_BUF_SIZE = 4096 - 256 - 768 = 3072

The hardware may then DMA up to page offset 256 + 3134 = 3390, which is
past the start of skb_shared_info at 3328.

In that case PP_FLAG_DMA_SYNC_DEV syncs only max_len bytes for the
device. In mtk_poll_rx(), build_skb(data, PAGE_SIZE) sets up shinfo on top
of the received bytes. For pktlen > 3072, skb_put() would move tail past
skb->end and hit skb_over_panic().

It isn't confirmed whether the MAC actually delivers frames between 3073
and 3134 bytes, since that depends on hardware frame length limits. With
the default CONFIG_MAX_SKB_FRAGS=17, this window isn't reached.

Would ring->buf_size <= MTK_PP_MAX_BUF_SIZE be a better condition here?
It would match the limits mtk_change_mtu() and mtk_xdp_setup() already
check against MTK_PP_MAX_BUF_SIZE.

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

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

Re: [PATCH net-next v2 1/2] 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 · 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 page_pool gate added in mtk_rx_alloc()
  (drivers/net/ethernet/mediatek/mtk_eth_soc.c:2827) checks…
- [Low] The commit message gives the reason for the gate as "On NETSYS v2
  and later every RX ring gets a page_pool ... The HW-LRO ring already…

--

net: ethernet: mtk_eth_soc: skip page_pool

With this change, mtk_rx_alloc() creates a page_pool for an RX ring only
when ring->frag_size fits in PAGE_SIZE. Rings with larger buffers fall
back to the existing netdev_alloc_frag()/mtk_max_lro_buf_alloc() path.

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 ad0fdc4133fb6..29370376f0322 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2819,7 +2819,12 @@ 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)) {
> +	/* page_pool hands out order-0 pages and the pool is created with
> +	 * MTK_PP_MAX_BUF_SIZE, so only rings whose buffers fit in a page can
> +	 * use it.  The larger rings (HW-LRO, and 9K jumbo once enabled) take
> +	 * the netdev_alloc_frag()/mtk_max_lro_buf_alloc() path below.
> +	 */

[Severity: Low]
Is the HW-LRO part of this comment and of the commit message accurate?
The commit message says:

    On NETSYS v2 and later every RX ring gets a page_pool.
    ...
    The HW-LRO ring already needs 12K buffers, and 9K jumbo adds more such
    rings; for those the descriptor length is larger than a page, so the
    pool cannot back them.

mtk_page_pool_enabled() returns mtk_is_netsys_v2_or_greater(eth). MTK_HWLRO
is set only in mt2701_data, mt7622_data, mt7623_data and mt7629_data, and
all of those are .version = 1. MT7981_CAPS, MT7986_CAPS and MT7988_CAPS
don't include it. So a HW-LRO ring is never allocated on a SoC that has
page_pool enabled.

At this commit, every ring that reaches the new check uses ETH_DATA_LEN,
and mtk_add_mac() limits max_mtu:

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

As a result, ring->frag_size <= PAGE_SIZE is always true here. The patch
doesn't change behavior until the following "add 9K jumbo" patch.

Could the commit message describe this as preparation for jumbo frames?
As written, it could be read as a fix for an existing HW-LRO overrun on
v2+, for example by stable backporters.

The next patch in the series renames mtk_max_lro_buf_alloc() to
mtk_max_buf_alloc() and enables 9K jumbo. After that, this comment still
says mtk_max_lro_buf_alloc() and "9K jumbo once enabled". Should that
patch also update the comment?

> +	if (mtk_page_pool_enabled(eth) && ring->frag_size <= PAGE_SIZE) {

[Severity: Medium]
Does this check match what the pool can actually hold? The comment refers
to MTK_PP_MAX_BUF_SIZE, but the condition compares frag_size with
PAGE_SIZE.

The pool uses the page_pool layout:

mtk_create_page_pool() {
    ...
		.offset = MTK_PP_HEADROOM,
		.max_len = MTK_PP_MAX_BUF_SIZE,
    ...
}

#define MTK_PP_MAX_BUF_SIZE	(PAGE_SIZE - MTK_PP_PAD)

MTK_PP_PAD is XDP_PACKET_HEADROOM (256) plus the aligned skb_shared_info.

The descriptor length is calculated for the netdev_alloc_frag() layout
instead:

mtk_max_buf_size() {
	int buf_size = frag_size - NET_SKB_PAD - NET_IP_ALIGN -
		       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
    ...
}

mtk_rx_alloc() then programs that value for page_pool buffers too:

	rxd->rxd2 = RX_DMA_PREP_PLEN0(ring->buf_size);

buf_size is larger than MTK_PP_MAX_BUF_SIZE once frag_size goes above
about PAGE_SIZE - 256 + NET_SKB_PAD + NET_IP_ALIGN (around 3906 with 4K
pages). Those rings still pass the frag_size <= PAGE_SIZE check.

For example, with the "add 9K jumbo" patch applied, consider an MT7988
with 4K pages, CONFIG_MAX_SKB_FRAGS between 42 and 45, and an MTU set
before ifup so that rx_buf_len is 3072:

    frag_size = 3200 + 768 = 3968   (passes this check)
    buf_size = 3134
    MTK_PP_MAX_BUF_SIZE = 4096 - 256 - 768 = 3072

The hardware may then DMA up to page offset 256 + 3134 = 3390, which is
past the start of skb_shared_info at 3328.

In that case PP_FLAG_DMA_SYNC_DEV syncs only max_len bytes for the
device. In mtk_poll_rx(), build_skb(data, PAGE_SIZE) sets up shinfo on top
of the received bytes. For pktlen > 3072, skb_put() would move tail past
skb->end and hit skb_over_panic().

It isn't confirmed whether the MAC actually delivers frames between 3073
and 3134 bytes, since that depends on hardware frame length limits. With
the default CONFIG_MAX_SKB_FRAGS=17, this window isn't reached.

Would ring->buf_size <= MTK_PP_MAX_BUF_SIZE be a better condition here?
It would match the limits mtk_change_mtu() and mtk_xdp_setup() already
check against MTK_PP_MAX_BUF_SIZE.

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

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

[PATCH net-next v2 2/2] 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 XGMII MACs, derived from the DT phy_mode at
  probe time so DSA can raise the conduit MTU before the netdev is
  opened;
- program XMAC_RX_CFG2 on the XGMII MACs;
- derive the shared RX buffer length from the largest MTU and size the
  RX rings from it;
- program the PDMA RX SDL max frame so the DMA engine accepts jumbo
  frames;
- reject a runtime MTU change that would need a different ring buffer
  size while DMA is running; the rings are sized at the next open.

The RX rings and their buffers are sized for the largest MTU of any MAC,
so the MTU limits of the MACs are now written from mtk_mac_config() and
mtk_change_mtu() with eth->rx_buf_len rather than a fixed 2K.

This is a forward-port of the MediaTek downstream jumbo series:
Mason Chang's "change default rx buffer length", "add 9k jumbo frame
support" and "add dynamic rx buffer adjustment support" (MediaTek
OpenWrt feed 999-eth-30/31/33, carried in
frank-w/BPI-Router-Linux 92bfdbdd4de1/b4c6cb1d5532/ca4c976dce22),
plus Frank Wunderlich's "try to fix crash the right way"
(a3207a383794).

Link: https://github.com/frank-w/BPI-Router-Linux/commit/b4c6cb1d5532
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 | 178 ++++++++++++++------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |   9 +-
 2 files changed, 138 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 29370376f..38d402c5e 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,43 @@ static int mtk_mac_prepare(struct phylink_config *config, unsigned int mode,
 	return 0;
 }
 
+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)) {
+		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 if (mtk_is_netsys_v3_or_greater(mac->hw) &&
+		   mac->id != MTK_GMAC1_ID) {
+		mcr_cur = mtk_r32(mac->hw, MTK_XMAC_RX_CFG2(mac->id));
+
+		if (val < MTK_MAX_RX_LENGTH_9K)
+			mcr_new = FIELD_PREP(MTK_XMAC_MAX_RX_MASK, val);
+		else
+			mcr_new = 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)
 {
@@ -647,6 +685,7 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
 		regmap_write(eth->ethsys, ETHSYS_SYSCFG0, val);
 
 		mac->interface = state->interface;
+		mtk_set_mcr_max_rx(mac, eth->rx_buf_len);
 	}
 
 	/* SGMII */
@@ -1242,22 +1281,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 max 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 +1319,8 @@ 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)
+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,
@@ -2310,7 +2348,8 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 			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++;
@@ -2812,8 +2851,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)
@@ -2857,7 +2896,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;
@@ -3253,6 +3293,35 @@ 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)
+		return MTK_MAX_RX_LENGTH;
+
+	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));
+}
+
 static int mtk_dma_init(struct mtk_eth *eth)
 {
 	int err;
@@ -3261,6 +3330,12 @@ 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 and mtk_mac_config() 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
@@ -3304,6 +3379,17 @@ static int mtk_dma_init(struct mtk_eth *eth)
 		mtk_w32(eth, 0x0, eth->soc->reg_map->qdma.hred);
 	}
 
+	/* Program the PDMA max RX frame (SDL) from rx_buf_len so jumbo
+	 * frames up to it are accepted by the DMA engine.
+	 */
+	if (mtk_is_netsys_v3_or_greater(eth)) {
+		u32 val = mtk_r32(eth, eth->soc->reg_map->pdma.rx_cfg);
+
+		mtk_w32(eth, val | ((MTK_PDMA_LRO_SDL + eth->rx_buf_len) <<
+			MTK_RX_CFG_SDL_OFFSET),
+			eth->soc->reg_map->pdma.rx_cfg);
+	}
+
 	return 0;
 }
 
@@ -3594,17 +3680,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);
@@ -3921,29 +3996,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)
 {
@@ -4354,6 +4406,7 @@ 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;
 	int max_mtu, i;
 
 	if (rcu_access_pointer(eth->prog) &&
@@ -4362,6 +4415,17 @@ static int mtk_change_mtu(struct net_device *dev, int new_mtu)
 		return -EINVAL;
 	}
 
+	/* The RX rings are shared and cannot be resized in place while DMA is
+	 * running.  Until the reallocation worker lands, reject a runtime MTU
+	 * change that would need a different ring buffer size.
+	 */
+	max_mtu = mtk_max_gmac_mtu(eth);
+	if (new_mtu > max_mtu)
+		max_mtu = new_mtu;
+	rx_buf_len = mtk_rx_buf_len_from_mtu(max_mtu);
+	if (refcount_read(&eth->dma_refcnt) && rx_buf_len != eth->rx_buf_len)
+		return -EBUSY;
+
 	mtk_set_mcr_max_rx(mac, length);
 	WRITE_ONCE(dev->mtu, new_mtu);
 
@@ -4369,6 +4433,15 @@ static int mtk_change_mtu(struct net_device *dev, int new_mtu)
 	for (i = 0; i < ARRAY_SIZE(eth->ppe); i++)
 		mtk_ppe_update_mtu(eth->ppe[i], max_mtu);
 
+	eth->rx_buf_len = rx_buf_len;
+	if (mtk_is_netsys_v3_or_greater(eth)) {
+		u32 val = mtk_r32(eth, eth->soc->reg_map->pdma.rx_cfg);
+
+		mtk_w32(eth, val | ((MTK_PDMA_LRO_SDL + eth->rx_buf_len) <<
+			MTK_RX_CFG_SDL_OFFSET),
+			eth->soc->reg_map->pdma.rx_cfg);
+	}
+
 	return 0;
 }
 
@@ -5014,8 +5087,15 @@ 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;
+	/* 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_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;
 
@@ -5156,6 +5236,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;
+
 	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..9aad21ea3 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
@@ -530,6 +531,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 +988,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 +1030,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 +1132,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 +1156,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 +1320,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 v2 2/2] net: ethernet: mtk_eth_soc: add 9K jumbo

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 8 · 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).

- [High] An XDP program can attach successfully and then never run.
- [Medium] This regression hits every SoC, not just MT7988, and the
  commit message does not say so.
- [Medium] The patch removes MT7628's max_mtu limit.
- [Medium] Two statements in the commit message do not match the code.
- [Medium] The PDMA RX SDL field in pdma.rx_cfg (0x6a10 on MT7988) is
  written as `mtk_w32(eth, val | ((MTK_PDMA_LRO_SDL + eth->rx_buf_len)…
- [Medium] The code breaks the invariant stated in the new mtk_dma_init()
  comment, that eth->rx_buf_len is derived at ring allocation "so ...…
- [Medium] While DMA is running, mtk_change_mtu() rejects any change
  whose computed rx_buf_len differs from eth->rx_buf_len (`!=`), not…
- [Medium] Once any MAC's MTU makes frag_size exceed PAGE_SIZE (MTU above
  ~3054 on MT7988), every RX ring is filled with high-order compound…
- [Medium] In the XGMII branch of mtk_set_mcr_max_rx(),
  MTK_XMAC_RX_CFG2(mac->id) is read into mcr_cur, but the new value is…
- [Low] The comment in mtk_rx_alloc() (lines 2861-2865) says large rings
  "take the netdev_alloc_frag()/mtk_max_lro_buf_alloc() path below" and…

--

net: ethernet: mtk_eth_soc: add 9K jumbo

This commit adds a MTK_NETSYS_RX_9K capability for MT7988. With it, the
XGMII MACs advertise a 9K max_mtu, and the driver programs XMAC_RX_CFG2
and the PDMA RX SDL. The shared RX buffer length now comes from the
largest MTU of any MAC, and runtime MTU changes that need a different
buffer size 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 29370376f0322..38d402c5ead73 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c

[ ... ]

> @@ -554,6 +555,43 @@ static int mtk_mac_prepare(struct phylink_config *config, unsigned int mode,
>  	return 0;
>  }
>  
> +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)) {
> +		mcr_cur = mtk_r32(mac->hw, MTK_MAC_MCR(mac->id));
> +		mcr_new = mcr_cur & ~MAC_MCR_MAX_RX_MASK;

[ ... ]

> +	} else if (mtk_is_netsys_v3_or_greater(mac->hw) &&
> +		   mac->id != MTK_GMAC1_ID) {
> +		mcr_cur = mtk_r32(mac->hw, MTK_XMAC_RX_CFG2(mac->id));
> +
> +		if (val < MTK_MAX_RX_LENGTH_9K)
> +			mcr_new = FIELD_PREP(MTK_XMAC_MAX_RX_MASK, val);
> +		else
> +			mcr_new = MTK_MAX_RX_LENGTH_9K;

[Severity: Medium]
Does this clear the rest of XMAC_RX_CFG2? The code reads mcr_cur, but it
builds mcr_new from the length field alone. Any bits outside
MTK_XMAC_MAX_RX_MASK (GENMASK(13, 0)) are written back as zero.

The GMAC branch above keeps the other bits with
mcr_cur & ~MAC_MCR_MAX_RX_MASK. Should the XGMII branch do the same?

This branch runs on the GMAC2/GMAC3 XGMII MACs from three places:
mtk_mac_config() on an interface change, mtk_change_mtu(), and
mtk_hw_init() after a reset. If the upper bits hold non-zero defaults,
the mcr_new != mcr_cur check will also trigger a write every time.

> +
> +		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)
>  {
> @@ -647,6 +685,7 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
>  		regmap_write(eth->ethsys, ETHSYS_SYSCFG0, val);
>  
>  		mac->interface = state->interface;
> +		mtk_set_mcr_max_rx(mac, eth->rx_buf_len);
>  	}

[Severity: Medium]
Can a MAC end up accepting frames larger than the RX ring buffers here?

This call is inside the if (mac->interface != state->interface) block.
mac->interface is not reset on close, so the call only runs on the first
mac_config for a given interface mode.

Take two MACs, A and B:

  A at MTU 9000 (or 2000 on MT7621), B at MTU 1500
  open both: mtk_dma_init() sets rx_buf_len to 9216 (or 2048), and
             mtk_mac_config(B) programs B's limit to 9216 (or MAX_RX_2048)
  close both
  set A's MTU to 1500: dma_refcnt is 0, so this is allowed, and only
             A's MAC is reprogrammed
  reopen: mtk_dma_init() recomputes rx_buf_len as 1536 and the rings
          shrink, but mtk_mac_config(B) skips this call because the
          interface did not change

B's MAC would still accept 9216 or 2048 byte frames into 1536 byte
buffers. That seems to contradict the new comment in mtk_dma_init():

    /* Derived here, at ring allocation, so eth->rx_buf_len always describes
     * the buffers the rings actually hold and mtk_mac_config() never lets
     * a MAC accept a frame the rings cannot take.
     */

Before this patch the buffers were always at least 2048 bytes, which
covered the largest MAC limit. The driver can't show what the PDMA does
with a frame longer than PLEN0, and mtk_poll_rx() has no handling for
frames that span multiple descriptors.

[ ... ]

> @@ -1280,9 +1319,8 @@ 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)
> +static void *mtk_max_buf_alloc(unsigned int size, gfp_t gfp_mask)
>  {

[Severity: Low]
This isn't a bug, but the comment above the page_pool check in
mtk_rx_alloc() still uses the old name and treats 9K jumbo as a future
case:

	/* page_pool hands out order-0 pages and the pool is created with
	 * MTK_PP_MAX_BUF_SIZE, so only rings whose buffers fit in a page can
	 * use it.  The larger rings (HW-LRO, and 9K jumbo once enabled) take
	 * the netdev_alloc_frag()/mtk_max_lro_buf_alloc() path below.
	 */

Should it be updated now that this patch renames the helper to
mtk_max_buf_alloc() and enables MTK_NETSYS_RX_9K for MT7988?

[ ... ]

> @@ -2812,8 +2851,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: High]
Can an attached XDP program silently stop running once frag_size grows
past PAGE_SIZE?

The normal ring's frag_size now follows eth->rx_buf_len. A page_pool is
only created when the buffer fits in a page:

	if (mtk_page_pool_enabled(eth) && ring->frag_size <= PAGE_SIZE) {

mtk_poll_rx() only calls mtk_xdp_run() inside the if (ring->page_pool)
branch. Without a page_pool, every packet goes to napi_gro_receive() and
the program never sees it.

The XDP admission checks only look at the MTU of a single device:

mtk_xdp_setup():
	if (dev->mtu > MTK_PP_MAX_BUF_SIZE) {
		NL_SET_ERR_MSG_MOD(extack, "MTU too large for XDP");
		return -EOPNOTSUPP;
	}

mtk_change_mtu():
	if (rcu_access_pointer(eth->prog) &&
	    length > MTK_PP_MAX_BUF_SIZE) {

On MT7988 there seem to be two ways to reach this:

1) GMAC1 (the DSA conduit, max_mtu 9198) is at MTU 9000, so rx_buf_len
   is 9216 and frag_size is around 9.6K. Attaching XDP to another MAC
   with MTU 1500 succeeds, but the program never runs on any MAC.

2) With XDP attached and the interfaces down, an MTU of 3055..3502
   passes the MTK_PP_MAX_BUF_SIZE check. The 1K rounding then gives
   rx_buf_len 4096 and frag_size around 4.5K, so the next open builds
   the rings without a page_pool.

Before this patch the normal ring's frag_size was always around 2.4K, so
page_pool, and therefore XDP, was always active on netsys v2 and later.
Should the XDP checks use the shared rx_buf_len instead of the MTU of
one device?

[ ... ]

> @@ -2857,7 +2896,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);

[Severity: Medium]
Is this memory cost intended when only one MAC uses a jumbo MTU?

Once frag_size exceeds PAGE_SIZE (MTU above roughly 3054 on MT7988),
every RX ring is filled from mtk_max_buf_alloc(). That includes
rx_ring_qdma, which mtk_get_rx_ring() never returns and NAPI never polls.

At rx_buf_len 9216, frag_size is around 9.6K, so each slot is an order-2
allocation (16 KiB). rx_ring_qdma and rx_ring[0] each have 2048 entries
(rx.dma_size), so about 64 MiB stays pinned while DMA is up. Around half
of that sits in the unused QDMA ring. Before this patch it was about
16 MiB of order-0 page_pool pages.

The refill in mtk_poll_rx() also becomes an order-2 GFP_ATOMIC
allocation for every packet:

	new_data = mtk_max_buf_alloc(ring->frag_size,
				     GFP_ATOMIC);

__GFP_NOWARN is set, so when memory is fragmented these allocations fail
without a warning and the packets are counted in rx_dropped. The
GFP_KERNEL fill at open can fail with -ENOMEM for the same reason.

[ ... ]

> @@ -3253,6 +3293,35 @@ 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;
> +
> +	if (length <= MTK_MAX_RX_LENGTH)
> +		return MTK_MAX_RX_LENGTH;

[Severity: Medium]
Does this change behaviour on every SoC, not just MT7988?

At the default MTU of 1500 the length is 1518, so this returns
MTK_MAX_RX_LENGTH (1536). mtk_max_frag_size() now uses that as the
buffer floor instead of the fixed MTK_MAX_RX_LENGTH_2K. All rings on
MT7621/7622/7623/7629/7981/7986 shrink from 2048 to 1536.

mtk_change_mtu() also gains this check:

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

Together, these make any runtime MTU change that needs a frame above
1536 bytes (MTU 1519..2030) fail with -EBUSY while any MAC is up. For
example, MTU 1600 gives length 1618 and rx_buf_len 2048. Those changes
were accepted before.

With DSA, dsa_user_change_mtu() raises the conduit MTU with
dev_set_mtu() and rolls back on failure. Raising a user port from 1500
to 1520 (conduit 1524, length 1542) on MT7621 with MT7530 would now fail
while the conduit is up.

The commit message describes this as an MT7988 jumbo feature. Should it
mention the smaller default buffer on the other SoCs, or should those
SoCs keep the 2K floor?

> +
> +	return DIV_ROUND_UP(length, 1024) * 1024;
> +}

[ ... ]

> @@ -3304,6 +3379,17 @@ static int mtk_dma_init(struct mtk_eth *eth)
>  		mtk_w32(eth, 0x0, eth->soc->reg_map->qdma.hred);
>  	}
>  
> +	/* Program the PDMA max RX frame (SDL) from rx_buf_len so jumbo
> +	 * frames up to it are accepted by the DMA engine.
> +	 */
> +	if (mtk_is_netsys_v3_or_greater(eth)) {
> +		u32 val = mtk_r32(eth, eth->soc->reg_map->pdma.rx_cfg);
> +
> +		mtk_w32(eth, val | ((MTK_PDMA_LRO_SDL + eth->rx_buf_len) <<
> +			MTK_RX_CFG_SDL_OFFSET),
> +			eth->soc->reg_map->pdma.rx_cfg);

[Severity: Medium]
Should the SDL field be cleared before the new value is ORed in?

The frame engine is only reset at probe and in error recovery, not on
close/open. mtk_dma_init() runs on every first open, and mtk_change_mtu()
does the same write on every MTU change. So the field holds the OR of
every value written and can only grow:

  open at MTU 1500:            0x3000 + 1536 = 0x3600
  close, set MTU 9000, reopen: 0x3600 | 0x5400 = 0x7600
  set MTU 1500 again:          stays 0x7600

A 1536 to 2048 change likewise gives 0x3600 | 0x3800 = 0x3e00.

There is also no field mask to go with MTK_PDMA_LRO_SDL and
MTK_RX_CFG_SDL_OFFSET. 0x3000 + 9216 = 0x5400 needs 15 bits. If the
field is narrower than that, does the shift by 16 set bit 30, which
belongs to another control field?

mtk_poll_rx() assumes one frame per descriptor. Could a DMA frame limit
that doesn't match buf_size cause the hardware to split frames?

[ ... ]

> @@ -4362,6 +4415,17 @@ static int mtk_change_mtu(struct net_device *dev, int new_mtu)
>  		return -EINVAL;
>  	}
>  
> +	/* The RX rings are shared and cannot be resized in place while DMA is
> +	 * running.  Until the reallocation worker lands, reject a runtime MTU
> +	 * change that would need a different ring buffer size.
> +	 */
> +	max_mtu = mtk_max_gmac_mtu(eth);
> +	if (new_mtu > max_mtu)
> +		max_mtu = new_mtu;
> +	rx_buf_len = mtk_rx_buf_len_from_mtu(max_mtu);
> +	if (refcount_read(&eth->dma_refcnt) && rx_buf_len != eth->rx_buf_len)
> +		return -EBUSY;

[Severity: Medium]
Should this only reject changes that need larger buffers, that is
rx_buf_len > eth->rx_buf_len?

max_mtu includes this device's old MTU, so lowering the largest MTU is
allowed once and eth->rx_buf_len stays at the old ring size. After that,
smaller changes that would fit in the existing buffers fail:

  DMA running, A at 9000, rx_buf_len 9216
  A 9000 -> 1500: max_mtu 9000, 9216 == 9216, allowed
  A 1500 -> 1400: 1536 != 9216, -EBUSY
  B 1500 -> 2000: 2048 != 9216, -EBUSY

>From then on, no MTU can be lowered until every MAC is closed. DSA user
port MTU changes that propagate to the conduit fail the same way.

> +
>  	mtk_set_mcr_max_rx(mac, length);
>  	WRITE_ONCE(dev->mtu, new_mtu);

[Severity: Medium]
The commit message says:

  The RX rings and their buffers are sized for the largest MTU of any MAC,
  so the MTU limits of the MACs are now written from mtk_mac_config() and
  mtk_change_mtu() with eth->rx_buf_len rather than a fixed 2K.

Is that accurate? This call still passes the per-device length.
mtk_hw_init() also still uses the per-device MTU:

	mtk_set_mcr_max_rx(netdev_priv(dev),
			   dev->mtu + MTK_RX_ETH_HLEN);

Only mtk_mac_config() uses eth->rx_buf_len, so the MAC limit depends on
which of the three call sites ran last. The baseline never wrote a fixed
2K either: both earlier call sites used dev->mtu + MTK_RX_ETH_HLEN.

The commit message also lists "add dynamic rx buffer adjustment support"
as part of this forward-port. However, the comment above refers to a
reallocation worker that hasn't landed yet, and runtime changes return
-EBUSY. Could the commit message be updated to match the code?

[ ... ]

> @@ -5014,8 +5087,15 @@ 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;
> +	/* 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_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;

[Severity: Medium]
Was dropping the MT7628 case intended? MT7628 used to get max_mtu 1518
(MTK_MAX_RX_LENGTH - MTK_RX_ETH_HLEN). Commit 4fd59792097a added that
limit on purpose ("MT762x HW, except for MT7628, supports frame length
up to 2048"). MT7628 now falls into the else branch and gets 2030.

Nothing else limits the frame size on MT7628. mtk_set_mcr_max_rx()
returns early for MTK_SOC_MT7628, and its RX descriptors are written as
RX_DMA_LSO with no PLEN0. With the interfaces down, mtk_change_mtu()
would now report success for MTUs 1519..2030 on hardware that cannot
receive frames above 1536 bytes.

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

Re: [PATCH net-next v2 2/2] net: ethernet: mtk_eth_soc: add 9K jumbo

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 8 · 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).

- [High] An XDP program can attach successfully and then never run.
- [Medium] This regression hits every SoC, not just MT7988, and the
  commit message does not say so.
- [Medium] The patch removes MT7628's max_mtu limit.
- [Medium] Two statements in the commit message do not match the code.
- [Medium] The PDMA RX SDL field in pdma.rx_cfg (0x6a10 on MT7988) is
  written as `mtk_w32(eth, val | ((MTK_PDMA_LRO_SDL + eth->rx_buf_len)…
- [Medium] The code breaks the invariant stated in the new mtk_dma_init()
  comment, that eth->rx_buf_len is derived at ring allocation "so ...…
- [Medium] While DMA is running, mtk_change_mtu() rejects any change
  whose computed rx_buf_len differs from eth->rx_buf_len (`!=`), not…
- [Medium] Once any MAC's MTU makes frag_size exceed PAGE_SIZE (MTU above
  ~3054 on MT7988), every RX ring is filled with high-order compound…
- [Medium] In the XGMII branch of mtk_set_mcr_max_rx(),
  MTK_XMAC_RX_CFG2(mac->id) is read into mcr_cur, but the new value is…
- [Low] The comment in mtk_rx_alloc() (lines 2861-2865) says large rings
  "take the netdev_alloc_frag()/mtk_max_lro_buf_alloc() path below" and…

--

net: ethernet: mtk_eth_soc: add 9K jumbo

This commit adds a MTK_NETSYS_RX_9K capability for MT7988. With it, the
XGMII MACs advertise a 9K max_mtu, and the driver programs XMAC_RX_CFG2
and the PDMA RX SDL. The shared RX buffer length now comes from the
largest MTU of any MAC, and runtime MTU changes that need a different
buffer size 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 29370376f0322..38d402c5ead73 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c

[ ... ]

> @@ -554,6 +555,43 @@ static int mtk_mac_prepare(struct phylink_config *config, unsigned int mode,
>  	return 0;
>  }
>  
> +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)) {
> +		mcr_cur = mtk_r32(mac->hw, MTK_MAC_MCR(mac->id));
> +		mcr_new = mcr_cur & ~MAC_MCR_MAX_RX_MASK;

[ ... ]

> +	} else if (mtk_is_netsys_v3_or_greater(mac->hw) &&
> +		   mac->id != MTK_GMAC1_ID) {
> +		mcr_cur = mtk_r32(mac->hw, MTK_XMAC_RX_CFG2(mac->id));
> +
> +		if (val < MTK_MAX_RX_LENGTH_9K)
> +			mcr_new = FIELD_PREP(MTK_XMAC_MAX_RX_MASK, val);
> +		else
> +			mcr_new = MTK_MAX_RX_LENGTH_9K;

[Severity: Medium]
Does this clear the rest of XMAC_RX_CFG2? The code reads mcr_cur, but it
builds mcr_new from the length field alone. Any bits outside
MTK_XMAC_MAX_RX_MASK (GENMASK(13, 0)) are written back as zero.

The GMAC branch above keeps the other bits with
mcr_cur & ~MAC_MCR_MAX_RX_MASK. Should the XGMII branch do the same?

This branch runs on the GMAC2/GMAC3 XGMII MACs from three places:
mtk_mac_config() on an interface change, mtk_change_mtu(), and
mtk_hw_init() after a reset. If the upper bits hold non-zero defaults,
the mcr_new != mcr_cur check will also trigger a write every time.

> +
> +		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)
>  {
> @@ -647,6 +685,7 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
>  		regmap_write(eth->ethsys, ETHSYS_SYSCFG0, val);
>  
>  		mac->interface = state->interface;
> +		mtk_set_mcr_max_rx(mac, eth->rx_buf_len);
>  	}

[Severity: Medium]
Can a MAC end up accepting frames larger than the RX ring buffers here?

This call is inside the if (mac->interface != state->interface) block.
mac->interface is not reset on close, so the call only runs on the first
mac_config for a given interface mode.

Take two MACs, A and B:

  A at MTU 9000 (or 2000 on MT7621), B at MTU 1500
  open both: mtk_dma_init() sets rx_buf_len to 9216 (or 2048), and
             mtk_mac_config(B) programs B's limit to 9216 (or MAX_RX_2048)
  close both
  set A's MTU to 1500: dma_refcnt is 0, so this is allowed, and only
             A's MAC is reprogrammed
  reopen: mtk_dma_init() recomputes rx_buf_len as 1536 and the rings
          shrink, but mtk_mac_config(B) skips this call because the
          interface did not change

B's MAC would still accept 9216 or 2048 byte frames into 1536 byte
buffers. That seems to contradict the new comment in mtk_dma_init():

    /* Derived here, at ring allocation, so eth->rx_buf_len always describes
     * the buffers the rings actually hold and mtk_mac_config() never lets
     * a MAC accept a frame the rings cannot take.
     */

Before this patch the buffers were always at least 2048 bytes, which
covered the largest MAC limit. The driver can't show what the PDMA does
with a frame longer than PLEN0, and mtk_poll_rx() has no handling for
frames that span multiple descriptors.

[ ... ]

> @@ -1280,9 +1319,8 @@ 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)
> +static void *mtk_max_buf_alloc(unsigned int size, gfp_t gfp_mask)
>  {

[Severity: Low]
This isn't a bug, but the comment above the page_pool check in
mtk_rx_alloc() still uses the old name and treats 9K jumbo as a future
case:

	/* page_pool hands out order-0 pages and the pool is created with
	 * MTK_PP_MAX_BUF_SIZE, so only rings whose buffers fit in a page can
	 * use it.  The larger rings (HW-LRO, and 9K jumbo once enabled) take
	 * the netdev_alloc_frag()/mtk_max_lro_buf_alloc() path below.
	 */

Should it be updated now that this patch renames the helper to
mtk_max_buf_alloc() and enables MTK_NETSYS_RX_9K for MT7988?

[ ... ]

> @@ -2812,8 +2851,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: High]
Can an attached XDP program silently stop running once frag_size grows
past PAGE_SIZE?

The normal ring's frag_size now follows eth->rx_buf_len. A page_pool is
only created when the buffer fits in a page:

	if (mtk_page_pool_enabled(eth) && ring->frag_size <= PAGE_SIZE) {

mtk_poll_rx() only calls mtk_xdp_run() inside the if (ring->page_pool)
branch. Without a page_pool, every packet goes to napi_gro_receive() and
the program never sees it.

The XDP admission checks only look at the MTU of a single device:

mtk_xdp_setup():
	if (dev->mtu > MTK_PP_MAX_BUF_SIZE) {
		NL_SET_ERR_MSG_MOD(extack, "MTU too large for XDP");
		return -EOPNOTSUPP;
	}

mtk_change_mtu():
	if (rcu_access_pointer(eth->prog) &&
	    length > MTK_PP_MAX_BUF_SIZE) {

On MT7988 there seem to be two ways to reach this:

1) GMAC1 (the DSA conduit, max_mtu 9198) is at MTU 9000, so rx_buf_len
   is 9216 and frag_size is around 9.6K. Attaching XDP to another MAC
   with MTU 1500 succeeds, but the program never runs on any MAC.

2) With XDP attached and the interfaces down, an MTU of 3055..3502
   passes the MTK_PP_MAX_BUF_SIZE check. The 1K rounding then gives
   rx_buf_len 4096 and frag_size around 4.5K, so the next open builds
   the rings without a page_pool.

Before this patch the normal ring's frag_size was always around 2.4K, so
page_pool, and therefore XDP, was always active on netsys v2 and later.
Should the XDP checks use the shared rx_buf_len instead of the MTU of
one device?

[ ... ]

> @@ -2857,7 +2896,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);

[Severity: Medium]
Is this memory cost intended when only one MAC uses a jumbo MTU?

Once frag_size exceeds PAGE_SIZE (MTU above roughly 3054 on MT7988),
every RX ring is filled from mtk_max_buf_alloc(). That includes
rx_ring_qdma, which mtk_get_rx_ring() never returns and NAPI never polls.

At rx_buf_len 9216, frag_size is around 9.6K, so each slot is an order-2
allocation (16 KiB). rx_ring_qdma and rx_ring[0] each have 2048 entries
(rx.dma_size), so about 64 MiB stays pinned while DMA is up. Around half
of that sits in the unused QDMA ring. Before this patch it was about
16 MiB of order-0 page_pool pages.

The refill in mtk_poll_rx() also becomes an order-2 GFP_ATOMIC
allocation for every packet:

	new_data = mtk_max_buf_alloc(ring->frag_size,
				     GFP_ATOMIC);

__GFP_NOWARN is set, so when memory is fragmented these allocations fail
without a warning and the packets are counted in rx_dropped. The
GFP_KERNEL fill at open can fail with -ENOMEM for the same reason.

[ ... ]

> @@ -3253,6 +3293,35 @@ 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;
> +
> +	if (length <= MTK_MAX_RX_LENGTH)
> +		return MTK_MAX_RX_LENGTH;

[Severity: Medium]
Does this change behaviour on every SoC, not just MT7988?

At the default MTU of 1500 the length is 1518, so this returns
MTK_MAX_RX_LENGTH (1536). mtk_max_frag_size() now uses that as the
buffer floor instead of the fixed MTK_MAX_RX_LENGTH_2K. All rings on
MT7621/7622/7623/7629/7981/7986 shrink from 2048 to 1536.

mtk_change_mtu() also gains this check:

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

Together, these make any runtime MTU change that needs a frame above
1536 bytes (MTU 1519..2030) fail with -EBUSY while any MAC is up. For
example, MTU 1600 gives length 1618 and rx_buf_len 2048. Those changes
were accepted before.

With DSA, dsa_user_change_mtu() raises the conduit MTU with
dev_set_mtu() and rolls back on failure. Raising a user port from 1500
to 1520 (conduit 1524, length 1542) on MT7621 with MT7530 would now fail
while the conduit is up.

The commit message describes this as an MT7988 jumbo feature. Should it
mention the smaller default buffer on the other SoCs, or should those
SoCs keep the 2K floor?

> +
> +	return DIV_ROUND_UP(length, 1024) * 1024;
> +}

[ ... ]

> @@ -3304,6 +3379,17 @@ static int mtk_dma_init(struct mtk_eth *eth)
>  		mtk_w32(eth, 0x0, eth->soc->reg_map->qdma.hred);
>  	}
>  
> +	/* Program the PDMA max RX frame (SDL) from rx_buf_len so jumbo
> +	 * frames up to it are accepted by the DMA engine.
> +	 */
> +	if (mtk_is_netsys_v3_or_greater(eth)) {
> +		u32 val = mtk_r32(eth, eth->soc->reg_map->pdma.rx_cfg);
> +
> +		mtk_w32(eth, val | ((MTK_PDMA_LRO_SDL + eth->rx_buf_len) <<
> +			MTK_RX_CFG_SDL_OFFSET),
> +			eth->soc->reg_map->pdma.rx_cfg);

[Severity: Medium]
Should the SDL field be cleared before the new value is ORed in?

The frame engine is only reset at probe and in error recovery, not on
close/open. mtk_dma_init() runs on every first open, and mtk_change_mtu()
does the same write on every MTU change. So the field holds the OR of
every value written and can only grow:

  open at MTU 1500:            0x3000 + 1536 = 0x3600
  close, set MTU 9000, reopen: 0x3600 | 0x5400 = 0x7600
  set MTU 1500 again:          stays 0x7600

A 1536 to 2048 change likewise gives 0x3600 | 0x3800 = 0x3e00.

There is also no field mask to go with MTK_PDMA_LRO_SDL and
MTK_RX_CFG_SDL_OFFSET. 0x3000 + 9216 = 0x5400 needs 15 bits. If the
field is narrower than that, does the shift by 16 set bit 30, which
belongs to another control field?

mtk_poll_rx() assumes one frame per descriptor. Could a DMA frame limit
that doesn't match buf_size cause the hardware to split frames?

[ ... ]

> @@ -4362,6 +4415,17 @@ static int mtk_change_mtu(struct net_device *dev, int new_mtu)
>  		return -EINVAL;
>  	}
>  
> +	/* The RX rings are shared and cannot be resized in place while DMA is
> +	 * running.  Until the reallocation worker lands, reject a runtime MTU
> +	 * change that would need a different ring buffer size.
> +	 */
> +	max_mtu = mtk_max_gmac_mtu(eth);
> +	if (new_mtu > max_mtu)
> +		max_mtu = new_mtu;
> +	rx_buf_len = mtk_rx_buf_len_from_mtu(max_mtu);
> +	if (refcount_read(&eth->dma_refcnt) && rx_buf_len != eth->rx_buf_len)
> +		return -EBUSY;

[Severity: Medium]
Should this only reject changes that need larger buffers, that is
rx_buf_len > eth->rx_buf_len?

max_mtu includes this device's old MTU, so lowering the largest MTU is
allowed once and eth->rx_buf_len stays at the old ring size. After that,
smaller changes that would fit in the existing buffers fail:

  DMA running, A at 9000, rx_buf_len 9216
  A 9000 -> 1500: max_mtu 9000, 9216 == 9216, allowed
  A 1500 -> 1400: 1536 != 9216, -EBUSY
  B 1500 -> 2000: 2048 != 9216, -EBUSY

>From then on, no MTU can be lowered until every MAC is closed. DSA user
port MTU changes that propagate to the conduit fail the same way.

> +
>  	mtk_set_mcr_max_rx(mac, length);
>  	WRITE_ONCE(dev->mtu, new_mtu);

[Severity: Medium]
The commit message says:

  The RX rings and their buffers are sized for the largest MTU of any MAC,
  so the MTU limits of the MACs are now written from mtk_mac_config() and
  mtk_change_mtu() with eth->rx_buf_len rather than a fixed 2K.

Is that accurate? This call still passes the per-device length.
mtk_hw_init() also still uses the per-device MTU:

	mtk_set_mcr_max_rx(netdev_priv(dev),
			   dev->mtu + MTK_RX_ETH_HLEN);

Only mtk_mac_config() uses eth->rx_buf_len, so the MAC limit depends on
which of the three call sites ran last. The baseline never wrote a fixed
2K either: both earlier call sites used dev->mtu + MTK_RX_ETH_HLEN.

The commit message also lists "add dynamic rx buffer adjustment support"
as part of this forward-port. However, the comment above refers to a
reallocation worker that hasn't landed yet, and runtime changes return
-EBUSY. Could the commit message be updated to match the code?

[ ... ]

> @@ -5014,8 +5087,15 @@ 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;
> +	/* 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_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;

[Severity: Medium]
Was dropping the MT7628 case intended? MT7628 used to get max_mtu 1518
(MTK_MAX_RX_LENGTH - MTK_RX_ETH_HLEN). Commit 4fd59792097a added that
limit on purpose ("MT762x HW, except for MT7628, supports frame length
up to 2048"). MT7628 now falls into the else branch and gets 2030.

Nothing else limits the frame size on MT7628. mtk_set_mcr_max_rx()
returns early for MTK_SOC_MT7628, and its RX descriptors are written as
RX_DMA_LSO with no PLEN0. With the interfaces down, mtk_change_mtu()
would now report success for MTUs 1519..2030 on hardware that cannot
receive frames above 1536 bytes.

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

Re: [PATCH net-next v2 2/2] net: ethernet: mtk_eth_soc: add 9K jumbo frame

[Sandeep Haemoon]

Thanks. All ten findings are valid; v3 fixes them.  The same applies to the
two findings in the review of 1/2.

- XDP: mtk_xdp_setup() and mtk_change_mtu() now compare the shared
  mtk_rx_buf_len()/eth->rx_buf_len against MTK_PP_MAX_BUF_SIZE, so a
  program can no longer attach when the rings have no page_pool, whether
  the large MTU is set on the same MAC or on another one.
- Every SoC: the RX buffer floor stays at 2K, so MTUs up to 2K keep
  working at runtime, DSA user ports included, and mtk_change_mtu() now
  rejects only a change that needs larger buffers (-EBUSY on growth), so
  lowering an MTU no longer locks the port out of further changes.
- MT7628: the 1518 max_mtu branch is restored; dropping it was not
  intended.
- Commit message: corrected. The series does not include the runtime
  reallocation, and mtk_hw_init() uses the shared length now, so the
  description matches the three call sites.
- PDMA RX SDL: the field is masked and replaced instead of OR-ed, so it
  no longer accumulates across opens.
- XMAC_RX_CFG2: read-modify-write, so the bits outside the length field
  survive.
- MAC limits: rewritten on every mac_config(), so a reopen after a
  smaller MTU narrows the MACs that still carry the old limit.
- Order-2 buffers: acknowledged as the cost of the downstream design and
  documented; the unused QDMA ring carrying half of it is a separate
  follow-up.
- Comment and commit text: fixed.

The page_pool gate in 1/2 is also changed as suggested there: it now
tests ring->buf_size <= MTK_PP_MAX_BUF_SIZE, since frag_size is computed
with the netdev_alloc_frag() layout and does not describe what the pool
can map.

v3: https://lore.kernel.org/all/patch-aad4cfa8d9522d1f7dbc9893ea12e813@loljews.com/

pw-bot: cr