Hello Marek and folks, chasing a race condition, I might have delved a little too deep into openwrt boot sequence :(
AFAICU, currently there's a /etc/hotplug.d/net/99-batman-adv script which tries to configure both bat0 , and any interfaces coming up that might have to be included there. Problem is, hotplug.d/net scripts are called when interface is "added" but not necessarilly brought up yet. So there's a "sleep 3s" that works in most cases as a window for slow hardware bring up the interface. ...in most cases :( on an old nanoloco2, creating 2 wifi-ifaces in ap mode and 1 in ibss mode, managed with batman-adv, the sleep 3s is not enough. A 6s timeout works around it, but maybe there's a better solution?
I've divided the script, putting the "add candidate interfaces to bat0" into /etc/hotplug.d/*iface*/99-batman-adv , which gets called precisely after an interface is actually brought up.
This works as expected, and i could even eliminate the original 3s sleep.
hotplug.d/iface script adds candidate interfaces to batX after they are brought up hotplug.d/net script configures batX as they appear,
Would that work in general? Or was the single hotplug.d/net implemented that way, for a reason i'm missing?
Thanks!
Gui
Boring bootlog at http://pastebin.com/59FEPttY
Proposed patch follows (against current batman-adv in openwrt 'packages' feed)
Signed-off-by: Gui Iribarren gui@altermundi.net
--- diff --git a/Makefile b/Makefile index d12665e..e37467f 100644 --- a/Makefile +++ b/Makefile @@ -113,9 +113,10 @@ define KernelPackage/batman-adv/conffiles endef
define KernelPackage/batman-adv/install - $(INSTALL_DIR) $(1)/etc/config $(1)/etc/hotplug.d/net $(1)/lib/batman-adv $(1)/usr/sbin + $(INSTALL_DIR) $(1)/etc/config $(1)/etc/hotplug.d/iface $(1)/etc/hotplug.d/net $(1)/lib/batman-adv $(1)/usr/sbin $(INSTALL_DATA) ./files/etc/config/batman-adv $(1)/etc/config $(INSTALL_DATA) ./files/lib/batman-adv/config.sh $(1)/lib/batman-adv + $(INSTALL_BIN) ./files/etc/hotplug.d/iface/99-batman-adv $(1)/etc/hotplug.d/iface $(INSTALL_BIN) ./files/etc/hotplug.d/net/99-batman-adv $(1)/etc/hotplug.d/net $(INSTALL_BIN) ./files/usr/sbin/batman-adv $(1)/usr/sbin $(BATCTL_INSTALL) diff --git a/files/etc/hotplug.d/iface/99-batman-adv b/files/etc/hotplug.d/iface/99-batman-adv new file mode 100644 index 0000000..a4f4ed0 --- /dev/null +++ b/files/etc/hotplug.d/iface/99-batman-adv @@ -0,0 +1,15 @@ +#!/bin/sh + +. /lib/batman-adv/config.sh + +bat_load_module +config_load batman-adv + +case "$ACTION" in + ifup) + [ -d /sys/class/net/$DEVICE/batman_adv/ ] && config_foreach bat_add_interface mesh "$DEVICE" + ;; + ifdown) + [ -d /sys/class/net/$DEVICE/batman_adv/ ] && config_foreach bat_del_interface mesh "$DEVICE" + ;; +esac diff --git a/files/etc/hotplug.d/net/99-batman-adv b/files/etc/hotplug.d/net/99-batman-adv index 42d4c29..f0c391f 100644 --- a/files/etc/hotplug.d/net/99-batman-adv +++ b/files/etc/hotplug.d/net/99-batman-adv @@ -8,9 +8,5 @@ config_load batman-adv case "$ACTION" in add) [ -d /sys/class/net/$INTERFACE/mesh/ ] && bat_config "$INTERFACE" - [ -d /sys/class/net/$INTERFACE/batman_adv/ ] && config_foreach bat_add_interface mesh "$INTERFACE" - ;; - remove) - [ -d /sys/class/net/$INTERFACE/batman_adv/ ] && config_foreach bat_del_interface mesh "$INTERFACE" ;; esac diff --git a/files/lib/batman-adv/config.sh b/files/lib/batman-adv/config.sh index cb78867..646e953 100644 --- a/files/lib/batman-adv/config.sh +++ b/files/lib/batman-adv/config.sh @@ -50,7 +50,6 @@ bat_add_interface() local interface="$2" local interfaces
- sleep 3s # some device (ath) is very lazy to start config_get interfaces $mesh interfaces for iface in $interfaces; do [ -f "/sys/class/net/$iface/batman_adv/mesh_iface" ] || {
On Friday, November 23, 2012 22:00:47 Gui Iribarren wrote:
Boring bootlog at http://pastebin.com/59FEPttY
Proposed patch follows (against current batman-adv in openwrt 'packages' feed)
Signed-off-by: Gui Iribarren gui@altermundi.net
I made the necessary changes myself after I successfully de-scrambled your patch. You will find the patch in OpenWrt trunk. Give it try! Next time, please configure your mail client to not wrap lines.
Thanks, Marek
On Monday 26 November 2012 18:40:20 Marek Lindner wrote:
On Friday, November 23, 2012 22:00:47 Gui Iribarren wrote:
Boring bootlog at http://pastebin.com/59FEPttY
Proposed patch follows (against current batman-adv in openwrt 'packages' feed)
Signed-off-by: Gui Iribarren gui@altermundi.net
I made the necessary changes myself after I successfully de-scrambled your patch. You will find the patch in OpenWrt trunk. Give it try! Next time, please configure your mail client to not wrap lines.
http://lxr.linux.no/linux+v2.6.30/Documentation/email-clients.txt
Kind regards, Sven
Really Sorry for the whitespace headaches :( hope to get it straight this time.
As we discussed on irc with Marek, here's a first stab at netifd integration. I'm surprised at how simple it actually is to add a custom "proto" with netifd
No race condition with this approach, at least on the hardware i have at hand (ubnt2 , and tplinks)
an example uci config scheme changes from
batman-adv.bat0=mesh batman-adv.bat0.interfaces='mesh0 mesh1 mesh2' batman-adv.bat0.bridge_loop_avoidance=1 network.mesh{0,1,2}=interface network.mesh{0,1,2}.proto=none network.mesh{0,1,2}.mtu=1528 wireless.@wifi-iface[]=wifi-iface wireless.@wifi-iface[].network=mesh{0,1,2} wireless.@wifi-iface[].mode=adhoc
to
batman-adv.bat0=mesh batman-adv.bat0.bridge_loop_avoidance=1 network.mesh{0,1,2}=interface network.mesh{0,1,2}.proto=batadv network.mesh{0,1,2}.mesh=bat0 network.mesh{0,1,2}.mtu=1528 wireless.@wifi-iface[1]=wifi-iface wireless.@wifi-iface[1].network=mesh{0,1,2} wireless.@wifi-iface[1].mode=adhoc
So /etc/config/batman-adv now deals only with bat0 tweakable parameters, /etc/config/network defines interfaces to be added to bat0 and /etc/config/wireless is unchanged
and netifd is responsible of avoiding race conditions of course, this makes batman-adv package (on openwrt) depend on netifd, but openwrt devs are pushing it that way (and it's finally looking like an elegant move after all ;) )
Cheers!
On Mon, Nov 26, 2012 at 7:45 AM, Sven Eckelmann sven@narfation.org wrote:
On Monday 26 November 2012 18:40:20 Marek Lindner wrote:
On Friday, November 23, 2012 22:00:47 Gui Iribarren wrote:
Boring bootlog at http://pastebin.com/59FEPttY
Proposed patch follows (against current batman-adv in openwrt 'packages' feed)
Signed-off-by: Gui Iribarren gui@altermundi.net
I made the necessary changes myself after I successfully de-scrambled your patch. You will find the patch in OpenWrt trunk. Give it try! Next time, please configure your mail client to not wrap lines.
http://lxr.linux.no/linux+v2.6.30/Documentation/email-clients.txt
Kind regards, Sven
I have tried batman-adv-netifd-proto-batadv.patch along with some Makefile adjustments to get it compile. Afaict it works.
On 12/02/2012 02:19 PM, Gui Iribarren wrote:
Really Sorry for the whitespace headaches :( hope to get it straight this time.
As we discussed on irc with Marek, here's a first stab at netifd integration. I'm surprised at how simple it actually is to add a custom "proto" with netifd
No race condition with this approach, at least on the hardware i have at hand (ubnt2 , and tplinks)
an example uci config scheme changes from
batman-adv.bat0=mesh batman-adv.bat0.interfaces='mesh0 mesh1 mesh2' batman-adv.bat0.bridge_loop_avoidance=1 network.mesh{0,1,2}=interface network.mesh{0,1,2}.proto=none network.mesh{0,1,2}.mtu=1528 wireless.@wifi-iface[]=wifi-iface wireless.@wifi-iface[].network=mesh{0,1,2} wireless.@wifi-iface[].mode=adhoc
to
batman-adv.bat0=mesh batman-adv.bat0.bridge_loop_avoidance=1 network.mesh{0,1,2}=interface network.mesh{0,1,2}.proto=batadv network.mesh{0,1,2}.mesh=bat0 network.mesh{0,1,2}.mtu=1528 wireless.@wifi-iface[1]=wifi-iface wireless.@wifi-iface[1].network=mesh{0,1,2} wireless.@wifi-iface[1].mode=adhoc
So /etc/config/batman-adv now deals only with bat0 tweakable parameters, /etc/config/network defines interfaces to be added to bat0 and /etc/config/wireless is unchanged
and netifd is responsible of avoiding race conditions of course, this makes batman-adv package (on openwrt) depend on netifd, but openwrt devs are pushing it that way (and it's finally looking like an elegant move after all ;) )
Cheers!
On Mon, Nov 26, 2012 at 7:45 AM, Sven Eckelmann sven@narfation.org wrote:
On Monday 26 November 2012 18:40:20 Marek Lindner wrote:
On Friday, November 23, 2012 22:00:47 Gui Iribarren wrote:
Boring bootlog at http://pastebin.com/59FEPttY
Proposed patch follows (against current batman-adv in openwrt 'packages' feed)
Signed-off-by: Gui Iribarren gui@altermundi.net
I made the necessary changes myself after I successfully de-scrambled your patch. You will find the patch in OpenWrt trunk. Give it try! Next time, please configure your mail client to not wrap lines.
http://lxr.linux.no/linux+v2.6.30/Documentation/email-clients.txt
Kind regards, Sven
On Sunday, December 09, 2012 19:10:56 Moritz Warning wrote:
I have tried batman-adv-netifd-proto-batadv.patch along with some Makefile adjustments to get it compile. Afaict it works.
Thanks for testing! Do you mind sharing & explaining these changes ? Would be a good idea to push the working variant, don't you think ?
Cheers, Marek
Here you go. :) It would be good idea if Gui had a look at it before it is applied.
On 12/09/2012 03:31 PM, Marek Lindner wrote:
On Sunday, December 09, 2012 19:10:56 Moritz Warning wrote:
I have tried batman-adv-netifd-proto-batadv.patch along with some Makefile adjustments to get it compile. Afaict it works.
Thanks for testing! Do you mind sharing & explaining these changes ? Would be a good idea to push the working variant, don't you think ?
Cheers, Marek
On Sun, Dec 09, 2012 at 05:03:11PM +0100, Moritz Warning wrote:
Here you go. :) It would be good idea if Gui had a look at it before it is applied.
Hey Moritz,
remember that this should all be GPLed stuff, therefore you sign is mandatory on this patch :) And since you are following up a patch from Gui, it would be nice if you could reuse the same commit message (or rearrange it..as you wish) and keep his sign as well.
Cheers,
Didn't test it but gave a quick glance, looks good to me :)
I'd also add 'netifd' as a dependency in the makefile
(and, well, increase the pkg revision, but that's marek job)
Thanks a lot!
On Sun, Dec 9, 2012 at 1:03 PM, Moritz Warning moritzwarning@web.de wrote:
Here you go. :) It would be good idea if Gui had a look at it before it is applied.
On 12/09/2012 03:31 PM, Marek Lindner wrote:
On Sunday, December 09, 2012 19:10:56 Moritz Warning wrote:
I have tried batman-adv-netifd-proto-batadv.patch along with some Makefile adjustments to get it compile. Afaict it works.
Thanks for testing! Do you mind sharing & explaining these changes ? Would be a good idea to push the working variant, don't you think ?
Cheers, Marek
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi.
I'd also add 'netifd' as a dependency in the makefile
You could but I do not think it is necessary. kmod-batman-adv works well without netifd, the onyl thing that would break is the support scripts.
Also netifd is pretty much guranteed to be present in current release so you can rely on it, but people who do not have it installed for wahtever reason might still want to use the batman kernel module, so I'd opt for not making it a hard dependency.
Cheers, Jow
On 12/09/2012 05:03 PM, Moritz Warning wrote:
Here you go. :) It would be good idea if Gui had a look at it before it is applied.
On 12/09/2012 03:31 PM, Marek Lindner wrote:
On Sunday, December 09, 2012 19:10:56 Moritz Warning wrote:
I have tried batman-adv-netifd-proto-batadv.patch along with some Makefile adjustments to get it compile. Afaict it works.
Thanks for testing! Do you mind sharing & explaining these changes ? Would be a good idea to push the working variant, don't you think ?
Cheers, Marek
The patch only adds necessary Makefile changes. It has to be GPL since Guis patch is GPL (afaik) as well.
Hi,
AFAICU, currently there's a /etc/hotplug.d/net/99-batman-adv script which tries to configure both bat0 , and any interfaces coming up that might have to be included there. Problem is, hotplug.d/net scripts are called when interface is "added" but not necessarilly brought up yet. So there's a "sleep 3s" that works in most cases as a window for slow hardware bring up the interface.
I like the idea of using the iface "ifup" hook for adding hard-interfaces. On the other hand I opt for keeping the net "remove" hook for removing them because batman-adv handles these ifup/ifdown blips internally. If you adjust that part I'll will merge your patch.
Excellent catch!
Cheers, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Gui Iribarren -
Jo-Philipp Wich -
Marek Lindner -
Moritz Warning -
Sven Eckelmann