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

[Jakub Kicinski <kuba@kernel.org>]

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?