[PATCH net-next v3 0/8] net: bcmgenet: support larger MTUs

[PATCH net-next v3 0/8] net: bcmgenet: support larger MTUs

[Nicolai Buchwitz]

Larger MTU aka. jumbo frames are requested often by Raspberry Pi users, but
the hardware has its quirks with larger frames, so it never got done. Dave
Stevenson's PoC [1] had the receive side working and Justin checked the RTL
what the hardware does with long frames, which got me past the rest.

GENET never sets dev->max_mtu, and the packet ready thresholds cut frames
off at their 2048 byte reset default. With this the receive dropped them as
fragmented and transmit never sent them out. See commit messages of the
patches for more details.

Patch 1 fixes a transmit error exit that left queued frames unpublished,
the padding in patch 6 adds two more of them. Patches 2 to 5 and 7
implement the MTU as far as one descriptor reaches (3564 bytes with 4K
pages). Patch 6 pads around a transmit quirk where a frame ending just past
the threshold stops the transmitter [3]. Patch 8 reassembles the fragments
the hardware produces above it, which gets the ceiling to 16347 [2].

Tested on a CM4 (GENET v5). MTU 1500 is unchanged, MTU 9000 reaches 991/987
Mbit/s. Pierre-Marin Leclercq tested on a Pi 4 (also GENET v5) with various
MTU sizes.

[1] https://github.com/raspberrypi/linux/pull/7614
[2] https://github.com/raspberrypi/linux/pull/7617#issuecomment-5669564355
[3] https://github.com/raspberrypi/linux/pull/7617#issuecomment-5735557324

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
Changes in v3 (all from clashiko):
- new patch 1: ring the doorbell on the transmit drop exits, the last
  frame of an xmit_more batch left the earlier ones unpublished
- patch 6 (pad) now precedes the max_mtu change, a bisect between them
  could stall the transmitter on large page kernels, the
  ENET_MAX_FRAME_LEN(mtu) helper moves into patch 5 for that
- patch 6: mention the checksum fallback in the commit message
- patch 8: read the status block before the FCS trim, pskb_trim() can
  release the page it lives in
- patch 8: drop a start descriptor without end that cannot hold the
  Ethernet header
- patch 8: threshold comment, note on the TX descriptor length
- Link to v2: https://lore.kernel.org/r/20261006-nb-genet-mtu-nn-v2-v2-0-c4be21151912@tipi-net.de

Changes in v2:
- patch 5: resync the PHY state machine after the MTU restart, a latched
  link event was lost and an internal PHY is never polled (v1-4)
- patch 6: move the checksum fallback here from patch 7, a bisect between
  them dropped checksummed frames on large page kernels
- patch 6: note the trailer limitation of the padding
- patch 7: pskb_trim() for the FCS trim, a reassembled frame is paged
- patch 7: latch frag_drop on a continuation without a start, the counter
  inflated per descriptor
- comment and commit message corrections in patches 2, 4 and 5
- Link to v1: https://lore.kernel.org/r/20261002-nb-genet-mtu-nn-v2-v1-0-96dc6d54cbee@tipi-net.de

To: Doug Berger <opendmb@gmail.com>
To: Florian Fainelli <florian.fainelli@broadcom.com>
To: Nicolai Buchwitz <nb@tipi-net.de>
To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
To: Andrew Lunn <andrew+netdev@lunn.ch>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Nicolai Buchwitz (8):
      net: bcmgenet: ring the doorbell when the last frame of a batch is dropped
      net: bcmgenet: let the caller decide whether to start the PHY
      net: bcmgenet: allow a continuation descriptor without the alignment pad
      net: bcmgenet: rename ENET_MAX_MTU_SIZE to ENET_MAX_FRAME_LEN
      net: bcmgenet: derive the receive buffer length from the MTU
      net: bcmgenet: pad transmit frames out of the packet ready window
      net: bcmgenet: allow the MTU to be changed
      net: bcmgenet: reassemble jumbo frames from status block fragments

 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 368 ++++++++++++++++++++++---
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  21 +-
 2 files changed, 342 insertions(+), 47 deletions(-)
---
base-commit: a5e7d8e446af9803e37a3b6a4d416fb41178348f
change-id: 20261002-nb-genet-mtu-nn-v2-402b5e6f306f

Best regards,
--  
Nicolai Buchwitz <nb@tipi-net.de>

[PATCH net-next v3 1/8] net: bcmgenet: ring the doorbell when the last frame of a batch is dropped

[Nicolai Buchwitz]

bcmgenet_xmit() only writes the producer index at the end of a batch. The
error exits return before that write. If the dropped frame was the last of
an xmit_more batch, the frames queued before it stay in the ring until the
next transmit. The queue is still running, so the watchdog does not fire.

Write the producer index on those exits too.

Fixes: ddd0ca5d60b3 ("net: bcmgenet: add support for xmit_more")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 4c9db2f9fc25..06ec8c211dd7 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2119,6 +2119,15 @@ static void bcmgenet_hide_tsb(struct sk_buff *skb)
 	__skb_pull(skb, sizeof(struct status_64));
 }
 
+static void bcmgenet_tx_kick(struct bcmgenet_priv *priv,
+			     struct bcmgenet_tx_ring *ring,
+			     struct netdev_queue *txq)
+{
+	if (!netdev_xmit_more() || netif_xmit_stopped(txq))
+		bcmgenet_tdma_ring_writel(priv, ring->index,
+					  ring->prod_index, TDMA_PROD_INDEX);
+}
+
 static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
@@ -2155,10 +2164,8 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* add the Transmit Status Block */
 	skb = bcmgenet_add_tsb(dev, skb, ring);
-	if (!skb) {
-		ret = NETDEV_TX_OK;
-		goto out;
-	}
+	if (!skb)
+		goto drop;
 
 	for (i = 0; i <= nr_frags; i++) {
 		tx_cb_ptr = bcmgenet_get_txcb(priv, ring);
@@ -2183,7 +2190,6 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 		if (ret) {
 			priv->mib.tx_dma_failed++;
 			netif_err(priv, tx_err, dev, "Tx DMA map failed\n");
-			ret = NETDEV_TX_OK;
 			goto out_unmap_frags;
 		}
 		dma_unmap_addr_set(tx_cb_ptr, dma_addr, mapping);
@@ -2225,10 +2231,7 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (ring->free_bds <= (MAX_SKB_FRAGS + 1))
 		netif_tx_stop_queue(txq);
 
-	if (!netdev_xmit_more() || netif_xmit_stopped(txq))
-		/* Packets are ready, update producer index */
-		bcmgenet_tdma_ring_writel(priv, ring->index,
-					  ring->prod_index, TDMA_PROD_INDEX);
+	bcmgenet_tx_kick(priv, ring, txq);
 out:
 	spin_unlock(&ring->lock);
 
@@ -2245,6 +2248,10 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	dev_kfree_skb(skb);
+drop:
+	/* The dropped frame may have been the last one of the batch */
+	bcmgenet_tx_kick(priv, ring, txq);
+	ret = NETDEV_TX_OK;
 	goto out;
 }
 

-- 
2.53.0

[PATCH net-next v3 2/8] net: bcmgenet: let the caller decide whether to start the PHY

[Nicolai Buchwitz]

bcmgenet_netif_stop() already takes stop_phy, bcmgenet_netif_start() does
not. The MTU change in a later patch leaves the PHY running while the
datapath goes down and comes back, and phy_start() expects a stopped PHY.

Add the same parameter to the start side.

No functional change.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 06ec8c211dd7..86c8f8f8fe15 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3355,7 +3355,7 @@ static void bcmgenet_get_hw_addr(struct bcmgenet_priv *priv,
 	put_unaligned_be16(addr_tmp, &addr[4]);
 }
 
-static void bcmgenet_netif_start(struct net_device *dev)
+static void bcmgenet_netif_start(struct net_device *dev, bool start_phy)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
 
@@ -3372,7 +3372,8 @@ static void bcmgenet_netif_start(struct net_device *dev)
 	/* Monitor link interrupts now */
 	bcmgenet_link_intr_enable(priv);
 
-	phy_start(dev->phydev);
+	if (start_phy)
+		phy_start(dev->phydev);
 }
 
 static int bcmgenet_open(struct net_device *dev)
@@ -3435,7 +3436,7 @@ static int bcmgenet_open(struct net_device *dev)
 
 	bcmgenet_phy_pause_set(dev, priv->rx_pause, priv->tx_pause);
 
-	bcmgenet_netif_start(dev);
+	bcmgenet_netif_start(dev, true);
 
 	netif_tx_start_all_queues(dev);
 
@@ -4319,7 +4320,7 @@ static int bcmgenet_resume(struct device *d)
 	if (!device_may_wakeup(d))
 		phy_resume(dev->phydev);
 
-	bcmgenet_netif_start(dev);
+	bcmgenet_netif_start(dev, true);
 
 	netif_device_attach(dev);
 

-- 
2.53.0

[PATCH net-next v3 3/8] net: bcmgenet: allow a continuation descriptor without the alignment pad

[Nicolai Buchwitz]

A frame longer than the packet ready threshold arrives in several
descriptors, each with its own status block. Only the first one also
carries the two alignment bytes. The length check assumes the pad is always
there, so a continuation holding a single byte looks a byte too short and
the whole frame is dropped.

Account for the pad on the first descriptor only.

The MTU cannot produce a frame past the threshold yet, so nothing hits this
today. It is preparation for the larger MTU.

Tested-by: Pierre-Marin Leclercq <pierremarinleclercq88@gmail.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 86c8f8f8fe15..9f9c3fde8725 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -53,7 +53,8 @@
 
 /* Page pool RX buffer layout:
  * RSB(64) + pad(2) | frame data | skb_shared_info
- * The HW writes the 64B RSB + 2B alignment padding before the frame.
+ * The HW writes the 64B RSB before every descriptor of a frame. Only the
+ * first one also gets the 2B alignment padding.
  */
 #define GENET_RSB_PAD		(sizeof(struct status_64) + 2)
 
@@ -2336,6 +2337,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 		unsigned int rx_offset, rx_size;
 		struct status_64 *status;
 		struct page *rx_page;
+		unsigned int min_len;
 		void *hard_start;
 		__be16 rx_csum;
 
@@ -2372,8 +2374,12 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 			  __func__, p_index, ring->c_index,
 			  ring->read_ptr, dma_length_status);
 
+		/* Only the first descriptor carries the alignment pad */
+		min_len = dma_flag & DMA_SOP ? GENET_RSB_PAD
+					     : sizeof(struct status_64);
+
 		/* Reject lengths that would underflow the SKB build path. */
