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: <kcqdranbwdvkgrambeg2f7cfxguzodtzgd2yrecu6scau27vzc@o6gxdpxuzotr> Date: Sun, 10 Dec 2023 13:00:46 +0000 From: Alvin Šipraga <ALSI@...g-olufsen.dk> To: Linus Walleij <linus.walleij@...aro.org> CC: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, Vladimir Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> Subject: Re: [PATCH net-next 2/2] net: dsa: realtek: Rewrite RTL8366RB MTU handling On Sat, Dec 09, 2023 at 11:37:35PM +0100, Linus Walleij wrote: > The MTU callbacks are in layer 1 size, so for example 1500 > bytes is a normal setting. Cache this size, and only add > the layer 2 framing right before choosing the setting. On > the CPU port this will however include the DSA tag since > this is transmitted from the parent ethernet interface! > > Add the layer 2 overhead such as ethernet and VLAN framing > and FCS before selecting the size in the register. > > This will make the code easier to understand. > > The rtl8366rb_max_mtu() callback returns a bogus MTU > just subtracting the CPU tag, which is the only thing > we should NOT subtract. Return the correct layer 1 > max MTU after removing headers and checksum. > > Signed-off-by: Linus Walleij <linus.walleij@...aro.org> Reviewed-by: Alvin Šipraga <alsi@...g-olufsen.dk> > --- > drivers/net/dsa/realtek/rtl8366rb.c | 48 +++++++++++++++++++++++-------------- > 1 file changed, 30 insertions(+), 18 deletions(-) > > diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c > index 887afd1392cb..e3b6a470ca67 100644 > --- a/drivers/net/dsa/realtek/rtl8366rb.c > +++ b/drivers/net/dsa/realtek/rtl8366rb.c > @@ -15,6 +15,7 @@ > #include <linux/bitops.h> > #include <linux/etherdevice.h> > #include <linux/if_bridge.h> > +#include <linux/if_vlan.h> > #include <linux/interrupt.h> > #include <linux/irqdomain.h> > #include <linux/irqchip/chained_irq.h> > @@ -929,15 +930,19 @@ static int rtl8366rb_setup(struct dsa_switch *ds) > if (ret) > return ret; > > - /* Set maximum packet length to 1536 bytes */ > + /* Set default maximum packet length to 1536 bytes */ > ret = regmap_update_bits(priv->map, RTL8366RB_SGCR, > RTL8366RB_SGCR_MAX_LENGTH_MASK, > RTL8366RB_SGCR_MAX_LENGTH_1536); > if (ret) > return ret; > - for (i = 0; i < RTL8366RB_NUM_PORTS; i++) > - /* layer 2 size, see rtl8366rb_change_mtu() */ > - rb->max_mtu[i] = 1532; > + for (i = 0; i < RTL8366RB_NUM_PORTS; i++) { > + if (i == priv->cpu_port) > + /* CPU port need to also accept the tag */ > + rb->max_mtu[i] = ETH_DATA_LEN + RTL8366RB_CPU_TAG_SIZE; > + else > + rb->max_mtu[i] = ETH_DATA_LEN; > + } > > /* Disable learning for all ports */ > ret = regmap_write(priv->map, RTL8366RB_PORT_LEARNDIS_CTRL, > @@ -1442,24 +1447,29 @@ static int rtl8366rb_change_mtu(struct dsa_switch *ds, int port, int new_mtu) > /* Roof out the MTU for the entire switch to the greatest > * common denominator: the biggest set for any one port will > * be the biggest MTU for the switch. > - * > - * The first setting, 1522 bytes, is max IP packet 1500 bytes, > - * plus ethernet header, 1518 bytes, plus CPU tag, 4 bytes. > - * This function should consider the parameter an SDU, so the > - * MTU passed for this setting is 1518 bytes. The same logic > - * of subtracting the DSA tag of 4 bytes apply to the other > - * settings. > */ > - max_mtu = 1518; > + max_mtu = ETH_DATA_LEN; > for (i = 0; i < RTL8366RB_NUM_PORTS; i++) { > if (rb->max_mtu[i] > max_mtu) > max_mtu = rb->max_mtu[i]; > } > - if (max_mtu <= 1518) > + > + /* Translate to layer 2 size. > + * Add ethernet and (possible) VLAN headers, and checksum to the size. > + * For ETH_DATA_LEN (1500 bytes) this will add up to 1522 bytes. > + */ > + max_mtu += VLAN_ETH_HLEN; > + max_mtu += ETH_FCS_LEN; > + > + if (max_mtu <= 1522) > len = RTL8366RB_SGCR_MAX_LENGTH_1522; > - else if (max_mtu > 1518 && max_mtu <= 1532) > + else if (max_mtu > 1522 && max_mtu <= 1536) > + /* This will be the most common default if using VLAN and > + * CPU tagging on a port as both VLAN and CPU tag will > + * result in 1518 + 4 + 4 = 1526 bytes. > + */ > len = RTL8366RB_SGCR_MAX_LENGTH_1536; > - else if (max_mtu > 1532 && max_mtu <= 1548) > + else if (max_mtu > 1536 && max_mtu <= 1552) > len = RTL8366RB_SGCR_MAX_LENGTH_1552; > else > len = RTL8366RB_SGCR_MAX_LENGTH_16000; > @@ -1471,10 +1481,12 @@ static int rtl8366rb_change_mtu(struct dsa_switch *ds, int port, int new_mtu) > > static int rtl8366rb_max_mtu(struct dsa_switch *ds, int port) > { > - /* The max MTU is 16000 bytes, so we subtract the CPU tag > - * and the max presented to the system is 15996 bytes. > + /* The max MTU is 16000 bytes, so we subtract the ethernet > + * headers with VLAN and checksum and arrive at > + * 16000 - 18 - 4 = 15978. This does not include the CPU tag > + * since that is added to the requested MTU by the DSA framework. > */ > - return 15996; > + return 16000 - VLAN_ETH_HLEN - ETH_FCS_LEN; > } > > static int rtl8366rb_get_vlan_4k(struct realtek_priv *priv, u32 vid, > > -- > 2.34.1 >
Powered by blists - more mailing lists