wg: Add netmap support

[markj]

Summary

When in netmap (emulated) mode, wireguard interfaces prepend or strip a
dummy ethernet header when interfacing with netmap. The netmap
application thus sees unencrypted, de-encapsulated frames with a fixed
header.

In this mode, netmap hooks the if_input and if_transmit routines of the
ifnet. Packets from the host TX ring are handled by wg_if_input(),
which simply hands them to the netisr layer; packets which would
otherwise be tunneled are intercepted in wg_output() and placed in the
host RX ring.

The "physical" TX ring is processed by wg_transmit(), which behaves
identically to wg_output() when netmap is not enabled, and packets
appear in the "physical" RX ring by hooking wg_deliver_in().

Sponsored by: Klara, Inc.
Sponsored by: Zenarmor

[vmaffione]

Looks pretty good to me, however I suggest adding some more comments to make easier to understand the four datapaths.
It's very confusing for me that I know the netmap code. I can only imagine for the casual reader...

Also the commit message says something like "... wg_transmit() inverts this ..." which makes the reader think that a packet will go through wg_output() first and then through wg_transmit(), but in reality the two functions belong to completely indendent datapaths, because wg_output() is related to the host RX ring, whereas wg_transmit() is related to the "hardware" TX ring. Same thing for the wg_if_input() (host TX ring) and wg_deliver_in() (hardware RX ring).
Maybe a little bit of rephrasing there?

sys/dev/wg/if_wg.c

1694

What about using the broadcast address here FF:FF:FF:FF:FF:FF?
If this packet ever makes it to a network switch, it would make sense to broadcast to all the switch ports because there is no specific information. Otherwise if 06:06:06:06:06:06 happens to exist, and the netmap application does not rewrite it with something real, well, the user would observer an inconsistent behaviour...

1695

Maybe drop a comment saying that here it must be if_input == freebsd_generic_rx_handler?

2214

Maybe drop a comment saying that currently this should only be called by nm_os_generic_xmit_frame()?

2252

Maybe drop a comment here that says that this is only expected to be called by nm_send_up(), to implement the host TX ring datapath?

2281

Maybe drop a comment here that says that here it must be if_transmit == netmap_transmit, and this packet will go to the host RX ring?

2294

Same comment as above... this packet will go to a netmap ring, and from there could be forwarded everywhere..
It would be better to use a destination broadcast address.

[markj]

Handle Vincenzo's feedback.

Simplify wg_transmit() a bit. In particular, it is presumed to be unused when
netmap is disabled.

[markj]

In D43460#991503, @vmaffione wrote:

Looks pretty good to me, however I suggest adding some more comments to make easier to understand the four datapaths.
It's very confusing for me that I know the netmap code. I can only imagine for the casual reader...

Also the commit message says something like "... wg_transmit() inverts this ..." which makes the reader think that a packet will go through wg_output() first and then through wg_transmit(), but in reality the two functions belong to completely indendent datapaths, because wg_output() is related to the host RX ring, whereas wg_transmit() is related to the "hardware" TX ring. Same thing for the wg_if_input() (host TX ring) and wg_deliver_in() (hardware RX ring).
Maybe a little bit of rephrasing there?

Thanks for the feedback! I'll update the review summary.

sys/dev/wg/if_wg.c

1694

Yes, that's a good idea. I made the same change in wg_xmit_netmap().

2214

Yes, and I changed it to assert that IFCAP_NETMAP is set.

[vmaffione]

Thanks.
I'm still wondering whether anyone would call wg_output(), though...

In addition to the hw TX/RX rings, have you tested that the host RX/TX rings are fully functional?
i.e., if you run bridge between the hw rings and the host rings like this

# bridge -i netmap:wg0 -i netmap:wg0^

with all the offloads disabled on wg0, you should be able to use wg0 as if netmap did not exist.
And while doing so, with some debug print you should also be able to check that wg_output() does not get called...

[markj]

In D43460#992332, @vmaffione wrote:

