Re: [LEDE-DEV] [PATCH] mac80211: Fix race condition leading to wifi interfaces not coming up at boot sometimes.

[Bastian Bittorf]

* Vittorio Gambaletta (VittGam)  [14.03.2017 07:55]:
> +mac80211_iw_interface_add() {
> + local phy="$1"
> + local ifname="$2"
> + local type="$3"
> + local wdsflag="$4"

please add a 'local ret' here, or better use the common 'local rc=0'
and later always "return $rc"

> +
> + iw phy "$phy" interface add "$ifname" type "$type" $wdsflag
> + ret="$?"
> +
> + [ "$ret" = 233 ] && {
> + # Device might have just been deleted, give the kernel some 
> time to finish cleaning it up
> + sleep 1
> +
> + iw phy "$phy" interface add "$ifname" type "$type" $wdsflag
> + ret="$?"
> + }
> +
> + [ "$ret" != 0 ] && {
> + wireless_setup_failed INTERFACE_CREATION_FAILED
> + return 1
> + }
> +
> + return 0
> +}

thank you for debugging this! bye, bastian

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

Re: [LEDE-DEV] [PATCH] mac80211: Fix race condition leading to wifi interfaces not coming up at boot sometimes.

[Vittorio G (VittGam)]

If you're wondering where the -ENFILE error comes from...

At first I thought it was obviously related to fs.file-max that
defaults to "just" 8192.

But it was not. It took me a while to figure out that the error
came from here instead...

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/core/dev.c?id=13baa00ad01bb3a9f893e3a08cbc2d072fc0c15d#n1070

In fact, -ENFILE is returned every time one tries to add a wireless
virtual interface with the same name as another existing one.
(Maybe the error code should be changed to -EEXIST when an explicit
interface name is requested and it turns out to be already taken,
like in this case... Time for a kernel patch, maybe!)

This issue was also very time-sensitive; when I added some debug
prints to iw, it disappeared...

On 14/03/2017 06:19:44 CET, Vittorio Gambaletta (VittGam) wrote:
> In the drv_mac80211_setup function, mac80211_interface_cleanup
> is called to ask the kernel to delete all existing interfaces
> for the phy that is being configured via netlink.
> 
> Later in the first function, mac80211_prepare_vif is called to
> set up the new interfaces as required.
> 
> But sometimes, when mac80211_prepare_vif (and so the relevant
> `iw phy x interface add y` command) runs, the kernel might still
> be cleaning up the old interface with the same ifname. It usually
> takes very few time to do that; possibly a few milliseconds of
> sleep in the script after detecting this error condition could be
> enough, but the busybox sh does not support sub-second sleep
> intervals.
> 
> When this happens, iw obviously fails to create the new interface;
> and the following message is printed in the system log, followed by
> subsequent failure messages from hostapd in case this would have been
> an AP interface.
> 
> Tue Mar 14 04:21:57 2017 daemon.notice netifd: radio1 (2767): command failed: 
> Too many open files in system (-23)
> 
> This was a long-standing issue existing since at least OpenWrt Backfire,
> and today I finally managed to debug and (hopefully) solve it.
> It was happening very few times on most devices; but it was happening
> a lot more frequently on fast platforms with multiple radios, such as
> the powerpc-based dual-ath9k-radio tl-wdr4900-v1.
> 
> Signed-off-by: Vittorio Gambaletta 
> 
> ---
>  .../mac80211/files/lib/netifd/wireless/mac80211.sh |   35 
> +---
>  1 file changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh 
> b/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
> index baa023e..5058b01 100644
> --- a/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
> +++ b/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
> @@ -411,6 +411,31 @@ mac80211_check_ap() {
>   has_ap=1
>  }
>  
> +mac80211_iw_interface_add() {
> + local phy="$1"
> + local ifname="$2"
> + local type="$3"
> + local wdsflag="$4"
> +
> + iw phy "$phy" interface add "$ifname" type "$type" $wdsflag
> + ret="$?"
> +
> + [ "$ret" = 233 ] && {
> + # Device might have just been deleted, give the kernel some 
> time to finish cleaning it up
> + sleep 1
> +
> + iw phy "$phy" interface add "$ifname" type "$type" $wdsflag
> + ret="$?"
> + }
> +
> + [ "$ret" != 0 ] && {
> + wireless_setup_failed INTERFACE_CREATION_FAILED
> + return 1
> + }
> +
> + return 0
> +}
> +
>  mac80211_prepare_vif() {
>   json_select config
>  
> @@ -437,7 +462,7 @@ mac80211_prepare_vif() {
>   # It is far easier to delete and create the desired interface
>   case "$mode" in
>   adhoc)
> - iw phy "$phy" interface add "$ifname" type adhoc
> + mac80211_iw_interface_add "$phy" "$ifname" adhoc || 
> return
>   ;;
>   ap)
>   # Hostapd will handle recreating the interface and
> @@ -451,21 +476,21 @@ mac80211_prepare_vif() {
>   mac80211_hostapd_setup_bss "$phy" "$ifname" "$macaddr" 
> "$type" || return
>  
>   [ -n "$hostapd_ctrl" ] || {
> - iw phy "$phy" interface add "$ifname" type __ap
> + mac80211_iw_interface_add "$phy" "$ifname" __ap 
> || return
>   
> hostapd_ctrl="${hostapd_ctrl:-/var/run/hostapd/$ifname}"
>   }
>   ;;
>   mesh)
> - iw phy "$phy" interface add "$ifname" type mp
> + mac80211_iw_interface_add "$phy" "$ifname" mp || return
>   ;;
>   monitor)
> - iw phy "$phy" interface add "$ifname" type monitor
> + mac80211_iw_interface_add "$phy" "$ifname" monitor || 
> return
>   ;;
>   sta)
>   local wdsflag=
>  

