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: <ZAmh54kNGDUEay3H@corigine.com>
Date:   Thu, 9 Mar 2023 10:07:51 +0100
From:   Simon Horman <simon.horman@...igine.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Luiz Angelo Daros de Luca <luizluca@...il.com>,
        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,
        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

On Wed, Mar 08, 2023 at 10:45:29PM -0800, Jakub Kicinski wrote:
> On Wed, 8 Mar 2023 14:10:59 -0300 Luiz Angelo Daros de Luca wrote:
> > > 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.
> 
> Could I trouble you for v5 with some form of this explanation in the
> commit message?

FWIIW, that would address my concern.