batman-adv: init ELP tweaking options only once
Commit Message
The ELP interval and throughput override interface settings are initialized
with default settings on every iface_enable() call. Thus, the user space
configuration is overridden as soon as an interface is going down and
coming up again.
This patch prevents this behavior by moving the configuration init to the
interface detection routine which runs only once per interface.
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
net/batman-adv/bat_v.c | 5 -----
net/batman-adv/bat_v_elp.c | 1 -
net/batman-adv/hard-interface.c | 7 +++++++
3 files changed, 7 insertions(+), 6 deletions(-)
Comments
On Sun, May 08, 2016 at 07:47:16AM +0800, Marek Lindner wrote:
> The ELP interval and throughput override interface settings are initialized
> with default settings on every iface_enable() call. Thus, the user space
> configuration is overridden as soon as an interface is going down and
> coming up again.
iface_enable() is only invoked when an interface is added to the mesh. Up and
Down should trigger a iface_activate/deactivate() only.
Have you seen the userspace settings being reverted ?
Cheers,
On Sunday, May 08, 2016 23:41:59 Antonio Quartulli wrote:
> On Sun, May 08, 2016 at 07:47:16AM +0800, Marek Lindner wrote:
> > The ELP interval and throughput override interface settings are
> > initialized
> > with default settings on every iface_enable() call. Thus, the user space
> > configuration is overridden as soon as an interface is going down and
> > coming up again.
>
> iface_enable() is only invoked when an interface is added to the mesh. Up
> and Down should trigger a iface_activate/deactivate() only.
>
> Have you seen the userspace settings being reverted ?
Without my patch:
root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
500
root@OpenWrt:/# echo 700 > /sys/class/net/eth1/batman_adv/elp_interval
[51835.004638] batman_adv: eth1: elp_interval: Changing from: 500 to: 700
root@OpenWrt:/# ifconfig eth1 down && ifconfig eth1 up
root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
500
With my patch:
root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
500
root@OpenWrt:/# echo 700 > /sys/class/net/eth1/batman_adv/elp_interval
[ 33.972946] batman_adv: eth1: elp_interval: Changing from: 500 to: 700
root@OpenWrt:/# ifconfig eth1 down && ifconfig eth1 up
root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
700
Cheers,
Marek
On Mon, May 09, 2016 at 03:32:19PM +0800, Marek Lindner wrote:
> On Sunday, May 08, 2016 23:41:59 Antonio Quartulli wrote:
> > On Sun, May 08, 2016 at 07:47:16AM +0800, Marek Lindner wrote:
> > > The ELP interval and throughput override interface settings are
> > > initialized
> > > with default settings on every iface_enable() call. Thus, the user space
> > > configuration is overridden as soon as an interface is going down and
> > > coming up again.
> >
> > iface_enable() is only invoked when an interface is added to the mesh. Up
> > and Down should trigger a iface_activate/deactivate() only.
> >
> > Have you seen the userspace settings being reverted ?
>
> Without my patch:
> root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
> 500
> root@OpenWrt:/# echo 700 > /sys/class/net/eth1/batman_adv/elp_interval
> [51835.004638] batman_adv: eth1: elp_interval: Changing from: 500 to: 700
> root@OpenWrt:/# ifconfig eth1 down && ifconfig eth1 up
> root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
> 500
Performed the same, but I cannot reproduce. I tested:
- origin/maint: 9685688ae7dd85804aec2f6ce760611551fe9635
- origin/master: 7608af0adebb29dc25bd8aa489ad8a2d0e4a6317
Are you sure you are not testing this with other patches applied ?
Cheers,
On Monday, May 09, 2016 18:45:33 Antonio Quartulli wrote:
> > Without my patch:
> > root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
> > 500
> > root@OpenWrt:/# echo 700 > /sys/class/net/eth1/batman_adv/elp_interval
> > [51835.004638] batman_adv: eth1: elp_interval: Changing from: 500 to: 700
> > root@OpenWrt:/# ifconfig eth1 down && ifconfig eth1 up
> > root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
> > 500
>
> Performed the same, but I cannot reproduce. I tested:
> - origin/maint: 9685688ae7dd85804aec2f6ce760611551fe9635
> - origin/master: 7608af0adebb29dc25bd8aa489ad8a2d0e4a6317
>
> Are you sure you are not testing this with other patches applied ?
root@OpenWrt:/# batctl -v
batctl f29682c [batman-adv: 7608af0]
root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
500
root@OpenWrt:/# echo 700 > /sys/class/net/eth1/batman_adv/elp_interval
[ 62.176281] batman_adv: eth1: elp_interval: Changing from: 500 to: 700
root@OpenWrt:/# ifconfig eth1 down && ifconfig eth1 up
root@OpenWrt:/# cat /sys/class/net/eth1/batman_adv/elp_interval
500
Am I right assuming you're not testing with Openwrt ? I suspect it is netifd
that removes the interface from batX entirely on ifconfig down. That might not
be the same on your system. I can reword the commit message to not mention
interface down if you like.
Cheers,
Marek
On Mon, May 09, 2016 at 09:10:44PM +0800, Marek Lindner wrote:
> Am I right assuming you're not testing with Openwrt ? I suspect it is netifd
> that removes the interface from batX entirely on ifconfig down. That might not
> be the same on your system. I can reword the commit message to not mention
> interface down if you like.
You are right - I was not using netifd and so my simple "ifdown & ifup" was not
enough to trigger the reset.
Still, I do understand that your point is to avoid resetting the elp_interval
and throughput_override more than once during the hard_iface life span.
This said, I think your patch is fine, but the commit message should be
re-arranged in order to avoid stating that "ifdown & ifup" is a way to trigger
the "misbehaviour".
Cheers,
@@ -70,11 +70,6 @@ static int batadv_v_iface_enable(struct batadv_hard_iface *hard_iface)
if (ret < 0)
batadv_v_elp_iface_disable(hard_iface);
- /* enable link throughput auto-detection by setting the throughput
- * override to zero
- */
- atomic_set(&hard_iface->bat_v.throughput_override, 0);
-
return ret;
}
@@ -344,7 +344,6 @@ int batadv_v_elp_iface_enable(struct batadv_hard_iface *hard_iface)
/* randomize initial seqno to avoid collision */
get_random_bytes(&random_seqno, sizeof(random_seqno));
atomic_set(&hard_iface->bat_v.elp_seqno, random_seqno);
- atomic_set(&hard_iface->bat_v.elp_interval, 500);
/* assume full-duplex by default */
hard_iface->bat_v.flags |= BATADV_FULL_DUPLEX;
@@ -683,6 +683,13 @@ batadv_hardif_add_interface(struct net_device *net_dev)
if (batadv_is_wifi_netdev(net_dev))
hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS;
+ /* enable link throughput auto-detection by setting the throughput
+ * override to zero
+ */
+ atomic_set(&hard_iface->bat_v.throughput_override, 0);
+
+ atomic_set(&hard_iface->bat_v.elp_interval, 500);
+
/* extra reference for return */
kref_init(&hard_iface->refcount);
kref_get(&hard_iface->refcount);