[LEDE-DEV] [PATCH] mac80211: Fix race condition leading to wifi interfaces not coming up at boot sometimes.

[Vittorio Gambaletta (VittGam)]

In the drv_mac80211_setup function, mac80211_interface_cleanup
is called to ask the kernel to delete all existing interfaces
for the phy that is being configured via netlink.

Later in the first function, mac80211_prepare_vif is called to
set up the new interfaces as required.

But sometimes, when mac80211_prepare_vif (and so the relevant
`iw phy x interface add y` command) runs, the kernel might still
be cleaning up the old interface with the same ifname. It usually
takes very few time to do that; possibly a few milliseconds of
sleep in the script after detecting this error condition could be
enough, but the busybox sh does not support sub-second sleep
intervals.

When this happens, iw obviously fails to create the new interface;
and the following message is printed in the system log, followed by
subsequent failure messages from hostapd in case this would have been
an AP interface.

Tue Mar 14 04:21:57 2017 daemon.notice netifd: radio1 (2767): command failed: 
Too many open files in system (-23)

This was a long-standing issue existing since at least OpenWrt Backfire,
and today I finally managed to debug and (hopefully) solve it.
It was happening very few times on most devices; but it was happening
a lot more frequently on fast platforms with multiple radios, such as
the powerpc-based dual-ath9k-radio tl-wdr4900-v1.

Signed-off-by: Vittorio Gambaletta 

---
 .../mac80211/files/lib/netifd/wireless/mac80211.sh |   35 +---
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh 
b/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
index baa023e..5058b01 100644
--- a/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
+++ b/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
@@ -411,6 +411,31 @@ mac80211_check_ap() {
has_ap=1
 }
 
+mac80211_iw_interface_add() {
+   local phy="$1"
+   local ifname="$2"
+   local type="$3"
+   local wdsflag="$4"
+
+   iw phy "$phy" interface add "$ifname" type "$type" $wdsflag
+   ret="$?"
+
+   [ "$ret" = 233 ] && {
+   # Device might have just been deleted, give the kernel some 
time to finish cleaning it up
+   sleep 1
+
+   iw phy "$phy" interface add "$ifname" type "$type" $wdsflag
+   ret="$?"
+   }
+
+   [ "$ret" != 0 ] && {
+   wireless_setup_failed INTERFACE_CREATION_FAILED
+   return 1
+   }
+
+   return 0
+}
+
 mac80211_prepare_vif() {
json_select config
 
@@ -437,7 +462,7 @@ mac80211_prepare_vif() {
# It is far easier to delete and create the desired interface
case "$mode" in
adhoc)
-   iw phy "$phy" interface add "$ifname" type adhoc
+   mac80211_iw_interface_add "$phy" "$ifname" adhoc || 
return
;;
ap)
# Hostapd will handle recreating the interface and
@@ -451,21 +476,21 @@ mac80211_prepare_vif() {
mac80211_hostapd_setup_bss "$phy" "$ifname" "$macaddr" 
"$type" || return
 
[ -n "$hostapd_ctrl" ] || {
-   iw phy "$phy" interface add "$ifname" type __ap
+   mac80211_iw_interface_add "$phy" "$ifname" __ap 
|| return

hostapd_ctrl="${hostapd_ctrl:-/var/run/hostapd/$ifname}"
}
;;
mesh)
-   iw phy "$phy" interface add "$ifname" type mp
+   mac80211_iw_interface_add "$phy" "$ifname" mp || return
;;
monitor)
-   iw phy "$phy" interface add "$ifname" type monitor
+   mac80211_iw_interface_add "$phy" "$ifname" monitor || 
return
;;
sta)
local wdsflag=
staidx="$(($staidx + 1))"
[ "$wds" -gt 0 ] && wdsflag="4addr on"
-   iw phy "$phy" interface add "$ifname" type managed 
$wdsflag
+   mac80211_iw_interface_add "$phy" "$ifname" managed 
"$wdsflag" || return
[ "$powersave" -gt 0 ] && powersave="on" || 
powersave="off"
iw "$ifname" set power_save "$powersave"
;;


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