-		if (unlikely(len > RX_BUF_LENGTH || len < GENET_RSB_PAD)) {
+		if (unlikely(len > RX_BUF_LENGTH || len < min_len)) {
 			netif_err(priv, rx_status, dev,
 				  "invalid packet length %d\n", len);
 			BCMGENET_STATS64_INC(stats, length_errors);

-- 
2.53.0

[PATCH net-next v3 4/8] net: bcmgenet: rename ENET_MAX_MTU_SIZE to ENET_MAX_FRAME_LEN

[Nicolai Buchwitz]

ENET_MAX_MTU_SIZE holds a frame length, not an MTU. Both users program it
into hardware that expects a frame length. The name is wrong once the MTU
is no longer fixed at ETH_DATA_LEN.

No functional change.

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
 drivers/net/ethernet/broadcom/genet/bcmgenet.h | 7 +++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 9f9c3fde8725..83ffc4a2a2a7 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2646,7 +2646,7 @@ static void init_umac(struct bcmgenet_priv *priv)
 			     UMAC_MIB_CTRL);
 	bcmgenet_umac_writel(priv, 0, UMAC_MIB_CTRL);
 
-	bcmgenet_umac_writel(priv, ENET_MAX_MTU_SIZE, UMAC_MAX_FRAME_LEN);
+	bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN, UMAC_MAX_FRAME_LEN);
 
 	/* init tx registers, enable TSB */
 	reg = bcmgenet_tbuf_ctrl_get(priv);
@@ -2752,7 +2752,7 @@ static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
 
 	/* Set flow period for ring != 0 */
 	if (index)
-		flow_period_val = ENET_MAX_MTU_SIZE << 16;
+		flow_period_val = ENET_MAX_FRAME_LEN << 16;
 
 	bcmgenet_tdma_ring_writel(priv, index, 0, TDMA_PROD_INDEX);
 	bcmgenet_tdma_ring_writel(priv, index, 0, TDMA_CONS_INDEX);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 86f2aed20dbe..501dd1256693 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -28,12 +28,11 @@
 /* which ring is descriptor based */
 #define DESC_INDEX				16
 
-/* Body(1500) + EH_SIZE(14) + VLANTAG(4) + BRCMTAG(6) + FCS(4) = 1528.
- * 1536 is multiple of 256 bytes
- */
 #define ENET_BRCM_TAG_LEN	6
 #define ENET_PAD		8
-#define ENET_MAX_MTU_SIZE	(ETH_DATA_LEN + ETH_HLEN + VLAN_HLEN + \
+
+/* Longest frame the MAC must accept for the default MTU */
+#define ENET_MAX_FRAME_LEN	(ETH_DATA_LEN + ETH_HLEN + VLAN_HLEN + \
 				 ENET_BRCM_TAG_LEN + ETH_FCS_LEN + ENET_PAD)
 #define DMA_MAX_BURST_LENGTH    0x10
 

-- 
2.53.0

[PATCH net-next v3 5/8] net: bcmgenet: derive the receive buffer length from the MTU

[Nicolai Buchwitz]

The receive buffer length is a fixed 2048 bytes. The packet ready
thresholds keep whatever value the reset left. Neither follows the MTU.

Compute the receive threshold from the MTU and program it into RBUF. The
buffer length follows from it, with the status block on top. The MTU is
still fixed at ETH_DATA_LEN, so the threshold comes out at the reset
default and the buffer only grows by the status block the hardware already
wrote. UMAC_MAX_FRAME_LEN and the flow period are derived from the MTU
as well.

Program the transmit threshold at its maximum as well. It sets how much of
a frame the MAC holds before it starts sending, and holding less buys
nothing. A later patch lowers it for the few MTUs that need the room.

Suggested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 88 +++++++++++++++++++++-----
 drivers/net/ethernet/broadcom/genet/bcmgenet.h | 14 +++-
 2 files changed, 84 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 83ffc4a2a2a7..4a4b0c46f898 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -48,18 +48,36 @@
 #define GENET_Q0_TX_BD_CNT	\
 	(TOTAL_DESC - priv->hw_params->tx_queues * priv->hw_params->tx_bds_per_q)
 
-#define RX_BUF_LENGTH		2048
 #define SKB_ALIGNMENT		32
 
+/* RBUF and TBUF hand a frame to the DMA once the threshold is reached. Both
+ * registers are 8 bit in units of 16 bytes and want a multiple of the 256
+ * byte burst size, so 0xf0 is the largest usable value.
+ */
+#define ENET_THLD_UNIT		16
+#define ENET_THLD_BURST		256
+#define ENET_THLD_DEFAULT	0x80
+#define ENET_THLD_MAX		0xf0
+
 /* Page pool RX buffer layout:
  * RSB(64) + pad(2) | frame data | skb_shared_info
  * The HW writes the 64B RSB before every descriptor of a frame. Only the
  * first one also gets the 2B alignment padding.
  */
-#define GENET_RSB_PAD		(sizeof(struct status_64) + 2)
+#define GENET_RBUF_ALIGN	2
+#define GENET_RSB_PAD		(sizeof(struct status_64) + GENET_RBUF_ALIGN)
 
-/* RX buffer plus the skb_shared_info napi_build_skb() places behind it */
-#define GENET_RX_BUF_SIZE	SKB_HEAD_ALIGN(RX_BUF_LENGTH)
+/* A descriptor never spans more than one page, which also holds
+ * skb_shared_info behind the frame, so on 4K pages the page bounds the
+ * threshold before the register does. Larger pages fit several descriptors.
+ */
+#define ENET_SHINFO_LEN		SKB_DATA_ALIGN(sizeof(struct skb_shared_info))
+#define ENET_THLD_PAGE_LEN	round_down(PAGE_SIZE - ENET_SHINFO_LEN - \
+					   sizeof(struct status_64), \
+					   ENET_THLD_BURST)
+#define ENET_THLD_MAX_LEN	min_t(unsigned int, \
+				      ENET_THLD_MAX * ENET_THLD_UNIT, \
+				      ENET_THLD_PAGE_LEN)
 
 /* Tx/Rx DMA register offset, skip 256 descriptors */
 #define WORDS_PER_BD(p)		(p->hw_params->words_per_bd)
@@ -2260,7 +2278,7 @@ static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring,
 			      struct enet_cb *cb)
 {
 	struct bcmgenet_priv *priv = ring->priv;
-	unsigned int size = GENET_RX_BUF_SIZE;
+	unsigned int size = SKB_HEAD_ALIGN(priv->rx_buf_len);
 	unsigned int offset;
 	dma_addr_t mapping;
 	struct page *page;
@@ -2275,7 +2293,7 @@ static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring,
 
 	/* page_pool handles DMA mapping via PP_FLAG_DMA_MAP */
 	mapping = page_pool_get_dma_addr(page) + offset;
-	dma_sync_single_for_device(&priv->pdev->dev, mapping, RX_BUF_LENGTH,
+	dma_sync_single_for_device(&priv->pdev->dev, mapping, priv->rx_buf_len,
 				   DMA_FROM_DEVICE);
 
 	cb->rx_page = page;
@@ -2354,10 +2372,10 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 		}
 
 		/* Sync the full buffer; the HW may have written anywhere
-		 * up to RX_BUF_LENGTH.
+		 * up to priv->rx_buf_len.
 		 */
 		page_pool_dma_sync_for_cpu(ring->page_pool, rx_page, rx_offset,
-					   RX_BUF_LENGTH);
+					   priv->rx_buf_len);
 
 		hard_start = page_address(rx_page) + rx_offset;
 		status = (struct status_64 *)hard_start;
@@ -2379,7 +2397,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 					     : sizeof(struct status_64);
 
 		/* Reject lengths that would underflow the SKB build path. */
-		if (unlikely(len > RX_BUF_LENGTH || len < min_len)) {
+		if (unlikely(len > priv->rx_buf_len || len < min_len)) {
 			netif_err(priv, rx_status, dev,
 				  "invalid packet length %d\n", len);
 			BCMGENET_STATS64_INC(stats, length_errors);
@@ -2630,6 +2648,44 @@ static void bcmgenet_link_intr_enable(struct bcmgenet_priv *priv)
 	bcmgenet_intrl2_0_writel(priv, int0_enable, INTRL2_CPU_MASK_CLEAR);
 }
 
+/* Receive threshold in register units. Covers the alignment bytes and the
+ * frame, but not the status block, which the hardware adds on top.
+ */
+static unsigned int bcmgenet_pkt_rdy_thld(unsigned int mtu)
+{
+	unsigned int len = GENET_RBUF_ALIGN + mtu + ETH_HLEN + VLAN_HLEN;
+
+	len = round_up(len, ENET_THLD_BURST) / ENET_THLD_UNIT;
+
+	/* Keep the reset default for the common MTUs */
+	return clamp_t(unsigned int, len, ENET_THLD_DEFAULT,
+		       ENET_THLD_MAX_LEN / ENET_THLD_UNIT);
+}
+
+/* A buffer has to hold everything the threshold lets the hardware deliver */
+static unsigned int bcmgenet_rx_buf_len(unsigned int mtu)
+{
+	return sizeof(struct status_64) +
+	       bcmgenet_pkt_rdy_thld(mtu) * ENET_THLD_UNIT;
+}
+
+/* Program the MTU dependent registers. Call with the MAC disabled. */
+static void bcmgenet_set_mtu_regs(struct bcmgenet_priv *priv, unsigned int mtu)
+{
+	u32 thld = bcmgenet_pkt_rdy_thld(mtu);
+
+	bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), UMAC_MAX_FRAME_LEN);
+
+	/* GENET v1 maps other registers at these offsets */
+	if (GENET_IS_V1(priv))
+		return;
+
+	bcmgenet_rbuf_writel(priv, thld, RBUF_PKT_RDY_THLD);
+	bcmgenet_writel(ENET_THLD_MAX,
+			priv->base + priv->hw_params->tbuf_offset +
+			TBUF_PKT_RDY_THLD);
+}
+
 static void init_umac(struct bcmgenet_priv *priv)
 {
 	struct device *kdev = &priv->pdev->dev;
@@ -2646,7 +2702,7 @@ static void init_umac(struct bcmgenet_priv *priv)
 			     UMAC_MIB_CTRL);
 	bcmgenet_umac_writel(priv, 0, UMAC_MIB_CTRL);
 
-	bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN, UMAC_MAX_FRAME_LEN);
+	bcmgenet_set_mtu_regs(priv, priv->dev->mtu);
 
 	/* init tx registers, enable TSB */
 	reg = bcmgenet_tbuf_ctrl_get(priv);
@@ -2752,7 +2808,7 @@ static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
 
 	/* Set flow period for ring != 0 */
 	if (index)
-		flow_period_val = ENET_MAX_FRAME_LEN << 16;
+		flow_period_val = ENET_MAX_FRAME_LEN(priv->dev->mtu) << 16;
 
 	bcmgenet_tdma_ring_writel(priv, index, 0, TDMA_PROD_INDEX);
 	bcmgenet_tdma_ring_writel(priv, index, 0, TDMA_CONS_INDEX);
