Bug 274428 - net/wireguard-tools: host service restart kldunloads if_wg, taking it away from VNET jails
Summary: net/wireguard-tools: host service restart kldunloads if_wg, taking it away fr...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Bernhard Froehlich
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-10-12 11:38 UTC by vedad
Modified: 2023-11-06 11:23 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (decke)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description vedad 2023-10-12 11:38:54 UTC
Hello,

Under a setup where wireguard is being used both on the host system and VNET
jails, doing a `service wireguard stop` or `service wireguard restart` on the
host system unconditionally kldunload's if_wg, thus taking away wireguard
tunnels from jails (the interfaces simply disappear from jails), requiring a
manual wireguard restart in jails (as long as if_wg is loaded).

On my own systems, i have simply commented the kldunload bit from the host rc.d
script as a workaround, but that's obviously not the right approach for the
port.

I actually don't know what the right approach would be.

Perhaps an rcvar setting could be introduced to prevent the kldunload?

Thanks,
Kind regards
Comment 1 crest 2023-10-30 12:06:14 UTC
Which WireGuard rc.d script are you using? The one I wrote from that's still stuck in PR/review, wg-quick, something else?

Is it also unloaded if if_wg.ko is loaded via if_wg_load="YES" in /boot/loader.conf or kld_list="if_wg ..." in /etc/rc.conf?
Comment 2 Zhenlei Huang freebsd_committer freebsd_triage 2023-11-03 14:49:04 UTC
I do not see a good reason to unload modules previously loaded on restarting services.
Comment 3 Zhenlei Huang freebsd_committer freebsd_triage 2023-11-03 14:51:52 UTC
(In reply to crest from comment #1)
> Which WireGuard rc.d script are you using? The one I wrote from that's still
> stuck in PR/review, wg-quick, something else?
Is it https://reviews.freebsd.org/D41318 ?
Comment 4 vedad 2023-11-05 18:02:51 UTC
(In reply to crest from comment #1)

I'm referring to the /usr/local/etc/rc.d/wireguard script, installed by the net/wireguard-tools port.

The offending lines are:

if kldstat -q -n if_wg; then
	   if ! kldunload if_wg > /dev/null 2>&1; then
			   warn "Can't unload if_wg module."
			   return 1
	   fi
fi

This will unconditionally klunload the if_wg module, whether it was loaded via /boot/loader.conf, manually, or by the above wireguard rc script.
Comment 5 Bernhard Froehlich freebsd_committer freebsd_triage 2023-11-05 21:03:06 UTC
I think the commit message from back then describes it pretty well:

net/wireguard-tools: Unload if_wg kernel module after stop if it was loaded

This helps to reload also the kernel module after an update with a simple
restart or stop/start of the service.


https://svnweb.freebsd.org/ports?view=revision&revision=569184
Comment 6 Zhenlei Huang freebsd_committer freebsd_triage 2023-11-06 08:19:06 UTC
(In reply to Bernhard Froehlich from comment #5)
> net/wireguard-tools: Unload if_wg kernel module after stop if it was loaded

> This helps to reload also the kernel module after an update with a simple
> restart or stop/start of the service.


> https://svnweb.freebsd.org/ports?view=revision&revision=569184

Then that will certainly interrupt wireguard tunnels in VNET jails.
Also it does not work inside jails.

Given,
 1. wireguard has been re-imported to base since 744bfb213144 (Import the WireGuard driver from zx2c4.com.)
 2. It now works greatly in VNET jails.
 3. It is stable enough and for a given support life time of release (e.g. 14.0), it should not be get updated frequently (except for security issues / serious bugs). 

IMO the previous commit https://svnweb.freebsd.org/ports?view=revision&revision=569184 helps reloading if_wg kernel module make more troubles than it resolves.

Users should be aware to reload modules and then restart all wireguard services in / out jails.
Comment 7 Bernhard Froehlich freebsd_committer freebsd_triage 2023-11-06 11:05:53 UTC
The environment this rc script is used in has changed quite a bit over time.

When it was written the main purpose was to test the wireguard-kmod port and make sure that a wireguard restart also uses the new kmod. We had frequent updates and I am not sure if it worked in VNET jails.

Now the wireguard-kmod port is almost gone so frequent updates might only happen on CURRENT where kernel and wg(4) are updated in sync - so a reboot is required anyway.

Looking a bit over the fence kldunload seems to be a common practice in similar ports:

net-mgmt/ng_ipacct
net/ndproxy

Considering all of it I think you are right and it makes sense to revert that commit.
Comment 8 commit-hook freebsd_committer freebsd_triage 2023-11-06 11:22:38 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=c83f22d493f0736384be0b389122208955dcfbee

commit c83f22d493f0736384be0b389122208955dcfbee
Author:     Bernhard Froehlich <decke@FreeBSD.org>
AuthorDate: 2023-11-06 11:09:20 +0000
Commit:     Bernhard Froehlich <decke@FreeBSD.org>
CommitDate: 2023-11-06 11:09:20 +0000

    net/wireguard-tools: Revert kldunload from rc.d scripts

    Unloading if_wg(4) kmod was added to rc.d scripts to make sure that a
    service restart also reload the kernel module.

    Now we don't have frequent updates anymore and the wireguard-kmod port
    will soon be gone but people using this script face issues in VNET jails.

    This commit reverts 562d171b9dacad8f63e6e4a45035824b10b59341

    PR:             274428
    Reported by:    vedad@kajtaz.net

 net/wireguard-tools/Makefile                   | 2 +-
 net/wireguard-tools/files/wireguard_lite.in    | 7 -------
 net/wireguard-tools/files/wireguard_wgquick.in | 7 -------
 3 files changed, 1 insertion(+), 15 deletions(-)