diff mbox series

[net-next,v3] net: dsa: realtek: rtl8365mb: add change_mtu

Message ID 20230218230636.5528-1-luizluca@gmail.com (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v3] net: dsa: realtek: rtl8365mb: add change_mtu | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: edumazet@google.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 77 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Luiz Angelo Daros de Luca Feb. 18, 2023, 11:06 p.m. UTC
The rtl8365mb was using a fixed MTU size of 1536, which was probably
inspired by the rtl8366rb's initial packet size. However, unlike that
family, the rtl8365mb family can specify the max packet size in bytes,
rather than in fixed steps. The max packet size now defaults to
VLAN_ETH_HLEN+ETH_DATA_LEN+ETH_FCS_LEN, which is 1522 bytes.

DSA calls change_mtu for the CPU port once the max MTU value among the
ports changes. As the max packet size is defined globally, the switch
is configured only when the call affects the CPU port.

The available specifications do not directly define the max supported
packet size, but it mentions a 16k limit. This driver will use the 0x3FFF
limit as it is used in the vendor API code. However, the switch sets the
max packet size to 16368 bytes (0x3FF0) after it resets.

change_mtu uses MTU size, or ethernet payload size, while the switch
works with frame size. The frame size is calculated considering the
ethernet header (14 bytes), a possible 802.1Q tag (4 bytes), the payload
size (MTU), and the Ethernet FCS (4 bytes). The CPU tag (8 bytes) is
consumed before the switch enforces the limit.

MTU was tested up to 2018 (with 802.1Q) as that is as far as mt7620
(where rtl8367s is stacked) can go. The register was manually
manipulated byte-by-byte to ensure the MTU to frame size conversion was
correct. For frames without 802.1Q tag, the frame size limit will be 4
bytes over the required size.

There is a jumbo register, enabled by default at 6k packet size.
However, the jumbo settings do not seem to limit nor expand the maximum
tested MTU (2018), even when jumbo is disabled. More tests are needed
with a device that can handle larger frames.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---

v2->v3:
- changed max frame size to 0x3FFF (used by vendor API)
- added info about how frame size is calculated, some more description
  about the tests performed and the 4 extra bytes when untagged frame is
  used.

v1->v2:
- dropped jumbo code as it was not changing the behavior (up to 2k MTU)
- fixed typos
- fixed code alignment
- renamed rtl8365mb_(change|max)_mtu to rtl8365mb_port_(change|max)_mtu

 drivers/net/dsa/realtek/rtl8365mb.c | 41 ++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 4 deletions(-)

Comments

Jakub Kicinski Feb. 21, 2023, 12:47 a.m. UTC | #1
On Sat, 18 Feb 2023 20:06:37 -0300 Luiz Angelo Daros de Luca wrote:
> The rtl8365mb was using a fixed MTU size of 1536, which was probably
> inspired by the rtl8366rb's initial packet size. However, unlike that
> family, the rtl8365mb family can specify the max packet size in bytes,
> rather than in fixed steps. The max packet size now defaults to
> VLAN_ETH_HLEN+ETH_DATA_LEN+ETH_FCS_LEN, which is 1522 bytes.
> 
> DSA calls change_mtu for the CPU port once the max MTU value among the
> ports changes. As the max packet size is defined globally, the switch
> is configured only when the call affects the CPU port.
> 
> The available specifications do not directly define the max supported
> packet size, but it mentions a 16k limit. This driver will use the 0x3FFF
> limit as it is used in the vendor API code. However, the switch sets the
> max packet size to 16368 bytes (0x3FF0) after it resets.
> 
> change_mtu uses MTU size, or ethernet payload size, while the switch
> works with frame size. The frame size is calculated considering the
> ethernet header (14 bytes), a possible 802.1Q tag (4 bytes), the payload
> size (MTU), and the Ethernet FCS (4 bytes). The CPU tag (8 bytes) is
> consumed before the switch enforces the limit.
> 
> MTU was tested up to 2018 (with 802.1Q) as that is as far as mt7620
> (where rtl8367s is stacked) can go. The register was manually
> manipulated byte-by-byte to ensure the MTU to frame size conversion was
> correct. For frames without 802.1Q tag, the frame size limit will be 4
> bytes over the required size.
> 
> There is a jumbo register, enabled by default at 6k packet size.
> However, the jumbo settings do not seem to limit nor expand the maximum
> tested MTU (2018), even when jumbo is disabled. More tests are needed
> with a device that can handle larger frames.

