netdev - Re: [RFC PATCH net-next 00/11] Nested VLANs

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

Message-ID: <20200527234849.GA18589@qmqm.qmqm.pl>
Date:   Thu, 28 May 2020 02:15:07 +0200
From:   Michał Mirosław <mirq-linux@...e.qmqm.pl>
To:     Edwin Peer <edwin.peer@...adcom.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next 00/11] Nested VLANs - decimate flags and add
 another

On Wed, May 27, 2020 at 02:25:01PM -0700, Edwin Peer wrote:
> This series began life as a modest attempt to fix two issues pertaining
> to VLANs nested inside Geneve tunnels and snowballed from there. The
> first issue, addressed by a simple one-liner, is that GSO is not enabled
> for upper VLAN devices on top of Geneve. The second issue, addressed by
> the balance of the series, deals largely with MTU handling. VLAN devices
> above L2 in L3 tunnels inherit the MTU of the underlying device. This
> causes IP fragmentation because the inner L2 cannot be expanded within
> the same maximum L3 size to accommodate the additional VLAN tag.
> 
> As a first attempt, a new flag was introduced to generalize what was
> already being done for MACsec devices. This flag was unconditionally
> set for all devices that have a size constrained L2, such as is the
> case for Geneve and VXLAN tunnel devices. This doesn't quite do the
> right thing, however, if the underlying device MTU happens to be
> configured to a lower MTU than is supported. Thus, the approach was
> further refined to set IFF_NO_VLAN_ROOM when changing MTU, based on
> whether the underlying device L2 still has room for VLAN tags, but
> stopping short of registering device notifiers to update upper device
> MTU whenever a lower device changes. VLAN devices will thus do the
> sensible thing if they are applied to an already configured device,
> but will not dynamically update whenever the underlying device's MTU
> is subsequently changed (this seemed a bridge too far).
[...]

Hi!

Good to see someone taking on the VLAN MTU mess.  :-)

Have you considered adding a 'vlan_headroom' field (or another name)
for a netdev instead of a flag? This would submit easily to device
aggregation (just take min from the slaves) and would also handle
nested VLANs gracefully (subtracting for every layer).

In patch 3 you seem to assume that if lower device reduces MTU below
its max, then its ok to push it up with VLAN headers. I don't think
this is apropriate when reducing MTU because of eg. PMTU limit for
a tunnel.

Best Regards,
Michał Mirosław