@@ -2762,7 +2818,7 @@ static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
 				  TDMA_FLOW_PERIOD);
 	bcmgenet_tdma_ring_writel(priv, index,
 				  ((size << DMA_RING_SIZE_SHIFT) |
-				   RX_BUF_LENGTH), DMA_RING_BUF_SIZE);
+				   priv->rx_buf_len), DMA_RING_BUF_SIZE);
 
 	/* Set start and end address, read and write pointers */
 	bcmgenet_tdma_ring_writel(priv, index, start_ptr * words_per_bd,
@@ -2781,8 +2837,9 @@ static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
 static int bcmgenet_rx_ring_create_pool(struct bcmgenet_priv *priv,
 					struct bcmgenet_rx_ring *ring)
 {
-	/* Buffers share a page. bcmgenet_rx_refill() syncs each one for the
-	 * device, PP_FLAG_DMA_SYNC_DEV would sync the whole page.
+	/* Buffers may share a page, depending on PAGE_SIZE and the MTU.
+	 * bcmgenet_rx_refill() syncs each one for the device,
+	 * PP_FLAG_DMA_SYNC_DEV would sync the whole page.
 	 */
 	struct page_pool_params pp_params = {
 		.order = 0,
@@ -2845,7 +2902,7 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
 	bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_CONS_INDEX);
 	bcmgenet_rdma_ring_writel(priv, index,
 				  ((size << DMA_RING_SIZE_SHIFT) |
-				   RX_BUF_LENGTH), DMA_RING_BUF_SIZE);
+				   priv->rx_buf_len), DMA_RING_BUF_SIZE);
 	bcmgenet_rdma_ring_writel(priv, index,
 				  (DMA_FC_THRESH_LO <<
 				   DMA_XOFF_THRESHOLD_SHIFT) |
@@ -4114,6 +4171,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
 	/* Mii wait queue */
 	init_waitqueue_head(&priv->wq);
 	bcmgenet_hfb_init(priv);
+	priv->rx_buf_len = bcmgenet_rx_buf_len(dev->mtu);
 	INIT_WORK(&priv->bcmgenet_irq_work, bcmgenet_irq_task);
 
 	priv->clk_wol = devm_clk_get_optional(&priv->pdev->dev, "enet-wol");
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 501dd1256693..33a23f899c4e 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -31,9 +31,13 @@
 #define ENET_BRCM_TAG_LEN	6
 #define ENET_PAD		8
 
-/* Longest frame the MAC must accept for the default MTU */
-#define ENET_MAX_FRAME_LEN	(ETH_DATA_LEN + ETH_HLEN + VLAN_HLEN + \
-				 ENET_BRCM_TAG_LEN + ETH_FCS_LEN + ENET_PAD)
+/* Longest frame the MAC must accept for a given MTU. ENET_PAD is slack the
+ * driver has always carried, it rounded the default up to 1536 from 1528.
+ */
+#define ENET_FRAME_OVERHEAD	(ETH_HLEN + VLAN_HLEN + ENET_BRCM_TAG_LEN + \
+				 ETH_FCS_LEN + ENET_PAD)
+#define ENET_MAX_FRAME_LEN(mtu)	((mtu) + ENET_FRAME_OVERHEAD)
+
 #define DMA_MAX_BURST_LENGTH    0x10
 
 /* misc. configuration */
@@ -218,6 +222,8 @@ struct bcmgenet_rx_stats64 {
 #define  RBUF_ALIGN_2B			(1 << 1)
 #define  RBUF_BAD_DIS			(1 << 2)
 
+#define RBUF_PKT_RDY_THLD		0x08
+
 #define RBUF_STATUS			0x0C
 #define  RBUF_STATUS_WOL		(1 << 0)
 #define  RBUF_STATUS_MPD_INTR_ACTIVE	(1 << 1)
@@ -248,6 +254,7 @@ struct bcmgenet_rx_stats64 {
 #define TBUF_CTRL			0x00
 #define  TBUF_64B_EN			(1 << 0)
 #define TBUF_BP_MC			0x0C
+#define TBUF_PKT_RDY_THLD		0x10
 #define TBUF_ENERGY_CTRL		0x14
 #define  TBUF_EEE_EN			(1 << 0)
 #define  TBUF_PM_EN			(1 << 1)
@@ -612,6 +619,7 @@ struct bcmgenet_priv {
 	void __iomem *rx_bds;
 	struct enet_cb *rx_cbs;
 	unsigned int num_rx_bds;
+	unsigned int rx_buf_len;
 	struct bcmgenet_rxnfc_rule rxnfc_rules[MAX_NUM_OF_FS_RULES];
 	struct list_head rxnfc_list;
 

-- 
2.53.0

Re: [PATCH net-next v3 5/8] net: bcmgenet: derive the receive buffer length from the MTU

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0

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] In bcmgenet_set_mtu_regs()
  (drivers/net/ethernet/broadcom/genet/bcmgenet.c), UMAC_MAX_FRAME_LEN…

--

net: bcmgenet: derive the receive buffer length from the MTU

This commit computes the RBUF packet ready threshold from the MTU and
derives the receive buffer length from it, replacing the fixed 2048 byte
RX_BUF_LENGTH. UMAC_MAX_FRAME_LEN and the TDMA flow period are now also
derived from the MTU, and the TBUF threshold is programmed at its maximum.

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 83ffc4a2a2a72..4a4b0c46f8986 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c

[ ... ]

> @@ -2630,6 +2648,44 @@ static void bcmgenet_link_intr_enable(struct bcmgenet_priv *priv)

[ ... ]

> +/* Program the MTU dependent registers. Call with the MAC disabled. */
> +static void bcmgenet_set_mtu_regs(struct bcmgenet_priv *priv, unsigned int mtu)
> +{
> +	u32 thld = bcmgenet_pkt_rdy_thld(mtu);
> +
> +	bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), UMAC_MAX_FRAME_LEN);

[Severity: Medium]

Will this drop standard size frames once the MTU is lowered?

Before this patch, UMAC_MAX_FRAME_LEN was always 1536. Now it is
ENET_MAX_FRAME_LEN(mtu), which is mtu + 36, and nothing keeps it from
going lower.

The commit message says:

  The MTU is still fixed at ETH_DATA_LEN

Only the upper bound is fixed, though. bcmgenet does not override
min_mtu, so ether_setup() still allows any MTU from 68 up to 1500:

net/ethernet/eth.c:ether_setup() {
    ...
	dev->min_mtu		= ETH_MIN_MTU;
	dev->max_mtu		= ETH_DATA_LEN;
    ...
}

This commit has no ndo_change_mtu, so __netif_set_mtu() only updates
dev->mtu. The next open or full resume then writes the smaller value:

init_umac()
  bcmgenet_set_mtu_regs(priv, priv->dev->mtu)
    bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), UMAC_MAX_FRAME_LEN)

Take an MTU below about 1482, for example 1400 for a tunnel overlay, or
1280. Full size 1514/1518 byte frames from LAN peers would then be over
the MAC limit. They would get DMA_RX_LG and be dropped in
bcmgenet_desc_rx():

	if (dma_flag & DMA_RX_LG)
		u64_stats_inc(&stats->length_errors);
	...
	page_pool_put_full_page(ring->page_pool, rx_page,
				true);

Before this patch, the MAC accepted these frames whatever the MTU was.
Full size UDP, broadcast, multicast or forwarded traffic that used to be
received would now be lost.

This still happens at the end of the series. The later commit "net:
bcmgenet: allow the MTU to be changed" adds bcmgenet_change_mtu(), which
applies the smaller limit straight away on a running interface.

That later commit does fix two related cases here. One is the stale,
too-small limit left after raising the MTU on a running interface. The
other is the WoL fast resume mismatch between TDMA_FLOW_PERIOD and
UMAC_MAX_FRAME_LEN.

Would it work to program max(mtu, ETH_DATA_LEN) into UMAC_MAX_FRAME_LEN,
and to update the commit message to match?

> +
> +	/* GENET v1 maps other registers at these offsets */
> +	if (GENET_IS_V1(priv))
> +		return;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-nb-genet-mtu-nn-v2-v3-0-74a796c019ce%40tipi-net.de

Re: [PATCH net-next v3 5/8] net: bcmgenet: derive the receive buffer length from the MTU

[Nicolai Buchwitz]

On 8.10.2026 11:40, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> 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] In bcmgenet_set_mtu_regs()
>   (drivers/net/ethernet/broadcom/genet/bcmgenet.c), UMAC_MAX_FRAME_LEN…
> 
> --
> 
> net: bcmgenet: derive the receive buffer length from the MTU
> 
> This commit computes the RBUF packet ready threshold from the MTU and
> derives the receive buffer length from it, replacing the fixed 2048 
> byte
> RX_BUF_LENGTH. UMAC_MAX_FRAME_LEN and the TDMA flow period are now also
> derived from the MTU, and the TBUF threshold is programmed at its 
> maximum.
> 
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
>> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> index 83ffc4a2a2a72..4a4b0c46f8986 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> 
> [ ... ]
> 
>> @@ -2630,6 +2648,44 @@ static void bcmgenet_link_intr_enable(struct 
>> bcmgenet_priv *priv)
> 
> [ ... ]
> 
>> +/* Program the MTU dependent registers. Call with the MAC disabled. 
>> */
>> +static void bcmgenet_set_mtu_regs(struct bcmgenet_priv *priv, 
>> unsigned int mtu)
>> +{
>> +	u32 thld = bcmgenet_pkt_rdy_thld(mtu);
>> +
>> +	bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), 
>> UMAC_MAX_FRAME_LEN);
> 
> [Severity: Medium]
> 
> Will this drop standard size frames once the MTU is lowered?
> 
> Before this patch, UMAC_MAX_FRAME_LEN was always 1536. Now it is
> ENET_MAX_FRAME_LEN(mtu), which is mtu + 36, and nothing keeps it from
> going lower.
> 
> The commit message says:
> 
>   The MTU is still fixed at ETH_DATA_LEN
> 
> Only the upper bound is fixed, though. bcmgenet does not override
> min_mtu, so ether_setup() still allows any MTU from 68 up to 1500:
> 
> net/ethernet/eth.c:ether_setup() {
>     ...
> 	dev->min_mtu		= ETH_MIN_MTU;
> 	dev->max_mtu		= ETH_DATA_LEN;
>     ...
> }
> 
> This commit has no ndo_change_mtu, so __netif_set_mtu() only updates
> dev->mtu. The next open or full resume then writes the smaller value:
> 
> init_umac()
>   bcmgenet_set_mtu_regs(priv, priv->dev->mtu)
>     bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), 
> UMAC_MAX_FRAME_LEN)
> 
> Take an MTU below about 1482, for example 1400 for a tunnel overlay, or
> 1280. Full size 1514/1518 byte frames from LAN peers would then be over
> the MAC limit. They would get DMA_RX_LG and be dropped in
> bcmgenet_desc_rx():
> 
> 	if (dma_flag & DMA_RX_LG)
> 		u64_stats_inc(&stats->length_errors);
> 	...
> 	page_pool_put_full_page(ring->page_pool, rx_page,
> 				true);
> 
> Before this patch, the MAC accepted these frames whatever the MTU was.
> Full size UDP, broadcast, multicast or forwarded traffic that used to 
> be
> received would now be lost.
> 
> This still happens at the end of the series. The later commit "net:
> bcmgenet: allow the MTU to be changed" adds bcmgenet_change_mtu(), 
> which
> applies the smaller limit straight away on a running interface.
> 
> That later commit does fix two related cases here. One is the stale,
> too-small limit left after raising the MTU on a running interface. The
> other is the WoL fast resume mismatch between TDMA_FLOW_PERIOD and
> UMAC_MAX_FRAME_LEN.
> 
> Would it work to program max(mtu, ETH_DATA_LEN) into 
> UMAC_MAX_FRAME_LEN,
> and to update the commit message to match?

