Re: [LEDE-DEV] [PATCH v2 4/5] base-files, mac80211, broadcom-wl: plug-and-play wifi detection

2016-10-13 Thread Jonas Gorski
On 12 October 2016 at 20:13, Christian Lamparter
 wrote:
> Hello,
>
> On Wednesday, October 12, 2016 1:32:38 PM CEST Jonas Gorski wrote:
>> On 11 October 2016 at 13:37, Christian Lamparter
>>  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 000..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

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH v2 4/5] base-files, mac80211, broadcom-wl: plug-and-play wifi detection

2016-10-12 Thread Christian Lamparter
Hello,

On Wednesday, October 12, 2016 1:32:38 PM CEST Jonas Gorski wrote:
> On 11 October 2016 at 13:37, Christian Lamparter
>  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 000..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] 


Re: [LEDE-DEV] [PATCH v2 4/5] base-files, mac80211, broadcom-wl: plug-and-play wifi detection

2016-10-12 Thread Mathias Kresin
2016-10-12 13:32 GMT+02:00 Jonas Gorski :
>> 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

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH v2 4/5] base-files, mac80211, broadcom-wl: plug-and-play wifi detection

2016-10-12 Thread Jonas Gorski
Hi,

On 11 October 2016 at 13:37, Christian Lamparter
 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 
> Signed-off-by: Mathias Kresin 
> Signed-off-by: Christian Lamparter 
>
> ---
> 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 000..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

[LEDE-DEV] [PATCH v2 4/5] base-files, mac80211, broadcom-wl: plug-and-play wifi detection

2016-10-11 Thread Christian Lamparter
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.

Acked-by: Jo-Philipp Wich 
Signed-off-by: Mathias Kresin 
Signed-off-by: Christian Lamparter 

---
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 000..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/l