All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/4] Better wifi handling
@ 2022-05-27 10:33 Angelo Compagnucci
  2022-05-27 10:33 ` [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line" Angelo Compagnucci
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Angelo Compagnucci @ 2022-05-27 10:33 UTC (permalink / raw)
  To: buildroot; +Cc: jagan, Angelo Compagnucci

While adding and testing for a new wifi driver, I could notice the sad
state of wifi interface bringup in buildroot, so I tried to have
something simple enough to work on the most common cases.

What problems we had:
* Wrong syntax for wpa_supplicant.conf when using default build
  options.
* Lack of a proper ifup/ifdown support to have wpa_supplicant
  automatically started.
* Due to this, the recommended way of configuring a wifi via
  the "interfaces" file wasn't working at all.

The series adds also a new wifi driver which I tested.

Angelo Compagnucci (4):
  package/wpa_supplicant: fixing "Invalid configuration line"
  package/wpa_supplicant: adding ifupdown support
  package/busybox: make udhcp discover faster
  package/rtl8723ds: new package

 package/Config.in                          |  1 +
 package/busybox/busybox.config             |  2 +-
 package/rtl8723ds/Config.in                | 10 +++
 package/rtl8723ds/rtl8723ds.mk             | 23 +++++++
 package/wpa_supplicant/ifupdown.sh         | 71 ++++++++++++++++++++++
 package/wpa_supplicant/wpa_supplicant.conf |  1 -
 package/wpa_supplicant/wpa_supplicant.mk   | 10 +++
 7 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 package/rtl8723ds/Config.in
 create mode 100644 package/rtl8723ds/rtl8723ds.mk
 create mode 100755 package/wpa_supplicant/ifupdown.sh

-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-05-27 10:33 [Buildroot] [PATCH 0/4] Better wifi handling Angelo Compagnucci
@ 2022-05-27 10:33 ` Angelo Compagnucci
  2022-06-01 19:21   ` Thomas Petazzoni via buildroot
  2022-05-27 10:33 ` [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support Angelo Compagnucci
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Angelo Compagnucci @ 2022-05-27 10:33 UTC (permalink / raw)
  To: buildroot; +Cc: jagan, Angelo Compagnucci

Default configuration file is wrong for the default compiling options.

Fixes:

Successfully initialized wpa_supplicant
Line 1: unknown global field 'ctrl_interface=/var/run/wpa_supplicant'.
Line 1: Invalid configuration line
'ctrl_interface=/var/run/wpa_supplicant'.
Failed to read or parse configuration '/etc/wpa_supplicant.conf'.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 package/wpa_supplicant/wpa_supplicant.conf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/package/wpa_supplicant/wpa_supplicant.conf b/package/wpa_supplicant/wpa_supplicant.conf
index 1994a6c739..f8a73d465f 100644
--- a/package/wpa_supplicant/wpa_supplicant.conf
+++ b/package/wpa_supplicant/wpa_supplicant.conf
@@ -1,4 +1,3 @@
-ctrl_interface=/var/run/wpa_supplicant
 ap_scan=1
 
 network={
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support
  2022-05-27 10:33 [Buildroot] [PATCH 0/4] Better wifi handling Angelo Compagnucci
  2022-05-27 10:33 ` [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line" Angelo Compagnucci
@ 2022-05-27 10:33 ` Angelo Compagnucci
  2022-06-01 19:25   ` Thomas Petazzoni via buildroot
  2022-06-06 15:09   ` Nicolas Cavallari
  2022-05-27 10:33 ` [Buildroot] [PATCH 3/4] package/busybox: make udhcp discover faster Angelo Compagnucci
  2022-05-27 10:33 ` [Buildroot] [PATCH 4/4] package/rtl8723ds: new package Angelo Compagnucci
  3 siblings, 2 replies; 19+ messages in thread
From: Angelo Compagnucci @ 2022-05-27 10:33 UTC (permalink / raw)
  To: buildroot; +Cc: jagan, Angelo Compagnucci

Actually, configuring a wifi interface as per "interfaces" man:

auto wlan0
iface wlan0 inet dhcp
wpa-conf /etc/wpa_supplicant.conf

doesn't work on buildroot because the line wpa-conf is ignored due to
the lack of a proper ifupdown script to handle the wpa_supplicant
initialization.

The provided file is a simplified version based on the one available
on debian.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 package/wpa_supplicant/ifupdown.sh       | 71 ++++++++++++++++++++++++
 package/wpa_supplicant/wpa_supplicant.mk | 10 ++++
 2 files changed, 81 insertions(+)
 create mode 100755 package/wpa_supplicant/ifupdown.sh

diff --git a/package/wpa_supplicant/ifupdown.sh b/package/wpa_supplicant/ifupdown.sh
new file mode 100755
index 0000000000..8eecf73436
--- /dev/null
+++ b/package/wpa_supplicant/ifupdown.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+# This file is executed by ifupdown in pre-up, post-up, pre-down and
+# post-down phases of network interface configuration.
+
+WPA_SUP_BIN="/usr/sbin/wpa_supplicant"
+
+if [ -n "$IF_WPA_MAINT_DEBUG" ]; then
+	set -x
+fi
+
+# quit if we're called for the loopback
+if [ "$IFACE" = lo ]; then
+	exit 0
+fi
+
+# allow wpa_supplicant interface to be specified via wpa-iface
+# useful for starting wpa_supplicant on one interface of a bridge
+if [ -n "$IF_WPA_IFACE" ]; then
+	WPA_IFACE="$IF_WPA_IFACE"
+else
+	WPA_IFACE="$IFACE"
+fi
+
+WPA_SUP_PIDFILE="/run/wpa_supplicant.${WPA_IFACE}.pid"
+
+# quit if executables are not installed
+if [ ! -x "$WPA_SUP_BIN" ]; then
+	exit 0
+fi
+
+do_start () {
+	if [ -n "$IF_WPA_CONF" ] && [ "$IF_WPA_CONF" != "managed" ]; then
+		if [ ! -s "$IF_WPA_CONF" ]; then
+			echo "cannot read contents of $IF_WPA_CONF"
+			exit 1
+		fi
+		WPA_SUP_CONF_CTRL_DIR=$(sed -n -e 's/[[:space:]]*#.*//g' -e 's/[[:space:]]\+.*$//g' \
+			-e 's/^ctrl_interface=\(DIR=\)\?\(.*\)/\2/p' "$IF_WPA_CONF")
+		if [ -n "$WPA_SUP_CONF_CTRL_DIR" ]; then
+			WPA_SUP_CONF="-c $IF_WPA_CONF -C $WPA_SUP_CONF_CTRL_DIR"
+		else
+			WPA_SUP_CONF="-c $IF_WPA_CONF"
+		fi
+	else
+		# specify the default ctrl_interface
+		WPA_SUP_CONF="-C $WPA_CTRL_DIR"
+	fi
+}
+
+case "$MODE" in
+	start)
+		do_start
+		case "$PHASE" in
+			post-up)
+				start-stop-daemon -S -q -x ${WPA_SUP_BIN} \
+				-- -B -i ${WPA_IFACE} ${WPA_SUP_CONF} -P ${WPA_SUP_PIDFILE}
+				;;
+		esac
+		;;
+
+	stop)
+		case "$PHASE" in
+			pre-down)
+				start-stop-daemon -K -p ${WPA_SUP_PIDFILE}
+				;;
+		esac
+		;;
+esac
+
+exit 0
diff --git a/package/wpa_supplicant/wpa_supplicant.mk b/package/wpa_supplicant/wpa_supplicant.mk
index c4cfe03371..d38bf572db 100644
--- a/package/wpa_supplicant/wpa_supplicant.mk
+++ b/package/wpa_supplicant/wpa_supplicant.mk
@@ -250,6 +250,14 @@ define WPA_SUPPLICANT_INSTALL_STAGING_CMDS
 	$(WPA_SUPPLICANT_INSTALL_STAGING_WPA_CLIENT_SO)
 endef
 
