Re: [LEDE-DEV] [PATCH] mac80211: Fix race condition leading to wifi interfaces not coming up at boot sometimes.
* 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.
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.
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