Sure. Will do in v4

---
pw-bot: cr

[PATCH net-next v3 6/8] net: bcmgenet: pad transmit frames out of the packet ready window

[Nicolai Buchwitz]

A frame that ends a few bytes past the transmit packet ready threshold
stops the transmitter as soon as a shorter frame follows. Tx DMA then
refuses to halt, so every later bcmgenet_init_dma() fails and the interface
cannot be opened again. IP fragmentation generates that pattern on every
datagram, full frames and a short tail.

The window starts one byte past the threshold and widens with it. On a CM4
it ends 28, 32 and 46 bytes past thresholds of 2560, 3584 and 3840. Link
speed makes no difference. Pad frames landing in it to 64 bytes past the
threshold.

Padding must not push a frame past what the peer accepts. Linux does not
bound how many VLAN tags a frame carries, so measure against the longest
frame the MAC has to accept rather than a tag count. For the MTUs where
such a frame would land in the window, lower the threshold instead. All
other MTUs keep the register maximum.

The MAC inserts a checksum only into a frame it holds in full. A frame
longer than the threshold gets its checksum computed in software.

The padding is appended to the frame, so a protocol that locates data from
the end of it, such as a DSA tail tag or a PRP trailer, sees the zeros
instead. Nothing below an MTU of 3809 is affected, since no frame reaches
the window there. Above it the alternatives are dropping the frame or
leaving the transmitter stalled.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Tested-by: Pierre-Marin Leclercq <pierremarinleclercq88@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 50 ++++++++++++++++++++++++--
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 4a4b0c46f898..17c8318c2038 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -59,6 +59,11 @@
 #define ENET_THLD_DEFAULT	0x80
 #define ENET_THLD_MAX		0xf0
 
+/* A frame ending just past the transmit threshold stops the transmitter once
+ * a shorter frame follows, so pad frames that land there this far past it.
+ */
+#define ENET_TX_SAFE_MARGIN	64
+
 /* Page pool RX buffer layout:
  * RSB(64) + pad(2) | frame data | skb_shared_info
  * The HW writes the 64B RSB before every descriptor of a frame. Only the
@@ -2176,6 +2181,29 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out;
 	}
 
+	/* The MAC only inserts a checksum into a frame it holds in full, and
+	 * silently drops a longer one, so fall back to software.
+	 */
+	if (unlikely(skb->len > priv->tx_thld_len) &&
+	    skb->ip_summed == CHECKSUM_PARTIAL) {
+		if (skb_checksum_help(skb)) {
+			BCMGENET_STATS64_INC((&ring->stats64), dropped);
+			dev_kfree_skb_any(skb);
+			goto drop;
+		}
+	}
+
+	/* Keep the frame out of the window just past the threshold */
+	if (unlikely(skb->len > priv->tx_thld_len &&
+		     skb->len < priv->tx_thld_len + ENET_TX_SAFE_MARGIN)) {
+		if (skb_put_padto(skb, priv->tx_thld_len + ENET_TX_SAFE_MARGIN)) {
+			BCMGENET_STATS64_INC((&ring->stats64), dropped);
+			goto drop;
+		}
+	}
+
+	nr_frags = skb_shinfo(skb)->nr_frags;
+
 	/* Retain how many bytes will be sent on the wire, without TSB inserted
 	 * by transmit checksum offload
 	 */
@@ -2662,6 +2690,23 @@ static unsigned int bcmgenet_pkt_rdy_thld(unsigned int mtu)
 		       ENET_THLD_MAX_LEN / ENET_THLD_UNIT);
 }
 
+/* Transmit threshold in register units. Frames landing in the window just
+ * past it are padded clear of it, so pick a threshold that leaves room for
+ * that padding inside the frame the MTU allows. Size the window against the
+ * longest frame the MAC has to accept, since the tag count is not bounded.
+ */
+static unsigned int bcmgenet_tx_pkt_rdy_thld(unsigned int mtu)
+{
+	unsigned int thld = ENET_THLD_MAX;
+
+	while (thld > ENET_THLD_DEFAULT &&
+	       ENET_MAX_FRAME_LEN(mtu) - ETH_FCS_LEN > thld * ENET_THLD_UNIT &&
+	       thld * ENET_THLD_UNIT + ENET_TX_SAFE_MARGIN > mtu + ETH_HLEN)
+		thld -= ENET_THLD_BURST / ENET_THLD_UNIT;
+
+	return thld;
+}
+
 /* A buffer has to hold everything the threshold lets the hardware deliver */
 static unsigned int bcmgenet_rx_buf_len(unsigned int mtu)
 {
@@ -2672,8 +2717,10 @@ static unsigned int bcmgenet_rx_buf_len(unsigned int mtu)
 /* Program the MTU dependent registers. Call with the MAC disabled. */
 static void bcmgenet_set_mtu_regs(struct bcmgenet_priv *priv, unsigned int mtu)
 {
+	u32 tx_thld = bcmgenet_tx_pkt_rdy_thld(mtu);
 	u32 thld = bcmgenet_pkt_rdy_thld(mtu);
 
+	priv->tx_thld_len = tx_thld * ENET_THLD_UNIT;
 	bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), UMAC_MAX_FRAME_LEN);
 
 	/* GENET v1 maps other registers at these offsets */
@@ -2681,8 +2728,7 @@ static void bcmgenet_set_mtu_regs(struct bcmgenet_priv *priv, unsigned int mtu)
 		return;
 
 	bcmgenet_rbuf_writel(priv, thld, RBUF_PKT_RDY_THLD);
-	bcmgenet_writel(ENET_THLD_MAX,
-			priv->base + priv->hw_params->tbuf_offset +
+	bcmgenet_writel(tx_thld, priv->base + priv->hw_params->tbuf_offset +
 			TBUF_PKT_RDY_THLD);
 }
 
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 33a23f899c4e..ac1d0aab5d75 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -620,6 +620,7 @@ struct bcmgenet_priv {
 	struct enet_cb *rx_cbs;
 	unsigned int num_rx_bds;
 	unsigned int rx_buf_len;
+	unsigned int tx_thld_len;
 	struct bcmgenet_rxnfc_rule rxnfc_rules[MAX_NUM_OF_FS_RULES];
 	struct list_head rxnfc_list;
 

-- 
2.53.0

Re: [PATCH net-next v3 6/8] net: bcmgenet: pad transmit frames out of the packet ready window

[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] In bcmgenet_xmit()
  (drivers/net/ethernet/broadcom/genet/bcmgenet.c), any frame whose…
- [Low] The commit message describes the transmit stall (Tx DMA will not
  halt, bcmgenet_init_dma() fails, the interface cannot be reopened) as…

--

net: bcmgenet: pad transmit frames out of the packet ready window

This patch picks a lower transmit packet ready threshold for MTUs where
the longest frame would end just past it. It pads frames that end within
64 bytes past the threshold so they clear the window. Frames longer than
the threshold that ask for checksum offload get their checksum computed
in software.

> A frame that ends a few bytes past the transmit packet ready threshold
> stops the transmitter as soon as a shorter frame follows. Tx DMA then
> refuses to halt, so every later bcmgenet_init_dma() fails and the interface
> cannot be opened again.

[Severity: Low]

Can this stall happen at this commit? genet does not set max_mtu or
provide ndo_change_mtu yet, so the MTU stays capped at the ether_setup()
default:

net/ethernet/eth.c:ether_setup() {
    ...
	dev->max_mtu		= ETH_DATA_LEN;
    ...
}

With an MTU of 1500, ENET_MAX_FRAME_LEN(mtu) - ETH_FCS_LEN stays well
below 3840. bcmgenet_tx_pkt_rdy_thld() therefore returns ENET_THLD_MAX
and tx_thld_len is 3840. Neither new branch in bcmgenet_xmit() can then
be reached.

The message does say "Nothing below an MTU of 3809 is affected". The new
code only becomes reachable with the later "net: bcmgenet: allow the MTU
to be changed" patch.

Could the message say that nothing reaches this code until the MTU limit
is raised later in the series?

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 4a4b0c46f8986..17c8318c20388 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c

[ ... ]

> @@ -2176,6 +2181,29 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
>  		goto out;
>  	}
>  

[ ... ]

> +	/* Keep the frame out of the window just past the threshold */
> +	if (unlikely(skb->len > priv->tx_thld_len &&
> +		     skb->len < priv->tx_thld_len + ENET_TX_SAFE_MARGIN)) {
> +		if (skb_put_padto(skb, priv->tx_thld_len + ENET_TX_SAFE_MARGIN)) {
> +			BCMGENET_STATS64_INC((&ring->stats64), dropped);
> +			goto drop;
> +		}
> +	}

[Severity: Medium]

What happens here when the GENET is the conduit for a DSA switch that
uses a tail tag? By the time the skb reaches bcmgenet_xmit(), the tagger
has already written the tag into the last bytes of the frame. This
skb_put_padto() then appends zeros after the tag.

The DSA core pads before tagging so that the conduit never has to add
bytes after a tail tag:

net/dsa/user.c:dsa_user_xmit() {
    ...
	if (dev->needed_tailroom)
		eth_skb_pad(skb);
    ...
}

For KSZ9477-class switches the tag holds the egress port mask, the
priority, and the override bit for link-local frames:

net/dsa/tag_ksz.c:ksz9477_xmit() {
    ...
	if (is_link_local_ether_addr(hdr->h_dest))
		val |= KSZ9477_TAIL_TAG_OVERRIDE;

	*tag = cpu_to_be16(val);
    ...
}

lan937x_xmit() also always sets LAN937X_TAIL_TAG_VALID.

After padding, would the switch read an all-zero tag in place of these
fields, with the real tag left inside the payload? HSR/PRP RCT trailers
look like they would be corrupted the same way.

The commit message says these protocols "see the zeros instead". The
switch, though, would read the zeros as forwarding instructions: no port
mask, no override bit and no valid bit. Depending on the switch, the
frame could be dropped, forwarded by address lookup or flooded, whatever
egress port the host chose. That could break standalone port isolation,
and link-local frames would lose their forced egress port.

This path opens up once later patches in the series raise max_mtu. If a
user port MTU is raised above roughly 3806, dsa_user_change_mtu() calls
dev_set_mtu(conduit, new_conduit_mtu), which moves the conduit into the
MTU range that has the window. After that, whoever sends the traffic
decides whether a frame lands in the window, for example through the UDP
datagram size, IP fragment tails, or forwarded traffic.

