[PATCH 0/5] net: dsa: b53: assorted jumbo frame fixes

[PATCH 0/5] net: dsa: b53: assorted jumbo frame fixes

[Jonas Gorski]

While investigating the capabilities of BCM63XX's integrated switch and
its DMA engine, I noticed a few issues in b53's jumbo frame code.

Mostly a confusion of MTU vs frame length, but also a few missing cases
for 100M switches.

Tested on BCM63XX and BCM53115 with intel 1G and realtek 1G NICs,
which support MTUs of 9000 or slightly above, but significantly less
than the 9716/9720 supported by BCM53115, so I couldn't verify the
actual maximum frame length.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
Jonas Gorski (5):
      net: dsa: b53: fix jumbo frame mtu check
      net: dsa: b53: fix max MTU for 1g switches
      net: dsa: b53: fix max MTU for BCM5325/BCM5365
      net: dsa: b53: allow lower MTUs on BCM5325/5365
      net: dsa: b53: fix jumbo frames on 10/100 ports

 drivers/net/dsa/b53/b53_common.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
---
base-commit: 8beee4d8dee76b67c75dc91fd8185d91e845c160
change-id: 20241003-b53_jumbo_fixes-95ff690b1d9e

Best regards,
-- 
Jonas Gorski <jonas.gorski@gmail.com>

[PATCH 1/5] net: dsa: b53: fix jumbo frame mtu check

[Jonas Gorski]

JMS_MIN_SIZE is the full ethernet frame length, while mtu is just the
data payload size. Comparing these two meant that mtus between 1500 and
1518 did not trigger enabling jumbo frames.

So instead compare the set mtu ETH_DATA_LEN, which is equal to
JMS_MIN_SIZE - ETH_HLEN - ETH_FCS_LEN;

Also do a check that the requested mtu is actually greater than the
minimum length, else we do not need to enable jumbo frames.

In practice this only introduced a very small range of mtus that did not
work properly. Newer chips allow 2000 byte large frames by default, and
older chips allow 1536 bytes long, which is equivalent to an mtu of
1514. So effectivly only mtus of 1515~1517 were broken.

Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 0783fc121bbbf979abe6c9985b10cf4379bf2a9b..57df00ad9dd4cedfe9e959ea779d48e3f8f36142 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2259,7 +2259,7 @@ static int b53_change_mtu(struct dsa_switch *ds, int port, int mtu)
 	if (!dsa_is_cpu_port(ds, port))
 		return 0;
 
-	enable_jumbo = (mtu >= JMS_MIN_SIZE);
+	enable_jumbo = (mtu > ETH_DATA_LEN);
 	allow_10_100 = (dev->chip_id == BCM583XX_DEVICE_ID);
 
 	return b53_set_jumbo(dev, enable_jumbo, allow_10_100);

-- 
2.43.0

Re: [PATCH 1/5] net: dsa: b53: fix jumbo frame mtu check

[Florian Fainelli]

On 10/4/2024 1:47 AM, Jonas Gorski wrote:
> JMS_MIN_SIZE is the full ethernet frame length, while mtu is just the
> data payload size. Comparing these two meant that mtus between 1500 and
> 1518 did not trigger enabling jumbo frames.
> 
> So instead compare the set mtu ETH_DATA_LEN, which is equal to
> JMS_MIN_SIZE - ETH_HLEN - ETH_FCS_LEN;
> 
> Also do a check that the requested mtu is actually greater than the
> minimum length, else we do not need to enable jumbo frames.
> 
> In practice this only introduced a very small range of mtus that did not
> work properly. Newer chips allow 2000 byte large frames by default, and
> older chips allow 1536 bytes long, which is equivalent to an mtu of
> 1514. So effectivly only mtus of 1515~1517 were broken.
> 
> Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[PATCH 2/5] net: dsa: b53: fix max MTU for 1g switches

[Jonas Gorski]

JMS_MAX_SIZE is the ethernet frame length, not the MTU, which is payload
without ethernet headers.

