Bug 263297 - [netinet] [742e721] Changes in udp_tun_func_t() breaks if_wg from net/wireguard-kmod
Summary: [netinet] [742e721] Changes in udp_tun_func_t() breaks if_wg from net/wiregua...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Bernhard Froehlich
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2022-04-15 16:54 UTC by Rainer Hurling
Modified: 2022-05-06 18:42 UTC (History)
10 users (show)

See Also:


Attachments
patch and Makefile addition (1.87 KB, text/plain)
2022-04-25 12:32 UTC, imbutler
no flags Details
Improved patch for if_wg.c, differentiating between the new and the old behaviour (2.14 KB, patch)
2022-04-26 18:15 UTC, Trond Endrestøl
no flags Details | Diff
Improved patch for if_wg.c, differentiating between the new and the old behaviour, added UTC timestamp for the original file (2.10 KB, patch)
2022-04-26 18:35 UTC, Trond Endrestøl
no flags Details | Diff
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format (2.18 KB, patch)
2022-04-27 06:40 UTC, Trond Endrestøl
no flags Details | Diff
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format (2.18 KB, patch)
2022-04-27 06:43 UTC, Trond Endrestøl
no flags Details | Diff
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format (2.18 KB, patch)
2022-04-27 06:44 UTC, Trond Endrestøl
no flags Details | Diff
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format (2.28 KB, patch)
2022-04-27 06:46 UTC, Trond Endrestøl
no flags Details | Diff
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format (2.28 KB, patch)
2022-05-01 12:43 UTC, Trond Endrestøl
no flags Details | Diff
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format (2.36 KB, patch)
2022-05-01 13:03 UTC, Trond Endrestøl
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Hurling freebsd_committer freebsd_triage 2022-04-15 16:54:02 UTC
On my 14.0-CURRENT amd64 box, it seems that since the commit of base r742e721 [1] ("allow udp_tun_func_t() to indicate it did not eat the packet"), the net/wireguard-kmod port build has stopped working:


[..snip..]
===>  Building for wireguard-kmod-0.0.20211105
machine -> /usr/src/sys/amd64/include
x86 -> /usr/src/sys/x86/include
awk -f /usr/src/sys/tools/makeobjops.awk /usr/src/sys/kern/device_if.m -h
awk -f /usr/src/sys/tools/makeobjops.awk /usr/src/sys/kern/bus_if.m -h
awk -f /usr/src/sys/tools/makeobjops.awk /usr/src/sys/net/ifdi_if.m -h
touch opt_global.h
Warning: Object directory not changed from original /usr/obj/usr/src/amd64.amd64/sys/RHURLIN/usr/ports/net/wireguard-kmod/work/wireguard-freebsd-0.0.20211105/src
cc  -O2 -pipe -fno-strict-aliasing -include compat.h  -Werror -D_KERNEL -DKLD_MODULE -nostdinc   -include /usr/obj/usr/src/amd64.amd64/sys/RHURLIN/usr/ports/net/wireguard-kmod/work/wireguard-freebsd-0.0.20211105/src/opt_global.h -I. -I/usr/src/sys -I/usr/src/sys/contrib/ck/include -fno-common  -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fdebug-prefix-map=./machine=/usr/src/sys/amd64/include -fdebug-prefix-map=./x86=/usr/src/sys/x86/include     -MD  -MF.depend.if_wg.o -MTif_wg.o -mcmodel=kernel -mno-red-zone -mno-mmx -mno-sse -msoft-float  -fno-asynchronous-unwind-tables -ffreestanding -fwrapv -fstack-protector -Wall -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wcast-qual -Wundef -Wno-pointer-sign -D__printf__=__freebsd_kprintf__ -Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas -Wno-error=tautological-compare -Wno-error=empty-body -Wno-error=parentheses-equality -Wno-error=unused-function -Wno-error=pointer-sign -Wno-error=shift-negative-value -Wno-address-of-packed-member -Wno-unused-but-set-variable -Wno-format-zero-length   -mno-aes -mno-avx  -std=iso9899:1999 -c if_wg.c -o if_wg.o
if_wg.c:711:37: error: incompatible function pointer types passing 'void (struct mbuf *, int, struct inpcb *, const struct sockaddr *, void *)' to parameter of type 'udp_tun_func_t' (aka '_Bool (*)(struct mbuf *, int, struct inpcb *, const struct sockaddr *, void *)') [-Werror,-Wincompatible-function-pointer-types]
        rc = udp_set_kernel_tunneling(so4, wg_input, NULL, sc);
                                           ^~~~~~~~