Instead of padding after the trailer, would it be safer to check
netdev_uses_dsa() here and drop the frame? Another option would be to
refuse conduit MTUs that reach the window.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-nb-genet-mtu-nn-v2-v3-0-74a796c019ce%40tipi-net.de

Re: [PATCH net-next v3 6/8] net: bcmgenet: pad transmit frames out of the packet ready window

[Nicolai Buchwitz]

Hi Clashiko

On 8.10.2026 11:40, netdev-bot+sashiko@kernel.org wrote:

> [...]

> [Severity: Medium]
> 
> What happens here when the GENET is the conduit for a DSA switch that
> uses a tail tag? By the time the skb reaches bcmgenet_xmit(), the 
> tagger
> has already written the tag into the last bytes of the frame. This
> skb_put_padto() then appends zeros after the tag.
> 
> The DSA core pads before tagging so that the conduit never has to add
> bytes after a tail tag:
> 
> net/dsa/user.c:dsa_user_xmit() {
>     ...
> 	if (dev->needed_tailroom)
> 		eth_skb_pad(skb);
>     ...
> }
> 
> For KSZ9477-class switches the tag holds the egress port mask, the
> priority, and the override bit for link-local frames:
> 
> net/dsa/tag_ksz.c:ksz9477_xmit() {
>     ...
> 	if (is_link_local_ether_addr(hdr->h_dest))
> 		val |= KSZ9477_TAIL_TAG_OVERRIDE;
> 
> 	*tag = cpu_to_be16(val);
>     ...
> }
> 
> lan937x_xmit() also always sets LAN937X_TAIL_TAG_VALID.
> 
> After padding, would the switch read an all-zero tag in place of these
> fields, with the real tag left inside the payload? HSR/PRP RCT trailers
> look like they would be corrupted the same way.
> 
> The commit message says these protocols "see the zeros instead". The
> switch, though, would read the zeros as forwarding instructions: no 
> port
> mask, no override bit and no valid bit. Depending on the switch, the
> frame could be dropped, forwarded by address lookup or flooded, 
> whatever
> egress port the host chose. That could break standalone port isolation,
> and link-local frames would lose their forced egress port.
> 
> This path opens up once later patches in the series raise max_mtu. If a
> user port MTU is raised above roughly 3806, dsa_user_change_mtu() calls
> dev_set_mtu(conduit, new_conduit_mtu), which moves the conduit into the
> MTU range that has the window. After that, whoever sends the traffic
> decides whether a frame lands in the window, for example through the 
> UDP
> datagram size, IP fragment tails, or forwarded traffic.
> 
> Instead of padding after the trailer, would it be safer to check
> netdev_uses_dsa() here and drop the frame? Another option would be to
> refuse conduit MTUs that reach the window.

I do not think genet is ever a DSA conduit with tail tags. At least none
of the boards I'm aware of + what appears in the tree...

Broadcom switches use a head tag, and genet already reserves
ENET_BRCM_TAG_LEN for it, so padding at the tail wont do any harm. On 
the
Broadcom SoCs that do have an integrated switch the conduit is 
SYSTEMPORT,
not genet, and that driver has the netdev_uses_dsa() checks.

Every tagger listed is a tail tagger for a switch family that is not 
paired
with genet. Reaching this would need an out of tree board wiring one 
behind
a genet SoC and raising the user port MTU past 3808 (not 3806)...

So I would keep the padding and the note in the commit message rather 
than
add a drop path for a configuration that does not exist?

The only other way (which I don't really like), would be to cap the MTU 
when
attached to dsa in _change_mtu():

	/* Padding a frame clear of the window would overwrite a DSA tail tag 
*/
	if (netdev_uses_dsa(dev) && new_mtu > ENET_MAX_PAD_FREE_MTU)
		return -EINVAL;

But if we do this, Clashiko would complain about the case where the dsa 
is
attached after the MTU is already set to something above the thresholds 
...

@Florian / Justin: Anything you are aware of in the stb universe?

Thanks,
Nicolai

[PATCH net-next v3 7/8] net: bcmgenet: allow the MTU to be changed

[Nicolai Buchwitz]

The driver never sets dev->max_mtu, so the MTU is stuck at ETH_DATA_LEN.

One descriptor reaches as far as the packet ready threshold, so derive the
maximum from it. The threshold registers are 8 bit in units of 16 bytes and
want a multiple of the 256 byte burst size. A descriptor is one page and
also holds skb_shared_info behind the frame. On 4K pages the page is the
tighter limit and leaves 3564 bytes. That includes room for a VLAN tag so a
VLAN interface can run at the parent MTU.

Resize the buffers and rewrite the registers in place. The PHY keeps
running and the link stays up.

A failed allocation retries at the previous size. If that fails too, take
the interface down. Running on rings that were never allocated is worse.

Suggested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Link: https://github.com/raspberrypi/linux/issues/5561
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Tested-by: Pierre-Marin Leclercq <pierremarinleclercq88@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 83 +++++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 17c8318c2038..6d21ac2d78d5 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -84,6 +84,12 @@
 				      ENET_THLD_MAX * ENET_THLD_UNIT, \
 				      ENET_THLD_PAGE_LEN)
 
+/* Largest MTU that fits one descriptor, with room for a VLAN tag so a VLAN
+ * interface can use the parent MTU.
+ */
+#define ENET_MAX_MTU		(ENET_THLD_MAX_LEN - GENET_RBUF_ALIGN - \
+				 ETH_HLEN - VLAN_HLEN)
+
 /* Tx/Rx DMA register offset, skip 256 descriptors */
 #define WORDS_PER_BD(p)		(p->hw_params->words_per_bd)
 #define DMA_DESC_SIZE		(WORDS_PER_BD(priv) * sizeof(u32))
@@ -3547,6 +3553,7 @@ static int bcmgenet_open(struct net_device *dev)
 
 	bcmgenet_netif_start(dev, true);
 
+	priv->datapath_up = true;
 	netif_tx_start_all_queues(dev);
 
 	return 0;
@@ -3605,7 +3612,11 @@ static int bcmgenet_close(struct net_device *dev)
 
 	netif_dbg(priv, ifdown, dev, "bcmgenet_close\n");
 
-	bcmgenet_netif_stop(dev, false);
+	/* A failed MTU change can have torn the datapath down already */
+	if (priv->datapath_up) {
+		bcmgenet_netif_stop(dev, false);
+		priv->datapath_up = false;
+	}
 
 	/* Really kill the PHY state machine and disconnect from it */
 	phy_disconnect(dev->phydev);
@@ -3853,6 +3864,71 @@ static int bcmgenet_change_carrier(struct net_device *dev, bool new_carrier)
 	return 0;
 }
 
+static int bcmgenet_change_mtu(struct net_device *dev, int new_mtu)
+{
+	struct bcmgenet_priv *priv = netdev_priv(dev);
+	unsigned int old_mtu = dev->mtu;
+	int ret;
+
+	if (!netif_running(dev)) {
+		WRITE_ONCE(dev->mtu, new_mtu);
+		priv->rx_buf_len = bcmgenet_rx_buf_len(new_mtu);
+		return 0;
+	}
+
+	/* The watchdog trips on an idle queue once the rings are gone */
+	netif_device_detach(dev);
+
+	/* Only the buffers and the MTU registers change, leave the PHY up */
+	bcmgenet_netif_stop(dev, false);
+	priv->datapath_up = false;
+
+	WRITE_ONCE(dev->mtu, new_mtu);
+	priv->rx_buf_len = bcmgenet_rx_buf_len(new_mtu);
+	bcmgenet_set_mtu_regs(priv, new_mtu);
+
+	ret = bcmgenet_init_dma(priv, true);
+	if (ret) {
+		/* Retry the size that was allocated a moment ago */
+		WRITE_ONCE(dev->mtu, old_mtu);
+		priv->rx_buf_len = bcmgenet_rx_buf_len(old_mtu);
+		bcmgenet_set_mtu_regs(priv, old_mtu);
+		if (bcmgenet_init_dma(priv, true)) {
+			/* Nothing left to run on. Take the interface down so
+			 * that close and suspend do not tear it down twice.
+			 */
+			netdev_err(dev, "failed to restore MTU %u, closing\n",
+				   old_mtu);
+			netif_close(dev);
+
+			/* Mark the device present again, __dev_open()
+			 * refuses a detached one. The queues stay stopped
+			 * because the interface is down by now.
+			 */
+			netif_device_attach(dev);
+			return ret;
+		}
+	}
+
+	bcmgenet_hfb_restore(priv);
+	bcmgenet_netif_start(dev, false);
+
+	/* bcmgenet_netif_start() only restores the link interrupt */
+	if (bcmgenet_has_mdio_intr(priv))
+		bcmgenet_intrl2_0_writel(priv, UMAC_IRQ_MDIO_EVENT,
+					 INTRL2_CPU_MASK_CLEAR);
+
+	/* A link event latched while the interrupts were off is gone. Internal
+	 * PHYs on GENET v1-v4 are not polled, so resync the state machine.
+	 */
+	phy_mac_interrupt(dev->phydev);
+
+	priv->datapath_up = true;
+	netif_device_attach(dev);
+
+	return ret;
+}
+
 static const struct net_device_ops bcmgenet_netdev_ops = {
 	.ndo_open		= bcmgenet_open,
 	.ndo_stop		= bcmgenet_close,
@@ -3864,6 +3940,7 @@ static const struct net_device_ops bcmgenet_netdev_ops = {
 	.ndo_set_features	= bcmgenet_set_features,
 	.ndo_get_stats64	= bcmgenet_get_stats64,
 	.ndo_change_carrier	= bcmgenet_change_carrier,
+	.ndo_change_mtu		= bcmgenet_change_mtu,
 };
 
 /* GENET hardware parameters/characteristics */
@@ -4217,7 +4294,11 @@ static int bcmgenet_probe(struct platform_device *pdev)
 	/* Mii wait queue */
 	init_waitqueue_head(&priv->wq);
 	bcmgenet_hfb_init(priv);
+
+	/* v1 cannot program the thresholds, so it stays at the default MTU */
 	priv->rx_buf_len = bcmgenet_rx_buf_len(dev->mtu);
+	if (!GENET_IS_V1(priv))
+		dev->max_mtu = ENET_MAX_MTU;
 	INIT_WORK(&priv->bcmgenet_irq_work, bcmgenet_irq_task);
 
 	priv->clk_wol = devm_clk_get_optional(&priv->pdev->dev, "enet-wol");
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index ac1d0aab5d75..a4933a5d3823 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -632,6 +632,7 @@ struct bcmgenet_priv {
 	unsigned autoneg_pause:1;
 	unsigned tx_pause:1;
 	unsigned rx_pause:1;
+	unsigned datapath_up:1;
 
 	/* MDIO bus variables */
 	wait_queue_head_t wq;

-- 
2.53.0

Re: [PATCH net-next v3 7/8] net: bcmgenet: allow the MTU to be changed

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 (1) · Medium: 1 (1) · Low: 2

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] bcmgenet_change_mtu() leaves the PHY state machine running,
  but it needs the MAC disabled while it reprograms the MAC.
- [Low] At this commit, ENET_MAX_MTU (bcmgenet.c:90) and
  bcmgenet_pkt_rdy_thld() (bcmgenet.c:2688) size one RX descriptor for 2…
- [Low] bcmgenet_change_mtu() leaves the PHY state machine running, but
  bcmgenet_netif_stop() -> bcmgenet_intr_disable() (line 3596) writes…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the new
  priv->datapath_up flag does not cover it.
- [Medium] This problem wasn't introduced by this patch, but
  bcmgenet_netif_stop() ignores the -ETIMEDOUT result of…

--

net: bcmgenet: allow the MTU to be changed

On GENET v2 and later, this commit sets dev->max_mtu from the RX packet
ready threshold. It also adds an ndo_change_mtu handler that tears down and
rebuilds the DMA rings in place while the PHY keeps running. A new
datapath_up flag lets bcmgenet_close() skip the teardown when a failed MTU
change has already taken the datapath down.

Link: https://github.com/raspberrypi/linux/issues/5561

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 17c8318c20388..6d21ac2d78d51 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -84,6 +84,12 @@
>  				      ENET_THLD_MAX * ENET_THLD_UNIT, \
>  				      ENET_THLD_PAGE_LEN)
>  
> +/* Largest MTU that fits one descriptor, with room for a VLAN tag so a VLAN
> + * interface can use the parent MTU.
> + */
> +#define ENET_MAX_MTU		(ENET_THLD_MAX_LEN - GENET_RBUF_ALIGN - \
> +				 ETH_HLEN - VLAN_HLEN)

