Re: [PATCH net-next 1/3] net: dsa: Configure the MTU for switch ports

[Florian Fainelli <f.fainelli@gmail.com>]

On 11/23/2019 12:46 PM, Vladimir Oltean wrote:
> 
> Correct. I was actually held back a bit while looking at Andrew's
> patch dc0fe7d47f9f ("net: dsa: Set the master device's MTU to account
> for DSA overheads") where he basically discarded errors, so that's the
> approach I took too (thinking that some DSA masters would not have ops
> for changing or reporting the MTU).
> 
>> I had prepared a patch series with Murali doing nearly the same thing
>> and targeting Broadcom switches nearly a year ago but since I never got
>> feedback whether this worked properly for the use case he was after, I
>> did not submit it since I did not need it personally and found it to be
>> a nice can of worms.
>>
> 
> Nice, do you mind if I take your series instead then?

Not at all, if it works, please go ahead, not sure how hard it is going
to be to rebase.

> 
>> Another thing that I had not gotten around testing was making sure that
>> when a slave_dev gets enslaved as a bridge port member, that bridge MTU
>> normalization would kick in and make sure that if you have say: port 0
>> configured with MTU 1500 and port 1 configured with MTU 9000, the bridge
>> would normalize to MTU 1500 as you would expect.
>>
> 
> Nope, that doesn't happen by default, at least in my implementation.
> Is there code in the bridge core for it?

net/bridge/br_if.c::br_mtu_auto_adjust() takes care of adjusting the
bridge master device's MTU based on the minimum MTU of all ports within
the bridge, but what it seems to be missing is ensuring that if bridge
ports are enslaved, and those bridge ports happen to be part of the same
switch id (similar decision path to setting skb->fwd_offload_mark), then
the bridge port's MTU should also be auto adjusted. mlxsw also supports
changing the MTU, so I am surprised this is not something they fixed
already.

> 
>> https://github.com/ffainelli/linux/commits/dsa-mtu
>>
>> This should be a DSA switch fabric notifier IMHO because changing the
>> MTU on an user port implies changing the MTU on every DSA port in
>> between plus the CPU port. Your approach here works for the first
>> upstream port, but not for the ones in between, and there can be more,
>> as is common with the ZII devel Rev. B and C boards.
> 
> Yes, correct. Your patch implements notifiers which is definitely
> good. I don't have a cascaded setup to test yet (my Turris Mox was
> supposed to arrive but for some reason it was returned to the seller
> by the shipping company...).

Andrew and Vivien may know whether it is possible to get you a ZII Devel
rev. B or C since those have 3 switches in the fabric and have
constituted nice test platforms. Not sure if there is a version with a
CPU port that is Gigabit capable though.
-- 
Florian