Message ID | 20200527212512.17901-1-edwin.peer@broadcom.com |
---|---|
Headers | show |
Series | Nested VLANs - decimate flags and add another | expand |
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
On Wed, May 27, 2020 at 5:15 PM Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote: > 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). Great idea, I think that would be much nicer. > 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. Indeed. For non-tunnel devices I think this behavior is still correct, because past the 1st hop (where device MTU should be appropriate), all of L2, including any VLANs, has been replaced by something else. But yes, tunnels probably do need to unconditionally reduce MTU, because PMTU is something more dynamic. I guess I kind of half thought about this for gre6, where this is what I did because PMTU is so much more in your face for IPv6. Regards, Edwin Peer