[Severity: Low]
At this commit, ENET_MAX_MTU and bcmgenet_pkt_rdy_thld() both size one RX
descriptor as GENET_RBUF_ALIGN + mtu + ETH_HLEN + VLAN_HLEN. Meanwhile,
bcmgenet_set_mtu_regs() programs the MAC limit with the larger
ENET_FRAME_OVERHEAD:

    bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), UMAC_MAX_FRAME_LEN);

If the MAC accepts a frame that is bigger than one descriptor, for example
QinQ at the parent MTU, that frame spans two descriptors. Would it then be
dropped in bcmgenet_desc_rx()?

    if (unlikely(!(dma_flag & DMA_EOP) || !(dma_flag & DMA_SOP))) {
        netif_err(priv, rx_status, dev,
                  "dropping fragmented packet!\n");

The forwarded FCS case doesn't look reachable here. reset_umac() in open
clears CMD_CRC_FWD, and bcmgenet_set_features() then reads crc_fwd_en back as
0. So only frames longer than the MTU plus one VLAN tag seem to be affected.

The later patch in this series, "net: bcmgenet: reassemble jumbo frames from
status block fragments", removes ENET_MAX_MTU and replaces the drop with
bcmgenet_add_frag() reassembly. That means this only applies to the
intermediate state at this commit.

> +
>  /* Tx/Rx DMA register offset, skip 256 descriptors */
>  #define WORDS_PER_BD(p)		(p->hw_params->words_per_bd)
>  #define DMA_DESC_SIZE		(WORDS_PER_BD(priv) * sizeof(u32))

[ ... ]

> @@ -3605,7 +3612,11 @@ static int bcmgenet_close(struct net_device *dev)
>  
>  	netif_dbg(priv, ifdown, dev, "bcmgenet_close\n");
>  
> -	bcmgenet_netif_stop(dev, false);
> +	/* A failed MTU change can have torn the datapath down already */
> +	if (priv->datapath_up) {
> +		bcmgenet_netif_stop(dev, false);
> +		priv->datapath_up = false;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but does datapath_up cover the
suspend/resume path?

Only bcmgenet_open() and bcmgenet_change_mtu() update the flag. The non-WoL
branch of bcmgenet_suspend() calls bcmgenet_netif_stop(dev, true). That
disables the NAPIs and frees rx_cbs/tx_cbs in bcmgenet_fini_dma() without
NULLing the pointers, and it leaves datapath_up set.

If bcmgenet_resume() then fails in bcmgenet_init_dma():

    ret = bcmgenet_init_dma(priv, false);
    if (ret) {
        netdev_err(dev, "failed to initialize DMA\n");
        goto out_clk_disable;
    }

then the interface is still running, but it has no rings and datapath_up is
still 1. A later bcmgenet_close() from ifdown, unregister or shutdown would
call bcmgenet_netif_stop() a second time.

The NAPIs are already disabled at that point. Can napi_disable() in
bcmgenet_disable_tx_napi() then spin forever with RTNL held? If it did
return, bcmgenet_fini_dma() would kfree() rx_cbs and tx_cbs a second time.

A failed bcmgenet_init_dma() on resume looks realistic. The earlier patch in
this series, "net: bcmgenet: pad transmit frames out of the packet ready
window", describes TX DMA refusing to halt so that every later
bcmgenet_init_dma() fails. The RX buffers also come from
page_pool_dev_alloc(), which doesn't sleep.

bcmgenet_change_mtu() adds another way to hit this, because it calls
bcmgenet_netif_stop() without checking datapath_up.

Would clearing datapath_up in the suspend teardown, and setting it again only
after a successful resume, close this gap?

>  
>  	/* Really kill the PHY state machine and disconnect from it */
>  	phy_disconnect(dev->phydev);
> @@ -3853,6 +3864,71 @@ static int bcmgenet_change_carrier(struct net_device *dev, bool new_carrier)
>  	return 0;
>  }
>  
> +static int bcmgenet_change_mtu(struct net_device *dev, int new_mtu)
> +{

[ ... ]

> +	/* The watchdog trips on an idle queue once the rings are gone */
> +	netif_device_detach(dev);
> +
> +	/* Only the buffers and the MTU registers change, leave the PHY up */
> +	bcmgenet_netif_stop(dev, false);

[Severity: Medium]
This is a pre-existing issue, but bcmgenet_netif_stop() ignores the
-ETIMEDOUT result of bcmgenet_dma_teardown():

    bcmgenet_dma_teardown(priv);

    /* Disable MAC transmit. TX DMA disabled must be done before this */
    umac_enable_set(priv, CMD_TX_EN, false);

If RDMA or TDMA doesn't halt in time, bcmgenet_fini_dma() still returns the
RX pages to the page pool and destroys the pools. Hardware descriptors may
still point at those pages. Could the device then DMA into freed pages, or
read freed skbs on TX?

MAC RX is disabled before the teardown, so this needs a real halt timeout to
trigger. This new caller lets an admin run the teardown repeatedly with ip
link set mtu.

> +	priv->datapath_up = false;
> +
> +	WRITE_ONCE(dev->mtu, new_mtu);
> +	priv->rx_buf_len = bcmgenet_rx_buf_len(new_mtu);
> +	bcmgenet_set_mtu_regs(priv, new_mtu);
> +
> +	ret = bcmgenet_init_dma(priv, true);

[Severity: Medium]
Is the MAC actually disabled at this point if the interface was opened
without a link?

After an open with no link, reset_umac() leaves CMD_SW_RESET set.
umac_enable_set() returns early while that bit is set:

    if (reg & CMD_SW_RESET) {
        spin_unlock_bh(&priv->reg_lock);
        return;
    }

So the CMD_RX_EN/CMD_TX_EN disable in bcmgenet_netif_stop() does nothing.
The PHY state machine keeps running and doesn't take RTNL. If the first
link-up arrives during the teardown and rebuild,
bcmgenet_mii_setup()->bcmgenet_mac_config() does:

    if (reg & CMD_SW_RESET) {
        reg &= ~CMD_SW_RESET;
        bcmgenet_umac_writel(priv, reg, UMAC_CMD);
        udelay(2);
        reg |= CMD_TX_EN | CMD_RX_EN;
    }

bcmgenet_set_mtu_regs() is documented as "Call with the MAC disabled".
Wouldn't it, along with the RBUF flush in bcmgenet_init_dma(), then run while
the MAC is receiving?

Nothing seems to serialize the adjust_link callback against this window.
reg_lock only covers single read-modify-write sequences.

> +	if (ret) {

[ ... ]

> +	bcmgenet_hfb_restore(priv);
> +	bcmgenet_netif_start(dev, false);
> +
> +	/* bcmgenet_netif_start() only restores the link interrupt */
> +	if (bcmgenet_has_mdio_intr(priv))
> +		bcmgenet_intrl2_0_writel(priv, UMAC_IRQ_MDIO_EVENT,
> +					 INTRL2_CPU_MASK_CLEAR);

[Severity: Low]
The PHY keeps running during the MTU change, but
bcmgenet_netif_stop()->bcmgenet_intr_disable() also masks
UMAC_IRQ_MDIO_EVENT. It is only unmasked here, after the whole teardown and
rebuild.

bcmgenet_mii_wait() depends on that interrupt to wake priv->wq:

    wait_event_timeout(priv->wq,
                       !(bcmgenet_umac_readl(priv, UMAC_MDIO_CMD)
                       & MDIO_START_BUSY),
                       HZ / 100);

Would every phylib MDIO access in this window wait the full 10 ms timeout
while holding phydev->lock? bcmgenet_mii_wait() always returns 0, so the
effect looks like added latency rather than a failure.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-nb-genet-mtu-nn-v2-v3-0-74a796c019ce%40tipi-net.de

Re: [PATCH net-next v3 7/8] net: bcmgenet: allow the MTU to be changed

[Nicolai Buchwitz]

On 8.10.2026 11:40, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 (1) · Medium: 1 (1) · Low: 2
> 
> 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] bcmgenet_change_mtu() leaves the PHY state machine running,
>   but it needs the MAC disabled while it reprograms the MAC.
> - [Low] At this commit, ENET_MAX_MTU (bcmgenet.c:90) and
>   bcmgenet_pkt_rdy_thld() (bcmgenet.c:2688) size one RX descriptor for 
> 2…
> - [Low] bcmgenet_change_mtu() leaves the PHY state machine running, but
>   bcmgenet_netif_stop() -> bcmgenet_intr_disable() (line 3596) writes…
> 
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but the new
>   priv->datapath_up flag does not cover it.
> - [Medium] This problem wasn't introduced by this patch, but
>   bcmgenet_netif_stop() ignores the -ETIMEDOUT result of…
> 
> --
> 
> net: bcmgenet: allow the MTU to be changed
> 
> On GENET v2 and later, this commit sets dev->max_mtu from the RX packet
> ready threshold. It also adds an ndo_change_mtu handler that tears down 
> and
> rebuilds the DMA rings in place while the PHY keeps running. A new
> datapath_up flag lets bcmgenet_close() skip the teardown when a failed 
> MTU
> change has already taken the datapath down.
> 
> Link: https://github.com/raspberrypi/linux/issues/5561
> 
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
>> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> index 17c8318c20388..6d21ac2d78d51 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> @@ -84,6 +84,12 @@
>>  				      ENET_THLD_MAX * ENET_THLD_UNIT, \
>>  				      ENET_THLD_PAGE_LEN)
>> 
>> +/* Largest MTU that fits one descriptor, with room for a VLAN tag so 
>> a VLAN
>> + * interface can use the parent MTU.
>> + */
>> +#define ENET_MAX_MTU		(ENET_THLD_MAX_LEN - GENET_RBUF_ALIGN - \
>> +				 ETH_HLEN - VLAN_HLEN)
> 
> [Severity: Low]
> At this commit, ENET_MAX_MTU and bcmgenet_pkt_rdy_thld() both size one 
> RX
> descriptor as GENET_RBUF_ALIGN + mtu + ETH_HLEN + VLAN_HLEN. Meanwhile,
> bcmgenet_set_mtu_regs() programs the MAC limit with the larger
> ENET_FRAME_OVERHEAD:

