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