# Form letter - net-next is closed

The merge window for v6.3 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.

Please repost when net-next reopens after Mar 6th.

RFC patches sent for review only are obviously welcome at any time.
Alvin Šipraga Feb. 23, 2023, 3:03 p.m. UTC | #2
On Sat, Feb 18, 2023 at 08:06:37PM -0300, Luiz Angelo Daros de Luca wrote:
> The rtl8365mb was using a fixed MTU size of 1536, which was probably
> inspired by the rtl8366rb's initial packet size. However, unlike that
> family, the rtl8365mb family can specify the max packet size in bytes,
> rather than in fixed steps. The max packet size now defaults to
> VLAN_ETH_HLEN+ETH_DATA_LEN+ETH_FCS_LEN, which is 1522 bytes.
> 
> DSA calls change_mtu for the CPU port once the max MTU value among the
> ports changes. As the max packet size is defined globally, the switch
> is configured only when the call affects the CPU port.
> 
> The available specifications do not directly define the max supported
> packet size, but it mentions a 16k limit. This driver will use the 0x3FFF
> limit as it is used in the vendor API code. However, the switch sets the
> max packet size to 16368 bytes (0x3FF0) after it resets.
> 
> change_mtu uses MTU size, or ethernet payload size, while the switch
> works with frame size. The frame size is calculated considering the
> ethernet header (14 bytes), a possible 802.1Q tag (4 bytes), the payload
> size (MTU), and the Ethernet FCS (4 bytes). The CPU tag (8 bytes) is
> consumed before the switch enforces the limit.
> 
> MTU was tested up to 2018 (with 802.1Q) as that is as far as mt7620
> (where rtl8367s is stacked) can go. The register was manually
> manipulated byte-by-byte to ensure the MTU to frame size conversion was
> correct. For frames without 802.1Q tag, the frame size limit will be 4
> bytes over the required size.
> 
> There is a jumbo register, enabled by default at 6k packet size.
> However, the jumbo settings do not seem to limit nor expand the maximum
> tested MTU (2018), even when jumbo is disabled. More tests are needed
> with a device that can handle larger frames.
> 
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> ---
> 
> v2->v3:
> - changed max frame size to 0x3FFF (used by vendor API)
> - added info about how frame size is calculated, some more description
>   about the tests performed and the 4 extra bytes when untagged frame is
>   used.
> 
> v1->v2:
> - dropped jumbo code as it was not changing the behavior (up to 2k MTU)
> - fixed typos
> - fixed code alignment
> - renamed rtl8365mb_(change|max)_mtu to rtl8365mb_port_(change|max)_mtu
> 
>  drivers/net/dsa/realtek/rtl8365mb.c | 41 ++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
> index da31d8b839ac..05642c617c3b 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb.c
> @@ -98,6 +98,7 @@
>  #include <linux/of_irq.h>
>  #include <linux/regmap.h>
>  #include <linux/if_bridge.h>
> +#include <linux/if_vlan.h>
>  
>  #include "realtek.h"
>  
> @@ -267,6 +268,7 @@
>  /* Maximum packet length register */
>  #define RTL8365MB_CFG0_MAX_LEN_REG	0x088C
>  #define   RTL8365MB_CFG0_MAX_LEN_MASK	0x3FFF
> +#define RTL8365MB_CFG0_MAX_LEN_MAX	0x3FFF
>  
>  /* Port learning limit registers */
>  #define RTL8365MB_LUT_PORT_LEARN_LIMIT_BASE		0x0A20
> @@ -1135,6 +1137,36 @@ static void rtl8365mb_phylink_mac_link_up(struct dsa_switch *ds, int port,
>  	}
>  }
>  
> +static int rtl8365mb_port_change_mtu(struct dsa_switch *ds, int port,
> +				     int new_mtu)
> +{
> +	struct realtek_priv *priv = ds->priv;
> +	int frame_size;
> +
> +	/* When a new MTU is set, DSA always sets the CPU port's MTU to the
> +	 * largest MTU of the slave ports. Because the switch only has a global
> +	 * RX length register, only allowing CPU port here is enough.
> +	 */
> +

You forgot to remove this spurious newline.

With that fixed you can add:

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Thanks!

> +	if (!dsa_is_cpu_port(ds, port))
> +		return 0;
> +
> +	frame_size = new_mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
> +
> +	dev_dbg(priv->dev, "changing mtu to %d (frame size: %d)\n",
> +		new_mtu, frame_size);
> +
> +	return regmap_update_bits(priv->map, RTL8365MB_CFG0_MAX_LEN_REG,
> +				  RTL8365MB_CFG0_MAX_LEN_MASK,
> +				  FIELD_PREP(RTL8365MB_CFG0_MAX_LEN_MASK,
> +					     frame_size));
> +}
> +
> +static int rtl8365mb_port_max_mtu(struct dsa_switch *ds, int port)
> +{
> +	return RTL8365MB_CFG0_MAX_LEN_MAX - VLAN_ETH_HLEN - ETH_FCS_LEN;
> +}
> +
>  static void rtl8365mb_port_stp_state_set(struct dsa_switch *ds, int port,
>  					 u8 state)
>  {
> @@ -1980,10 +2012,7 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
>  		p->index = i;
>  	}
>  
> -	/* Set maximum packet length to 1536 bytes */
> -	ret = regmap_update_bits(priv->map, RTL8365MB_CFG0_MAX_LEN_REG,
> -				 RTL8365MB_CFG0_MAX_LEN_MASK,
> -				 FIELD_PREP(RTL8365MB_CFG0_MAX_LEN_MASK, 1536));
> +	ret = rtl8365mb_port_change_mtu(ds, cpu->trap_port, ETH_DATA_LEN);
>  	if (ret)
>  		goto out_teardown_irq;
>  
> @@ -2103,6 +2132,8 @@ static const struct dsa_switch_ops rtl8365mb_switch_ops_smi = {
>  	.get_eth_mac_stats = rtl8365mb_get_mac_stats,
>  	.get_eth_ctrl_stats = rtl8365mb_get_ctrl_stats,
>  	.get_stats64 = rtl8365mb_get_stats64,
> +	.port_change_mtu = rtl8365mb_port_change_mtu,
> +	.port_max_mtu = rtl8365mb_port_max_mtu,
>  };
>  
>  static const struct dsa_switch_ops rtl8365mb_switch_ops_mdio = {
> @@ -2124,6 +2155,8 @@ static const struct dsa_switch_ops rtl8365mb_switch_ops_mdio = {
>  	.get_eth_mac_stats = rtl8365mb_get_mac_stats,
>  	.get_eth_ctrl_stats = rtl8365mb_get_ctrl_stats,
>  	.get_stats64 = rtl8365mb_get_stats64,
> +	.port_change_mtu = rtl8365mb_port_change_mtu,
> +	.port_max_mtu = rtl8365mb_port_max_mtu,
>  };
>  
>  static const struct realtek_ops rtl8365mb_ops = {
> -- 
> 2.39.1
>
diff mbox series

Patch

diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index da31d8b839ac..05642c617c3b 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -98,6 +98,7 @@ 
 #include <linux/of_irq.h>
 #include <linux/regmap.h>
 #include <linux/if_bridge.h>
+#include <linux/if_vlan.h>
 
 #include "realtek.h"
 
@@ -267,6 +268,7 @@ 
 /* Maximum packet length register */
 #define RTL8365MB_CFG0_MAX_LEN_REG	0x088C
 #define   RTL8365MB_CFG0_MAX_LEN_MASK	0x3FFF
+#define RTL8365MB_CFG0_MAX_LEN_MAX	0x3FFF
 
 /* Port learning limit registers */
 #define RTL8365MB_LUT_PORT_LEARN_LIMIT_BASE		0x0A20
@@ -1135,6 +1137,36 @@  static void rtl8365mb_phylink_mac_link_up(struct dsa_switch *ds, int port,
 	}
 }
 
+static int rtl8365mb_port_change_mtu(struct dsa_switch *ds, int port,
+				     int new_mtu)
+{
+	struct realtek_priv *priv = ds->priv;
+	int frame_size;
+
+	/* When a new MTU is set, DSA always sets the CPU port's MTU to the
+	 * largest MTU of the slave ports. Because the switch only has a global
+	 * RX length register, only allowing CPU port here is enough.
+	 */
+
+	if (!dsa_is_cpu_port(ds, port))
+		return 0;
+
+	frame_size = new_mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
+
+	dev_dbg(priv->dev, "changing mtu to %d (frame size: %d)\n",
+		new_mtu, frame_size);
+
+	return regmap_update_bits(priv->map, RTL8365MB_CFG0_MAX_LEN_REG,
+				  RTL8365MB_CFG0_MAX_LEN_MASK,
+				  FIELD_PREP(RTL8365MB_CFG0_MAX_LEN_MASK,
+					     frame_size));
+}
+
+static int rtl8365mb_port_max_mtu(struct dsa_switch *ds, int port)
+{
+	return RTL8365MB_CFG0_MAX_LEN_MAX - VLAN_ETH_HLEN - ETH_FCS_LEN;
+}
+
 static void rtl8365mb_port_stp_state_set(struct dsa_switch *ds, int port,
 					 u8 state)
 {
@@ -1980,10 +2012,7 @@  static int rtl8365mb_setup(struct dsa_switch *ds)
 		p->index = i;
 	}
 
-	/* Set maximum packet length to 1536 bytes */
-	ret = regmap_update_bits(priv->map, RTL8365MB_CFG0_MAX_LEN_REG,
-				 RTL8365MB_CFG0_MAX_LEN_MASK,
-				 FIELD_PREP(RTL8365MB_CFG0_MAX_LEN_MASK, 1536));
+	ret = rtl8365mb_port_change_mtu(ds, cpu->trap_port, ETH_DATA_LEN);
 	if (ret)
 		goto out_teardown_irq;
 
@@ -2103,6 +2132,8 @@  static const struct dsa_switch_ops rtl8365mb_switch_ops_smi = {
 	.get_eth_mac_stats = rtl8365mb_get_mac_stats,
 	.get_eth_ctrl_stats = rtl8365mb_get_ctrl_stats,
 	.get_stats64 = rtl8365mb_get_stats64,
+	.port_change_mtu = rtl8365mb_port_change_mtu,
+	.port_max_mtu = rtl8365mb_port_max_mtu,
 };
 
 static const struct dsa_switch_ops rtl8365mb_switch_ops_mdio = {
@@ -2124,6 +2155,8 @@  static const struct dsa_switch_ops rtl8365mb_switch_ops_mdio = {
 	.get_eth_mac_stats = rtl8365mb_get_mac_stats,
 	.get_eth_ctrl_stats = rtl8365mb_get_ctrl_stats,
 	.get_stats64 = rtl8365mb_get_stats64,
+	.port_change_mtu = rtl8365mb_port_change_mtu,
+	.port_max_mtu = rtl8365mb_port_max_mtu,
 };
 
 static const struct realtek_ops rtl8365mb_ops = {