linux-stable v4.14.217 causes skb_mark_not_on_list() build failure

[Kamal Mostafa]

[Impact]
wireguard-dkms won't build on bionic 4.15 kernels with upstream commit 5440233ac430 ("net: use skb_list_del_init() to remove from RX sublists") applied.

[Fix]
Detect whether skb_mark_not_on_list is present in kernel headers by creating a config.h file and using it to either define or not the same function on compat.h.

[Test case]
Build the dkms module against a 4.15 and 5.4 kernel, as the 5.4 kernel has the commit.

[Regression potential]
wireguard-dkms could stop building for some kernel versions.

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

The upstream linux-stable release v4.14.217 contains the following commit (also attached here):

5440233ac430 net: use skb_list_del_init() to remove from RX sublists

... which introduces skb_mark_not_on_list() to linux v4.14. This will cause a wireguard-linux-compat build failure for v4.14-based kernels, as well as Ubuntu Bionic (v4.15) which takes ports of the 4.14-series patch sets:

    <<DKMSDIR>>/build/wireguard/1.0.20201112/build/compat/compat.h:830:20: error:
        redefinition of 'skb_mark_not_on_list'
        static inline void skb_mark_not_on_list(struct sk_buff *skb)
    In file included from <<DKMSDIR>>/build/wireguard/1.0.20201112/build/compat/compat.h:789:0,
    ./include/linux/skbuff.h:1346:20: note: previous definition of 'skb_mark_not_on_list' was here
        static inline void skb_mark_not_on_list(struct sk_buff *skb)

For Bionic, the situation is complicated since Bionic's v4.15 KERNEL_VERSION 'SUBLEVEL' does not encode the SUBLEVEL number (217) from the 4.14 series, so a simple switch on SUBLEVEL will not be sufficient for Bionic.

We're holding out the port of the noted stable commit from Bionic, pending resolution of this.

[Jason A. Donenfeld (zx2c4)]

This was fixed in the latest upstream wireguard-linux-compat release on Jan 24.

[Thadeu Lima de Souza Cascardo (cascardo)]

Hi, Jason.

Thanks for pointing out the fix. I checked it and it wasn't going to work for either older or newer kernels, as we wanted the dkms to be able to be built against either of them.

As I have some experience doing that kind of check for other dkms packages, inspired by my previous work with openvswitch, I decided to work it out for this one-off case. Unfortunately, I don't have the time budget right now to do it extensively for all of wireguard-linux-compat, which would be a more acceptable solution for you as its upstream, I suppose.

As we haven't applied the referred linux commit yet, upstream wireguard-linux-compat still doesn't require a fix for building against Ubuntu kernels. As I understand it, once we ship something including that commit, it would break its build, and some solution would be necessary. May I suggest looking into using UTS_UBUNTU_RELEASE_ABI? Would that work for you?

Thanks for your work on wireguard.
Cascardo.

[Jason A. Donenfeld (zx2c4)]

Due to inconsistent use of ubuntu-specific identifiers and complexity introduced HWE and such, wireguard-linux-compat develops against the latest kernels for each of the Ubuntu releases -- listed on https://www.wireguard.com/build-status/ , ctrl+F for ubuntu. This already amounts to ~7 kernels. So the thing to do here would be to add !defined(ISUBUNTU1804) to the relevant ifdef:

#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 19, 10) && LINUX_VERSION_CODE >= KERNEL_VERSION(4, 15, 0) && !defined(ISRHEL8) && !defined(ISUBUNTU1804)) || LINUX_VERSION_CODE < KERNEL_VERSION(4, 14, 217)

Afterwards, if you want to apply additional patches downstream so that this works on older kernels within each Ubuntu release, that would make sense. But upstream should first always be made to work against the latest kernel version in each Ubuntu release. If you have advanced knowledge that something is about to break (because of this or that backport), then please push a patch upstream for that.

apw@ knows how this works, if you want to talk to somebody internal about it. Otherwise I'm zx2c4 on Freenode and happy to help.

[Brian Murray (brian-murray)]

Hello Kamal, or anyone else affected,

Accepted wireguard-linux-compat into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/wireguard-linux-compat/1.0.20201112-1~18.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Kamal Mostafa (kamalmostafa)]

Looks like that bionic package FTBFS: https://launchpad.net/ubuntu/+source/wireguard-linux-compat/1.0.20201112-1~18.04.2
... due to the other change (debian/compat). We have alerted Alberto Milone; verification of this will have to wait until that gets fixed.

[Robie Basak (racb)]

Hello Kamal, or anyone else affected,

Accepted wireguard-linux-compat into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/wireguard-linux-compat/1.0.20201112-1~18.04.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Kamal Mostafa (kamalmostafa)]

Unfortunately, wireguard-linux-compat 1.0.20201112-1~18.04.3 still does *not* fix it. The problem is that the fix tries to check /lib/modules/.../skbuff.h which isn't installed during the in-tree build of wireguard DKMS:

+srctree="/lib/modules/$kernelver/build"
+if grep -q skb_mark_not_on_list $srctree/include/linux/skbuff.h
+then
+ echo '#define SKB_MARK_NOT_ON_LIST' >> "$1"
+fi

The check needs to look for the skb_mark_not_on_list definition in the source tree copy of skbuff.h instead (or in addition to?).

@Thadeu will rework the fix accordingly.

[Jason A. Donenfeld (zx2c4)]

I wish you'd not waste time on this downstream stuff. wireguard-linux-compat v1.0.20210219 has the proper fix (along with other important fixes). Simply import the package from debian and be done with it.

[Brian Murray (brian-murray)]

The fix for this bug has been awaiting testing feedback in the -proposed repository for bionic for more than 90 days. Please test this fix and update the bug appropriately with the results. In the event that the fix for this bug is still not verified 15 days from now, the package will be removed from the -proposed repository.

[Andy Whitcroft (apw)]

The version of wireguard-linux-compat in the proposed pocket of Bionic that was purported to fix this bug report has been removed because the bugs that were to be fixed by the upload were not verified in a timely (105 days) fashion.