Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 15 Oct 2022 19:23:49 +0100
From:      "Alexander V. Chernikov" <melifaro@ipfw.ru>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        freebsd-arch <freebsd-arch@freebsd.org>
Subject:   Re: Re-importing WireGuard driver and utilities
Message-ID:  <04EAB90E-06B7-43DC-8D70-8A403E789458@ipfw.ru>
In-Reply-To: <742c4fe8-4c25-d7e5-1df3-b2851d90e630@FreeBSD.org>
References:  <742c4fe8-4c25-d7e5-1df3-b2851d90e630@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 13 Oct 2022, at 18:55, John Baldwin <jhb@FreeBSD.org> wrote:
>=20
> Over the past several months, I have spent some time reviewing the
> WireGuard driver including its interactions with the rest of the
> kernel and its use of crypto in the kernel.  This work was sponsored
> by the FreeBSD Foundation and had a few goals:
>=20
> 1) Review the driver generally and how it interacted with other parts
>   of the kernel.
>=20
> 2) Review the use of crypto in the driver and evaluate the feasibility
>   of using existing kernel crypto code where possible.  Specifically
>   included in this goal was aiming to make use of accelerated
>   software crypto via the ossl(4) driver for datapath crypto.
>=20
> 3) Work with upstream to fix any issues encoutered and evaluate the
>   potential for reintegrating into the source tree.
>=20
> In terms of the driver in general, I found a few minor nits and
> developed fixes for those that were accepted upstream.  I did make one
> non-trivial change (also accepted upstream) to fix a CPU scheduling
> inefficiency that dated back to the originally-imported driver.
> Initially, each input or output packet resulted in scheduling
> encryption or decryption tasks on every CPU in the system.  I modified
> this part of the driver to follow the Linux driver and instead
> schedule a single task on a single CPU (chosen via round-robin) for
> each packet.  This improved throughput in my (simple) tests over epair
> interfaces far more than switching to use ossl(4) for the datapath
> crypto.  Another user also reported that this change alone reduced the
> power overhead of FreeBSD VMs using WireGuard in ESXi to be nearly on
> par with Linux due to avoiding wasted CPU cycles on tasks that did no
> actual work.
>=20
> For the crypto used in the driver, I have extended existing crypto
> APIs in the kernel to support the algorithms used by WireGuard that
> weren't already present (and these changes are already in main).  In
> general I have not imported any new crypto code into the kernel, but
> have added wrappers (often with APIs compatible with Linux) for
> existing (but previously unused) routines from libsodium.
>=20
> That said, in my review of the driver I also found that the existing
> crypto implementations in the WireGuard driver were fine from a
> correctness standpoint.  Still, I prefer to minimize the number of
> copies of crypto algorithm implementations when possible to minimize
> future maintenance.
>=20
> One bit of crypto is still used directly by the WireGuard driver which
> is the use of the Blake2 hashes.  In particular, the construction of
> the Blake2 hashes requires the length of the desired digest as an
> input into initializing the authentication state (similar to how
> CBC-MAC used in AES-CCM encodes L in block 0).  Here FreeBSD's
> in-kernel Blake2 implementation is somewhat broken as it hardcodes
> only a single digest length.  While OCF itself supports a notion of
> variable digest lengths via the csp_mlen field, the software
> auth_xform interface does not pass this length down to the Init() or
> SetKey() routines, so the auth_xform wrappers of Blake2 are not able
> to support variable digest lengths.  This could be fixed in the future
> by altering the auth_xform interface.  However, WireGuard's use of
> Blake2 is not a good fit for OCF.  Instead, adding an explicit
> "library" API around the existing Blake2 implementation is probably
> the most straightforward path if we want to eliminate the last of
> WireGuard's internal crypto.
>=20
> On the third question, I feel that the current driver is stable and
> suitable for bring back into the source tree.
>=20
> One question compared to the previous driver import is how to manage
> the configuration of WireGuard tunnels.  The previous driver added new
> commands to ifconfig(8) that largely mapped one to one with commands
> passed to the upstream wg(8) tool.  Upstream WireGuard has since
> relicensed their upstream wg(8) tool under a dual MIT/GPL license
> permitting direct use on FreeBSD.  Using the existing tooling means
> that existing recipes for configuring WireGuard on Linux using wg(8)
> also apply directly on FreeBSD.  It also provides a more seamless
> transition path for existing users of the WireGuard driver and utility
> in ports vs adding FreeBSD-specific extensions to ifconfig(8).
>=20
> Given all that, Kyle Evans, Ed Maste, and myself would like to move
> forward with importing the current WireGuard driver and wg(8) utility
> into FreeBSD.  =46rom upstream's perspective, the driver should simply
> move into the tree and be maintained directly in the tree (e.g. as
> sys/dev/wg).  The wg(8) tool, on the other hand, will be continued to
> be maintained by upstream, and so would be imported as normal
> third-party software into contrib/.
I love the separation boundary (provided that upstream is happy with =
it). Having wg(8) as a separate utility, that=E2=80=99s maintained by =
the software author is also great.
Do you folks have any opinion on the interface used to control the =
driver? Currently, wg(8) uses netlink on Linux, ioctl+nvlist on FreeBSD =
and some custom ioctls on OpenBSD.
Netlink support recently landed to main & there are plans to merge it to =
13-S so it=E2=80=99s available in 13.2.
Any thoughts on switching our interface to netlink? I=E2=80=99m happy to =
write the interface & tests if there are no objections.

>=20
> I have uploaded the main driver for review (along with some followup
> local cleanup changes) to https://reviews.freebsd.org/D36909.  The
> cleanups are included in the phabricator stack off of that review.
>=20
> --=20
> John Baldwin
>=20




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?04EAB90E-06B7-43DC-8D70-8A403E789458>