It’s in a non performance critical path with no side effects if not used, so worst case it’s not useful to other consumers.

LGTM

In D26137#612610, @zarychtam_plan-b.pwste.edu.pl wrote:

Thanks for in-kernel Wireguard. That's really great news before 13-STABLE is branched !
Everything works fine for me allowing to tunnel both legacy IP and IPv6 over legacy IP link. I was not able to utilise IPv6 address as tunnel endpoint so far. It failed with such an error: "wg0: wg_peer_add bad length for endpoint 28". Will tunnelling over IPv6 be supported in future?

• build fixes for tier 2 & 3 architectures

• more dead code GC
• add header licenses

• garbage collect dead code

In D26137#611002, @peter_libassi.se wrote:

In D26137#610778, @sg2342_googlemail.com wrote:

moved the test setup to a different machine and after 1 hour and 19 minutes of running the test setup i got a panic here:

Stefan, I'm on r367980 with diff 79843 and I manually removed the mfree line in wg_encap since the latest diff 79919 could not be used. I've run your test for over 6 hours now without any panic. I even added iperf3 --udp and bombarded the server over the wg link for one hour. Only difference what I understand is that i'm on a bare metal server and you run in a bhyve/vale instance. Could what you see now instead be an issue with the virtualization layer?

I don't have time to test right now, but this is an analogous double free fix in the wg_encap path that I did earlier in the wg_decap path.

• don't prematurely free in wg_encap

• fix BPF on TX

• update uio_bio structure to support scatter gather as well as improve interop with linux

• fix BPF issue
• avoid socket operations when link is down
• fix use after free

In D26137#601504, @sg2342_googlemail.com wrote:

with the same setup (on FreeBSD: while true; do ifconfig wg0 create .....; ping -c 1 PEERIP; sleep 1; ifconfig wg0 destroy; done and on the Linux peer: ping -f FreeBSDwgIP) i can also get a different panic: here the gtaskqueue_drain thread got to wg_deliver_in(...) but peer->p_sc->sc_socket->so_so4 is 0x0

Fatal trap 12: page fault while in kernel mode
cpuid = 1; apic id = 01
fault virtual address   = 0xd8
fault code              = supervisor read data, page not present
instruction pointer     = 0x20:0xffffffff823123ef
stack pointer           = 0x28:0xfffffe004c8daa60
frame pointer           = 0x28:0xfffffe004c8dab00
code segment            = base 0x0, limit 0xfffff, type 0x1b
                        = DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags        = interrupt enabled, resume, IOPL = 0
current process         = 0 (if_io_tqg_1)
trap number             = 12
panic: page fault
cpuid = 1
time = 1603769864
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe004c8da710
vpanic() at vpanic+0x182/frame 0xfffffe004c8da760
panic() at panic+0x43/frame 0xfffffe004c8da7c0
trap_fatal() at trap_fatal+0x387/frame 0xfffffe004c8da820
trap_pfault() at trap_pfault+0x97/frame 0xfffffe004c8da880
trap() at trap+0x2ab/frame 0xfffffe004c8da990
calltrap() at calltrap+0x8/frame 0xfffffe004c8da990
--- trap 0xc, rip = 0xffffffff823123ef, rsp = 0xfffffe004c8daa60, rbp = 0xfffffe004c8dab00 ---
wg_deliver_in() at wg_deliver_in+0x24f/frame 0xfffffe004c8dab00
gtaskqueue_run_locked() at gtaskqueue_run_locked+0xa7/frame 0xfffffe004c8dab80
gtaskqueue_thread_loop() at gtaskqueue_thread_loop+0x94/frame 0xfffffe004c8dabb0
fork_exit() at fork_exit+0x80/frame 0xfffffe004c8dabf0
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe004c8dabf0
--- trap 0, rip = 0, rsp = 0, rbp = 0 ---
KDB: enter: panic
Uptime: 32m27s
Dumping 555 out of 8062 MB:..3%..12%..21%..32%..41%..52%..61%..72%..81%..93%

