Details

Reviewers

tuexen
glebius
rscheff
kp
peter.lei_ieee.org
melifaro
zlei

Group Reviewers

network
transport

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.

parameters.

Test Plan

see D55183

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 71503
Build 68386: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

glebius added inline comments.Dec 11 2025, 5:21 PM

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 mentioned this in D54190: if_clone: don't overwrite dump_nl of drivers.Dec 12 2025, 12:37 PM

pouria planned changes to this revision.Dec 12 2025, 12:39 PM

pouria added inline comments.

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.

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

Harbormaster completed remote builds in B69198: Diff 167915.Dec 12 2025, 12:42 PM

pouria edited the summary of this revision. (Show Details)Dec 12 2025, 12:44 PM

pouria added a parent revision: D54190: if_clone: don't overwrite dump_nl of drivers.Dec 12 2025, 1:12 PM

glebius added inline comments.Dec 12 2025, 4:47 PM

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 added inline comments.Dec 12 2025, 5:23 PM

sys/net/if_geneve.c
1676	So, to check if I'm understanding correctly: 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. Then I should use a separate internal flag for my own uses? Am I understand correctly?

glebius added inline comments.Dec 12 2025, 5:26 PM

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.

Replace IFF_DRV_RUNNING with GENEVE_FLAG_RUNNING, address @glebius note

Harbormaster completed remote builds in B69217: Diff 167941.Dec 12 2025, 9:15 PM

pouria added a reviewer: manpages.Dec 13 2025, 12:38 PM

crest_freebsd_rlwinm.de added a subscriber: crest_freebsd_rlwinm.de.Dec 15 2025, 3:29 PM

zlei added a subscriber: zlei.Dec 16 2025, 4:03 AM

zlei added inline comments.

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 added inline comments.Dec 16 2025, 4:24 PM

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.

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.

Harbormaster completed remote builds in B69472: Diff 168596.Dec 26 2025, 8:07 AM

zlei mentioned this in D54387: gre: use its own internal flag.Dec 29 2025, 4:53 AM

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

Harbormaster completed remote builds in B69563: Diff 168865.Jan 1 2026, 11:26 PM

emaste added a subscriber: emaste.Jan 14 2026, 8:52 PM

pouria added inline comments.Feb 7 2026, 6:42 PM