+ifeq ($(BR2_PACKAGE_IFUPDOWN_SCRIPTS),y)
+define WPA_SUPPLICANT_INSTALL_IFUP_SCRIPTS
+	$(INSTALL) -m 0755 -D package/wpa_supplicant/ifupdown.sh \
+		$(TARGET_DIR)/etc/network/if-up.d/wpasupplicant
+	ln -sf ../if-up.d/wpasupplicant $(TARGET_DIR)/etc/network/if-down.d/wpasupplicant
+endef
+endif
+
 define WPA_SUPPLICANT_INSTALL_TARGET_CMDS
 	$(INSTALL) -m 0755 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/wpa_supplicant \
 		$(TARGET_DIR)/usr/sbin/wpa_supplicant
@@ -259,8 +267,10 @@ define WPA_SUPPLICANT_INSTALL_TARGET_CMDS
 	$(WPA_SUPPLICANT_INSTALL_PASSPHRASE)
 	$(WPA_SUPPLICANT_INSTALL_DBUS)
 	$(WPA_SUPPLICANT_INSTALL_WPA_CLIENT_SO)
+	$(WPA_SUPPLICANT_INSTALL_IFUP_SCRIPTS)
 endef
 
+
 define WPA_SUPPLICANT_INSTALL_INIT_SYSTEMD
 	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant.service \
 		$(TARGET_DIR)/usr/lib/systemd/system/wpa_supplicant.service
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Buildroot] [PATCH 3/4] package/busybox: make udhcp discover faster
  2022-05-27 10:33 [Buildroot] [PATCH 0/4] Better wifi handling Angelo Compagnucci
  2022-05-27 10:33 ` [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line" Angelo Compagnucci
  2022-05-27 10:33 ` [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support Angelo Compagnucci
@ 2022-05-27 10:33 ` Angelo Compagnucci
  2022-06-01 19:26   ` Thomas Petazzoni via buildroot
  2022-05-27 10:33 ` [Buildroot] [PATCH 4/4] package/rtl8723ds: new package Angelo Compagnucci
  3 siblings, 1 reply; 19+ messages in thread
From: Angelo Compagnucci @ 2022-05-27 10:33 UTC (permalink / raw)
  To: buildroot; +Cc: jagan, Angelo Compagnucci

Instead of waiting almost 10 seconds foreground (3 discovery packets for
3 seconds retry delay) at each boot, make only one request then fork to
background. This way, the behavior is the same for working interfaces,
but it's way faster for interfaces where the address cannot be obtained
straight away.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 package/busybox/busybox.config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/busybox/busybox.config b/package/busybox/busybox.config
index 52cb8ffcd8..2409cbcce1 100644
--- a/package/busybox/busybox.config
+++ b/package/busybox/busybox.config
@@ -1022,7 +1022,7 @@ CONFIG_UDHCP_DEBUG=0
 CONFIG_UDHCPC_SLACK_FOR_BUGGY_SERVERS=80
 CONFIG_FEATURE_UDHCP_RFC3397=y
 CONFIG_FEATURE_UDHCP_8021Q=y
-CONFIG_IFUPDOWN_UDHCPC_CMD_OPTIONS="-b -R -O search"
+CONFIG_IFUPDOWN_UDHCPC_CMD_OPTIONS="-t1 -b -R -O search"
 
 #
 # Print Utilities
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Buildroot] [PATCH 4/4] package/rtl8723ds: new package
  2022-05-27 10:33 [Buildroot] [PATCH 0/4] Better wifi handling Angelo Compagnucci
                   ` (2 preceding siblings ...)
  2022-05-27 10:33 ` [Buildroot] [PATCH 3/4] package/busybox: make udhcp discover faster Angelo Compagnucci
@ 2022-05-27 10:33 ` Angelo Compagnucci
  2022-06-01 19:55   ` Thomas Petazzoni via buildroot
  3 siblings, 1 reply; 19+ messages in thread
From: Angelo Compagnucci @ 2022-05-27 10:33 UTC (permalink / raw)
  To: buildroot; +Cc: jagan, Angelo Compagnucci

This package adds a driver for Realtek RTL8723DS wifi chip.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 package/Config.in              |  1 +
 package/rtl8723ds/Config.in    | 10 ++++++++++
 package/rtl8723ds/rtl8723ds.mk | 23 +++++++++++++++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 package/rtl8723ds/Config.in
 create mode 100644 package/rtl8723ds/rtl8723ds.mk

diff --git a/package/Config.in b/package/Config.in
index 8892134133..52671dbf89 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -565,6 +565,7 @@ endmenu
 	source "package/rtl8189es/Config.in"
 	source "package/rtl8189fs/Config.in"
 	source "package/rtl8723bs/Config.in"
+	source "package/rtl8723ds/Config.in"
 	source "package/rtl8723bu/Config.in"
 	source "package/rtl8812au-aircrack-ng/Config.in"
 	source "package/rtl8821au/Config.in"
diff --git a/package/rtl8723ds/Config.in b/package/rtl8723ds/Config.in
new file mode 100644
index 0000000000..ef7dd39a68
--- /dev/null
+++ b/package/rtl8723ds/Config.in
@@ -0,0 +1,10 @@
+config BR2_PACKAGE_RTL8723DS
+	bool "rtl8723ds"
+	depends on BR2_LINUX_KERNEL
+	help
+	  rtl8723bs wifi driver
+
+	  https://github.com/lwfinger/rtl8723ds
+
+comment "rtl8723ds needs a Linux kernel to be built"
+	depends on !BR2_LINUX_KERNEL
diff --git a/package/rtl8723ds/rtl8723ds.mk b/package/rtl8723ds/rtl8723ds.mk
new file mode 100644
index 0000000000..dea403c8d6
--- /dev/null
+++ b/package/rtl8723ds/rtl8723ds.mk
@@ -0,0 +1,23 @@
+################################################################################
+#
+# rtl8723ds
+#
+################################################################################
+
+RTL8723DS_VERSION = 76146e85847beb2427b1d4958fa275822f2b04ab
+RTL8723DS_SITE = $(call github,lwfinger,rtl8723ds,$(RTL8723DS_VERSION))
+RTL8723DS_LICENSE = GPL-2.0, proprietary (*.bin firmware blobs)
+
+RTL8723DS_MODULE_MAKE_OPTS = \
+	CONFIG_RTL8723DS=m \
+	KVER=$(LINUX_VERSION_PROBED) \
+	KSRC=$(LINUX_DIR)
+
+define RTL8723DS_LINUX_CONFIG_FIXUPS
+	$(call KCONFIG_ENABLE_OPT,CONFIG_NET)
+	$(call KCONFIG_ENABLE_OPT,CONFIG_WIRELESS)
+	$(call KCONFIG_ENABLE_OPT,CONFIG_CFG80211)
+endef
+
+$(eval $(kernel-module))
+$(eval $(generic-package))
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-05-27 10:33 ` [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line" Angelo Compagnucci
@ 2022-06-01 19:21   ` Thomas Petazzoni via buildroot
  2022-06-01 21:55     ` Angelo Compagnucci
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-06-01 19:21 UTC (permalink / raw)
  To: Angelo Compagnucci; +Cc: jagan, buildroot

On Fri, 27 May 2022 12:33:32 +0200
Angelo Compagnucci <angelo@amarulasolutions.com> wrote:

> Default configuration file is wrong for the default compiling options.
> 
> Fixes:
> 
> Successfully initialized wpa_supplicant
> Line 1: unknown global field 'ctrl_interface=/var/run/wpa_supplicant'.
> Line 1: Invalid configuration line
> 'ctrl_interface=/var/run/wpa_supplicant'.
> Failed to read or parse configuration '/etc/wpa_supplicant.conf'.
> 
> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>

Indeed, this option only makes sense when
BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE=y I suppose. However, in this
case, it makes sense a lot of sense to have this option in the config
file.

Should we have some kind of logic to add this line when
BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE=y ?

Maybe something like this:

# ctrl_interface=/var/run/wpa_supplicant # BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE

and a bit of $(SED) magic in the .mk file ?

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support
  2022-05-27 10:33 ` [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support Angelo Compagnucci
@ 2022-06-01 19:25   ` Thomas Petazzoni via buildroot
  2022-06-01 22:06     ` Angelo Compagnucci
  2022-06-06 15:09   ` Nicolas Cavallari
  1 sibling, 1 reply; 19+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-06-01 19:25 UTC (permalink / raw)
  To: Angelo Compagnucci; +Cc: jagan, buildroot

Hello,

On Fri, 27 May 2022 12:33:33 +0200
Angelo Compagnucci <angelo@amarulasolutions.com> wrote:

> Actually, configuring a wifi interface as per "interfaces" man:
> 
> auto wlan0
> iface wlan0 inet dhcp
> wpa-conf /etc/wpa_supplicant.conf

Do you have a link to an interfaces manpage that documents wpa-conf?

Is this supported by the Busybox ifupdown?

> diff --git a/package/wpa_supplicant/ifupdown.sh b/package/wpa_supplicant/ifupdown.sh
> new file mode 100755
> index 0000000000..8eecf73436
> --- /dev/null
> +++ b/package/wpa_supplicant/ifupdown.sh
> @@ -0,0 +1,71 @@
> +#!/bin/sh
> +
> +# This file is executed by ifupdown in pre-up, post-up, pre-down and
> +# post-down phases of network interface configuration.
> +
> +WPA_SUP_BIN="/usr/sbin/wpa_supplicant"
> +
> +if [ -n "$IF_WPA_MAINT_DEBUG" ]; then
> +	set -x
> +fi

Where is IF_WPA_MAINT_DEBUG supposed to be defined?

> +# allow wpa_supplicant interface to be specified via wpa-iface
> +# useful for starting wpa_supplicant on one interface of a bridge
> +if [ -n "$IF_WPA_IFACE" ]; then
> +	WPA_IFACE="$IF_WPA_IFACE"
> +else
> +	WPA_IFACE="$IFACE"
> +fi

I'm curious to understand how wpa-iface ends up in IP_WPA_IFACE. I
guess I'm missing a piece of the puzzla.

> +WPA_SUP_PIDFILE="/run/wpa_supplicant.${WPA_IFACE}.pid"
> +
> +# quit if executables are not installed
> +if [ ! -x "$WPA_SUP_BIN" ]; then
> +	exit 0
> +fi

This can be removed in the context of Buildroot. We tend to not check
for the installation of executables from the same package, as it's
quite useless.

> +
> +do_start () {
> +	if [ -n "$IF_WPA_CONF" ] && [ "$IF_WPA_CONF" != "managed" ]; then
> +		if [ ! -s "$IF_WPA_CONF" ]; then
> +			echo "cannot read contents of $IF_WPA_CONF"
> +			exit 1
> +		fi
> +		WPA_SUP_CONF_CTRL_DIR=$(sed -n -e 's/[[:space:]]*#.*//g' -e 's/[[:space:]]\+.*$//g' \
> +			-e 's/^ctrl_interface=\(DIR=\)\?\(.*\)/\2/p' "$IF_WPA_CONF")
> +		if [ -n "$WPA_SUP_CONF_CTRL_DIR" ]; then
> +			WPA_SUP_CONF="-c $IF_WPA_CONF -C $WPA_SUP_CONF_CTRL_DIR"

The manpage of wpa_supplicant says:

       -C ctrl_interface
              Path to ctrl_interface socket (Per interface. Only used if -c is not).

so passing -C when -c is passed does not make sense. Or am I missing
something?

> +		else
> +			WPA_SUP_CONF="-c $IF_WPA_CONF"
> +		fi
> +	else
> +		# specify the default ctrl_interface
> +		WPA_SUP_CONF="-C $WPA_CTRL_DIR"

How is WPA_CTRL_DIR defined?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 3/4] package/busybox: make udhcp discover faster
  2022-05-27 10:33 ` [Buildroot] [PATCH 3/4] package/busybox: make udhcp discover faster Angelo Compagnucci
@ 2022-06-01 19:26   ` Thomas Petazzoni via buildroot
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-06-01 19:26 UTC (permalink / raw)
  To: Angelo Compagnucci; +Cc: jagan, buildroot

On Fri, 27 May 2022 12:33:34 +0200
Angelo Compagnucci <angelo@amarulasolutions.com> wrote:

> Instead of waiting almost 10 seconds foreground (3 discovery packets for
> 3 seconds retry delay) at each boot, make only one request then fork to
> background. This way, the behavior is the same for working interfaces,
> but it's way faster for interfaces where the address cannot be obtained
> straight away.
> 
> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
> ---
>  package/busybox/busybox.config | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/busybox/busybox.config b/package/busybox/busybox.config
> index 52cb8ffcd8..2409cbcce1 100644
> --- a/package/busybox/busybox.config
> +++ b/package/busybox/busybox.config
> @@ -1022,7 +1022,7 @@ CONFIG_UDHCP_DEBUG=0
>  CONFIG_UDHCPC_SLACK_FOR_BUGGY_SERVERS=80
>  CONFIG_FEATURE_UDHCP_RFC3397=y
>  CONFIG_FEATURE_UDHCP_8021Q=y
> -CONFIG_IFUPDOWN_UDHCPC_CMD_OPTIONS="-b -R -O search"
> +CONFIG_IFUPDOWN_UDHCPC_CMD_OPTIONS="-t1 -b -R -O search"
>  
>  #
>  # Print Utilities

I will not commit this myself as I want another Buildroot maintainer to
have a look, but it seems reasonable to me:

Acked-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 4/4] package/rtl8723ds: new package
  2022-05-27 10:33 ` [Buildroot] [PATCH 4/4] package/rtl8723ds: new package Angelo Compagnucci
@ 2022-06-01 19:55   ` Thomas Petazzoni via buildroot
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-06-01 19:55 UTC (permalink / raw)
  To: Angelo Compagnucci; +Cc: jagan, buildroot

On Fri, 27 May 2022 12:33:35 +0200
Angelo Compagnucci <angelo@amarulasolutions.com> wrote:

> This package adds a driver for Realtek RTL8723DS wifi chip.
> 
> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
> ---
>  package/Config.in              |  1 +
>  package/rtl8723ds/Config.in    | 10 ++++++++++
>  package/rtl8723ds/rtl8723ds.mk | 23 +++++++++++++++++++++++
>  3 files changed, 34 insertions(+)

This was missing an entry in the DEVELOPERS file.

> diff --git a/package/Config.in b/package/Config.in
> index 8892134133..52671dbf89 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -565,6 +565,7 @@ endmenu
>  	source "package/rtl8189es/Config.in"
>  	source "package/rtl8189fs/Config.in"
>  	source "package/rtl8723bs/Config.in"
> +	source "package/rtl8723ds/Config.in"
>  	source "package/rtl8723bu/Config.in"

Alphabetic ordering was not correct here (reported by check-package),
so I fixed that up.


> +RTL8723DS_VERSION = 76146e85847beb2427b1d4958fa275822f2b04ab
> +RTL8723DS_SITE = $(call github,lwfinger,rtl8723ds,$(RTL8723DS_VERSION))
> +RTL8723DS_LICENSE = GPL-2.0, proprietary (*.bin firmware blobs)

There are no firmware files in this Git repository, so it seems like
"proprietary (*.bin firmware blobs)" was copy/pasted from other rtlxxxx
packages. I dropped that, and kept only GPL-2.0.

Of course, if you have some evidence showing that there are some
proprietary firmware files in this package, let me know and we'll fix
things back.

Applied to next with those changes. Thanks!

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-06-01 19:21   ` Thomas Petazzoni via buildroot
@ 2022-06-01 21:55     ` Angelo Compagnucci
  2022-06-06 12:42       ` Arnout Vandecappelle
  0 siblings, 1 reply; 19+ messages in thread
From: Angelo Compagnucci @ 2022-06-01 21:55 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: jagan, buildroot


[-- Attachment #1.1: Type: text/plain, Size: 1735 bytes --]

On Wed, Jun 1, 2022 at 9:21 PM Thomas Petazzoni <
thomas.petazzoni@bootlin.com> wrote:

> On Fri, 27 May 2022 12:33:32 +0200
> Angelo Compagnucci <angelo@amarulasolutions.com> wrote:
>
> > Default configuration file is wrong for the default compiling options.
> >
> > Fixes:
> >
> > Successfully initialized wpa_supplicant
> > Line 1: unknown global field 'ctrl_interface=/var/run/wpa_supplicant'.
> > Line 1: Invalid configuration line
> > 'ctrl_interface=/var/run/wpa_supplicant'.
> > Failed to read or parse configuration '/etc/wpa_supplicant.conf'.
> >
> > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
>
> Indeed, this option only makes sense when
> BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE=y I suppose. However, in this
> case, it makes sense a lot of sense to have this option in the config
> file.
>
> Should we have some kind of logic to add this line when
> BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE=y ?
>
> Maybe something like this:
>
> # ctrl_interface=/var/run/wpa_supplicant #
> BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE
>
> and a bit of $(SED) magic in the .mk file ?
>

No need to have this logic in my opinion:
* the option has a sensible default when enabled
* If a user really needs to change the default, he can add that line
manually in an overlay file.


>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
>


-- 

Angelo Compagnucci

Software Engineer

angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL

Via le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 (0)42 243 5310
info@amarulasolutions.com

www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|

[-- Attachment #1.2: Type: text/html, Size: 6367 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support
  2022-06-01 19:25   ` Thomas Petazzoni via buildroot
@ 2022-06-01 22:06     ` Angelo Compagnucci
  0 siblings, 0 replies; 19+ messages in thread
From: Angelo Compagnucci @ 2022-06-01 22:06 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: jagan, buildroot


[-- Attachment #1.1: Type: text/plain, Size: 4047 bytes --]

On Wed, Jun 1, 2022 at 9:25 PM Thomas Petazzoni <
thomas.petazzoni@bootlin.com> wrote:

> Hello,
>
> On Fri, 27 May 2022 12:33:33 +0200
> Angelo Compagnucci <angelo@amarulasolutions.com> wrote:
>
> > Actually, configuring a wifi interface as per "interfaces" man:
> >
> > auto wlan0
> > iface wlan0 inet dhcp
> > wpa-conf /etc/wpa_supplicant.conf
>
> Do you have a link to an interfaces manpage that documents wpa-conf?
>

Not really, I had a look at the source code for busybox and general
debian/ifupdown documentation. The better info I could find is this
stackoverflow page
https://unix.stackexchange.com/questions/262094/where-is-wpa-conf-documented


> Is this supported by the Busybox ifupdown?
>

Yes it is.


>
> > diff --git a/package/wpa_supplicant/ifupdown.sh
> b/package/wpa_supplicant/ifupdown.sh
> > new file mode 100755
> > index 0000000000..8eecf73436
> > --- /dev/null
> > +++ b/package/wpa_supplicant/ifupdown.sh
> > @@ -0,0 +1,71 @@
> > +#!/bin/sh
> > +
> > +# This file is executed by ifupdown in pre-up, post-up, pre-down and
> > +# post-down phases of network interface configuration.
> > +
> > +WPA_SUP_BIN="/usr/sbin/wpa_supplicant"
> > +
> > +if [ -n "$IF_WPA_MAINT_DEBUG" ]; then
> > +     set -x
> > +fi
>
> Where is IF_WPA_MAINT_DEBUG supposed to be defined?
>

In your console before running the script manually if you need to debug it.

> +# allow wpa_supplicant interface to be specified via wpa-iface
> > +# useful for starting wpa_supplicant on one interface of a bridge
> > +if [ -n "$IF_WPA_IFACE" ]; then
> > +     WPA_IFACE="$IF_WPA_IFACE"
> > +else
> > +     WPA_IFACE="$IFACE"
> > +fi
>
> I'm curious to understand how wpa-iface ends up in IP_WPA_IFACE. I
> guess I'm missing a piece of the puzzla.
>

If you add an "echo $@" at the beginning of the script, you could see that
when "wpa-conf" option is defined, those variables are injected into the
environment by busybox while calling the ifup/ifdown scripts.


>
> > +WPA_SUP_PIDFILE="/run/wpa_supplicant.${WPA_IFACE}.pid"
> > +
> > +# quit if executables are not installed
> > +if [ ! -x "$WPA_SUP_BIN" ]; then
> > +     exit 0
> > +fi
>
> This can be removed in the context of Buildroot. We tend to not check
> for the installation of executables from the same package, as it's
> quite useless.
>

Fair enough.


> > +
> > +do_start () {
> > +     if [ -n "$IF_WPA_CONF" ] && [ "$IF_WPA_CONF" != "managed" ]; then
> > +             if [ ! -s "$IF_WPA_CONF" ]; then
> > +                     echo "cannot read contents of $IF_WPA_CONF"
> > +                     exit 1
> > +             fi
> > +             WPA_SUP_CONF_CTRL_DIR=$(sed -n -e 's/[[:space:]]*#.*//g'
> -e 's/[[:space:]]\+.*$//g' \
> > +                     -e 's/^ctrl_interface=\(DIR=\)\?\(.*\)/\2/p'
> "$IF_WPA_CONF")
> > +             if [ -n "$WPA_SUP_CONF_CTRL_DIR" ]; then
> > +                     WPA_SUP_CONF="-c $IF_WPA_CONF -C
> $WPA_SUP_CONF_CTRL_DIR"
>
> The manpage of wpa_supplicant says:
>
>        -C ctrl_interface
>               Path to ctrl_interface socket (Per interface. Only used if
> -c is not).
>
> so passing -C when -c is passed does not make sense. Or am I missing
> something?
>

Not entirely sure, need to check on this.  From my testing, that is working
fine. Let me review that.


> > +             else
> > +                     WPA_SUP_CONF="-c $IF_WPA_CONF"
> > +             fi
> > +     else
> > +             # specify the default ctrl_interface
> > +             WPA_SUP_CONF="-C $WPA_CTRL_DIR"
>
> How is WPA_CTRL_DIR defined?
>

It should be injected too, but I'll do a recheck to be completely sure.


> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
>


-- 

Angelo Compagnucci

Software Engineer

angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL

Via le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 (0)42 243 5310
info@amarulasolutions.com

www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|

[-- Attachment #1.2: Type: text/html, Size: 10360 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-06-01 21:55     ` Angelo Compagnucci
@ 2022-06-06 12:42       ` Arnout Vandecappelle
  2022-06-07 10:21         ` Angelo Compagnucci
  0 siblings, 1 reply; 19+ messages in thread
From: Arnout Vandecappelle @ 2022-06-06 12:42 UTC (permalink / raw)
  To: Angelo Compagnucci, Thomas Petazzoni; +Cc: jagan, buildroot



On 01/06/2022 23:55, Angelo Compagnucci wrote:
> 
> 
> On Wed, Jun 1, 2022 at 9:21 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com 
> <mailto:thomas.petazzoni@bootlin.com>> wrote:
> 
>     On Fri, 27 May 2022 12:33:32 +0200
>     Angelo Compagnucci <angelo@amarulasolutions.com
>     <mailto:angelo@amarulasolutions.com>> wrote:
> 
>      > Default configuration file is wrong for the default compiling options.
>      >
>      > Fixes:
>      >
>      > Successfully initialized wpa_supplicant
>      > Line 1: unknown global field 'ctrl_interface=/var/run/wpa_supplicant'.
>      > Line 1: Invalid configuration line
>      > 'ctrl_interface=/var/run/wpa_supplicant'.
>      > Failed to read or parse configuration '/etc/wpa_supplicant.conf'.
>      >
>      > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com
>     <mailto:angelo@amarulasolutions.com>>
> 
>     Indeed, this option only makes sense when
>     BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE=y I suppose. However, in this
>     case, it makes sense a lot of sense to have this option in the config
>     file.
> 
>     Should we have some kind of logic to add this line when
>     BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE=y ?
> 
>     Maybe something like this:
> 
>     # ctrl_interface=/var/run/wpa_supplicant # BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE
> 
>     and a bit of $(SED) magic in the .mk file ?
> 
> 
> No need to have this logic in my opinion:
> * the option has a sensible default when enabled

  Does it? AFAIK, if ctrl_interface is not specified in the config file or on 
the command line with -C, then no control socket will be created.

> * If a user really needs to change the default, he can add that line manually in 
> an overlay file.

  Yeah, but we prefer that if a user selects 
BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE, that the control socket actually appears 
in the expected place (/var/run/wpa_supplicant).

  Regards,
  Arnout

> 
> 
>     Best regards,
> 
>     Thomas
>     -- 
>     Thomas Petazzoni, co-owner and CEO, Bootlin
>     Embedded Linux and Kernel engineering and training
>     https://bootlin.com <https://bootlin.com>
> 
> 
> 
> -- 
> 
> Angelo Compagnucci
> 
> Software Engineer
> 
> angelo@amarulasolutions.com <mailto:angelo@amarulasolutions.com>
> __________________________________
> Amarula Solutions SRL
> 
> Via le Canevare 30, 31100 Treviso, Veneto, IT
> 
> T. +39 (0)42 243 5310
> info@amarulasolutions.com <mailto:info@amarulasolutions.com>
> 
> www.amarulasolutions.com <http://www.amarulasolutions.com/>
> 
> [`as] https://www.amarulasolutions.com <https://www.amarulasolutions.com>|
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support
  2022-05-27 10:33 ` [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support Angelo Compagnucci
  2022-06-01 19:25   ` Thomas Petazzoni via buildroot
@ 2022-06-06 15:09   ` Nicolas Cavallari
  2022-06-07 15:00     ` Angelo Compagnucci
  1 sibling, 1 reply; 19+ messages in thread
From: Nicolas Cavallari @ 2022-06-06 15:09 UTC (permalink / raw)
  To: Angelo Compagnucci, buildroot; +Cc: jagan

Hello,

On 27/05/2022 12:33, Angelo Compagnucci wrote:
> Actually, configuring a wifi interface as per "interfaces" man:
> 
> auto wlan0
> iface wlan0 inet dhcp
> wpa-conf /etc/wpa_supplicant.conf
> 
> doesn't work on buildroot because the line wpa-conf is ignored due to
> the lack of a proper ifupdown script to handle the wpa_supplicant
> initialization.
> 
> The provided file is a simplified version based on the one available
> on debian.
> 
> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
> ---
>   package/wpa_supplicant/ifupdown.sh       | 71 ++++++++++++++++++++++++
>   package/wpa_supplicant/wpa_supplicant.mk | 10 ++++
>   2 files changed, 81 insertions(+)
>   create mode 100755 package/wpa_supplicant/ifupdown.sh
> 
> diff --git a/package/wpa_supplicant/ifupdown.sh b/package/wpa_supplicant/ifupdown.sh
> new file mode 100755
> index 0000000000..8eecf73436
> --- /dev/null
> +++ b/package/wpa_supplicant/ifupdown.sh
> @@ -0,0 +1,71 @@
> +#!/bin/sh
> +
> +# This file is executed by ifupdown in pre-up, post-up, pre-down and
> +# post-down phases of network interface configuration.

It is also executed for every interface regardless of if wpa-* options are 
specified or not. ifupdown works by turning all options into IF_* environment 
variables and running all hooks with it.

Right now this hook starts wpa_supplicant on every non-lo interface.
It probably needs a if [ -z "$IF_WPA_CONF" ]; then exit 0; fi somewhere.
The earliest the better.

> +
> +WPA_SUP_BIN="/usr/sbin/wpa_supplicant"
> +
> +if [ -n "$IF_WPA_MAINT_DEBUG" ]; then
> +	set -x
> +fi

It is really necessary to support the "wpa-maint-debug yes" 
/etc/network/interfaces option ?  This script is simple enough, unlike Debian's.

> +
> +# quit if we're called for the loopback
> +if [ "$IFACE" = lo ]; then
> +	exit 0
> +fi

Special-casing lo can be removed by filtering on -z IF_WPA_CONF.  Nobody is 
going to specify a wpa-conf on the lo interface, and if they do, they should 
assume the consequences.

> +
> +# allow wpa_supplicant interface to be specified via wpa-iface
> +# useful for starting wpa_supplicant on one interface of a bridge
> +if [ -n "$IF_WPA_IFACE" ]; then
> +	WPA_IFACE="$IF_WPA_IFACE"
> +else
> +	WPA_IFACE="$IFACE"
> +fi
> +
> +WPA_SUP_PIDFILE="/run/wpa_supplicant.${WPA_IFACE}.pid"
> +
> +# quit if executables are not installed
> +if [ ! -x "$WPA_SUP_BIN" ]; then
> +	exit 0
> +fi
> +
> +do_start () {
> +	if [ -n "$IF_WPA_CONF" ] && [ "$IF_WPA_CONF" != "managed" ]; then
> +		if [ ! -s "$IF_WPA_CONF" ]; then
> +			echo "cannot read contents of $IF_WPA_CONF"
> +			exit 1
> +		fi
> +		WPA_SUP_CONF_CTRL_DIR=$(sed -n -e 's/[[:space:]]*#.*//g' -e 's/[[:space:]]\+.*$//g' \
> +			-e 's/^ctrl_interface=\(DIR=\)\?\(.*\)/\2/p' "$IF_WPA_CONF")
> +		if [ -n "$WPA_SUP_CONF_CTRL_DIR" ]; then
> +			WPA_SUP_CONF="-c $IF_WPA_CONF -C $WPA_SUP_CONF_CTRL_DI > +		else
> +			WPA_SUP_CONF="-c $IF_WPA_CONF"
> +		fi
> +	else
> +		# specify the default ctrl_interface
> +		WPA_SUP_CONF="-C $WPA_CTRL_DIR"
> +	fi
> +}

Debian's ifupdown script need wpa_supplicant to have a ctrl_interface because it 
want to use wpa_cli for all the other wpa-* options, and wpa_cli works by 
connecting to a ctrl_interface.

This script does not use wpa_cli so all the ctrl_interface handling can be 
removed, unless you want to implement the other options later.

By the way, "wpa-conf "managed" is mostly used with the other wpa-* options to 
configure the ssid/passphrase within /etc/network/interfaces, which is something 
your script does not support, so you can remove the first 'if'.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-06-06 12:42       ` Arnout Vandecappelle
@ 2022-06-07 10:21         ` Angelo Compagnucci
  2022-06-07 11:48           ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 19+ messages in thread
From: Angelo Compagnucci @ 2022-06-07 10:21 UTC (permalink / raw)
  To: Arnout Vandecappelle; +Cc: jagan, Thomas Petazzoni, buildroot


[-- Attachment #1.1: Type: text/plain, Size: 3672 bytes --]

On Mon, Jun 6, 2022 at 2:42 PM Arnout Vandecappelle <arnout@mind.be> wrote:

>
>
> On 01/06/2022 23:55, Angelo Compagnucci wrote:
> >
> >
> > On Wed, Jun 1, 2022 at 9:21 PM Thomas Petazzoni <
> thomas.petazzoni@bootlin.com
> > <mailto:thomas.petazzoni@bootlin.com>> wrote:
> >
> >     On Fri, 27 May 2022 12:33:32 +0200
> >     Angelo Compagnucci <angelo@amarulasolutions.com
> >     <mailto:angelo@amarulasolutions.com>> wrote:
> >
> >      > Default configuration file is wrong for the default compiling
> options.
> >      >
> >      > Fixes:
> >      >
> >      > Successfully initialized wpa_supplicant
> >      > Line 1: unknown global field
> 'ctrl_interface=/var/run/wpa_supplicant'.
> >      > Line 1: Invalid configuration line
> >      > 'ctrl_interface=/var/run/wpa_supplicant'.
> >      > Failed to read or parse configuration '/etc/wpa_supplicant.conf'.
> >      >
> >      > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com
> >     <mailto:angelo@amarulasolutions.com>>
> >
> >     Indeed, this option only makes sense when
> >     BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE=y I suppose. However, in this
> >     case, it makes sense a lot of sense to have this option in the config
> >     file.
> >
> >     Should we have some kind of logic to add this line when
> >     BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE=y ?
> >
> >     Maybe something like this:
> >
> >     # ctrl_interface=/var/run/wpa_supplicant #
> BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE
> >
> >     and a bit of $(SED) magic in the .mk file ?
> >
> >
> > No need to have this logic in my opinion:
> > * the option has a sensible default when enabled
>
>   Does it? AFAIK, if ctrl_interface is not specified in the config file or
> on
> the command line with -C, then no control socket will be created.
>
> > * If a user really needs to change the default, he can add that line
> manually in
> > an overlay file.
>
>   Yeah, but we prefer that if a user selects
> BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE, that the control socket actually
> appears
> in the expected place (/var/run/wpa_supplicant).
>

The problem here is that wpa_supplicant.conf file will always be provided
in a user overlay, because to connect to a wifi a proper `network=`
configuration settings must be provided.
Would we really like to mess with a user overlay provided configuration
file? I think that a user must ensure the configuration file is in good
shape for the service to work.


>
>   Regards,
>   Arnout
>
> >
> >
> >     Best regards,
> >
> >     Thomas
> >     --
> >     Thomas Petazzoni, co-owner and CEO, Bootlin
> >     Embedded Linux and Kernel engineering and training
> >     https://bootlin.com <https://bootlin.com>
> >
> >
> >
> > --
> >
> > Angelo Compagnucci
> >
> > Software Engineer
> >
> > angelo@amarulasolutions.com <mailto:angelo@amarulasolutions.com>
> > __________________________________
> > Amarula Solutions SRL
> >
> > Via le Canevare 30, 31100 Treviso, Veneto, IT
> >
> > T. +39 (0)42 243 5310
> > info@amarulasolutions.com <mailto:info@amarulasolutions.com>
> >
> > www.amarulasolutions.com <http://www.amarulasolutions.com/>
> >
> > [`as] https://www.amarulasolutions.com <https://www.amarulasolutions.com
> >|
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
>


-- 

Angelo Compagnucci

Software Engineer

angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL

Via le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 (0)42 243 5310
info@amarulasolutions.com

www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|

[-- Attachment #1.2: Type: text/html, Size: 9896 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-06-07 10:21         ` Angelo Compagnucci
@ 2022-06-07 11:48           ` Thomas Petazzoni via buildroot
  2022-06-07 12:16             ` Angelo Compagnucci
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-06-07 11:48 UTC (permalink / raw)
  To: Angelo Compagnucci; +Cc: jagan, buildroot

On Tue, 7 Jun 2022 12:21:22 +0200
Angelo Compagnucci <angelo@amarulasolutions.com> wrote:

> The problem here is that wpa_supplicant.conf file will always be provided
> in a user overlay, because to connect to a wifi a proper `network=`
> configuration settings must be provided.

Then why do we provide an example configuration file? :-)

> Would we really like to mess with a user overlay provided configuration
> file? I think that a user must ensure the configuration file is in good
> shape for the service to work.

If the wpa_supplicant.conf provided by Buildroot is overridden by a
file in the rootfs overlay, then whatever we do in
WPA_SUPPLICANT_INSTALL_TARGET_HOOKS will have no effect, the file in
the rootfs overlay will win. So we are not "messing up" with the file
provided by the user.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-06-07 11:48           ` Thomas Petazzoni via buildroot
@ 2022-06-07 12:16             ` Angelo Compagnucci
  2022-06-07 13:12               ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 19+ messages in thread
From: Angelo Compagnucci @ 2022-06-07 12:16 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: jagan, buildroot


[-- Attachment #1.1: Type: text/plain, Size: 2002 bytes --]

On Tue, Jun 7, 2022 at 1:48 PM Thomas Petazzoni <
thomas.petazzoni@bootlin.com> wrote:

> On Tue, 7 Jun 2022 12:21:22 +0200
> Angelo Compagnucci <angelo@amarulasolutions.com> wrote:
>
> > The problem here is that wpa_supplicant.conf file will always be provided
> > in a user overlay, because to connect to a wifi a proper `network=`
> > configuration settings must be provided.
>
> Then why do we provide an example configuration file? :-)
>

Is the one we provide really an example configuration file?  An example
configuration file usually looks quite different!


> > Would we really like to mess with a user overlay provided configuration
> > file? I think that a user must ensure the configuration file is in good
> > shape for the service to work.
>
> If the wpa_supplicant.conf provided by Buildroot is overridden by a
> file in the rootfs overlay, then whatever we do in
> WPA_SUPPLICANT_INSTALL_TARGET_HOOKS will have no effect, the file in
> the rootfs overlay will win. So we are not "messing up" with the file
> provided by the user.
>

Indeed. It looks useless doing some configuration file mangling only to
have it overwritten. We don't do that for other parameters, like autoscan
for example, which in turn we should do following this logic.
I think that the provided configuration file should work with provided
default options, or we should consider all the optional values too, they
are many more for wpa_supplicant. What happens if I configure EAP, or AP
mode or the hotspot? Should we have "example" configuration sections
enabled by those options?


> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
>


-- 

Angelo Compagnucci

Software Engineer

angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL

Via le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 (0)42 243 5310
info@amarulasolutions.com

www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|

[-- Attachment #1.2: Type: text/html, Size: 6658 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-06-07 12:16             ` Angelo Compagnucci
@ 2022-06-07 13:12               ` Thomas Petazzoni via buildroot
  2022-06-07 13:22                 ` Angelo Compagnucci
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-06-07 13:12 UTC (permalink / raw)
  To: Angelo Compagnucci; +Cc: jagan, buildroot

On Tue, 7 Jun 2022 14:16:19 +0200
Angelo Compagnucci <angelo@amarulasolutions.com> wrote:

> > Then why do we provide an example configuration file? :-)
> 
> Is the one we provide really an example configuration file?  An example
> configuration file usually looks quite different!

Sorry I don't follow you. We provide and install
package/wpa_supplicant/wpa_supplicant.conf, which is an example
configuration.

Or perhaps your question was ironic/cynical, and I didn't get the joke?

> Indeed. It looks useless doing some configuration file mangling only to
> have it overwritten. We don't do that for other parameters, like autoscan
> for example, which in turn we should do following this logic.
> I think that the provided configuration file should work with provided
> default options, or we should consider all the optional values too, they
> are many more for wpa_supplicant. What happens if I configure EAP, or AP
> mode or the hotspot? Should we have "example" configuration sections
> enabled by those options?

I understand your point, and I agree it's difficult and probably too
complicated to have all the logic to adjust the example configuration
file to all possible sub-options of wpa_supplicant. We have to draw the
line somewhere and it's not very clear cut where to draw this line.

I just though a working wpa_cli / wpa_supplicant combo would be useful
to have as an out-of-the box experience when
BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE is enabled. I guess this was the
intention of the currently provided wpa_supplicant.conf.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line"
  2022-06-07 13:12               ` Thomas Petazzoni via buildroot
@ 2022-06-07 13:22                 ` Angelo Compagnucci
  0 siblings, 0 replies; 19+ messages in thread
From: Angelo Compagnucci @ 2022-06-07 13:22 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: jagan, buildroot


[-- Attachment #1.1: Type: text/plain, Size: 2802 bytes --]

On Tue, Jun 7, 2022 at 3:12 PM Thomas Petazzoni <
thomas.petazzoni@bootlin.com> wrote:

> On Tue, 7 Jun 2022 14:16:19 +0200
> Angelo Compagnucci <angelo@amarulasolutions.com> wrote:
>
> > > Then why do we provide an example configuration file? :-)
> >
> > Is the one we provide really an example configuration file?  An example
> > configuration file usually looks quite different!
>
> Sorry I don't follow you. We provide and install
> package/wpa_supplicant/wpa_supplicant.conf, which is an example
> configuration.
>
> Or perhaps your question was ironic/cynical, and I didn't get the joke?
>

It was! Sorry! The example configuration file for wpa_supplicant is 2K+
line with explained each one of the configuration options.
Our is a highly opinionated minimal configuration file to run the service,
which in turn doesn't work due to an incompatible configuration line!


>
> > Indeed. It looks useless doing some configuration file mangling only to
> > have it overwritten. We don't do that for other parameters, like autoscan
> > for example, which in turn we should do following this logic.
> > I think that the provided configuration file should work with provided
> > default options, or we should consider all the optional values too, they
> > are many more for wpa_supplicant. What happens if I configure EAP, or AP
> > mode or the hotspot? Should we have "example" configuration sections
> > enabled by those options?
>
> I understand your point, and I agree it's difficult and probably too
> complicated to have all the logic to adjust the example configuration
> file to all possible sub-options of wpa_supplicant. We have to draw the
> line somewhere and it's not very clear cut where to draw this line.
>
> I just though a working wpa_cli / wpa_supplicant combo would be useful
> to have as an out-of-the box experience when
> BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE is enabled. I guess this was the
> intention of the currently provided wpa_supplicant.conf.
>

It could, but the outcome was poor, due to the fact the configuration
option kills the service with an error.
I still think we should stick with something that works for our default and
minimal configuration and let the user decide what to do.
I think it's cleaner to have a service that works "out of the box" and
complains with an error when you enable some options not properly
configured.


>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
>


-- 

Angelo Compagnucci

Software Engineer

angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL

Via le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 (0)42 243 5310
info@amarulasolutions.com

www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|

[-- Attachment #1.2: Type: text/html, Size: 7584 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support
  2022-06-06 15:09   ` Nicolas Cavallari
@ 2022-06-07 15:00     ` Angelo Compagnucci
  0 siblings, 0 replies; 19+ messages in thread
From: Angelo Compagnucci @ 2022-06-07 15:00 UTC (permalink / raw)
  To: Nicolas Cavallari; +Cc: jagan, buildroot


[-- Attachment #1.1: Type: text/plain, Size: 4581 bytes --]

On Mon, Jun 6, 2022 at 5:09 PM Nicolas Cavallari <
Nicolas.Cavallari@green-communications.fr> wrote:

> Hello,
>
> On 27/05/2022 12:33, Angelo Compagnucci wrote:
> > Actually, configuring a wifi interface as per "interfaces" man:
> >
> > auto wlan0
> > iface wlan0 inet dhcp
> > wpa-conf /etc/wpa_supplicant.conf
> >
> > doesn't work on buildroot because the line wpa-conf is ignored due to
> > the lack of a proper ifupdown script to handle the wpa_supplicant
> > initialization.
> >
> > The provided file is a simplified version based on the one available
> > on debian.
> >
> > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
> > ---
> >   package/wpa_supplicant/ifupdown.sh       | 71 ++++++++++++++++++++++++
> >   package/wpa_supplicant/wpa_supplicant.mk | 10 ++++
> >   2 files changed, 81 insertions(+)
> >   create mode 100755 package/wpa_supplicant/ifupdown.sh
> >
> > diff --git a/package/wpa_supplicant/ifupdown.sh
> b/package/wpa_supplicant/ifupdown.sh
> > new file mode 100755
> > index 0000000000..8eecf73436
> > --- /dev/null
> > +++ b/package/wpa_supplicant/ifupdown.sh
> > @@ -0,0 +1,71 @@
> > +#!/bin/sh
> > +
> > +# This file is executed by ifupdown in pre-up, post-up, pre-down and
> > +# post-down phases of network interface configuration.
>
> It is also executed for every interface regardless of if wpa-* options are
> specified or not. ifupdown works by turning all options into IF_*
> environment
> variables and running all hooks with it.
>
> Right now this hook starts wpa_supplicant on every non-lo interface.
> It probably needs a if [ -z "$IF_WPA_CONF" ]; then exit 0; fi somewhere.
> The earliest the better.
>

Not entirely sure, wpa_supplicant can be used on Ethernet interfaces too.
Better to check if the IF_WPA_CONF has a value or exit silently.


>
> > +
> > +WPA_SUP_BIN="/usr/sbin/wpa_supplicant"
> > +
> > +if [ -n "$IF_WPA_MAINT_DEBUG" ]; then
> > +     set -x
> > +fi
>
> It is really necessary to support the "wpa-maint-debug yes"
> /etc/network/interfaces option ?  This script is simple enough, unlike
> Debian's.
>

Right.


>
> > +
> > +# quit if we're called for the loopback
> > +if [ "$IFACE" = lo ]; then
> > +     exit 0
> > +fi
>
> Special-casing lo can be removed by filtering on -z IF_WPA_CONF.  Nobody
> is
> going to specify a wpa-conf on the lo interface, and if they do, they
> should
> assume the consequences.
>

Right.


>
> > +
> > +# allow wpa_supplicant interface to be specified via wpa-iface
> > +# useful for starting wpa_supplicant on one interface of a bridge
> > +if [ -n "$IF_WPA_IFACE" ]; then
> > +     WPA_IFACE="$IF_WPA_IFACE"
> > +else
> > +     WPA_IFACE="$IFACE"
> > +fi
> > +
> > +WPA_SUP_PIDFILE="/run/wpa_supplicant.${WPA_IFACE}.pid"
> > +
> > +# quit if executables are not installed
> > +if [ ! -x "$WPA_SUP_BIN" ]; then
> > +     exit 0
> > +fi
> > +
> > +do_start () {
> > +     if [ -n "$IF_WPA_CONF" ] && [ "$IF_WPA_CONF" != "managed" ]; then
> > +             if [ ! -s "$IF_WPA_CONF" ]; then
> > +                     echo "cannot read contents of $IF_WPA_CONF"
> > +                     exit 1
> > +             fi
> > +             WPA_SUP_CONF_CTRL_DIR=$(sed -n -e 's/[[:space:]]*#.*//g'
> -e 's/[[:space:]]\+.*$//g' \
> > +                     -e 's/^ctrl_interface=\(DIR=\)\?\(.*\)/\2/p'
> "$IF_WPA_CONF")
> > +             if [ -n "$WPA_SUP_CONF_CTRL_DIR" ]; then
> > +                     WPA_SUP_CONF="-c $IF_WPA_CONF -C
> $WPA_SUP_CONF_CTRL_DI > +              else
> > +                     WPA_SUP_CONF="-c $IF_WPA_CONF"
> > +             fi
> > +     else
> > +             # specify the default ctrl_interface
> > +             WPA_SUP_CONF="-C $WPA_CTRL_DIR"
> > +     fi
> > +}
>
> Debian's ifupdown script need wpa_supplicant to have a ctrl_interface
> because it
> want to use wpa_cli for all the other wpa-* options, and wpa_cli works by
> connecting to a ctrl_interface.
>
> This script does not use wpa_cli so all the ctrl_interface handling can be
> removed, unless you want to implement the other options later.
>
> By the way, "wpa-conf "managed" is mostly used with the other wpa-*
> options to
> configure the ssid/passphrase within /etc/network/interfaces, which is
> something
> your script does not support, so you can remove the first 'if'.
>

Right.


-- 

Angelo Compagnucci

Software Engineer

angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL

Via le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 (0)42 243 5310
info@amarulasolutions.com

www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|

[-- Attachment #1.2: Type: text/html, Size: 10306 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-06-07 15:01 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27 10:33 [Buildroot] [PATCH 0/4] Better wifi handling Angelo Compagnucci
2022-05-27 10:33 ` [Buildroot] [PATCH 1/4] package/wpa_supplicant: fixing "Invalid configuration line" Angelo Compagnucci
2022-06-01 19:21   ` Thomas Petazzoni via buildroot
2022-06-01 21:55     ` Angelo Compagnucci
2022-06-06 12:42       ` Arnout Vandecappelle
2022-06-07 10:21         ` Angelo Compagnucci
2022-06-07 11:48           ` Thomas Petazzoni via buildroot
2022-06-07 12:16             ` Angelo Compagnucci
2022-06-07 13:12               ` Thomas Petazzoni via buildroot
2022-06-07 13:22                 ` Angelo Compagnucci
2022-05-27 10:33 ` [Buildroot] [PATCH 2/4] package/wpa_supplicant: adding ifupdown support Angelo Compagnucci
2022-06-01 19:25   ` Thomas Petazzoni via buildroot
2022-06-01 22:06     ` Angelo Compagnucci
2022-06-06 15:09   ` Nicolas Cavallari
2022-06-07 15:00     ` Angelo Compagnucci
2022-05-27 10:33 ` [Buildroot] [PATCH 3/4] package/busybox: make udhcp discover faster Angelo Compagnucci
2022-06-01 19:26   ` Thomas Petazzoni via buildroot
2022-05-27 10:33 ` [Buildroot] [PATCH 4/4] package/rtl8723ds: new package Angelo Compagnucci
2022-06-01 19:55   ` Thomas Petazzoni via buildroot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.