__curthread () at /usr/src/sys/amd64/include/pcpu_aux.h:55
55              __asm("movq %%gs:%P1,%0" : "=r" (td) : "n" (offsetof(struct pcpu,
(kgdb) #0  __curthread () at /usr/src/sys/amd64/include/pcpu_aux.h:55
#1  doadump (textdump=1) at /usr/src/sys/kern/kern_shutdown.c:394
#2  0xffffffff80be05b0 in kern_reboot (howto=260)
    at /usr/src/sys/kern/kern_shutdown.c:481
#3  0xffffffff80be09fa in vpanic (fmt=<optimized out>, ap=<optimized out>)
    at /usr/src/sys/kern/kern_shutdown.c:912
#4  0xffffffff80be0763 in panic (fmt=<unavailable>)
    at /usr/src/sys/kern/kern_shutdown.c:838
#5  0xffffffff8102b2b7 in trap_fatal (frame=0xfffffe004c8da9a0, eva=216)
    at /usr/src/sys/amd64/amd64/trap.c:915
#6  0xffffffff8102b357 in trap_pfault (frame=0xfffffe004c8da9a0, 
    usermode=<optimized out>, signo=<optimized out>, ucode=<optimized out>)
    at /usr/src/sys/amd64/amd64/trap.c:732
#7  0xffffffff8102a94b in trap (frame=0xfffffe004c8da9a0)
    at /usr/src/sys/amd64/amd64/trap.c:398
#8  <signal handler called>
#9  0xffffffff823123ef in wg_deliver_in (peer=0xfffff80164e98000)
    at /usr/src/sys/dev/if_wg/module/if_wg_session.c:1572
#10 0xffffffff80c2aa27 in gtaskqueue_run_locked (queue=0xfffff8000347bc00)
    at /usr/src/sys/kern/subr_gtaskqueue.c:371
#11 0xffffffff80c2a824 in gtaskqueue_thread_loop (arg=<optimized out>)
    at /usr/src/sys/kern/subr_gtaskqueue.c:547
#12 0xffffffff80b9b9c0 in fork_exit (
    callout=0xffffffff80c2a790 <gtaskqueue_thread_loop>, 
    arg=0xfffffe004ca97020, frame=0xfffffe004c8dac00)
    at /usr/src/sys/kern/kern_fork.c:1052
#13 <signal handler called>
(kgdb) 

------------------------------------------------------------------------

Thank you.

In general if you're super eager to merge some change that for which there's no pressing reason to merge it because I haven't had time to MFV. I'd rather that you simply volunteer the time to do the MFV yourself. Thanks.

• build fixes :-/

• fix ifwg.c compile
• avoid enqueueing tasks when link is down
• wait for tasks to complete before detach

In D26137#597241, @sg2342_googlemail.com wrote:

Another kernel panic triggered by interface destruction: incoming upd traffic from the wg peer arrives in wg_input() where sc is already gone.

Fatal trap 9: general protection fault while in kernel mode
cpuid = 0; apic id = 00
instruction pointer     = 0x20:0xffffffff80cefaad
stack pointer           = 0x28:0xfffffe000eb13610
frame pointer           = 0x28:0xfffffe000eb13610
code segment            = base 0x0, limit 0xfffff, type 0x1b
                        = DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags        = interrupt enabled, resume, IOPL = 0
current process         = 12 (irq43: virtio_pci1)
trap number             = 9
panic: general protection fault
cpuid = 0
time = 1602732063
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe000eb13320
vpanic() at vpanic+0x182/frame 0xfffffe000eb13370
panic() at panic+0x43/frame 0xfffffe000eb133d0
trap_fatal() at trap_fatal+0x387/frame 0xfffffe000eb13430
trap() at trap+0xa4/frame 0xfffffe000eb13540
calltrap() at calltrap+0x8/frame 0xfffffe000eb13540
--- trap 0x9, rip = 0xffffffff80cefaad, rsp = 0xfffffe000eb13610, rbp = 0xfffffe000eb13610 ---
if_inc_counter() at if_inc_counter+0xd/frame 0xfffffe000eb13610
wg_input() at wg_input+0xa3/frame 0xfffffe000eb13650
udp_append() at udp_append+0x81/frame 0xfffffe000eb136c0
udp_input() at udp_input+0xa2f/frame 0xfffffe000eb13790
ip_input() at ip_input+0x194/frame 0xfffffe000eb13820
netisr_dispatch_src() at netisr_dispatch_src+0xb1/frame 0xfffffe000eb13880
ether_demux() at ether_demux+0x16e/frame 0xfffffe000eb138b0
ether_nh_input() at ether_nh_input+0x408/frame 0xfffffe000eb13910
netisr_dispatch_src() at netisr_dispatch_src+0xb1/frame 0xfffffe000eb13970
ether_input() at ether_input+0xa1/frame 0xfffffe000eb139d0
vtnet_rxq_input() at vtnet_rxq_input+0x200/frame 0xfffffe000eb13a10
vtnet_rxq_eof() at vtnet_rxq_eof+0x63d/frame 0xfffffe000eb13ae0
vtnet_rx_vq_process() at vtnet_rx_vq_process+0x97/frame 0xfffffe000eb13b20
ithread_loop() at ithread_loop+0x279/frame 0xfffffe000eb13bb0
fork_exit() at fork_exit+0x80/frame 0xfffffe000eb13bf0
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe000eb13bf0
--- trap 0, rip = 0, rsp = 0, rbp = 0 ---
KDB: enter: panic

• rebase
• fix WGC_SET priv_check to work in jails
• mark link down before starting detach

This will get merged in with the next MFV.

We can continue any further discussion on the PR itself. The only thing left resolve is whether or not to heed the environment variable that Brian mentioned.

@cy This all looks fine, but for anything that isn't an urgent bug I greatly prefer that it makes it way in by MFV. Thanks.

• add sparse page array support to uiobiomove

@asomers can you let me what more you'd like to see in the documentation.

• comment new functions in vfs_aio.c
• add man page for uio_bio (includes uiobiomove) and VOP_UBOP.

• Fix run_send_keepalive panic

In D26137#596298, @peter_libassi.se wrote:

It seems that the new wg interface is not completely jail-ready yet. I'm exposing the wg interface in devfs.rules with
[devfsrules_jail_wg=10]
add include $devfsrules_jail_vnet
add path 'wg*' unhide

Inside the jail i can create the wg interface. However i'm not allowed to add peers.

ifconfig wg0 create .... gives: ifconfig: failed to install peer

wg setconf wg0 ... gives: Unable to modify interface: Operation not permitted

Could it be that the wg peer structures are not exposed to the jail?

Once you can get ZTS to pass it LGTM.

In D26726#595827, @asomers wrote:

Could we get some documentation for the uiobiomove, aio_qasync, and VOP_UBOP ?

• Don't advertise checksum offload

In D26137#592895, @sg2342_googlemail.com wrote:

This might be out of scope of this review:
the WGC_SET ioctl is not priv(9) checked (and there is no PRIV_NET_WG entry in sys/priv.h)

• rebase against master
• don't print the first allowedip repeatedly
• don't print the private key for unprivileged users
• priv_check WGC_SET

In D26137#587341, @sg2342_googlemail.com wrote:

In D26137#587248, @mmacy wrote:

In D26137#586721, @sg2342_googlemail.com wrote:

According to ifconfig(8): Cloned interfaces are members of their interface family group by default.

The use of iflib_clone_register() in module.c prevents that this is done for if_wg.

How so? iflib_clone_register calls if_clone_simple which is what vxlan does as well.

iflib_clone_register does set

ifc_flags_set(ip->ip_ifc, IFC_NOGROUP);

• Don't disable groups in iflib

• handle empty peer list
• limit ioctl command set to WGC_GET, WGC_SET

In D26137#586721, @sg2342_googlemail.com wrote:

According to ifconfig(8): Cloned interfaces are members of their interface family group by default.

The use of iflib_clone_register() in module.c prevents that this is done for if_wg.

• don't fail status on alias

In D26137#586901, @peter_libassi.se wrote:

Bad news, remember https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247853
comment 6 item2 and comment 8. Local access to wg-host services was early an issue, then with D26137 this issue was solved and do still work.

Now i've found as it seems that issue is the other way around, services on a remote host is not accessible. Below my test setup:

bsd1 em0:172.16.0.150/24 --- bsd2 em0:172.16.0.179/24

bsd2 ue0:172.16.42.1/24 ---bsd22 172.16.42.2

bsd1 wg0:192.168.3.1/24 ----- bsd2 wg0:192.168.3.2/24

ping and traceroute works to all IP addresses
bsd1 ssh to bsd2 192.168.3.2 works
bsd1 ssh to bsd2 172.16.42.1 works
bsd1 ssh to bsd22 172.16.42.2 does not work

I tried these routes over wg0, same result:
route add -inet 172.16.42.0/24 192.168.3.2
route add -inet 172.16.42.0/24 -interface wg0

bsd1 ssh to bsd22 works (of course) if I change the route to
route add -inet 172.16.42.0/24 172.16.0.179
i.e no wireguard, instead via local lan em0

tested on r365550 with Diff 76838

• make fpu kern thread check work on arm64

• start slimming down ioctl interface with the idea to eventually just use WGC_{GET, SET}

In D26137#585962, @sg2342_googlemail.com wrote:

if the wg interface has an ipv6 address, SIOCIFDESTROY can panic the kernel.

looking at the backtraces, i guess what happens is:
mld_fasttimo() tries to emit a ipv6 MLD packet and wg_route_lookup() touches
memory that was just released by the thread that was responsible for device
destruction.

minimal code to reproduce (keys and addresses are irrelevant):

• work around mld bug

In D26137#586328, @peter_libassi.se wrote:

Hostname in the endpoint directive does not resolve to an IP address:

ifconfig wg0 create \
private-key wN4PXBViEY7uAwn7HVD+Z3Qn3E8yB4wD60jSki5+SF4= \
listen-port 3333 \
peer public-key cu2RbOX8183OLuKo7CbTNhLelGluVeiQ7jFhy50kxA8= \
endpoint bsd2:3333 \
allowed-ips 192.168.3.0/24

ifconfig: Name does not resolve

userland command 'getaddrinfo bsd2' returns:
dgram inet udp 172.16.0.179 0
stream inet tcp 172.16.0.179 0
seqpacket inet sctp 172.16.0.179 0

• allow non-numeric endpoints

Also I think wg(8) need an update after the recent fixes. wg setconf stopped working, It does not add peers, ifconfig wg0 peer-list gives:
ifconfig: can't get peer list size

that it's generally accepted that ifconfig is the standard tool used for all networking configuration, but duplicating all of wg(8) in to ifconfig is tedious and feels a bit redundant.

Probably not important for a v1, but it might be nice to eventually move wg(8) functionality to libifconfig and use the linked library to avoid reimplementing the wheel too much.

• fix allowedips in peer-list output

• place output of successful diagnostics under bootverbose
• fix tcpdump: WARNING: wg0: That device doesn't support promiscuous mode complaints

#
What are the design decisions made here? Is it to implement wg(8) 'setconf' functionality into ifconfig then drop wg(8) and then adopt wg-quick(8) to use ifconfig? if so thats fine and correct IMHO. This would then also require implementation of wg(8) 'show' which gives traffic stats and latest handshake per peer, a good place for that could then be netstat? How about the key management in wg(8)?

• disable debug noise by default

In D26137#585115, @peter_libassi.se wrote:

tested on r364973

Resolved issues:

1. The issue described in comment 8 in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247853

• incorporate HRS feedback

• fix clean build issues due to circular dependencies

https://github.com/mattmacy/networking/tree/projects/wireguard_merge_update is a WIP with updates for the provided feedback along with some observed build issues.

In D26137#584256, @olivier wrote:

Comparing wireguard userland vs kernel module on a small device: +300% (from 117Mb/s to 483Mb/s)

• PC Engines APU2C4 (quad core AMD GX-412T Processor 1 GHz)
• 3 Intel i210AT Gigabit Ethernet ports
• FreeBSD 13-head r365033
• Wireguard kernel: D26137
• Wireguard userland: 1.0.20200827
• 2000 flows of UDP packets
• 500Bytes UDP load => packet size: 528B => Ethernet frame size:542B

rebase

• whitespace