| Message ID | 20240818203134.1512793-2-geomatsi@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | package/wpa_supplicant: follow-up fixes for v2.11 | expand |
Hello Sergey, On Sun, 18 Aug 2024 23:31:29 +0300 Sergey Matyukevich <geomatsi@gmail.com> wrote: > diff --git a/package/wpa_supplicant/Config.in b/package/wpa_supplicant/Config.in > index 92953f69f0..e013f5a9c1 100644 > --- a/package/wpa_supplicant/Config.in > +++ b/package/wpa_supplicant/Config.in > @@ -42,7 +42,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_WEXT > config BR2_PACKAGE_WPA_SUPPLICANT_WIRED > bool "Enable wired support" > depends on BR2_TOOLCHAIN_HAS_THREADS # libnl > - depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_6 > + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_7 Thanks, but it seems a bit drastic. If I looked at the code: ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_WIRED),y) WPA_SUPPLICANT_DEPENDENCIES += host-pkgconf libnl WPA_SUPPLICANT_CONFIG_ENABLE += \ CONFIG_LIBNL32 \ CONFIG_DRIVER_WIRED \ CONFIG_MACSEC \ CONFIG_DRIVER_MACSEC_LINUX so it forces CONFIG_DRIVER_MACSEC_LINUX, what about making that dependent on kernel headers >= 5.7, rather than preventing the whole wired driver from being compiled? Or, in fact even better, in ./src/drivers/driver_macsec_linux.c, change: #if LIBNL_VER_NUM >= LIBNL_VER(3, 6) #define LIBNL_HAS_OFFLOAD #endif to: #if LIBNL_VER_NUM >= LIBNL_VER(3, 6) && LINUX_VERSION_CODE >= KERNEL_VERSION(5,7,0) #define LIBNL_HAS_OFFLOAD #endif And that should only enable the offload support... when it can really be supported. Could you check this? Thanks! Thomas
Hello Thomas, On Mon, Aug 19, 2024 at 12:59:36PM +0200, Thomas Petazzoni wrote: > Hello Sergey, > > On Sun, 18 Aug 2024 23:31:29 +0300 > Sergey Matyukevich <geomatsi@gmail.com> wrote: > > > diff --git a/package/wpa_supplicant/Config.in b/package/wpa_supplicant/Config.in > > index 92953f69f0..e013f5a9c1 100644 > > --- a/package/wpa_supplicant/Config.in > > +++ b/package/wpa_supplicant/Config.in > > @@ -42,7 +42,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_WEXT > > config BR2_PACKAGE_WPA_SUPPLICANT_WIRED > > bool "Enable wired support" > > depends on BR2_TOOLCHAIN_HAS_THREADS # libnl > > - depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_6 > > + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_7 > > Thanks, but it seems a bit drastic. > > If I looked at the code: > > ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_WIRED),y) > WPA_SUPPLICANT_DEPENDENCIES += host-pkgconf libnl > WPA_SUPPLICANT_CONFIG_ENABLE += \ > CONFIG_LIBNL32 \ > CONFIG_DRIVER_WIRED \ > CONFIG_MACSEC \ > CONFIG_DRIVER_MACSEC_LINUX > > so it forces CONFIG_DRIVER_MACSEC_LINUX, what about making that > dependent on kernel headers >= 5.7, rather than preventing the whole > wired driver from being compiled? > > Or, in fact even better, in ./src/drivers/driver_macsec_linux.c, change: > > #if LIBNL_VER_NUM >= LIBNL_VER(3, 6) > #define LIBNL_HAS_OFFLOAD > #endif > > to: > > #if LIBNL_VER_NUM >= LIBNL_VER(3, 6) && LINUX_VERSION_CODE >= KERNEL_VERSION(5,7,0) > #define LIBNL_HAS_OFFLOAD > #endif > > And that should only enable the offload support... when it can really > be supported. Could you check this? I like the second option with explicit kernel and libnl version check. I build-tested several configurations and it works just fine: : diff --git a/src/drivers/driver_macsec_linux.c b/src/drivers/driver_macsec_linux.c : index c86715498..9ad24183e 100644 : --- a/src/drivers/driver_macsec_linux.c : +++ b/src/drivers/driver_macsec_linux.c : @@ -19,6 +19,7 @@ : #include <netlink/route/link.h> : #include <netlink/route/link/macsec.h> : #include <linux/if_macsec.h> : +#include <linux/version.h> : #include <inttypes.h> : : #include "utils/common.h" : @@ -32,7 +33,7 @@ : : #define UNUSED_SCI 0xffffffffffffffff : : -#if LIBNL_VER_NUM >= LIBNL_VER(3, 6) : +#if (LIBNL_VER_NUM >= LIBNL_VER(3, 6) && LINUX_VERSION_CODE >= KERNEL_VERSION(5,7,0)) : #define LIBNL_HAS_OFFLOAD : #endif I will send a fix to hostapd mailing list. If it is not acceptable for whatever reason, we can fallback to the separate Buildroot option for MACSEC. BTW, the idea and draft is yours, so let me know if I should keep your authorship of the hostapd patch. Regards, Sergey
Hello Sergey, On Mon, 19 Aug 2024 23:07:38 +0300 Sergey Matyukevich <geomatsi@gmail.com> wrote: > > And that should only enable the offload support... when it can really > > be supported. Could you check this? > > I like the second option with explicit kernel and libnl version check. > I build-tested several configurations and it works just fine: Excellent! > I will send a fix to hostapd mailing list. If it is not acceptable for > whatever reason, we can fallback to the separate Buildroot option for > MACSEC. Sounds like a good plan. > BTW, the idea and draft is yours, so let me know if I should keep > your authorship of the hostapd patch. I have written/contributed enough patches that I no longer really care about authorship. Feel free to stupid the hostapd patch under your name, it's going to make things easier (and in fact you did all the testing, I just threw some random untested one-liner code in an e-mail). Thanks for all your work! Thomas
diff --git a/package/wpa_supplicant/Config.in b/package/wpa_supplicant/Config.in index 92953f69f0..e013f5a9c1 100644 --- a/package/wpa_supplicant/Config.in +++ b/package/wpa_supplicant/Config.in @@ -42,7 +42,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_WEXT config BR2_PACKAGE_WPA_SUPPLICANT_WIRED bool "Enable wired support" depends on BR2_TOOLCHAIN_HAS_THREADS # libnl - depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_6 + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_7 select BR2_PACKAGE_LIBNL select BR2_PACKAGE_WPA_SUPPLICANT_EAP help @@ -50,8 +50,8 @@ config BR2_PACKAGE_WPA_SUPPLICANT_WIRED supplicant can be used with Ethernet. This also enables support for MACSEC. -comment "wired macsec support needs a toolchain w/ headers >= 4.6" - depends on !BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_6 +comment "wired macsec support needs a toolchain w/ headers >= 5.7" + depends on !BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_7 comment "wired support needs a toolchain w/ threads" depends on !BR2_TOOLCHAIN_HAS_THREADS
In wpa_supplicant v2.11 macsec support in wired driver requires enum 'macsec_offload' if libnl version is >= v3.6, see the commit: - https://w1.fi/cgit/hostap/commit/?id=40c139664439b2576e1506fbca14a7b79425a9dd Buildroot provides libnl version v3.9.0, so enum 'macsec_offload' shall be available in order to successfully build wpa_supplicant wired driver with macsec support. However that enum has only been added to Linux header if_link.h since kernel v5.7, see commits: - https://github.com/torvalds/linux/commit/21114b7feec29e4425a3ac48a037569c016a46c8 - https://github.com/torvalds/linux/commit/76564261a7db80c5f5c624e0122a28787f266bdf That is why we have to tighten restriction on Linux headers for wired driver from v4.6 to v5.7. Fixes: - http://autobuild.buildroot.net/results/b59d5bc5bd17683a3a1e3577c40c802e81911f84/ Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com> --- package/wpa_supplicant/Config.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)