netdev - Re: [PATCH net-next v4] net: dsa: realtek: rtl8365mb: add change

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJq09z7U75Qe_oW3vbNmG=QaKFQW_zuFyNqjg0HAPPHh3t71Qg@mail.gmail.com>
Date:   Wed, 8 Mar 2023 14:10:59 -0300
From:   Luiz Angelo Daros de Luca <luizluca@...il.com>
To:     Simon Horman <simon.horman@...igine.com>
Cc:     netdev@...r.kernel.org, linus.walleij@...aro.org,
        alsi@...g-olufsen.dk, andrew@...n.ch, vivien.didelot@...il.com,
        f.fainelli@...il.com, olteanv@...il.com, davem@...emloft.net,
        kuba@...nel.org, pabeni@...hat.com, robh+dt@...nel.org,
        krzk+dt@...nel.org, arinc.unal@...nc9.com,
        Alexander Duyck <alexanderduyck@...com>
Subject: Re: [PATCH net-next v4] net: dsa: realtek: rtl8365mb: add change_mtu

> Hi Luiz,

Hi Simon,

>> +     ret = rtl8365mb_port_change_mtu(ds, cpu->trap_port, ETH_DATA_LEN);

This call just synchronizes the switch frame size with the default MTU (1500).

> Perhaps I am misreading this, perhaps it was discussed elsewhere (I did
> look), and perhaps it's not important. But prior to this
> patch a value of 1536 is used. Whereas with this patch the
> value, calculated in rtl8365mb_port_change_mtu, is
> ETH_DATA_LEN + VLAN_ETH_HLEN + ETH_FCS_LEN = 1500 + 18 + 4 = 1522.

That value, as mentioned in the commit message, probably came from
rtl8366rb driver jumbo frame settings.
The "rtl8366rb family" has 4 levels of jumbo frame size:

#define RTL8366RB_SGCR_MAX_LENGTH_1522          RTL8366RB_SGCR_MAX_LENGTH(0x0)
#define RTL8366RB_SGCR_MAX_LENGTH_1536          RTL8366RB_SGCR_MAX_LENGTH(0x1)
#define RTL8366RB_SGCR_MAX_LENGTH_1552          RTL8366RB_SGCR_MAX_LENGTH(0x2)
#define RTL8366RB_SGCR_MAX_LENGTH_16000         RTL8366RB_SGCR_MAX_LENGTH(0x3)

The first one might be the sum you did. I don't know what 1536 and
1552 are for. However, if those cases increase the MTU as well, the
code will handle it.
During my tests, changing those similar values or disabling jumbo
frames wasn't enough to change the switch behavior. As "rtl8365mb
family" can control frame size byte by byte, I believe it ignores the
old jumbo registers.

The 1522 size is already in use by other drivers. If there is
something that requires more room without increasing the MTU, like
QinQ, we would need to add that extra length to the
rtl8365mb_port_change_mtu formula and not the initial value. If not,
the switch will have different frame limits when the user leaves the
default 1500 MTU or when it changes and reverts the MTU size.

Regards,