netdev - Re: [PATCH net] net: dsa: mv88e6xxx: Respect other ports when setting chip-wide MTU

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] [day] [month] [year] [list]

Message-ID: <20240716141755.5dq32slyd5agum7z@skbuf>
Date: Tue, 16 Jul 2024 17:17:55 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Martin Willi <martin@...ongswan.org>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
	Chris Packham <chris.packham@...iedtelesis.co.nz>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net] net: dsa: mv88e6xxx: Respect other ports when
 setting chip-wide MTU

Hi Martin,

On Tue, Jul 16, 2024 at 02:08:08PM +0200, Martin Willi wrote:
> DSA chips not supporting per-port jumbo frame size configurations use a
> chip-wide setting. In the commit referenced with the Fixes tag, the
> setting is applied just for the last port changing its MTU. This may
> result in two issues:
> 
>   * Starting with commit b9c587fed61c ("dsa: mv88e6xxx: Include tagger
>     overhead when setting MTU for DSA and CPU ports"), the CPU port
>     accounts for tagger overhead. If a user port is configured after
>     the CPU port, the chip-wide setting may be reduced again, as the
>     user port does not account for tagger overhead.
>   * If different user ports use different MTUs (say within different
>     L2 domains), setting the lower MTU after the higher MTU may result
>     in a chip-wide setting for the lower MTU, only.
> 
> Any of the above may result in clearing MV88E6185_G1_CTL1_MAX_FRAME_1632
> while it is actually required for the current configuration on some (CPU)
> ports. Specifically, on a MV88E6097 this results in dropped frames when
> setting the MTU to 1500 and sending local full-sized frames over a user
> port.
> 
> To respect the MTU requirements of all CPU and user ports, get the maximum
> frame size requirements over all ports when updating the chip-wide
> setting.
> 
> Fixes: 1baf0fac10fb ("net: dsa: mv88e6xxx: Use chip-wide max frame size for MTU")
> Signed-off-by: Martin Willi <martin@...ongswan.org>
> ---

There is actually a much simpler solution which I advise you to take.

We already know, by construction, that the MTU applied to the CPU port
is the largest among the MTUs of all user ports.

So you can program to hardware a chip-wide value which corresponds only
to the MTU of the CPU port.

You can see that rtl8365mb_port_change_mtu(), qca8k_port_change_mtu(),
mt7530_port_change_mtu(), ksz8_change_mtu(), ksz9477_change_mtu()
already do this. And we also have rtl8366rb_change_mtu() which takes the
long route, as you do, and which could be simplified.

This fix should work, I believe:

-	else if (chip->info->ops->set_max_frame_size)
+	else if (chip->info->ops->set_max_frame_size && dsa_is_cpu_port(ds, port))
 		ret = chip->info->ops->set_max_frame_size(chip, new_mtu);

BTW, I now realize b53_change_mtu() suffers from the same problem.
It would be great if you could also send a patch fixing that driver in
the same way.