[PATCH net] net: bgmac: keep the maximum RX frame within one RX buffer

[PATCH net] net: bgmac: keep the maximum RX frame within one RX buffer

[Rosen Penev]

Each RX descriptor holds at most BGMAC_RX_MAX_FRAME_SIZE (1536) bytes of
frame data. The MAC, however, is programmed to accept up to
32 + ETHER_MAX_LEN = 1550 bytes in bgmac_chip_init(), and up to
32 + mtu = 1564 bytes after bgmac_change_mtu(). max_mtu is set to
BGMAC_RX_MAX_FRAME_SIZE - ETH_FCS_LEN, which leaves out the Ethernet
header and any VLAN tag, so a frame at max_mtu does not fit in one buffer
either.

The DMA engine is the same dma64 engine used by b43 and brcmsmac, which
continue a frame that does not fit into the next descriptor's buffer.
bgmac_dma_rx_read() does not handle that: it consumes only the first
descriptor and parses the continuation buffer as a new frame, taking its
RX header from frame data the sender controls.

Program UMAC_MAX_FRAME_LEN from the MTU plus a VLAN tagged Ethernet header
and the FCS, as bcmgenet does for the same UniMAC register, and limit
max_mtu so that the resulting frame fits in one RX buffer. At the default
MTU of 1500 this gives 1522 bytes. A DSA conduit carrying a 4 byte
Broadcom tag (MTU 1504) gives 1526 bytes. max_mtu drops from 1532 to 1514.

bgmac_chip_init() runs on every ndo_open and used to reset the limit to
the fixed default. Use the current MTU instead.

bgmac_change_mtu() also never updated net_dev->mtu. When a driver
provides ndo_change_mtu, the core leaves that to the driver, so the MTU
seen by the stack never changed. Set it.

Only compile tested. Not tested on hardware. Found with a review on a
different commit by Sashiko.

Fixes: dd4544f05469 ("bgmac: driver for GBit MAC core on BCMA bus")
Fixes: 752e5fcc2e77 ("bgmac: reduce max frame size to support just MTU 1500")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 15 +++++++++++----
 drivers/net/ethernet/broadcom/bgmac.h |  6 ++++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 669eb7ce7d40..f29568a4190a 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1110,6 +1110,13 @@ static void bgmac_enable(struct bgmac *bgmac)
 	}
 }
 
+/* The MAC counts the FCS in the maximum frame length. */
+static void bgmac_set_max_frame_len(struct bgmac *bgmac, int mtu)
+{
+	bgmac_umac_write(bgmac, UMAC_MAX_FRAME_LEN,
+			 mtu + VLAN_ETH_HLEN + ETH_FCS_LEN);
+}
+
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
 static void bgmac_chip_init(struct bgmac *bgmac)
 {
@@ -1131,7 +1138,7 @@ static void bgmac_chip_init(struct bgmac *bgmac)
 	else
 		bgmac_umac_cmd_maskset(bgmac, ~CMD_LCL_LOOP_EN, 0, false);
 
-	bgmac_umac_write(bgmac, UMAC_MAX_FRAME_LEN, 32 + ETHER_MAX_LEN);
+	bgmac_set_max_frame_len(bgmac, bgmac->net_dev->mtu);
 
 	bgmac_chip_intrs_on(bgmac);
 
@@ -1267,7 +1274,8 @@ static int bgmac_change_mtu(struct net_device *net_dev, int mtu)
 {
 	struct bgmac *bgmac = netdev_priv(net_dev);
 
-	bgmac_umac_write(bgmac, UMAC_MAX_FRAME_LEN, 32 + mtu);
+	bgmac_set_max_frame_len(bgmac, mtu);
+	WRITE_ONCE(net_dev->mtu, mtu);
 	return 0;
 }
 
@@ -1555,8 +1563,7 @@ int bgmac_enet_probe(struct bgmac *bgmac)
 	net_dev->hw_features = net_dev->features;
 	net_dev->vlan_features = net_dev->features;
 
-	/* Omit FCS from max MTU size */
-	net_dev->max_mtu = BGMAC_RX_MAX_FRAME_SIZE - ETH_FCS_LEN;
+	net_dev->max_mtu = BGMAC_RX_MAX_MTU;
 
 	bgmac->in_init = false;
 
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 6fee9a41839c..ace65789289f 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -2,6 +2,7 @@
 #ifndef _BGMAC_H
 #define _BGMAC_H
 
+#include <linux/if_vlan.h>
 #include <linux/netdevice.h>
 
 #include "unimac.h"
@@ -330,6 +331,9 @@
 						 BGMAC_RX_FRAME_OFFSET)
 #define BGMAC_RX_MAX_FRAME_SIZE			1536
 #define BGMAC_RX_BUF_SIZE			(BGMAC_RX_FRAME_OFFSET + BGMAC_RX_MAX_FRAME_SIZE)
+/* Largest MTU whose VLAN tagged frames, FCS included, fit in one RX buffer */
+#define BGMAC_RX_MAX_MTU			(BGMAC_RX_MAX_FRAME_SIZE - VLAN_ETH_HLEN - \
+						 ETH_FCS_LEN)
 #define BGMAC_RX_ALLOC_SIZE			(SKB_DATA_ALIGN(BGMAC_RX_BUF_SIZE + BGMAC_RX_BUF_OFFSET) + \
 						 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
 
@@ -363,8 +367,6 @@
 #define BGMAC_CHIPCTL_7_IF_TYPE_MII		0x00000040
 #define BGMAC_CHIPCTL_7_IF_TYPE_RGMII		0x00000080
 
-#define ETHER_MAX_LEN	(ETH_FRAME_LEN + ETH_FCS_LEN)
-
 /* Feature Flags */
 #define BGMAC_FEAT_TX_MASK_SETUP	BIT(0)
 #define BGMAC_FEAT_RX_MASK_SETUP	BIT(1)
-- 
2.56.0