Add Support for Geneve (RFC8926)

[pouria]

Summary

if_geneve: Add geneve support (RFC8926)
geneve creates a generic network virtualization tunnel
interface for Tentant Systems over an L3 (IP/UDP) underlay network that
provides a Layer 2 (ethernet) or Layer 3 service using the geneve protocol.
This implementation is based on RFC8926.

• IPv4 and IPv6 is fully supported for both unicast and multicast underlay.
• Per-VNET geneve tunnel is implemented.
• RXCSUM/TXCSUM/TSO offloading capabilities are implemented.
• NETLINK/WITHOUT_NETLINK are fully supported.
• ifconfig updated to support interface creation and modification using NETLINK.
• Manual for geneve(4) is added and ifconfig(8) is updated to include geneve

parameters.

I can breakdown this revision if required.
To test, please apply D54109 + D54190 patches.
Final commit for this revision must use ECN_COMPLETE from D53516 to be compliant
with RFC.

Test Plan

kyua test -k /usr/tests/sys/net/Kyuafile if_geneve

if_geneve:ether_ipv4  ->  passed  [0.169s]
if_geneve:ether_ipv4_multicast  ->  passed  [0.191s]
if_geneve:ether_ipv4_multicast_without_dev  ->  passed  [0.189s]
if_geneve:ether_ipv6  ->  passed  [0.178s]
if_geneve:ether_ipv6_blind_options  ->  passed  [0.327s]
if_geneve:ether_ipv6_external  ->  passed  [0.160s]
if_geneve:ether_ipv6_multicast  ->  passed  [0.179s]
if_geneve:ether_ipv6_multicast_without_dev  ->  passed  [0.173s]
if_geneve:inherit_ipv4  ->  passed  [0.171s]
if_geneve:inherit_ipv4_multicast  ->  passed  [0.202s]
if_geneve:inherit_ipv4_multicast_without_dev  ->  passed  [0.201s]
if_geneve:inherit_ipv6  ->  passed  [0.169s]
if_geneve:inherit_ipv6_multicast  ->  passed  [0.194s]
if_geneve:inherit_ipv6_multicast_without_dev  ->  passed  [0.192s]

[pouria]

Resolve PEP8 test warnings for netlink_route.py

[pouria]

Fix lint warning on if_geneve.sh test and remove changes to netlink_route.py to avoid error that are not mine

[glebius]

sys/net/if_clone.c

543 ↗

(On Diff #167856)

This looks like a bugfix to if_clone that is not related to Geneve. Is that correct?

sys/net/if_geneve.c

134

Can this be different to interface's vnet?

192

Can that be different to sc->gnv_ifp->if_vnet?

192

Can this be different to sc->gnv_ifp->if_vnet?

1676

I'd recommend to use some internal flag in softc for this purpose. The IFF_DRV_RUNNING is idea from the early SMP work on the network stack that did not pan out, IMHO. Just don't use it in new interface implementations.

2931

Are we compiling this file at all if neither INET and INET6 are defined? You have added glue to sys/conf/files correctly. So why do you need this (and other below) ifdef?

3176

Are we always sending with zero ip_id, even when IP_DF is not set? IMHO, ip_fillid() needs to be called, unless this is intentional.

3181

mcast can be bool and assigned as mcast = (m->m_flags & (M_MCAST | M_BCAST))

[pouria]

sys/net/if_clone.c

543 ↗

(On Diff #167856)

Yes, I asked about the reason in the original revision D39032 .
I will split this change into a separate revision.

sys/net/if_geneve.c

134

Yes, it will be used in geneve_socket_lookup to make sure we have the right vnet without any knowledge about interface.
However, after fixing the ucred of thread in D54109, this shouldn't be the case any more, so->so_vnet will be correctly set by socreate. I will remove it right away.

192

No, you're right, I can replace it with gnv_ifp, but I will remove because of fix on ucred. thank you

1676

sys/net/if_ethersubr.c use it for ether_output and ether_input_internal functions and my geneve l2 mode depends on those functions. AFAICU, It requires changes to ethersubr and its dependent modules. Correct me if I'm wrong, but I can look at it on a separate revision that is the case.

2931

The protocol itself does not require to be dependent on the INET or INET6 macros and other interfaces like if_vxlan do the same despite gluing. However, it makes no sense to me either if someone use GENEVE protocol without supporting one of the INET or INET6.
I prefer to remove it myself too, I simply followed others' footsteps on interface modules.
If that's the case, I will remove it.

3176

ip_output.c:370 will set it if we don't set the IP_FORWARDING and IP_RAWOUTPUT flags and I didn't. Please correct me if I'm wrong.

3181

Done.

[pouria]

• if_clone change is separated into D54190
• all the INET and INET6 macros are removed.
• remove vnet members from sc and gnvso due to fix on D54109
• replace int mcast with bool mcast

[glebius]

sys/net/if_geneve.c

1676

Right. But ether_output() doesn't know about geneve internal locking and checks the flag in a racy manner. This idea of a shared flag between drivers and stack is from the very early days of SMPng and IMHO it was incorrect in principle and it definitely doesn't work right now due to Ethernet layer not using internal driver locking. This is true for any driver.

A properly written driver today will not panic if Ethernet layer calls into it at any point after if_attach().

3176

You are correct, thanks!

[pouria]

sys/net/if_geneve.c

1676

So, to check if I'm understanding correctly:

1. In order to make it work I have to set the IFF_DRV_RUNNING flag anyway, but instead of geneve_init I must do this at geneve_clone_create point to make the ether layer believe that the interface is always ready and then forget about the IFF_DRV_RUNNING flag until ifdetach happens.
2. Then I should use a separate internal flag for my own uses?

Am I understand correctly?

[glebius]

sys/net/if_geneve.c

1676

Yes, exactly that. You may set it in geneve_init() as well, doesn't really matter. But there is no sense in ever clearing the flag, as it doesn't give a reliable protection against entering the driver with the flag cleared.

[pouria]

• Replace IFF_DRV_RUNNING with GENEVE_FLAG_RUNNING, address @glebius note

[zlei]

sys/net/if_geneve.c

1676

I'm recently fighting with ifp->if_drv_flags & IFF_DRV_RUNNING. I think the flag should ( if ever wanted ) be write only by the driver and protected by driver locks, but not by the net stack. For sync issue, the net stack should use atomic_load but not merely a test by ifp->if_drv_flags & IFF_DRV_RUNNING.

[glebius]

sys/net/if_geneve.c

1676

IMHO, the idea of the flag was not correct in the first place. A new properly written driver shall not rely on it as a measure to prevent the stack from talking to the driver.

[pouria]

Rebase and update according to 0bd0c3295ac09f759f2816b73cbd2d950e3bef7e .

• There is no ether_reassign, therefore, geneve_reassign removed for good.

All geneve tests are passed and reassignment is also tested.

[pouria]

Fix ability to modifying generic and link-specific attributes at the same time in geneve_clone_modify_nl