Right, and as you have noticed patch 8 removes ENET_MAX_MTU and 
reassembles instead.
I can amend the commit message and mention hat the one descriptor limit 
is intermediate ...

> 
>     bcmgenet_umac_writel(priv, ENET_MAX_FRAME_LEN(mtu), 
> UMAC_MAX_FRAME_LEN);
> 
> If the MAC accepts a frame that is bigger than one descriptor, for 
> example
> QinQ at the parent MTU, that frame spans two descriptors. Would it then 
> be
> dropped in bcmgenet_desc_rx()?
> 
>     if (unlikely(!(dma_flag & DMA_EOP) || !(dma_flag & DMA_SOP))) {
>         netif_err(priv, rx_status, dev,
>                   "dropping fragmented packet!\n");
> 
> The forwarded FCS case doesn't look reachable here. reset_umac() in 
> open
> clears CMD_CRC_FWD, and bcmgenet_set_features() then reads crc_fwd_en 
> back as
> 0. So only frames longer than the MTU plus one VLAN tag seem to be 
> affected.
> 
> The later patch in this series, "net: bcmgenet: reassemble jumbo frames 
> from
> status block fragments", removes ENET_MAX_MTU and replaces the drop 
> with
> bcmgenet_add_frag() reassembly. That means this only applies to the
> intermediate state at this commit.
> 
>> +
>>  /* Tx/Rx DMA register offset, skip 256 descriptors */
>>  #define WORDS_PER_BD(p)		(p->hw_params->words_per_bd)
>>  #define DMA_DESC_SIZE		(WORDS_PER_BD(priv) * sizeof(u32))
> 
> [ ... ]
> 
>> @@ -3605,7 +3612,11 @@ static int bcmgenet_close(struct net_device 
>> *dev)
>> 
>>  	netif_dbg(priv, ifdown, dev, "bcmgenet_close\n");
>> 
>> -	bcmgenet_netif_stop(dev, false);
>> +	/* A failed MTU change can have torn the datapath down already */
>> +	if (priv->datapath_up) {
>> +		bcmgenet_netif_stop(dev, false);
>> +		priv->datapath_up = false;
>> +	}
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does datapath_up cover 
> the
> suspend/resume path?
> 
> Only bcmgenet_open() and bcmgenet_change_mtu() update the flag. The 
> non-WoL
> branch of bcmgenet_suspend() calls bcmgenet_netif_stop(dev, true). That
> disables the NAPIs and frees rx_cbs/tx_cbs in bcmgenet_fini_dma() 
> without
> NULLing the pointers, and it leaves datapath_up set.
> 
> If bcmgenet_resume() then fails in bcmgenet_init_dma():
> 
>     ret = bcmgenet_init_dma(priv, false);
>     if (ret) {
>         netdev_err(dev, "failed to initialize DMA\n");
>         goto out_clk_disable;
>     }
> 
> then the interface is still running, but it has no rings and 
> datapath_up is
> still 1. A later bcmgenet_close() from ifdown, unregister or shutdown 
> would
> call bcmgenet_netif_stop() a second time.
> 
> The NAPIs are already disabled at that point. Can napi_disable() in
> bcmgenet_disable_tx_napi() then spin forever with RTNL held? If it did
> return, bcmgenet_fini_dma() would kfree() rx_cbs and tx_cbs a second 
> time.
> 
> A failed bcmgenet_init_dma() on resume looks realistic. The earlier 
> patch in
> this series, "net: bcmgenet: pad transmit frames out of the packet 
> ready
> window", describes TX DMA refusing to halt so that every later
> bcmgenet_init_dma() fails. The RX buffers also come from
> page_pool_dev_alloc(), which doesn't sleep.
> 
> bcmgenet_change_mtu() adds another way to hit this, because it calls
> bcmgenet_netif_stop() without checking datapath_up.
> 
> Would clearing datapath_up in the suspend teardown, and setting it 
> again only
> after a successful resume, close this gap?

Will do.

> 
>> 
>>  	/* Really kill the PHY state machine and disconnect from it */
>>  	phy_disconnect(dev->phydev);
>> @@ -3853,6 +3864,71 @@ static int bcmgenet_change_carrier(struct 
>> net_device *dev, bool new_carrier)
>>  	return 0;
>>  }
>> 
>> +static int bcmgenet_change_mtu(struct net_device *dev, int new_mtu)
>> +{
> 
> [ ... ]
> 
>> +	/* The watchdog trips on an idle queue once the rings are gone */
>> +	netif_device_detach(dev);
>> +
>> +	/* Only the buffers and the MTU registers change, leave the PHY up 
>> */
>> +	bcmgenet_netif_stop(dev, false);
> 
> [Severity: Medium]
> This is a pre-existing issue, but bcmgenet_netif_stop() ignores the
> -ETIMEDOUT result of bcmgenet_dma_teardown():

Yes, but can we please fix this in a separate patch / series? I already 
have
list of other issues from the new sashiko bug list, where this would fit 
in.

> 
>     bcmgenet_dma_teardown(priv);
> 
>     /* Disable MAC transmit. TX DMA disabled must be done before this 
> */
>     umac_enable_set(priv, CMD_TX_EN, false);
> 
> If RDMA or TDMA doesn't halt in time, bcmgenet_fini_dma() still returns 
> the
> RX pages to the page pool and destroys the pools. Hardware descriptors 
> may
> still point at those pages. Could the device then DMA into freed pages, 
> or
> read freed skbs on TX?
> 
> MAC RX is disabled before the teardown, so this needs a real halt 
> timeout to
> trigger. This new caller lets an admin run the teardown repeatedly with 
> ip
> link set mtu.
> 
>> +	priv->datapath_up = false;
>> +
>> +	WRITE_ONCE(dev->mtu, new_mtu);
>> +	priv->rx_buf_len = bcmgenet_rx_buf_len(new_mtu);
>> +	bcmgenet_set_mtu_regs(priv, new_mtu);
>> +
>> +	ret = bcmgenet_init_dma(priv, true);
> 
> [Severity: Medium]
> Is the MAC actually disabled at this point if the interface was opened
> without a link?

No, it is not. reset_umac() leaves CMD_SW_RESET set and 
umac_enable_set()
returns early while it is.

But IMHO the window is very narrow, as the MAC is held in reset, and the
link has to come up mid rebuild. Nothing serialises that, agreed.

> 
> After an open with no link, reset_umac() leaves CMD_SW_RESET set.
> umac_enable_set() returns early while that bit is set:
> 
>     if (reg & CMD_SW_RESET) {
>         spin_unlock_bh(&priv->reg_lock);
>         return;
>     }
> 
> So the CMD_RX_EN/CMD_TX_EN disable in bcmgenet_netif_stop() does 
> nothing.
> The PHY state machine keeps running and doesn't take RTNL. If the first
> link-up arrives during the teardown and rebuild,
> bcmgenet_mii_setup()->bcmgenet_mac_config() does:
> 
>     if (reg & CMD_SW_RESET) {
>         reg &= ~CMD_SW_RESET;
>         bcmgenet_umac_writel(priv, reg, UMAC_CMD);
>         udelay(2);
>         reg |= CMD_TX_EN | CMD_RX_EN;
>     }
> 
> bcmgenet_set_mtu_regs() is documented as "Call with the MAC disabled".
> Wouldn't it, along with the RBUF flush in bcmgenet_init_dma(), then run 
> while
> the MAC is receiving?
> 
> Nothing seems to serialize the adjust_link callback against this 
> window.
> reg_lock only covers single read-modify-write sequences.
> 
>> +	if (ret) {
> 
> [ ... ]
> 
>> +	bcmgenet_hfb_restore(priv);
>> +	bcmgenet_netif_start(dev, false);
>> +
>> +	/* bcmgenet_netif_start() only restores the link interrupt */
>> +	if (bcmgenet_has_mdio_intr(priv))
>> +		bcmgenet_intrl2_0_writel(priv, UMAC_IRQ_MDIO_EVENT,
>> +					 INTRL2_CPU_MASK_CLEAR);
> 
> [Severity: Low]
> The PHY keeps running during the MTU change, but
> bcmgenet_netif_stop()->bcmgenet_intr_disable() also masks
> UMAC_IRQ_MDIO_EVENT. It is only unmasked here, after the whole teardown 
> and
> rebuild.
> 
> bcmgenet_mii_wait() depends on that interrupt to wake priv->wq:
> 
>     wait_event_timeout(priv->wq,
>                        !(bcmgenet_umac_readl(priv, UMAC_MDIO_CMD)
>                        & MDIO_START_BUSY),
>                        HZ / 100);
> 
> Would every phylib MDIO access in this window wait the full 10 ms 
> timeout
> while holding phydev->lock? bcmgenet_mii_wait() always returns 0, so 
> the
> effect looks like added latency rather than a failure.

It would, but bcmgenet_mii_wait() returns 0 either way, so it is added
latency during the MTU change and nothing more ...


> 
> [ ... ]

[PATCH net-next v3 8/8] net: bcmgenet: reassemble jumbo frames from status block fragments

[Nicolai Buchwitz]

The hardware does not truncate a frame longer than the packet ready
threshold. It splits the frame across descriptors and writes a status block
at the start of each one. The first fragment then arrives with SOP and no
EOP and is dropped as fragmented. This caps the MTU.

Strip the status blocks and reassemble the fragments. Only the last block
holds the checksum of the whole frame. Broadcom confirmed from the RTL that
every GENET revision splits long frames this way, not just the v5 this was
tested on.

The MAC only checksums a frame it holds in full, so anything longer than
the threshold falls back to software. At jumbo sizes the larger frame saves
more per packet overhead than the checksum costs.

UMAC_MAX_FRAME_LEN is 14 bit and counts the FCS. That puts the maximum MTU
at 16347.

The TX descriptor length field is wider than DMA_BUFLENGTH_MASK suggests,
that mask only extracts the length from the RX status word. Single
descriptor frames of up to 8998 bytes went out intact.

Suggested-by: Justin Chen <justin.chen@broadcom.com>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Tested-by: Pierre-Marin Leclercq <pierremarinleclercq88@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 133 ++++++++++++++++++++-----
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |   2 +
 2 files changed, 111 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 6d21ac2d78d5..80215b5c0063 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -84,11 +84,8 @@
 				      ENET_THLD_MAX * ENET_THLD_UNIT, \
 				      ENET_THLD_PAGE_LEN)
 
-/* Largest MTU that fits one descriptor, with room for a VLAN tag so a VLAN
- * interface can use the parent MTU.
- */
-#define ENET_MAX_MTU		(ENET_THLD_MAX_LEN - GENET_RBUF_ALIGN - \
-				 ETH_HLEN - VLAN_HLEN)
+/* UMAC_MAX_FRAME_LEN is 14 bits wide and counts the FCS */
+#define ENET_MAX_JUMBO_MTU	(GENMASK(13, 0) - ENET_FRAME_OVERHEAD)
 
 /* Tx/Rx DMA register offset, skip 256 descriptors */
 #define WORDS_PER_BD(p)		(p->hw_params->words_per_bd)