sbin/ifconfig/ifgeneve.c
233 ↗	(On Diff #168865)	D54443: @zlei This is an example of netlink implementation of ifconfig for geneve. For the same reason, we need the `nl_parsed_geneve` here. That's why I want the `nl_parsed_gre` to be public KPI.

pouria added a reviewer: zlei.Feb 7 2026, 8:53 PM

I'm going to breakdown this revision to be easier to review.
So, sorry for noise in advance.

Rebase geneve to main and limit this revision kernel only.
ifconfig netlink helper: D55174
ifconfig geneve implementation (netlink): D55184
ifconfig geneve implementation (without netlink): D55185
ifconfig(8) manual for geneve: D55181
geneve(4) manual: D55182
geneve tests: D55183
update geneve to follow RFC 6040: D55186

If you want to test all of the Geneve patches together,
you can find them with all patches applied on my github:
https://github.com/spmzt/freebsd-src/tree/geneve_total

pouria edited the summary of this revision. (Show Details)Feb 8 2026, 4:25 PM

pouria edited the test plan for this revision. (Show Details)

pouria removed a reviewer: manpages.

pouria removed a parent revision: D53516: Update ip_ecn to RFC 6040.

pouria removed a subscriber: ziaee.

pouria added a child revision: D55182: geneve(4): Add geneve manual.Feb 8 2026, 4:27 PM

pouria added a child revision: D55183: geneve: Add geneve tests.

pouria added a child revision: D55184: ifconfig: Add support for geneve (netlink).

pouria added a child revision: D55186: geneve: follow RFC 6040 and use ECN_COMPLETE.Feb 8 2026, 4:30 PM

pouria marked 12 inline comments as done.Feb 8 2026, 4:50 PM

pouria mentioned this in D55602: Pouria's geneve report for Status/2026Q.Mar 1 2026, 1:01 PM

@glebius Do you think removing nvlist/non-netlink support will help this revision get reviewed faster?
since it will reduce the code size.

Sorry if I wasn't clear - I have no objections against pushing this in. I'm not able to do a protocol level review, but wrt the interaction with the rest of the network stack all looks good to me.

P.S. My only comment was about use of IFF_DRV_RUNNING, which IMHO is not a real protection from stack calling into the driver. You can improve that later, though.

This revision is now accepted and ready to land.Mon, Mar 16, 6:47 PM

Rebase to main, apply and close D55186.

This revision now requires review to proceed.Thu, Mar 19, 10:37 AM

Harbormaster completed remote builds in B71503: Diff 173889.Thu, Mar 19, 10:37 AM

In D54172#1277853, @pouria wrote:

Do you think removing nvlist/non-netlink support will help this revision get reviewed faster?
since it will reduce the code size.

FYI, deleting nvlist/non-netlink support, removes ~550 LOC.

zarychtam_plan-b.pwste.edu.pl added a subscriber: zarychtam_plan-b.pwste.edu.pl.Thu, Mar 19, 1:38 PM

I have not looked through the related RFC. So this quick review is not completed.

sys/net/if.h
258	I'd like to test this HW offload feature for GENEVE, but I think the ethernet drivers in-tree are not ready for that yet. @pouria Do you have any WIP updates for any drivers ?
sys/net/if_geneve.c
97	The sx lock `geneve_sx` is only used in this file. Make it static to limit the scope.
612	I guess a memcmp() is sufficient ?
2102	The in-tree drivers are approaching to netlink based config. Shall we still support nvlist ? @kp How do you think about this ?
3079	No need for parentheses in case labels.
3093	Ditto.
3106	I believe the default branch is unreachable.
3200	Shall the drivers be updated to support the inner csum offload ?
3236	Unreachable code. The `ip_output()` shall handle the mbuf, and free it if error occurs. So this is not required.
3384	Ditto.
4533	Shall this move to VNET_SYSUNINIT ?

Address @zlei comments

Harbormaster completed remote builds in B71828: Diff 174551.Mon, Mar 30, 4:00 PM

Thank you for your review.

sys/net/if.h
258	You're right. There are drivers that should support GENEVE hw offload, but not implemented yet. I want to work on it after geneve lands. I'm currently busy with v6 and routing stack.
sys/net/if_geneve.c
612	It'll be used in line 785 to order entries on the ftable. It's basically the same mechanism used in if_vxlan.
2102	np from me. I've a branch prepared without nvlist. I also like to remove the nvlist support too.
3106	In that case, IMHO, we shouldn't panic.
3200	Of course, but I don't want to update them on this revision. I prepared the CSUM requirements for geneve here and I'll update other drivers in another revision.
4533	It should, but please see the D54175.

Add Support for Geneve (RFC8926)
Needs ReviewPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 173889

sys/conf/NOTES

sys/conf/files

sys/kern/kern_jail.c

sys/modules/Makefile

sys/modules/if_geneve/Makefile

sys/net/if.h

sys/net/if.c

sys/net/if_geneve.h

sys/net/if_geneve.c

sys/net/if_strings.h

sys/netlink/route/interface.h

sys/sys/mbuf.h

sys/sys/priv.h

Add Support for Geneve (RFC8926)Needs ReviewPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 173889

sys/conf/NOTES

sys/conf/files

sys/kern/kern_jail.c

sys/modules/Makefile

sys/modules/if_geneve/Makefile

sys/net/if.h

sys/net/if.c

sys/net/if_geneve.h

sys/net/if_geneve.c

sys/net/if_strings.h

sys/netlink/route/interface.h

sys/sys/mbuf.h

sys/sys/priv.h

Add Support for Geneve (RFC8926)
Needs ReviewPublic
Actions

Revision Contents
Changeset List