Re: [PATCH net] net: dsa: apply MTU normalization for ports that join a LAG under a bridge too

On Wed, Aug 11, 2021 at 03:38:33PM -0700, Jakub Kicinski wrote:

On Wed, 11 Aug 2021 19:44:41 +0300 Vladimir Oltean wrote:

quoted

On Wed, Aug 11, 2021 at 03:45:20PM +0300, Vladimir Oltean wrote:

quoted

We want the MTU normalization logic to apply each time
dsa_port_bridge_join is called, so instead of chasing all callers of
that function, we should move the call within the bridge_join function
itself.

Fixes: 185c9a760a61 ("net: dsa: call dsa_port_bridge_join when joining a LAG that is already in a bridge")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---  

I forgot to rebase this patch on top of 'net' and now I notice that it
conflicts with the switchdev_bridge_port_offload() work.

Do we feel that the issue this patch fixes isn't too big and the patch
can go into net-next, to avoid conflicts and all that?

The commit message doesn't really describe the impact so hard to judge,
but either way you want to go - we'll need a repost so it can be build
tested.

The impact is that if a DSA interface joins a bonding/team that is in a
bridge and the bonding/team had an MTU of 9000 bytes, the DSA interface
will still have what it had before (probably 1500). I found this through
code inspection, didn't hit an actual bug or anything like that. It
seems fairly low impact to me, and a rare occurrence in any case. The
common path is for the DSA interface to first join the bond, then the
bond to join the bridge anyway, and that would work I think.

Later edit: I just realized, while typing, that it's going to be more
complicated than that, so I'm going to just drop the patch at least for
now. While bond_enslave() does in fact make the lower interface inherit
the bond's MTU, that's not what we want here. DSA switches want their
bridged ports to have the same MTU, and they change it dynamically
whenever one MTU changes, but they don't change the MTU of any upper
interfaces they might have. So with the normalization logic as applied
by this patch, ports that join a bond will have an MTU of 9000, but the
bond itself will still have 1500, since
(a) DSA doesn't change it
(b) the bonding driver has a NETDEV_CHANGEMTU handler implementation
    which just wonders whether it should let DSA lowers change their MTU
    in the first place or not.

Conflicts are not a huge deal. Obviously always best to wait for trees
to merge if that fixes things, but if net has dependency on net-next
you should just describe what you want the resolution to look like we
should be able to execute :)

Yeah, thanks, I noticed you duly applied the instructions in the last
conflicting patch I sent exactly as described, and I appreciate it.
But I don't like conflicts either way, they're a pain to deal with when
you're backporting patches, since you can never get a clean cherry pick
list, so I try to avoid them myself now if I can.