According to the datasheets maximum supported frame length for most
gigabyte swithes is 9720 bytes, so convert that to the expected MTU when
using VLAN tagged frames.

Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 57df00ad9dd4cedfe9e959ea779d48e3f8f36142..6fed3eb15ad9b257c6fc3da20ce91b5e7129884c 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -27,6 +27,7 @@
 #include <linux/phylink.h>
 #include <linux/etherdevice.h>
 #include <linux/if_bridge.h>
+#include <linux/if_vlan.h>
 #include <net/dsa.h>
 
 #include "b53_regs.h"
@@ -224,6 +225,8 @@ static const struct b53_mib_desc b53_mibs_58xx[] = {
 
 #define B53_MIBS_58XX_SIZE	ARRAY_SIZE(b53_mibs_58xx)
 
+#define B53_MAX_MTU		(9720 - ETH_HLEN - VLAN_HLEN - ETH_FCS_LEN)
+
 static int b53_do_vlan_op(struct b53_device *dev, u8 op)
 {
 	unsigned int i;
@@ -2267,7 +2270,7 @@ static int b53_change_mtu(struct dsa_switch *ds, int port, int mtu)
 
 static int b53_get_max_mtu(struct dsa_switch *ds, int port)
 {
-	return JMS_MAX_SIZE;
+	return B53_MAX_MTU;
 }
 
 static const struct phylink_mac_ops b53_phylink_mac_ops = {

-- 
2.43.0

Re: [PATCH 2/5] net: dsa: b53: fix max MTU for 1g switches

[Florian Fainelli]

On 10/4/2024 1:47 AM, Jonas Gorski wrote:
> JMS_MAX_SIZE is the ethernet frame length, not the MTU, which is payload
> without ethernet headers.
> 
> According to the datasheets maximum supported frame length for most
> gigabyte swithes is 9720 bytes, so convert that to the expected MTU when
> using VLAN tagged frames.

Only if you need to resubmit:

s/gigabyte swithes/gigabit switches/g

> 
> Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[PATCH 3/5] net: dsa: b53: fix max MTU for BCM5325/BCM5365

[Jonas Gorski]

BCM5325/BCM5365 do not support jumbo frames, so we should not report a
jumbo frame mtu for them. But they do support so called "oversized"
frames up to 1536 bytes long by default, so report an appropriate MTU.

Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 6fed3eb15ad9b257c6fc3da20ce91b5e7129884c..e8b20bfa8b83ea7ac643bd5d005e2983747bd478 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -225,6 +225,7 @@ static const struct b53_mib_desc b53_mibs_58xx[] = {
 
 #define B53_MIBS_58XX_SIZE	ARRAY_SIZE(b53_mibs_58xx)
 
+#define B53_MAX_MTU_25		(1536 - ETH_HLEN - VLAN_HLEN - ETH_FCS_LEN)
 #define B53_MAX_MTU		(9720 - ETH_HLEN - VLAN_HLEN - ETH_FCS_LEN)
 
 static int b53_do_vlan_op(struct b53_device *dev, u8 op)
@@ -2270,6 +2271,11 @@ static int b53_change_mtu(struct dsa_switch *ds, int port, int mtu)
 
 static int b53_get_max_mtu(struct dsa_switch *ds, int port)
 {
+	struct b53_device *dev = ds->priv;
+
+	if (is5325(dev) || is5365(dev))
+		return B53_MAX_MTU_25;
+
 	return B53_MAX_MTU;
 }
 

-- 
2.43.0

Re: [PATCH 3/5] net: dsa: b53: fix max MTU for BCM5325/BCM5365

[Florian Fainelli]

On 10/4/2024 1:47 AM, Jonas Gorski wrote:
> BCM5325/BCM5365 do not support jumbo frames, so we should not report a
> jumbo frame mtu for them. But they do support so called "oversized"
> frames up to 1536 bytes long by default, so report an appropriate MTU.
> 
> Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[PATCH 4/5] net: dsa: b53: allow lower MTUs on BCM5325/5365

[Jonas Gorski]

While BCM5325/5365 do not support jumbo frames, they do support slightly
oversized frames, so do not error out if requesting a supported MTU for
them.

Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index e8b20bfa8b83ea7ac643bd5d005e2983747bd478..5b83f9b6cdac3de6c5e6e2164c78146d694674cd 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2258,7 +2258,7 @@ static int b53_change_mtu(struct dsa_switch *ds, int port, int mtu)
 	bool allow_10_100;
 
 	if (is5325(dev) || is5365(dev))
-		return -EOPNOTSUPP;
+		return 0;
 
 	if (!dsa_is_cpu_port(ds, port))
 		return 0;

-- 
2.43.0

Re: [PATCH 4/5] net: dsa: b53: allow lower MTUs on BCM5325/5365

[Florian Fainelli]

On 10/4/2024 1:47 AM, Jonas Gorski wrote:
> While BCM5325/5365 do not support jumbo frames, they do support slightly
> oversized frames, so do not error out if requesting a supported MTU for
> them.
> 
> Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>

Could be squashed with the previous commit, though no strong opinion 
either way.
-- 
Florian

[PATCH 5/5] net: dsa: b53: fix jumbo frames on 10/100 ports

[Jonas Gorski]

All modern chips support and need the 10_100 bit set for supporting jumbo
frames on 10/100 ports, so instead of enabling it only for 583XX enable
it for everything except bcm63xx, where the bit is writeable, but does
nothing.

Tested on BCM53115, where jumbo frames were dropped at 10/100 speeds
without the bit set.

Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 5b83f9b6cdac3de6c5e6e2164c78146d694674cd..c39cb119e760db5fcbfaaf44abe033f6977e7005 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2264,7 +2264,7 @@ static int b53_change_mtu(struct dsa_switch *ds, int port, int mtu)
 		return 0;
 
 	enable_jumbo = (mtu > ETH_DATA_LEN);
-	allow_10_100 = (dev->chip_id == BCM583XX_DEVICE_ID);
+	allow_10_100 = !is63xx(dev);
 
 	return b53_set_jumbo(dev, enable_jumbo, allow_10_100);
 }

-- 
2.43.0

Re: [PATCH 5/5] net: dsa: b53: fix jumbo frames on 10/100 ports

[Florian Fainelli]

On 10/4/2024 1:47 AM, Jonas Gorski wrote:
> All modern chips support and need the 10_100 bit set for supporting jumbo
> frames on 10/100 ports, so instead of enabling it only for 583XX enable
> it for everything except bcm63xx, where the bit is writeable, but does
> nothing.
> 
> Tested on BCM53115, where jumbo frames were dropped at 10/100 speeds
> without the bit set.
> 
> Fixes: 6ae5834b983a ("net: dsa: b53: add MTU configuration support")
 > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH 0/5] net: dsa: b53: assorted jumbo frame fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 04 Oct 2024 10:47:16 +0200 you wrote:
> While investigating the capabilities of BCM63XX's integrated switch and
> its DMA engine, I noticed a few issues in b53's jumbo frame code.
> 
> Mostly a confusion of MTU vs frame length, but also a few missing cases
> for 100M switches.
> 
> Tested on BCM63XX and BCM53115 with intel 1G and realtek 1G NICs,
> which support MTUs of 9000 or slightly above, but significantly less
> than the 9716/9720 supported by BCM53115, so I couldn't verify the
> actual maximum frame length.
> 
> [...]

Here is the summary with links:
  - [1/5] net: dsa: b53: fix jumbo frame mtu check
    https://git.kernel.org/netdev/net/c/42fb3acf6826
  - [2/5] net: dsa: b53: fix max MTU for 1g switches
    https://git.kernel.org/netdev/net/c/680a8217dc00
  - [3/5] net: dsa: b53: fix max MTU for BCM5325/BCM5365
    https://git.kernel.org/netdev/net/c/ca8c1f71c101
  - [4/5] net: dsa: b53: allow lower MTUs on BCM5325/5365
    https://git.kernel.org/netdev/net/c/e4b294f88a32
  - [5/5] net: dsa: b53: fix jumbo frames on 10/100 ports
    https://git.kernel.org/netdev/net/c/2f3dcd0d39af

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html