@@ -2187,8 +2184,8 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out;
 	}
 
-	/* The MAC only inserts a checksum into a frame it holds in full, and
-	 * silently drops a longer one, so fall back to software.
+	/* The MAC holds a frame to insert its checksum, but only as much as
+	 * its FIFO takes. Longer frames are dropped silently.
 	 */
 	if (unlikely(skb->len > priv->tx_thld_len) &&
 	    skb->ip_summed == CHECKSUM_PARTIAL) {
@@ -2338,6 +2335,54 @@ static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring,
 	return 0;
 }
 
+/* Drop the frame being collected. Its remaining descriptors carry no SOP,
+ * so they are dropped quietly until the next one does.
+ */
+static void bcmgenet_discard_frags(struct bcmgenet_rx_ring *ring)
+{
+	ring->frag_drop = true;
+
+	if (!ring->frag_head)
+		return;
+
+	dev_kfree_skb_any(ring->frag_head);
+	ring->frag_head = NULL;
+}
+
+/* A frame longer than the threshold arrives in several descriptors, each with
+ * its own status block. Only the first one carries a header, so hand the page
+ * of every later one to the frame already being collected. Returns the frame
+ * once EOP is in, NULL while more descriptors are expected or once the frame
+ * had to be dropped.
+ */
+static struct sk_buff *bcmgenet_add_frag(struct bcmgenet_rx_ring *ring,
+					 struct page *page,
+					 unsigned int offset,
+					 unsigned int size,
+					 unsigned int dma_flag,
+					 unsigned int len)
+{
+	struct sk_buff *head = ring->frag_head;
+
+	if (unlikely(skb_shinfo(head)->nr_frags >= MAX_SKB_FRAGS)) {
+		BCMGENET_STATS64_INC((&ring->stats64), fragmented_errors);
+		bcmgenet_discard_frags(ring);
+		page_pool_put_full_page(ring->page_pool, page, true);
+		return NULL;
+	}
+
+	skb_add_rx_frag(head, skb_shinfo(head)->nr_frags, page,
+			offset + sizeof(struct status_64),
+			len - sizeof(struct status_64), size);
+
+	if (!(dma_flag & DMA_EOP))
+		return NULL;
+
+	ring->frag_head = NULL;
+
+	return head;
+}
+
 /* bcmgenet_desc_rx - descriptor based rx process.
  * this could be called from bottom half, or from NAPI polling method.
  */
@@ -2402,6 +2447,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 
 		if (bcmgenet_rx_refill(ring, cb)) {
 			BCMGENET_STATS64_INC(stats, dropped);
+			bcmgenet_discard_frags(ring);
 			goto next;
 		}
 
@@ -2426,24 +2472,41 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 			  __func__, p_index, ring->c_index,
 			  ring->read_ptr, dma_length_status);
 
-		/* Only the first descriptor carries the alignment pad */
-		min_len = dma_flag & DMA_SOP ? GENET_RSB_PAD
-					     : sizeof(struct status_64);
+		/* Only the first descriptor carries the alignment pad. A head
+		 * with more to come must hold the Ethernet header.
+		 */
+		if (dma_flag & DMA_SOP) {
+			min_len = GENET_RSB_PAD;
+			if (!(dma_flag & DMA_EOP))
+				min_len += ETH_HLEN;
+		} else {
+			min_len = sizeof(struct status_64);
+		}
 
 		/* Reject lengths that would underflow the SKB build path. */
 		if (unlikely(len > priv->rx_buf_len || len < min_len)) {
 			netif_err(priv, rx_status, dev,
 				  "invalid packet length %d\n", len);
 			BCMGENET_STATS64_INC(stats, length_errors);
+			bcmgenet_discard_frags(ring);
 			page_pool_put_full_page(ring->page_pool, rx_page,
 						true);
 			goto next;
 		}
 
-		if (unlikely(!(dma_flag & DMA_EOP) || !(dma_flag & DMA_SOP))) {
-			netif_err(priv, rx_status, dev,
-				  "dropping fragmented packet!\n");
-			BCMGENET_STATS64_INC(stats, fragmented_errors);
+		/* A new SOP resynchronizes after an incomplete frame */
+		if (dma_flag & DMA_SOP) {
+			if (ring->frag_head) {
+				BCMGENET_STATS64_INC(stats, fragmented_errors);
+				bcmgenet_discard_frags(ring);
+			}
+			ring->frag_drop = false;
+		} else if (unlikely(!ring->frag_head)) {
+			/* Rest of a dropped frame, or no SOP seen yet */
+			if (!ring->frag_drop) {
+				BCMGENET_STATS64_INC(stats, fragmented_errors);
+				ring->frag_drop = true;
+			}
 			page_pool_put_full_page(ring->page_pool, rx_page,
 						true);
 			goto next;
@@ -2473,17 +2536,27 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 						DMA_RX_RXER)) == DMA_RX_RXER)
 				u64_stats_inc(&stats->errors);
 			u64_stats_update_end(&stats->syncp);
+			bcmgenet_discard_frags(ring);
 			page_pool_put_full_page(ring->page_pool, rx_page,
 						true);
 			goto next;
 		} /* error packet */
 
+		if (!(dma_flag & DMA_SOP)) {
+			skb = bcmgenet_add_frag(ring, rx_page, rx_offset,
+						rx_size, dma_flag, len);
+			if (!skb)
+				goto next;
+			goto deliver;
+		}
+
 		/* Build SKB from the page - data starts at hard_start,
 		 * frame begins after RSB(64) + pad(2) = 66 bytes.
 		 */
 		skb = napi_build_skb(hard_start, rx_size);
 		if (unlikely(!skb)) {
 			BCMGENET_STATS64_INC(stats, dropped);
+			bcmgenet_discard_frags(ring);
 			page_pool_put_full_page(ring->page_pool, rx_page,
 						true);
 			goto next;
@@ -2495,17 +2568,26 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
 		skb_reserve(skb, GENET_RSB_PAD);
 		__skb_put(skb, len - GENET_RSB_PAD);
 
-		if (priv->crc_fwd_en) {
-			skb_trim(skb, skb->len - ETH_FCS_LEN);
+		if (unlikely(!(dma_flag & DMA_EOP))) {
+			ring->frag_head = skb;
+			goto next;
+		}
+
+deliver:
+		/* The FCS trim may release the page with the last status block */
+		rx_csum = (__force __be16)(status->rx_csum & 0xffff);
+
+		if (priv->crc_fwd_en &&
+		    unlikely(pskb_trim(skb, skb->len - ETH_FCS_LEN))) {
+			BCMGENET_STATS64_INC(stats, dropped);
+			dev_kfree_skb_any(skb);
+			goto next;
 		}
 
 		/* Set up checksum offload */
-		if (dev->features & NETIF_F_RXCSUM) {
-			rx_csum = (__force __be16)(status->rx_csum & 0xffff);
-			if (rx_csum) {
-				skb->csum = (__force __wsum)ntohs(rx_csum);
-				skb->ip_summed = CHECKSUM_COMPLETE;
-			}
+		if ((dev->features & NETIF_F_RXCSUM) && rx_csum) {
+			skb->csum = (__force __wsum)ntohs(rx_csum);
+			skb->ip_summed = CHECKSUM_COMPLETE;
 		}
 
 		len = skb->len;
@@ -2613,6 +2695,8 @@ static void bcmgenet_free_rx_buffers(struct bcmgenet_priv *priv)
 			cb = ring->cbs + i;
 			bcmgenet_free_rx_cb(cb, ring->page_pool);
 		}
+		/* a partial frame still holds pages of this pool */
+		bcmgenet_discard_frags(ring);
 	}
 }
 
@@ -2683,7 +2767,8 @@ static void bcmgenet_link_intr_enable(struct bcmgenet_priv *priv)
 }
 
 /* Receive threshold in register units. Covers the alignment bytes and the
- * frame, but not the status block, which the hardware adds on top.
+ * frame up to the register maximum, but not the status block, which the
+ * hardware adds on top. A longer frame arrives in several descriptors.
  */
 static unsigned int bcmgenet_pkt_rdy_thld(unsigned int mtu)
 {
@@ -4298,7 +4383,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
 	/* v1 cannot program the thresholds, so it stays at the default MTU */
 	priv->rx_buf_len = bcmgenet_rx_buf_len(dev->mtu);
 	if (!GENET_IS_V1(priv))
-		dev->max_mtu = ENET_MAX_MTU;
+		dev->max_mtu = ENET_MAX_JUMBO_MTU;
 	INIT_WORK(&priv->bcmgenet_irq_work, bcmgenet_irq_task);
 
 	priv->clk_wol = devm_clk_get_optional(&priv->pdev->dev, "enet-wol");
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index a4933a5d3823..97c27b7920d5 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -581,6 +581,8 @@ struct bcmgenet_rx_ring {
 	unsigned int	cb_ptr;		/* Rx ring initial CB ptr */
 	unsigned int	end_ptr;	/* Rx ring end CB ptr */
 	unsigned int	old_discards;
+	struct sk_buff	*frag_head;	/* frame being reassembled */
+	bool		frag_drop;	/* discarding until the next SOP */
 	struct bcmgenet_net_dim dim;
 	u32		rx_max_coalesced_frames;
 	u32		rx_coalesce_usecs;

-- 
2.53.0

Re: [PATCH net-next v3 0/8] net: bcmgenet: support larger MTUs

[netdev-bot+sinfo]

Hi!

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

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next v3 0/8] net: bcmgenet: support larger MTUs

[Nicolai Buchwitz]

On 7.10.2026 10:56, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.

No, it's not. The one commit carrying a fixes tag shouldn't be reachable 
without the mtu series, but is kinda required for it. I added the tag 
for the sake of completeness though.

> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

(response was only sent to the bot but Suie is not happy with that, so I 
resend it to everyone)