Date:   Wed, 27 May 2020 14:25:01 -0700
From:   Edwin Peer <edwin.peer@...adcom.com>
To:     netdev@...r.kernel.org
Cc:     Edwin Peer <edwin.peer@...adcom.com>, edumazet@...gle.com,
        linville@...driver.com, shemminger@...tta.com,
        mirq-linux@...e.qmqm.pl, jesse.brandeburg@...el.com,
        jchapman@...alix.com, david@...ve.works, j.vosburgh@...il.com,
        vfalico@...il.com, andy@...yhouse.net, sridhar.samudrala@...el.com,
        jiri@...lanox.com, geoff@...radead.org, mokuno@...sony.co.jp,
        msink@...monline.ru, mporter@...nel.crashing.org,
        inaky.perez-gonzalez@...el.com, jwi@...ux.ibm.com,
        kgraul@...ux.ibm.com, ubraun@...ux.ibm.com,
        grant.likely@...retlab.ca, hadi@...erus.ca, dsahern@...nel.org,
        shrijeet@...il.com, jon.mason@...el.com, dave.jiang@...el.com,
        saeedm@...lanox.com, hadarh@...lanox.com, ogerlitz@...lanox.com,
        allenbh@...il.com, michael.chan@...adcom.com
Subject: [RFC PATCH net-next 00/11] Nested VLANs - decimate flags and add another

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).

Aggregate devices presented the next challenge. Transitively propagating
IFF_NO_VLAN_ROOM via bonds, teams and the like seemed similar in
principle to the handling of IFF_XMIT_DST_RELEASE (only the opposite),
but IFF_XMIT_DST_RELEASE_PERM evaded understanding. Ultimately this flag
failed to justify its existence, allowing the new flag to take its place
and avoid taking up the last bit in the enum.

Finally, an audit of the other net devices in the tree was conducted to
discover where else this new behavior may be appropriate. At this point
it was also discovered that GRE devices would happily allow VLANs to be
added even when L3 is being tunneled in L3, hence restricting VLANs to
ARPHRD_ETHER devices. Between ARPHRD_ETHER and IFF_NO_VLAN_ROOM, it now
seemed only a hop and a skip to eliminate NET_F_VLAN_CHALLENGED too, but
alas there are still a few holdouts that would appear to require more of
a moonshot to address.

Edwin Peer (11):
  net: geneve: enable vlan offloads
  net: do away with the IFF_XMIT_DST_RELEASE_PERM flag
  net: vlan: add IFF_NO_VLAN_ROOM to constrain MTU
  net: geneve: constrain upper VLAN MTU using IFF_NO_VLAN_ROOM
  net: vxlan: constrain upper VLAN MTU using IFF_NO_VLAN_ROOM
  net: l2tp_eth: constrain upper VLAN MTU using IFF_NO_VLAN_ROOM
  net: gre: constrain upper VLAN MTU using IFF_NO_VLAN_ROOM
  net: distribute IFF_NO_VLAN_ROOM into aggregate devs
  net: ntb_netdev: support VLAN using IFF_NO_VLAN_ROOM
  net: vlan: disallow non-Ethernet devices
  net: leverage IFF_NO_VLAN_ROOM to limit NETIF_F_VLAN_CHALLENGED

 Documentation/networking/netdev-features.rst  |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_main.c     |   3 +-
 drivers/net/bonding/bond_main.c               |  15 ++-
 drivers/net/ethernet/intel/e100.c             |  15 ++-
 .../net/ethernet/mellanox/mlxsw/switchx2.c    |  52 ++++++--
 drivers/net/ethernet/toshiba/ps3_gelic_net.c  |  12 +-
 drivers/net/ethernet/wiznet/w5100.c           |   6 +-
 drivers/net/ethernet/wiznet/w5300.c           |   6 +-
 drivers/net/ethernet/xilinx/ll_temac_main.c   |   1 -
 drivers/net/geneve.c                          |  17 ++-
 drivers/net/ifb.c                             |   4 +
 drivers/net/loopback.c                        |   1 -
 drivers/net/macsec.c                          |   6 +-
 drivers/net/net_failover.c                    |  31 +++--
 drivers/net/ntb_netdev.c                      |   8 +-
 drivers/net/rionet.c                          |   3 +
 drivers/net/sb1000.c                          |   1 +
 drivers/net/team/team.c                       |  16 +--
 drivers/net/vrf.c                             |   4 +-
 drivers/net/vxlan.c                           |  10 +-
 drivers/net/wimax/i2400m/netdev.c             |   5 +-
 drivers/s390/net/qeth_l2_main.c               |  12 +-
 include/linux/if_vlan.h                       |  48 ++++++++
 include/linux/netdevice.h                     |  12 +-
 net/8021q/vlan.c                              |   2 +-
 net/8021q/vlan_dev.c                          |   9 ++
 net/8021q/vlan_netlink.c                      |   2 +
 net/core/dev.c                                |   2 +-
 net/ipv4/ip_tunnel.c                          |   2 +
 net/ipv6/ip6_gre.c                            |   4 +-
 net/l2tp/l2tp_eth.c                           | 114 ++++++++++--------
 net/sched/sch_teql.c                          |   3 +
 32 files changed, 290 insertions(+), 140 deletions(-)

-- 
2.26.2

Powered by blists - more mailing lists