/usr/src/sys/netinet/udp_var.h:181:65: note: passing argument to parameter 'f' here
int             udp_set_kernel_tunneling(struct socket *so, udp_tun_func_t f,
                                                                           ^
if_wg.c:722:37: error: incompatible function pointer types passing 'void (struct mbuf *, int, struct inpcb *, const struct sockaddr *, void *)' to parameter of type 'udp_tun_func_t' (aka '_Bool (*)(struct mbuf *, int, struct inpcb *, const struct sockaddr *, void *)') [-Werror,-Wincompatible-function-pointer-types]
        rc = udp_set_kernel_tunneling(so6, wg_input, NULL, sc);
                                           ^~~~~~~~
/usr/src/sys/netinet/udp_var.h:181:65: note: passing argument to parameter 'f' here
int             udp_set_kernel_tunneling(struct socket *so, udp_tun_func_t f,
                                                                           ^
2 errors generated.
*** Error code 1


[1] https://freshbsd.org/freebsd/src/commit/88162f7abd61206c98432f2c0de869a59be13854
Comment 1 Rainer Hurling freebsd_committer freebsd_triage 2022-04-19 19:21:44 UTC
Sorry for this unfortunately partly incorrect report. Obviously I even accidentally quoted a wrong commit :(

The original error description is correct, but incomplete. The described error in building net/wireguard-kmod occurs from base commit #742e721 of April 12 (https://freshbsd.org/freebsd/src/commit/742e7210d00b359d81b9c778ab520003704e9b6c). Prior to this commit, up to and including https://freshbsd.org/freebsd/src/commit/b93f47eaeef75b2b99fc1c501fb7cffac34068c7 wireguard-kmod builds as expected.

The error behavior can also be reproduced in Poudriere if the jail for 14.0-CURRENT is built before or from the corresponding commit.
Comment 2 Mark Johnston freebsd_committer freebsd_triage 2022-04-22 15:45:55 UTC
I guess the port will need a patch to handle both flavours of the tunnel function, since stable/13 still has the old definition.  Can we bump __FreeBSD_version and submit a patch to the upstream wireguard repo?
Comment 3 commit-hook freebsd_committer freebsd_triage 2022-04-22 18:07:23 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e68b35e40881a1bd858e1b4b5003123a484fd7cd

commit e68b35e40881a1bd858e1b4b5003123a484fd7cd
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2022-04-22 18:04:38 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2022-04-22 18:04:38 +0000

    Bump __FreeBSD_version for udp_tun_func_t() prototype change

    742e7210d0 changed the prototype of udp_tun_func_t(). Bump
    __FreeBSD_version so that external modules can #ifdef for it as
    required.

    PR:             263297
    Sponsored by:   Rubicon Communications, LLC ("Netgate")

 sys/sys/param.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 4 John Baldwin freebsd_committer freebsd_triage 2022-04-22 21:45:12 UTC
I can work on a patch for upstream.
Comment 5 imbutler 2022-04-25 12:32:46 UTC
Created attachment 233478 [details]
patch and Makefile addition

I added the attached patch and added ..

.if ${OSVERSION} > 1400056
EXTRA_PATCHES+= ${PATCHDIR}/extra-patch-if__wg.c
.endif

 .. to the Makefile. Compiles and seems to run ok
Comment 6 Rainer Hurling freebsd_committer freebsd_triage 2022-04-25 16:48:09 UTC
(In reply to imbutler from comment #5)
Thanks for the patch, really appreciated!

I tried to update 14.0-CURRENT from commit #b93f47eaee from April, 15th to #67fc95025cc from today. 

The patch file itself is applied without any problems. 

However, the if condition in net/wireguard-kmod's Makefile does not execute the patch when updating from OSVERSION<=1400056, i.e. before the modified udp_tun_func_t() function is present.

The ports framework seems to refer to the still current (installed) OSVERSION instead of the future OSVERSION currently under construction when checking for net/wireguard-kmod?
Comment 7 Trond Endrestøl 2022-04-26 18:15:39 UTC
Created attachment 233513 [details]
Improved patch for if_wg.c, differentiating between the new and the old behaviour
Comment 8 Trond Endrestøl 2022-04-26 18:35:52 UTC
Created attachment 233516 [details]
Improved patch for if_wg.c, differentiating between the new and the old behaviour, added UTC timestamp for the original file
Comment 9 Rainer Hurling freebsd_committer freebsd_triage 2022-04-26 20:20:58 UTC
Sorry that my comment #6 was apparently not expressed clearly enough. I tried to describe the following initial situation where the if condition in the Makefile does not apply:

When 14.0-CURRENT is to be updated from an OSVERSION <=1400056 (strictly speaking from base commit #b93f47ea or earlier to a base commit #742e721 or later), the extra-patch file is not applied, whereupon the build of net/wireguard-kmod, which is also pushed after BUILD_KERNEL, occurs with the error described in comment #1 and comment #2, and the kernel update cannot be completed.

To apply the extra patch in this situation, a query is needed to detect what the target version of the new kernel is. Or am I completely misinterpreting something here?
Comment 10 Kyle Evans freebsd_committer freebsd_triage 2022-04-26 22:30:03 UTC
(In reply to Rainer Hurling from comment #9)

The last patch supplied would be OK to apply unconditionally (patch-*), just need to put in `make makepatch` form.
Comment 11 Rainer Hurling freebsd_committer freebsd_triage 2022-04-27 05:45:57 UTC
(In reply to Kyle Evans from comment #10)

Thanks Kyle!
That was exactly my problem. I had commented out the if condition in the Makefile, but still used the patch as an 'extra patch'. After renaming it to files/patch-if__wg.c, it now works to build from an OSVERSION <= 1400056 to a current version while also successfully rebuilding net/wireguard-kmod. All seems fine now.

My answer took a while, as I reverted a box to a commit before #742e721 to test from there, just to be on the safe side ;)
Comment 12 Trond Endrestøl 2022-04-27 06:40:32 UTC
Created attachment 233522 [details]
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format
Comment 13 Trond Endrestøl 2022-04-27 06:43:19 UTC
Created attachment 233523 [details]
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format
Comment 14 Trond Endrestøl 2022-04-27 06:44:56 UTC
Created attachment 233524 [details]
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format
Comment 15 Trond Endrestøl 2022-04-27 06:46:02 UTC
Created attachment 233525 [details]
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format
Comment 16 Trond Endrestøl 2022-05-01 12:43:09 UTC
Created attachment 233628 [details]
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format

Finally.
Comment 17 Trond Endrestøl 2022-05-01 13:03:18 UTC
Created attachment 233630 [details]
Improved patch for if_wg.c, differentiating between the new and the old behaviour, now adhering to the makepatch format

I can't get anything right today.
Comment 18 Bernhard Froehlich freebsd_committer freebsd_triage 2022-05-05 08:29:40 UTC
Thanks to everyone helping on that issue! I have prepared a commit for the port with the patch from Comment 17 but also asked upstream what they think about it. So expect a commit soon.
Comment 19 commit-hook freebsd_committer freebsd_triage 2022-05-06 18:37:36 UTC
A commit in branch main references this bug:

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

commit f8ba180a92c464ae9d4c8621c90149a3bcae25b4
Author:     Bernhard Froehlich <decke@FreeBSD.org>
AuthorDate: 2022-05-06 18:36:02 +0000
Commit:     Bernhard Froehlich <decke@FreeBSD.org>
CommitDate: 2022-05-06 18:37:09 +0000

    net/wireguard-kmod: Fix build on 14-current after udp_tun_func_t() change

    PR:             263297
    Submitted by:   Trond.Endrestol@ximalas.info

 net/wireguard-kmod/Makefile                  |  1 +
 net/wireguard-kmod/files/patch-if_wg.c (new) | 84 ++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
Comment 20 Bernhard Froehlich freebsd_committer freebsd_triage 2022-05-06 18:42:16 UTC
Committed. Thanks!