Skip to contentSkip to conversation

wpa_supplicant: Add NL80211 support option

PATCH New Jean-Baptiste Theou 6 participants 19 messages Aug 8, 2014 – Jan 10, 2016

lore
Share & tools
lore

Jean-Baptiste Theou

author
#0 lore raw

PATCH v2 1/1 wpa_supplicant: Add NL80211 support option

When you select wpa_supplicant, having an visual indication about the support or not of NL80211 is important.

And even if libnl is available, you may want to disable the support of NL80211 inside wpa_supplicant.

---
 package/wpa_supplicant/Config.in         | 7 +++++++
 package/wpa_supplicant/wpa_supplicant.mk | 6 ++++++
 2 files changed, 13 insertions(+)
package/wpa_supplicant/Config.in +7 -0
index 81c61ac..1d43acd 100644
@@ -22,6 +22,13 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
 	help
 	  Enable support for EAP.
 
+config BR2_PACKAGE_WPA_SUPPLICANT_NL80211
+	bool "Enable NL80211"
+	default y if BR2_PACKAGE_LIBNL
+	select BR2_PACKAGE_LIBNL
+	help
+	  Enable support for NL80211.
+
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
 	help
package/wpa_supplicant/wpa_supplicant.mk +6 -0
index 0ca2ce5..4ea2a3a 100644
@@ -42,6 +42,12 @@ else
 	WPA_SUPPLICANT_CONFIG_DISABLE += CONFIG_DRIVER_NL80211
 endif
 
+ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_NL80211),y)
+	WPA_SUPPLICANT_CONFIG_ENABLE += CONFIG_DRIVER_NL80211
+else
+	WPA_SUPPLICANT_CONFIG_DISABLE += CONFIG_DRIVER_NL80211
+endif
+
 # Trailing underscore on purpose to not enable CONFIG_EAPOL_TEST
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_EAP),y)
 	WPA_SUPPLICANT_CONFIG_ENABLE += CONFIG_EAP_
Signature · 1 line
2.0.3
#1 lore raw

PATCH v2 1/1 wpa_supplicant: Add NL80211 support option

Dear Jean-Baptiste Theou,

Quoted 5 lines · Jean-Baptiste Theou

When you select wpa_supplicant, having an visual indication about the support or not of NL80211 is important.

And even if libnl is available, you may want to disable the support of NL80211 inside wpa_supplicant.

You forgot your Signed-off-by here. But we anyway have more comments below.

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 30

Quoted 6 lines
package/wpa_supplicant/Config.in +6 -0
+config BR2_PACKAGE_WPA_SUPPLICANT_NL80211
+	bool "Enable NL80211"
+	default y if BR2_PACKAGE_LIBNL
+	select BR2_PACKAGE_LIBNL
+	help
+	  Enable support for NL80211.

In which cases would you want to *not* have NL80211 support if you already have libnl enabled? What is the reason/use-case?

If the NL80211 support in wpa_supplicant itself adds a significant binary size to the filesystem, then it might be useful to have an option such as the one you propose. But in this case, you should include some numbers in your commit log.

Also, your patch keeps the automatic dependency handling of libnl, and adds explicitly dependency handling on top of it, which doesn't make a lot of sense: it should really be either one or the other solution, not both.

You can also see slide 146 and following of
http://free-electrons.com/doc/training/buildroot/buildroot-slides.pdf
for more details about automatic or explicit handling of optional dependencies.

In the mean time, we'll mark your patch as Rejected in patchwork. Don't hesitate to resubmit a patch with a better explanation, and a cleanup of automatic vs. explicit handling of the libnl dependency.

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH v2 1/1 wpa_supplicant: Add NL80211 support option

Quoted 12 lines · Thomas Petazzoni

Dear Jean-Baptiste Theou,

Quoted 5 lines · Jean-Baptiste Theou

When you select wpa_supplicant, having an visual indication about the support or not of NL80211 is important.

And even if libnl is available, you may want to disable the support of NL80211 inside wpa_supplicant.

You forgot your Signed-off-by here. But we anyway have more comments below.

Quoted 9 lines

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 30

Quoted 6 lines
package/wpa_supplicant/Config.in +6 -0
+config BR2_PACKAGE_WPA_SUPPLICANT_NL80211
+	bool "Enable NL80211"
+	default y if BR2_PACKAGE_LIBNL
+	select BR2_PACKAGE_LIBNL
+	help
+	  Enable support for NL80211.

In which cases would you want to *not* have NL80211 support if you already have libnl enabled? What is the reason/use-case?

The author made it clear that it is not the main reason. The main reason is that wpa_supplicant's usefulness is pretty reduced if nl80211 is not enabled;

With a default kernel configuration, only nl80211 is supported since the deprecated wext compatibility is disabled by default, so you may even end up with a wpa_supplicant binary that can not manage any wifi device if you forgot about enabling libnl to have nl80211.

And wpa_supplicant is not entirely useless without nl80211 either: you may want to only use the wired driver to do 802.1x on Ethernet networks, or you may only need the wext driver because you are using some old/unmaintained out-of-tree linux driver that only knows about wext.

In either case, it is useful to have an option to enable nl80211, and I would even suggest it to be enabled by default (and removing the automatic dependency, of course). I'm quite sure that some features which are already optional (such as HS20 or WPS) are useless/dead code without nl80211.

#3 lore raw

PATCH v2 1/1 wpa_supplicant: Add NL80211 support option

Dear Nicolas Cavallari,

Quoted 24 lines · Nicolas Cavallari
Quoted 9 lines

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 30

Quoted 6 lines
package/wpa_supplicant/Config.in +6 -0
+config BR2_PACKAGE_WPA_SUPPLICANT_NL80211
+	bool "Enable NL80211"
+	default y if BR2_PACKAGE_LIBNL
+	select BR2_PACKAGE_LIBNL
+	help
+	  Enable support for NL80211.

In which cases would you want to *not* have NL80211 support if you already have libnl enabled? What is the reason/use-case?

The author made it clear that it is not the main reason. The main reason is that wpa_supplicant's usefulness is pretty reduced if nl80211 is not enabled;

With a default kernel configuration, only nl80211 is supported since the deprecated wext compatibility is disabled by default, so you may even end up with a wpa_supplicant binary that can not manage any wifi device if you forgot about enabling libnl to have nl80211.

And wpa_supplicant is not entirely useless without nl80211 either: you may want to only use the wired driver to do 802.1x on Ethernet networks, or you may only need the wext driver because you are using some old/unmaintained out-of-tree linux driver that only knows about wext.

Quoted 5 lines

In either case, it is useful to have an option to enable nl80211, and I would even suggest it to be enabled by default (and removing the automatic dependency, of course). I'm quite sure that some features which are already optional (such as HS20 or WPS) are useless/dead code without nl80211.

Then fair enough: what we need is a patch that adds an option as suggested by the original author, but also remove the "automatic optional dependency" handling that is currently in wpa_supplicant.mk.

Would you be willing to work on such a patch?

Thanks for the feedback!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH 1/2 wpa_supplicant: Add an explicit option to enable nl80211.

Currently, nl80211 support is conditional with libnl being enabled, using implicit dependencies. This causes problems since it is not obvious and wpa_supplicant without nl80211 isn't what most user expects.

If nl80211 isn't enabled, then buildroot only enables the wext driver, which will only work if some deprecated kernel feature isn't left disabled, or if using a outdated out-of-tree linux driver which doesn't use the cfg80211 infrastructure.

This makes nl80211 support an explicit option, which
"select BR2_PACKAGE_LIBNL" accordingly. To handle upgrades nicely, it would have been nice to have "default y if BR2_PACKAGE_LIBNL", but Kconfig treats this as a circular dependency. So instead, this enables the option by default, which is less worse than not enabling nl80211 when it was previously implicitly enabled.

Signed-off-by Nicolas Cavallari <[email protected]>
---
 package/wpa_supplicant/Config.in         | 16 ++++++++++++++++
 package/wpa_supplicant/wpa_supplicant.mk |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)
package/wpa_supplicant/Config.in +16 -0
index f32a867..1824f95 100644
@@ -8,6 +8,22 @@ config BR2_PACKAGE_WPA_SUPPLICANT
 
 if BR2_PACKAGE_WPA_SUPPLICANT
 
+config BR2_PACKAGE_WPA_SUPPLICANT_NL80211
+	bool "Enable nl80211 support"
+	default y
+	select BR2_PACKAGE_LIBNL
+	help
+	  Enable support for nl80211.  This is the current wireless API for
+	  Linux, supported by all wireless drivers in vanilla Linux, but may
+	  not be supported by some out-of-tree Linux wireless drivers.
+	  wpa_supplicant will still fall back to using the Wireless Extensions
+	  (wext) API with these drivers.
+
+	  If this option is disabled, then only the deprecated wext API will
+	  be supported, with far less features.  Linux may supports using
+	  wext with modern drivers using a compatibility layer, but it must
+	  be enabled in the kernel configuration.
+
 config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT
 	bool "Enable AP mode"
 	help
package/wpa_supplicant/wpa_supplicant.mk +1 -1
index eb4278a..6e25280 100644
@@ -32,7 +32,7 @@ WPA_SUPPLICANT_CONFIG_DISABLE = \
 # libnl-3 needs -lm (for rint) and -lpthread if linking statically
 # And library order matters hence stick -lnl-3 first since it's appended
 # in the wpa_supplicant Makefiles as in LIBS+=-lnl-3 ... thus failing
-ifeq ($(BR2_PACKAGE_LIBNL),y)
+ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_NL80211),y)
 ifeq ($(BR2_STATIC_LIBS),y)
 WPA_SUPPLICANT_LIBS += -lnl-3 -lm -lpthread
 endif
Signature · 1 line
2.1.4
#5 lore raw

PATCH 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

Hotspot 2.0 uses ANQP whose implementation needs offchannel/ remain-on-channel capabilities from the driver, which only exist in the nl80211 driver. Compiling would work, but the feature would not work.

---
 package/wpa_supplicant/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/wpa_supplicant/Config.in +1 -0
index 1824f95..c163bbc 100644
@@ -40,6 +40,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
 
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211
 	help
 	  Enable Hotspot 2.0 and IEEE 802.11u interworking functionality.
Signature · 1 line
2.1.4
#6 lore raw

PATCH 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

Hi Nicolas,

Quoted 3 lines · Nicolas Cavallari

Hotspot 2.0 uses ANQP whose implementation needs offchannel/ remain-on-channel capabilities from the driver, which only exist in the nl80211 driver. Compiling would work, but the feature would not work.

Your sign-off is missing.

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 43

Quoted 12 lines
 package/wpa_supplicant/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/wpa_supplicant/Config.in +1 -0
index 1824f95..c163bbc 100644
@@ -40,6 +40,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
 
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211

Please add the BR2_PACKAGE_LIBNL dependency here as well.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#7 lore raw

PATCH 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

Dear Baruch Siach,

Quoted 9 lines · Baruch Siach
Quoted 9 lines
 package/wpa_supplicant/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/wpa_supplicant/Config.in +0 -0
index 1824f95..c163bbc 100644
@@ -40,6 +40,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
Quoted 5 lines

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 43

Quoted 3 lines
package/wpa_supplicant/Config.in +1 -0
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211

Please add the BR2_PACKAGE_LIBNL dependency here as well.

Not needed since it's a "depends on", so BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT will anyway only be visible if BR2_PACKAGE_WPA_SUPPLICANT_NL80211 is enabled, and if BR2_PACKAGE_WPA_SUPPLICANT_NL80211 is enabled, then it has selected BR2_PACKAGE_LIBNL.

Am I missing something?

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#8 lore raw

PATCH 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

Quoted 7 lines · Thomas Petazzoni
Quoted 5 lines · Baruch Siach

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 43

Quoted 3 lines

config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT

package/wpa_supplicant/Config.in +1 -0
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211

Please add the BR2_PACKAGE_LIBNL dependency here as well.

Quoted 7 lines

Not needed since it's a "depends on", so BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT will anyway only be visible if BR2_PACKAGE_WPA_SUPPLICANT_NL80211 is enabled, and if BR2_PACKAGE_WPA_SUPPLICANT_NL80211 is enabled, then it has selected BR2_PACKAGE_LIBNL.

Am I missing something?

I was thinking the same thing.

#9 lore raw

PATCH 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

In reply to Thomas Petazzoni

Hi Thomas,

Quoted 16 lines · Thomas Petazzoni
Quoted 9 lines · Baruch Siach
Quoted 9 lines
 package/wpa_supplicant/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/wpa_supplicant/Config.in +0 -0
index 1824f95..c163bbc 100644
@@ -40,6 +40,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
Quoted 5 lines

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 43

Quoted 3 lines
package/wpa_supplicant/Config.in +1 -0
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211

Please add the BR2_PACKAGE_LIBNL dependency here as well.

Quoted 7 lines

Not needed since it's a "depends on", so BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT will anyway only be visible if BR2_PACKAGE_WPA_SUPPLICANT_NL80211 is enabled, and if BR2_PACKAGE_WPA_SUPPLICANT_NL80211 is enabled, then it has selected BR2_PACKAGE_LIBNL.

Am I missing something?

Right. The dependency of hotspot on libnl is no apparent at all, though. IMO, having here

select BR2_PACKAGE_WPA_SUPPLICANT_NL80211
depends on BR2_TOOLCHAIN_HAS_THREADS # libnl

and maybe also a dependency comment, would be better.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#10 lore raw

PATCH 1/2 wpa_supplicant: Add an explicit option to enable nl80211.

In reply to Nicolas Cavallari

Hi Nicolas,

Quoted 27 lines · Nicolas Cavallari

Currently, nl80211 support is conditional with libnl being enabled, using implicit dependencies. This causes problems since it is not obvious and wpa_supplicant without nl80211 isn't what most user expects.

If nl80211 isn't enabled, then buildroot only enables the wext driver, which will only work if some deprecated kernel feature isn't left disabled, or if using a outdated out-of-tree linux driver which doesn't use the cfg80211 infrastructure.

This makes nl80211 support an explicit option, which
"select BR2_PACKAGE_LIBNL" accordingly. To handle upgrades nicely, it would have been nice to have "default y if BR2_PACKAGE_LIBNL", but Kconfig treats this as a circular dependency. So instead, this enables the option by default, which is less worse than not enabling nl80211 when it was previously implicitly enabled.

Signed-off-by Nicolas Cavallari <[email protected]>
---
 package/wpa_supplicant/Config.in         | 16 ++++++++++++++++
 package/wpa_supplicant/wpa_supplicant.mk |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)
package/wpa_supplicant/Config.in +0 -0
index f32a867..1824f95 100644

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT · line 14

Quoted 7 lines
package/wpa_supplicant/Config.in +4 -0
@@ -8,6 +8,22 @@ config BR2_PACKAGE_WPA_SUPPLICANT
 
 if BR2_PACKAGE_WPA_SUPPLICANT
 
+config BR2_PACKAGE_WPA_SUPPLICANT_NL80211
+	bool "Enable nl80211 support"
+	default y
+	select BR2_PACKAGE_LIBNL

BR2_PACKAGE_LIBNL depends on BR2_TOOLCHAIN_HAS_THREADS. You need to propagate this dependency here, since Kconfig doesn't do that automatically. See
http://nightly.buildroot.org/manual.html#depends-on-vs-select.

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#11 lore raw

PATCH 1/2 wpa_supplicant: Add an explicit option to enable nl80211.

Quoted 33 lines · Baruch Siach

Hi Nicolas,

Quoted 30 lines · Nicolas Cavallari

Currently, nl80211 support is conditional with libnl being enabled, using implicit dependencies. This causes problems since it is not obvious and wpa_supplicant without nl80211 isn't what most user expects.

If nl80211 isn't enabled, then buildroot only enables the wext driver, which will only work if some deprecated kernel feature isn't left disabled, or if using a outdated out-of-tree linux driver which doesn't use the cfg80211 infrastructure.

This makes nl80211 support an explicit option, which
"select BR2_PACKAGE_LIBNL" accordingly. To handle upgrades nicely, it would have been nice to have "default y if BR2_PACKAGE_LIBNL", but Kconfig treats this as a circular dependency. So instead, this enables the option by default, which is less worse than not enabling nl80211 when it was previously implicitly enabled.

Signed-off-by Nicolas Cavallari <[email protected]>
---
 package/wpa_supplicant/Config.in         | 16 ++++++++++++++++
 package/wpa_supplicant/wpa_supplicant.mk |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)
package/wpa_supplicant/Config.in +0 -0
index f32a867..1824f95 100644
@@ -8,6 +8,22 @@ config BR2_PACKAGE_WPA_SUPPLICANT
 
 if BR2_PACKAGE_WPA_SUPPLICANT
Quoted 8 lines

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT · line 14

Quoted 4 lines
package/wpa_supplicant/Config.in +4 -0
+config BR2_PACKAGE_WPA_SUPPLICANT_NL80211
+	bool "Enable nl80211 support"
+	default y
+	select BR2_PACKAGE_LIBNL

BR2_PACKAGE_LIBNL depends on BR2_TOOLCHAIN_HAS_THREADS. You need to propagate this dependency here, since Kconfig doesn't do that automatically. See
http://nightly.buildroot.org/manual.html#depends-on-vs-select.

libnl need threads ?

apparently, no it doesn't if configure is run with --disable-pthreads. But this is not supported by buildroot.

will respin accordingly.

#12 lore raw

PATCH v2 1/2 wpa_supplicant: Add an explicit option to enable nl80211.

Currently, nl80211 support is conditionnal with libnl being enabled, using implicit dependencies. This causes problems since it is not obvious and wpa_supplicant without nl80211 isn't what most user expects.

If nl80211 isn't enabled, then buildroot only enables the wext driver, which will only work if some deprecated kernel feature isn't left disabled, or if using a outdated out-of-tree linux driver which doesn't use the cfg80211 infrastructure.

This makes nl80211 support an explicit option, which
"select BR2_PACKAGE_LIBNL" accordingly. To handle upgrades nicely, it would have been nice to have "default y if BR2_PACKAGE_LIBNL", but Kconfig treats this as a circular dependency. So instead, this enables the option by default, which is less worse than not enabling nl80211 when it was previously implicitely enabled.

Signed-off-by Nicolas Cavallari <[email protected]>

---
v2: Added depends on BR2_TOOLCHAIN_HAS_THREADS # libnl

 package/wpa_supplicant/Config.in         | 17 +++++++++++++++++
 package/wpa_supplicant/wpa_supplicant.mk |  2 +-
 2 files changed, 18 insertions(+), 1 deletion(-)
package/wpa_supplicant/Config.in +17 -0
index f32a867..29dc4b9 100644
@@ -8,6 +8,23 @@ config BR2_PACKAGE_WPA_SUPPLICANT
 
 if BR2_PACKAGE_WPA_SUPPLICANT
 
+config BR2_PACKAGE_WPA_SUPPLICANT_NL80211
+	bool "Enable nl80211 support"
+	default y
+	select BR2_PACKAGE_LIBNL
+	depends on BR2_TOOLCHAIN_HAS_THREADS # libnl
+	help
+	  Enable support for nl80211.  This is the current wireless API for
+	  Linux, supported by all wireless drivers in vanilla Linux, but may
+	  not be supported by some out-of-tree Linux wireless drivers.
+	  wpa_supplicant will still fall back to using the Wireless Extensions
+	  (wext) API with these drivers.
+
+	  If this option is disabled, then only the deprecated wext API will
+	  be supported, with far less features.  Linux may supports using
+	  wext with modern drivers using a compatibility layer, but it must
+	  be enabled in the kernel configuration.
+
 config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT
 	bool "Enable AP mode"
 	help
package/wpa_supplicant/wpa_supplicant.mk +1 -1
index eb4278a..6e25280 100644
@@ -32,7 +32,7 @@ WPA_SUPPLICANT_CONFIG_DISABLE = \
 # libnl-3 needs -lm (for rint) and -lpthread if linking statically
 # And library order matters hence stick -lnl-3 first since it's appended
 # in the wpa_supplicant Makefiles as in LIBS+=-lnl-3 ... thus failing
-ifeq ($(BR2_PACKAGE_LIBNL),y)
+ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_NL80211),y)
 ifeq ($(BR2_STATIC_LIBS),y)
 WPA_SUPPLICANT_LIBS += -lnl-3 -lm -lpthread
 endif
