Re: [Buildroot] [PATCH 1/2] package/wpa_supplicant: wired driver needs headers >= 5.7

[Sergey Matyukevich <geomatsi@gmail.com>]

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
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot