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

[Simon Horman <simon.horman@corigine.com>]

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.