| Message ID | 20221004110421.137795-4-angelo@amarulasolutions.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Configure default wifi through kconfig | expand |
On 04/10/2022 13:04, Angelo Compagnucci wrote: > Configure a default basic wifi setup able to automatically connect > to the selected access point. I think this feature is indeed useful. IIUC, currently wpa_supplicant actually isn't started, even though we do install a service file. > > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> > --- > package/wpa_supplicant/wpa_supplicant.mk | 10 ++++++++++ > system/Config.in | 1 + > 2 files changed, 11 insertions(+) > > diff --git a/package/wpa_supplicant/wpa_supplicant.mk b/package/wpa_supplicant/wpa_supplicant.mk > index 60ae290e26..8611fc6577 100644 > --- a/package/wpa_supplicant/wpa_supplicant.mk > +++ b/package/wpa_supplicant/wpa_supplicant.mk > @@ -277,6 +277,15 @@ define WPA_SUPPLICANT_ENABLE_WIFI > echo "}"; \ > ) >> $(TARGET_DIR)/etc/wpa_supplicant.conf > endef > +define WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD > + $(INSTALL) -m 0755 -d $(TARGET_DIR)/etc/wpa_supplicant/ > + ln -sf ../wpa_supplicant.conf \ > + $(TARGET_DIR)/etc/wpa_supplicant/wpa_supplicant-$(BR2_SYSTEM_DHCP).conf If the interface-specific service is used rather than the global one, I think the idea is to be able to use separate conf files for them. So I think it makes more sense to copy than symlink. That said, I'm not sure if it's really useful to use the per-interface service. The global one should work as well, no? I think the only thing we're missing is to enable it for the wifi interface. Also, I don't think it's a good idea to reuse BR2_SYSTEM_DHCP for this. There are quite a few boards that have both ethernet and wifi and ideally we want both to come up. So I think it's worth adding a BR2_SYSTEM_WLAN to specify the wifi interface - and perhaps automatically select wpa_supplicant if that option is set (although we could just as well use iwd instead of supplicant, of course). > +endef > +define WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD_PRESET > + $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset wpa_supplicant\@$(BR2_SYSTEM_DHCP).service I'm not sure what this is supposed to do. Doesn't it just undo the disable we do in 50-wpa_supplicant.preset? Wouldn't it be better to just remove that preset somehow? > +endef > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD_PRESET > endif > > define WPA_SUPPLICANT_INSTALL_TARGET_CMDS > @@ -304,6 +313,7 @@ define WPA_SUPPLICANT_INSTALL_INIT_SYSTEMD > $(TARGET_DIR)/usr/lib/systemd/system/wpa_supplicant-wired@.service > $(INSTALL) -D -m 644 $(WPA_SUPPLICANT_PKGDIR)/50-wpa_supplicant.preset \ > $(TARGET_DIR)/usr/lib/systemd/system-preset/50-wpa_supplicant.preset > + $(WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD) > endef > > $(eval $(generic-package)) > diff --git a/system/Config.in b/system/Config.in > index 647072b965..761a5a95c2 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -421,6 +421,7 @@ comment "automatic network configuration via DHCP needs ifupdown or busybox or n > config BR2_SYSTEM_CONNECT_WIFI > bool "Connect to a default wifi access point" > depends on BR2_SYSTEM_DHCP != "" && BR2_PACKAGE_WPA_SUPPLICANT > + select BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE if BR2_PACKAGE_SYSTEMD This needs some explanation. I don't see the service files requiring this anywhere. On the other hand, the global service file does start wpa_supplicant with the -u option so it would need dbus. Regards, Arnout > > config BR2_SYSTEM_CONNECT_WIFI_SSID > string "Access point SSID"
Hi Arnout, On Sat, Sep 30, 2023 at 6:51 PM Arnout Vandecappelle <arnout@mind.be> wrote: > > > > On 04/10/2022 13:04, Angelo Compagnucci wrote: > > Configure a default basic wifi setup able to automatically connect > > to the selected access point. > > I think this feature is indeed useful. IIUC, currently wpa_supplicant actually > isn't started, even though we do install a service file. I do not think I'm going to respin another version of this patch due to the fact the other patches in the series were rejected. This patch is indeed not useful without the others: what's the point of having systemd configured for wpa_supplicant if in any case the user must configure an overlay to add a working wpa_supplicant.conf file? The series was meant to offer an easy way to configure the wifi in the same way we do for the ethernet. Moreover, if we configure wpa_supplicant here, it's not going to work without clearly explain somewhere that the user must implement its own wpa_supplicant.conf file. Imho it's way more confusing than offering a completely DIY solution as we do now. In the first case we need to explain somewhere what to do, in the latter the user will figure it out. > > > > > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> > > --- > > package/wpa_supplicant/wpa_supplicant.mk | 10 ++++++++++ > > system/Config.in | 1 + > > 2 files changed, 11 insertions(+) > > > > diff --git a/package/wpa_supplicant/wpa_supplicant.mk b/package/wpa_supplicant/wpa_supplicant.mk > > index 60ae290e26..8611fc6577 100644 > > --- a/package/wpa_supplicant/wpa_supplicant.mk > > +++ b/package/wpa_supplicant/wpa_supplicant.mk > > @@ -277,6 +277,15 @@ define WPA_SUPPLICANT_ENABLE_WIFI > > echo "}"; \ > > ) >> $(TARGET_DIR)/etc/wpa_supplicant.conf > > endef > > +define WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD > > + $(INSTALL) -m 0755 -d $(TARGET_DIR)/etc/wpa_supplicant/ > > + ln -sf ../wpa_supplicant.conf \ > > + $(TARGET_DIR)/etc/wpa_supplicant/wpa_supplicant-$(BR2_SYSTEM_DHCP).conf > > If the interface-specific service is used rather than the global one, I think > the idea is to be able to use separate conf files for them. So I think it makes > more sense to copy than symlink. > > That said, I'm not sure if it's really useful to use the per-interface > service. The global one should work as well, no? I think the only thing we're > missing is to enable it for the wifi interface. > > Also, I don't think it's a good idea to reuse BR2_SYSTEM_DHCP for this. There > are quite a few boards that have both ethernet and wifi and ideally we want both > to come up. So I think it's worth adding a BR2_SYSTEM_WLAN to specify the wifi > interface - and perhaps automatically select wpa_supplicant if that option is > set (although we could just as well use iwd instead of supplicant, of course). > > > +endef > > +define WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD_PRESET > > + $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset wpa_supplicant\@$(BR2_SYSTEM_DHCP).service > > I'm not sure what this is supposed to do. Doesn't it just undo the disable we > do in 50-wpa_supplicant.preset? Wouldn't it be better to just remove that preset > somehow? > > > +endef > > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD_PRESET > > endif > > > > define WPA_SUPPLICANT_INSTALL_TARGET_CMDS > > @@ -304,6 +313,7 @@ define WPA_SUPPLICANT_INSTALL_INIT_SYSTEMD > > $(TARGET_DIR)/usr/lib/systemd/system/wpa_supplicant-wired@.service > > $(INSTALL) -D -m 644 $(WPA_SUPPLICANT_PKGDIR)/50-wpa_supplicant.preset \ > > $(TARGET_DIR)/usr/lib/systemd/system-preset/50-wpa_supplicant.preset > > + $(WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD) > > endef > > > > $(eval $(generic-package)) > > diff --git a/system/Config.in b/system/Config.in > > index 647072b965..761a5a95c2 100644 > > --- a/system/Config.in > > +++ b/system/Config.in > > @@ -421,6 +421,7 @@ comment "automatic network configuration via DHCP needs ifupdown or busybox or n > > config BR2_SYSTEM_CONNECT_WIFI > > bool "Connect to a default wifi access point" > > depends on BR2_SYSTEM_DHCP != "" && BR2_PACKAGE_WPA_SUPPLICANT > > + select BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE if BR2_PACKAGE_SYSTEMD > > This needs some explanation. I don't see the service files requiring this > anywhere. > > On the other hand, the global service file does start wpa_supplicant with the > -u option so it would need dbus. > > > Regards, > Arnout > > > > > config BR2_SYSTEM_CONNECT_WIFI_SSID > > string "Access point SSID"
diff --git a/package/wpa_supplicant/wpa_supplicant.mk b/package/wpa_supplicant/wpa_supplicant.mk index 60ae290e26..8611fc6577 100644 --- a/package/wpa_supplicant/wpa_supplicant.mk +++ b/package/wpa_supplicant/wpa_supplicant.mk @@ -277,6 +277,15 @@ define WPA_SUPPLICANT_ENABLE_WIFI echo "}"; \ ) >> $(TARGET_DIR)/etc/wpa_supplicant.conf endef +define WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD + $(INSTALL) -m 0755 -d $(TARGET_DIR)/etc/wpa_supplicant/ + ln -sf ../wpa_supplicant.conf \ + $(TARGET_DIR)/etc/wpa_supplicant/wpa_supplicant-$(BR2_SYSTEM_DHCP).conf +endef +define WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD_PRESET + $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset wpa_supplicant\@$(BR2_SYSTEM_DHCP).service +endef +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD_PRESET endif define WPA_SUPPLICANT_INSTALL_TARGET_CMDS @@ -304,6 +313,7 @@ define WPA_SUPPLICANT_INSTALL_INIT_SYSTEMD $(TARGET_DIR)/usr/lib/systemd/system/wpa_supplicant-wired@.service $(INSTALL) -D -m 644 $(WPA_SUPPLICANT_PKGDIR)/50-wpa_supplicant.preset \ $(TARGET_DIR)/usr/lib/systemd/system-preset/50-wpa_supplicant.preset + $(WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD) endef $(eval $(generic-package)) diff --git a/system/Config.in b/system/Config.in index 647072b965..761a5a95c2 100644 --- a/system/Config.in +++ b/system/Config.in @@ -421,6 +421,7 @@ comment "automatic network configuration via DHCP needs ifupdown or busybox or n config BR2_SYSTEM_CONNECT_WIFI bool "Connect to a default wifi access point" depends on BR2_SYSTEM_DHCP != "" && BR2_PACKAGE_WPA_SUPPLICANT + select BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE if BR2_PACKAGE_SYSTEMD config BR2_SYSTEM_CONNECT_WIFI_SSID string "Access point SSID"
Configure a default basic wifi setup able to automatically connect to the selected access point. Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> --- package/wpa_supplicant/wpa_supplicant.mk | 10 ++++++++++ system/Config.in | 1 + 2 files changed, 11 insertions(+)