Thanks.
I'm still wondering whether anyone would call wg_output(), though...

In addition to the hw TX/RX rings, have you tested that the host RX/TX rings are fully functional?
i.e., if you run bridge between the hw rings and the host rings like this

# bridge -i netmap:wg0 -i netmap:wg0^

with all the offloads disabled on wg0, you should be able to use wg0 as if netmap did not exist.

Indeed, this was my smoke test while developing the patch.

And while doing so, with some debug print you should also be able to check that wg_output() does not get called...

wg_output() is the host transmit path. If I ping the remote wg interface, wg_output() is called via

KDB: stack backtrace:                                                                                                                                                                                                                                                                                                         
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe0051f83a10                                                                                                                                                                                                                                                
wg_output() at wg_output+0x1f/frame 0xfffffe0051f83a50                                                                                                                                                                                                                                                                        
ip_output() at ip_output+0x140a/frame 0xfffffe0051f83b50                                                                                                                                                                                                                                                                      
rip_send() at rip_send+0x46e/frame 0xfffffe0051f83bc0                                                                                                                                                                                                                                                                         
sosend_generic() at sosend_generic+0x5fe/frame 0xfffffe0051f83c70                                                                                                                                                                                                                                                             
sousrsend() at sousrsend+0x79/frame 0xfffffe0051f83cd0                                                                                                                                                                                                                                                                        
kern_sendit() at kern_sendit+0x1c0/frame 0xfffffe0051f83d60                                                                                                                                                                                                                                                                   
sendit() at sendit+0xb7/frame 0xfffffe0051f83db0                                                                                                                                                                                                                                                                              
sys_sendto() at sys_sendto+0x4d/frame 0xfffffe0051f83e00                                                                                                                                                                                                                                                                      
amd64_syscall() at amd64_syscall+0x153/frame 0xfffffe0051f83f30                                                                                                                                                                                                                                                               
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe0051f83f30

[vmaffione]

In D43460#992353, @markj wrote:

In D43460#992332, @vmaffione wrote:

Thanks.
I'm still wondering whether anyone would call wg_output(), though...

In addition to the hw TX/RX rings, have you tested that the host RX/TX rings are fully functional?
i.e., if you run bridge between the hw rings and the host rings like this

# bridge -i netmap:wg0 -i netmap:wg0^

with all the offloads disabled on wg0, you should be able to use wg0 as if netmap did not exist.

Indeed, this was my smoke test while developing the patch.

ok, thanks

And while doing so, with some debug print you should also be able to check that wg_output() does not get called...

wg_output() is the host transmit path. If I ping the remote wg interface, wg_output() is called via

KDB: stack backtrace:                                                                                                                                                                                                                                                                                                         
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe0051f83a10                                                                                                                                                                                                                                                
wg_output() at wg_output+0x1f/frame 0xfffffe0051f83a50                                                                                                                                                                                                                                                                        
ip_output() at ip_output+0x140a/frame 0xfffffe0051f83b50                                                                                                                                                                                                                                                                      
rip_send() at rip_send+0x46e/frame 0xfffffe0051f83bc0                                                                                                                                                                                                                                                                         
sosend_generic() at sosend_generic+0x5fe/frame 0xfffffe0051f83c70                                                                                                                                                                                                                                                             
sousrsend() at sousrsend+0x79/frame 0xfffffe0051f83cd0                                                                                                                                                                                                                                                                        
kern_sendit() at kern_sendit+0x1c0/frame 0xfffffe0051f83d60                                                                                                                                                                                                                                                                   
sendit() at sendit+0xb7/frame 0xfffffe0051f83db0                                                                                                                                                                                                                                                                              
sys_sendto() at sys_sendto+0x4d/frame 0xfffffe0051f83e00                                                                                                                                                                                                                                                                      
amd64_syscall() at amd64_syscall+0x153/frame 0xfffffe0051f83f30                                                                                                                                                                                                                                                               
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe0051f83f30

got it, thanks