Signature · 1 line
2.1.4
#13 lore raw

PATCH v2 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

Hotspot 2.0 uses ANQP whose implementation needs offchannel/ remain-on-channel capabilities from the driver, which only exist in the nl80211 driver. Compiling would work, but the feature would not work.

Signed-off-by Nicolas Cavallari <[email protected]>

---

v2: added signoff.

 package/wpa_supplicant/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/wpa_supplicant/Config.in +1 -0
index 29dc4b9..22cf7f8 100644
@@ -41,6 +41,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
 
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211
 	help
 	  Enable Hotspot 2.0 and IEEE 802.11u interworking functionality.
Signature · 1 line
2.1.4
#14 lore raw

PATCH v2 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

Hi Nicolas,

Quoted 16 lines · Nicolas Cavallari

Hotspot 2.0 uses ANQP whose implementation needs offchannel/ remain-on-channel capabilities from the driver, which only exist in the nl80211 driver. Compiling would work, but the feature would not work.

Signed-off-by Nicolas Cavallari <[email protected]>

---

v2: added signoff.

 package/wpa_supplicant/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/wpa_supplicant/Config.in +0 -0
index 29dc4b9..22cf7f8 100644

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 47

Quoted 7 lines
package/wpa_supplicant/Config.in +1 -0
@@ -41,6 +41,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
 
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211
 	help
 	  Enable Hotspot 2.0 and IEEE 802.11u interworking functionality.

First I'm not sure on the procedure to test the Hotspot 2.0 but I think we should at least add a comment like:

comment "HS20 needs NL80211 support"
depends on !BR2_PACKAGE_WPA_SUPPLICANT_NL80211

Because right now if you unselect NL80211 WPA option the HS20 just disappears.

Or maybe selecting NL80211 would be even better if absolutely required at runtime.

Regards,
Gary

#15 lore raw

PATCH v2 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

Quoted 30 lines · Gary Bisson

Hi Nicolas,

Quoted 16 lines · Nicolas Cavallari

Hotspot 2.0 uses ANQP whose implementation needs offchannel/ remain-on-channel capabilities from the driver, which only exist in the nl80211 driver. Compiling would work, but the feature would not work.

Signed-off-by Nicolas Cavallari <[email protected]>

---

v2: added signoff.

 package/wpa_supplicant/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/wpa_supplicant/Config.in +0 -0
index 29dc4b9..22cf7f8 100644

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 47

Quoted 7 lines
package/wpa_supplicant/Config.in +1 -0
@@ -41,6 +41,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
 
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211
 	help
 	  Enable Hotspot 2.0 and IEEE 802.11u interworking functionality.

First I'm not sure on the procedure to test the Hotspot 2.0 but I think we should at least add a comment like:

Quoted 8 lines

comment "HS20 needs NL80211 support"
depends on !BR2_PACKAGE_WPA_SUPPLICANT_NL80211

Because right now if you unselect NL80211 WPA option the HS20 just disappears.

Or maybe selecting NL80211 would be even better if absolutely required at runtime.

The more i think of it, the more I feel like the current behavior is acceptable as is and this patch is unnecessary.

Sure, using a vanilla wpa_supplicant, the features needed by Hotspot 2.0 are only implemented by the nl80211 driver, but there may be other non-vanilla drivers added by out-of-tree patches which also implement it.

These patches may be in a buildroot global patch directory. We shouldn't force nl80211 to be enabled to enable Hotspot 2.0 because this is the only possibility that we know.

#16 lore raw

PATCH v2 2/2 wpa_supplicant: Have Hotspot 2.0 depend on nl80211

Nicolas, All,

On 2015-09-17 19:21 +0200, Nicolas Cavallari spake thusly:

Quoted 44 lines · Gary Bisson
Quoted 30 lines

Hi Nicolas,

Quoted 16 lines · Nicolas Cavallari

Hotspot 2.0 uses ANQP whose implementation needs offchannel/ remain-on-channel capabilities from the driver, which only exist in the nl80211 driver. Compiling would work, but the feature would not work.

Signed-off-by Nicolas Cavallari <[email protected]>

---

