| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230223150348.z3ufj5qqw6qv4s2m@bang-olufsen.dk>
Date: Thu, 23 Feb 2023 15:03:44 +0000
From: Alvin Šipraga <ALSI@...g-olufsen.dk>
To: Luiz Angelo Daros de Luca <luizluca@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"andrew@...n.ch" <andrew@...n.ch>,
"vivien.didelot@...il.com" <vivien.didelot@...il.com>,
"f.fainelli@...il.com" <f.fainelli@...il.com>,
"olteanv@...il.com" <olteanv@...il.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
"arinc.unal@...nc9.com" <arinc.unal@...nc9.com>
Subject: Re: [PATCH net-next v3] net: dsa: realtek: rtl8365mb: add change_mtu
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@...il.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@...g-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
>
Powered by blists - more mailing lists