Message ID | 20161011113740.28392-4-chunkeey@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | Mathias Kresin |
Headers | show |
Hi, On 11 October 2016 at 13:37, Christian Lamparter <chunkeey@googlemail.com> wrote: > Currently, the wifi detection script is executed as part of > the (early) boot process. Pluggable wifi USB devices, which > are inserted at a later time are not automatically > detected and therefore they don't show up in LuCI. > > A user has to deal with wifi detection manually, or restart > the router. > > However, the current "sleep 1" window - which the boot > process waits for wifi devices to "settle down" - is too > short to detect wifi devices for some routers anyway. > > For example, this can happen with USB WLAN devices on the > WNDR4700. This is because the usb controller needs to load > its firmware from UBI and initialize, before it can operate. > > The issue can be seen on a BT HomeHub 5A as well as soon as > the caldata are on an ubi volume. This is because the ath9k > card has to be initialized by owl-loader first. Which has to > wait for the firmware extraction script to retrieve the pci > initialization values inside the caldata. > > This patch moves the wifi detection to hotplug scripts. > For mac80211, the wifi detection will now automatically > run any time a "ieee80211" device is added. Likewise > broadcom-wl's script checks for new "net" devices which > have the "wl$NUMBERS" moniker. > > File locking has been added to the detection scripts in > order to prevent races during discovery. The locking code > will protect against adding more open wireless network > configurations. In case "wifi detect" is run manually > by the user at a bad time. > > All changes to base-files, mac80211 and broadcom-wl packages > have been integrated into a single patch to play nice with > git bisect. > > Thanks to Martin Blumenstingl for helping with the implementation > and testing of the patch. Funny enough I did something similar recently, but didn't submit it here yet because there is some ugliness. A few comments ... > > Acked-by: Jo-Philipp Wich <jo@mein.io> > Signed-off-by: Mathias Kresin <dev@kresin.me> > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > > --- > v1 -> v2: > - filter for broadcom devices (Mathias) > - integrate uci changes (no more /tmp/wireless.tmp) > - move locking to detect_mac80211 and detect_broadcom > --- > --- > package/base-files/files/etc/init.d/boot | 9 --------- > .../broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect | 5 +++++ > package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh | 6 ++++++ > package/kernel/mac80211/Makefile | 2 ++ > package/kernel/mac80211/files/lib/wifi/mac80211.sh | 7 +++++++ > package/kernel/mac80211/files/mac80211.hotplug | 5 +++++ > 6 files changed, 25 insertions(+), 9 deletions(-) > create mode 100644 package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect > create mode 100644 package/kernel/mac80211/files/mac80211.hotplug > > diff --git a/package/base-files/files/etc/init.d/boot b/package/base-files/files/etc/init.d/boot > index 904f7db..1d61f2f 100755 > --- a/package/base-files/files/etc/init.d/boot > +++ b/package/base-files/files/etc/init.d/boot > @@ -38,15 +38,6 @@ boot() { > > /sbin/kmodloader > > - # allow wifi modules time to settle > - sleep 1 > - > - /sbin/wifi detect > /tmp/wireless.tmp > - [ -s /tmp/wireless.tmp ] && { > - cat /tmp/wireless.tmp >> /etc/config/wireless > - } > - rm -f /tmp/wireless.tmp > - > /bin/config_generate > uci_apply_defaults > > diff --git a/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect b/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect > new file mode 100644 > index 0000000..6ced270 > --- /dev/null > +++ b/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect > @@ -0,0 +1,5 @@ > +#!/bin/sh > + > +[ "${ACTION}" = "add" ] && [ "${INTERFACE%%[0-9]*}" = "wl" ] { > + /sbin/wifi detect > +} > diff --git a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh > index 1881b46..264e01b 100644 > --- a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh > +++ b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh > @@ -446,10 +446,16 @@ EOF > eval "$nas_cmd" > } > > +detect_unlock_broadcom_trap() { > + lock -u /var/lock/wifi-detect-broadcom.lock > +} > > detect_broadcom() { > local i=-1 > > + trap detect_unlock_broadcom_trap EXIT > + lock /var/lock/wifi-detect-broadcom.lock > + > while grep -qs "^ *wl$((++i)):" /proc/net/dev; do > local channel type > > diff --git a/package/kernel/mac80211/Makefile b/package/kernel/mac80211/Makefile > index 91c9362..f320326 100644 > --- a/package/kernel/mac80211/Makefile > +++ b/package/kernel/mac80211/Makefile > @@ -1732,6 +1732,8 @@ define KernelPackage/cfg80211/install > $(INSTALL_DIR) $(1)/lib/wifi $(1)/lib/netifd/wireless > $(INSTALL_DATA) ./files/lib/wifi/mac80211.sh $(1)/lib/wifi > $(INSTALL_BIN) ./files/lib/netifd/wireless/mac80211.sh $(1)/lib/netifd/wireless > + $(INSTALL_DIR) $(1)/etc/hotplug.d/ieee80211 Don't you also need to update /etc/hotplug.json to pass through ieee80211 events? > + $(INSTALL_DATA) ./files/mac80211.hotplug $(1)/etc/hotplug.d/ieee80211/00-wifi-detect > endef > > define KernelPackage/ipw2100/install > diff --git a/package/kernel/mac80211/files/lib/wifi/mac80211.sh b/package/kernel/mac80211/files/lib/wifi/mac80211.sh > index 253de40..44ba511 100644 > --- a/package/kernel/mac80211/files/lib/wifi/mac80211.sh > +++ b/package/kernel/mac80211/files/lib/wifi/mac80211.sh > @@ -58,7 +58,14 @@ check_mac80211_device() { > [ "$phy" = "$dev" ] && found=1 > } > > +detect_mac80211_unlock_trap() { > + lock -u /var/lock/wifi-detect-mac80211.lock > +} > + > detect_mac80211() { > + trap detect_mac80211_unlock_trap EXIT > + lock /var/lock/wifi-detect-mac80211.lock > + > devidx=0 > config_load wireless > while :; do > diff --git a/package/kernel/mac80211/files/mac80211.hotplug b/package/kernel/mac80211/files/mac80211.hotplug > new file mode 100644 > index 0000000..581be3d > --- /dev/null > +++ b/package/kernel/mac80211/files/mac80211.hotplug > @@ -0,0 +1,5 @@ > +#!/bin/sh > + > +[ "${ACTION}" = "add" ] && { > + /sbin/wifi detect You need to fork here, else you will get a deadlock with wifi drivers that request firmware on interface up instead of probe time, e.g. rt2x00-usb. > +} > -- > 2.9.3 Regards Jonas
2016-10-12 13:32 GMT+02:00 Jonas Gorski <jonas.gorski@gmail.com>: >> diff --git a/package/kernel/mac80211/Makefile b/package/kernel/mac80211/Makefile >> index 91c9362..f320326 100644 >> --- a/package/kernel/mac80211/Makefile >> +++ b/package/kernel/mac80211/Makefile >> @@ -1732,6 +1732,8 @@ define KernelPackage/cfg80211/install >> $(INSTALL_DIR) $(1)/lib/wifi $(1)/lib/netifd/wireless >> $(INSTALL_DATA) ./files/lib/wifi/mac80211.sh $(1)/lib/wifi >> $(INSTALL_BIN) ./files/lib/netifd/wireless/mac80211.sh $(1)/lib/netifd/wireless >> + $(INSTALL_DIR) $(1)/etc/hotplug.d/ieee80211 > > Don't you also need to update /etc/hotplug.json to pass through > ieee80211 events? Since https://git.lede-project.org/4f3c1e779364394a7e8f9f45ee824e0dff556cec events from all subsystems are passed through. >> --- /dev/null >> +++ b/package/kernel/mac80211/files/mac80211.hotplug >> @@ -0,0 +1,5 @@ >> +#!/bin/sh >> + >> +[ "${ACTION}" = "add" ] && { >> + /sbin/wifi detect > > You need to fork here, else you will get a deadlock with wifi drivers > that request firmware on interface up instead of probe time, e.g. > rt2x00-usb. I've to admit, I fail to see where or how a deadlock could happen here. Using a Lantiq board with a RT3062F (PCI, firmware request on up as well), I've moved the firmware file rt2860.bin out of /lib/firmware/ and added a hotplug script to copy it back. I wasn't able to trigger any deadlocks. Well, it might be possible that my test setup isn't close enough to the example with the rt2x00-usb to trigger a deadlock. Anyway, would you please give an example where/how exactly the deadlock could happen! Mathias
Hello, On Wednesday, October 12, 2016 1:32:38 PM CEST Jonas Gorski wrote: > On 11 October 2016 at 13:37, Christian Lamparter > <chunkeey@googlemail.com> wrote: > > Currently, the wifi detection script is executed as part of > > the (early) boot process. Pluggable wifi USB devices, which > > are inserted at a later time are not automatically > > detected and therefore they don't show up in LuCI. > > > > A user has to deal with wifi detection manually, or restart > > the router. > > > > However, the current "sleep 1" window - which the boot > > process waits for wifi devices to "settle down" - is too > > short to detect wifi devices for some routers anyway. > > > > For example, this can happen with USB WLAN devices on the > > WNDR4700. This is because the usb controller needs to load > > its firmware from UBI and initialize, before it can operate. > > > > The issue can be seen on a BT HomeHub 5A as well as soon as > > the caldata are on an ubi volume. This is because the ath9k > > card has to be initialized by owl-loader first. Which has to > > wait for the firmware extraction script to retrieve the pci > > initialization values inside the caldata. > > > > This patch moves the wifi detection to hotplug scripts. > > For mac80211, the wifi detection will now automatically > > run any time a "ieee80211" device is added. Likewise > > broadcom-wl's script checks for new "net" devices which > > have the "wl$NUMBERS" moniker. > > > > File locking has been added to the detection scripts in > > order to prevent races during discovery. The locking code > > will protect against adding more open wireless network > > configurations. In case "wifi detect" is run manually > > by the user at a bad time. > > > > All changes to base-files, mac80211 and broadcom-wl packages > > have been integrated into a single patch to play nice with > > git bisect. > > > > Thanks to Martin Blumenstingl for helping with the implementation > > and testing of the patch. > > Funny enough I did something similar recently, but didn't submit it > here yet because there is some ugliness. A few comments ... Actually, I know that more people have tried this before and posted different versions and patches to the MLs. So chances are there will be more attempts after this one. [...] > > diff --git a/package/kernel/mac80211/Makefile b/package/kernel/mac80211/Makefile > > index 91c9362..f320326 100644 > > --- a/package/kernel/mac80211/Makefile > > +++ b/package/kernel/mac80211/Makefile > > @@ -1732,6 +1732,8 @@ define KernelPackage/cfg80211/install > > $(INSTALL_DIR) $(1)/lib/wifi $(1)/lib/netifd/wireless > > $(INSTALL_DATA) ./files/lib/wifi/mac80211.sh $(1)/lib/wifi > > $(INSTALL_BIN) ./files/lib/netifd/wireless/mac80211.sh $(1)/lib/netifd/wireless > > + $(INSTALL_DIR) $(1)/etc/hotplug.d/ieee80211 > > Don't you also need to update /etc/hotplug.json to pass through > ieee80211 events? No need for that. It's already handled there by [0]. 64 [ "if", 65 [ "isdir", "/etc/hotplug.d/%SUBSYSTEM%" ], 66 [ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ] 67 ] If the /etc/hotplug.d/ieee80211 directory exits, hotplug-call will look for scripts in the directory and execute them. > > + $(INSTALL_DATA) ./files/mac80211.hotplug $(1)/etc/hotplug.d/ieee80211/00-wifi-detect > > endef > > > > define KernelPackage/ipw2100/install > > diff --git a/package/kernel/mac80211/files/lib/wifi/mac80211.sh b/package/kernel/mac80211/files/lib/wifi/mac80211.sh > > index 253de40..44ba511 100644 > > --- a/package/kernel/mac80211/files/lib/wifi/mac80211.sh > > +++ b/package/kernel/mac80211/files/lib/wifi/mac80211.sh > > @@ -58,7 +58,14 @@ check_mac80211_device() { > > [ "$phy" = "$dev" ] && found=1 > > } > > > > +detect_mac80211_unlock_trap() { > > + lock -u /var/lock/wifi-detect-mac80211.lock > > +} > > + > > detect_mac80211() { > > + trap detect_mac80211_unlock_trap EXIT > > + lock /var/lock/wifi-detect-mac80211.lock > > + > > devidx=0 > > config_load wireless > > while :; do > > diff --git a/package/kernel/mac80211/files/mac80211.hotplug b/package/kernel/mac80211/files/mac80211.hotplug > > new file mode 100644 > > index 0000000..581be3d > > --- /dev/null > > +++ b/package/kernel/mac80211/files/mac80211.hotplug > > @@ -0,0 +1,5 @@ > > +#!/bin/sh > > + > > +[ "${ACTION}" = "add" ] && { > > + /sbin/wifi detect > > You need to fork here, else you will get a deadlock with wifi drivers > that request firmware on interface up instead of probe time, e.g. > rt2x00-usb. Are you sure that the detect_mac80211 and friends actually do bring up the interfaces? From what I can tell, it's just iw phy info and a bit of readlink [1]. Still, I can add a "/sbin/wifi config &" there. Regards, Christian [0] <https://git.lede-project.org/?p=source.git;a=blob;f=package/system/procd/files/hotplug.json;h=e5f8d967e8c16c8dc85a437a5259c0ba840d766c;hb=HEAD#l64> [1] <https://git.lede-project.org/?p=source.git;a=blob;f=package/kernel/mac80211/files/lib/wifi/mac80211.sh;h=9b15de55a8986fa9d354ccb5c92d65edf2b4f514;hb=8869dd47ca8ca70213acf349abc263bd6ec4eed0#l61>
On 12 October 2016 at 20:13, Christian Lamparter <chunkeey@googlemail.com> wrote: > Hello, > > On Wednesday, October 12, 2016 1:32:38 PM CEST Jonas Gorski wrote: >> On 11 October 2016 at 13:37, Christian Lamparter >> <chunkeey@googlemail.com> wrote: (snip) >> > diff --git a/package/kernel/mac80211/Makefile b/package/kernel/mac80211/Makefile >> > index 91c9362..f320326 100644 >> > --- a/package/kernel/mac80211/Makefile >> > +++ b/package/kernel/mac80211/Makefile >> > @@ -1732,6 +1732,8 @@ define KernelPackage/cfg80211/install >> > $(INSTALL_DIR) $(1)/lib/wifi $(1)/lib/netifd/wireless >> > $(INSTALL_DATA) ./files/lib/wifi/mac80211.sh $(1)/lib/wifi >> > $(INSTALL_BIN) ./files/lib/netifd/wireless/mac80211.sh $(1)/lib/netifd/wireless >> > + $(INSTALL_DIR) $(1)/etc/hotplug.d/ieee80211 >> >> Don't you also need to update /etc/hotplug.json to pass through >> ieee80211 events? > No need for that. It's already handled there by [0]. > > 64 [ "if", > 65 [ "isdir", "/etc/hotplug.d/%SUBSYSTEM%" ], > 66 [ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ] > 67 ] > > If the /etc/hotplug.d/ieee80211 directory exits, hotplug-call will > look for scripts in the directory and execute them. Ah indeed, that commit must have been flown past me. Thanks for pointing it out. >> > + $(INSTALL_DATA) ./files/mac80211.hotplug $(1)/etc/hotplug.d/ieee80211/00-wifi-detect >> > endef >> > >> > define KernelPackage/ipw2100/install >> > diff --git a/package/kernel/mac80211/files/lib/wifi/mac80211.sh b/package/kernel/mac80211/files/lib/wifi/mac80211.sh >> > index 253de40..44ba511 100644 >> > --- a/package/kernel/mac80211/files/lib/wifi/mac80211.sh >> > +++ b/package/kernel/mac80211/files/lib/wifi/mac80211.sh >> > @@ -58,7 +58,14 @@ check_mac80211_device() { >> > [ "$phy" = "$dev" ] && found=1 >> > } >> > >> > +detect_mac80211_unlock_trap() { >> > + lock -u /var/lock/wifi-detect-mac80211.lock >> > +} >> > + >> > detect_mac80211() { >> > + trap detect_mac80211_unlock_trap EXIT >> > + lock /var/lock/wifi-detect-mac80211.lock >> > + Just noticed, would broadcom and mac80211 detection running at the same time ok? If not, the lock should probably the same for both. >> > devidx=0 >> > config_load wireless >> > while :; do >> > diff --git a/package/kernel/mac80211/files/mac80211.hotplug b/package/kernel/mac80211/files/mac80211.hotplug >> > new file mode 100644 >> > index 0000000..581be3d >> > --- /dev/null >> > +++ b/package/kernel/mac80211/files/mac80211.hotplug >> > @@ -0,0 +1,5 @@ >> > +#!/bin/sh >> > + >> > +[ "${ACTION}" = "add" ] && { >> > + /sbin/wifi detect >> >> You need to fork here, else you will get a deadlock with wifi drivers >> that request firmware on interface up instead of probe time, e.g. >> rt2x00-usb. > Are you sure that the detect_mac80211 and friends actually do bring up > the interfaces? From what I can tell, it's just iw phy info and a bit > of readlink [1]. > > Still, I can add a "/sbin/wifi config &" there. Indeed, I tried to reproduce what I saw then and couldn't, so disregard my comments here. Regards Jonas
diff --git a/package/base-files/files/etc/init.d/boot b/package/base-files/files/etc/init.d/boot index 904f7db..1d61f2f 100755 --- a/package/base-files/files/etc/init.d/boot +++ b/package/base-files/files/etc/init.d/boot @@ -38,15 +38,6 @@ boot() { /sbin/kmodloader - # allow wifi modules time to settle - sleep 1 - - /sbin/wifi detect > /tmp/wireless.tmp - [ -s /tmp/wireless.tmp ] && { - cat /tmp/wireless.tmp >> /etc/config/wireless - } - rm -f /tmp/wireless.tmp - /bin/config_generate uci_apply_defaults diff --git a/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect b/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect new file mode 100644 index 0000000..6ced270 --- /dev/null +++ b/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect @@ -0,0 +1,5 @@ +#!/bin/sh + +[ "${ACTION}" = "add" ] && [ "${INTERFACE%%[0-9]*}" = "wl" ] { + /sbin/wifi detect +} diff --git a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh index 1881b46..264e01b 100644 --- a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh +++ b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh @@ -446,10 +446,16 @@ EOF eval "$nas_cmd" } +detect_unlock_broadcom_trap() { + lock -u /var/lock/wifi-detect-broadcom.lock +} detect_broadcom() { local i=-1 + trap detect_unlock_broadcom_trap EXIT + lock /var/lock/wifi-detect-broadcom.lock + while grep -qs "^ *wl$((++i)):" /proc/net/dev; do local channel type diff --git a/package/kernel/mac80211/Makefile b/package/kernel/mac80211/Makefile index 91c9362..f320326 100644 --- a/package/kernel/mac80211/Makefile +++ b/package/kernel/mac80211/Makefile @@ -1732,6 +1732,8 @@ define KernelPackage/cfg80211/install $(INSTALL_DIR) $(1)/lib/wifi $(1)/lib/netifd/wireless $(INSTALL_DATA) ./files/lib/wifi/mac80211.sh $(1)/lib/wifi $(INSTALL_BIN) ./files/lib/netifd/wireless/mac80211.sh $(1)/lib/netifd/wireless + $(INSTALL_DIR) $(1)/etc/hotplug.d/ieee80211 + $(INSTALL_DATA) ./files/mac80211.hotplug $(1)/etc/hotplug.d/ieee80211/00-wifi-detect endef define KernelPackage/ipw2100/install diff --git a/package/kernel/mac80211/files/lib/wifi/mac80211.sh b/package/kernel/mac80211/files/lib/wifi/mac80211.sh index 253de40..44ba511 100644 --- a/package/kernel/mac80211/files/lib/wifi/mac80211.sh +++ b/package/kernel/mac80211/files/lib/wifi/mac80211.sh @@ -58,7 +58,14 @@ check_mac80211_device() { [ "$phy" = "$dev" ] && found=1 } +detect_mac80211_unlock_trap() { + lock -u /var/lock/wifi-detect-mac80211.lock +} + detect_mac80211() { + trap detect_mac80211_unlock_trap EXIT + lock /var/lock/wifi-detect-mac80211.lock + devidx=0 config_load wireless while :; do diff --git a/package/kernel/mac80211/files/mac80211.hotplug b/package/kernel/mac80211/files/mac80211.hotplug new file mode 100644 index 0000000..581be3d --- /dev/null +++ b/package/kernel/mac80211/files/mac80211.hotplug @@ -0,0 +1,5 @@ +#!/bin/sh + +[ "${ACTION}" = "add" ] && { + /sbin/wifi detect +}