v2: added signoff.

 package/wpa_supplicant/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/wpa_supplicant/Config.in +0 -0
index 29dc4b9..22cf7f8 100644

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_EAP · line 47

Quoted 7 lines
package/wpa_supplicant/Config.in +1 -0
@@ -41,6 +41,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_EAP
 
 config BR2_PACKAGE_WPA_SUPPLICANT_HOTSPOT
 	bool "Enable HS20"
+	depends on BR2_PACKAGE_WPA_SUPPLICANT_NL80211
 	help
 	  Enable Hotspot 2.0 and IEEE 802.11u interworking functionality.

First I'm not sure on the procedure to test the Hotspot 2.0 but I think we should at least add a comment like:

Quoted 8 lines

comment "HS20 needs NL80211 support"
depends on !BR2_PACKAGE_WPA_SUPPLICANT_NL80211

Because right now if you unselect NL80211 WPA option the HS20 just disappears.

Or maybe selecting NL80211 would be even better if absolutely required at runtime.

The more i think of it, the more I feel like the current behavior is acceptable as is and this patch is unnecessary.

Quoted 7 lines

Sure, using a vanilla wpa_supplicant, the features needed by Hotspot 2.0 are only implemented by the nl80211 driver, but there may be other non-vanilla drivers added by out-of-tree patches which also implement it.

These patches may be in a buildroot global patch directory. We shouldn't force nl80211 to be enabled to enable Hotspot 2.0 because this is the only possibility that we know.

So, given your feedback, I'm marking this patch as "Rejected" in our Patchwork.

Thanks! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#17 lore raw

PATCH v2 1/2 wpa_supplicant: Add an explicit option to enable nl80211.

In reply to Nicolas Cavallari

Hi Nicolas,

Quoted 9 lines · Nicolas Cavallari

Currently, nl80211 support is conditionnal with libnl being enabled, using implicit dependencies. This causes problems since it is not obvious and wpa_supplicant without nl80211 isn't what most user expects.

If nl80211 isn't enabled, then buildroot only enables the wext driver, which will only work if some deprecated kernel feature isn't left disabled, or if using a outdated out-of-tree linux driver which doesn't use the cfg80211 infrastructure.

Quoted 6 lines

This makes nl80211 support an explicit option, which
"select BR2_PACKAGE_LIBNL" accordingly. To handle upgrades nicely, it would have been nice to have "default y if BR2_PACKAGE_LIBNL", but Kconfig treats this as a circular dependency. So instead, this enables the option by default, which is less worse than not enabling nl80211 when it was previously implicitely enabled.

Actually I like the fact that it is enabled by default. As you said, most users now want NL80211.

Only one remark, why are there two spaces after every dots in your commit logs. Is it a custom git hook? ;-)

Reviewed-by Gary Bisson <[email protected]>
Tested-by Gary Bisson <[email protected]>

Regards,
Gary

#18 lore raw

PATCH 1/2 wpa_supplicant: Add an explicit option to enable nl80211.

In reply to Nicolas Cavallari

Dear Nicolas Cavallari,

Quoted 16 lines · Nicolas Cavallari

Currently, nl80211 support is conditional with libnl being enabled, using implicit dependencies. This causes problems since it is not obvious and wpa_supplicant without nl80211 isn't what most user expects.

If nl80211 isn't enabled, then buildroot only enables the wext driver, which will only work if some deprecated kernel feature isn't left disabled, or if using a outdated out-of-tree linux driver which doesn't use the cfg80211 infrastructure.

This makes nl80211 support an explicit option, which
"select BR2_PACKAGE_LIBNL" accordingly. To handle upgrades nicely, it would have been nice to have "default y if BR2_PACKAGE_LIBNL", but Kconfig treats this as a circular dependency. So instead, this enables the option by default, which is less worse than not enabling nl80211 when it was previously implicitly enabled.

Quoted 5 lines
Signed-off-by Nicolas Cavallari <[email protected]>
---
 package/wpa_supplicant/Config.in         | 16 ++++++++++++++++
 package/wpa_supplicant/wpa_supplicant.mk |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

I've applied after doing some minor fixes:

[Thomas:
- rewrap Config.in help text
- add comment about thread dependency.]

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com