* [B.A.T.M.A.N.] [PATCH] batman-adv: init ELP tweaking options only once
@ 2016-05-07 23:47 Marek Lindner
2016-05-08 15:41 ` Antonio Quartulli
0 siblings, 1 reply; 6+ messages in thread
From: Marek Lindner @ 2016-05-07 23:47 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Marek Lindner
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(-)
diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index c16cd44..3c5d251 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -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;
}
diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 8909d1e..cf0262b 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -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;
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index db2009d..dd6a5a2 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -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);
--
2.8.0.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: init ELP tweaking options only once
2016-05-07 23:47 [B.A.T.M.A.N.] [PATCH] batman-adv: init ELP tweaking options only once Marek Lindner
@ 2016-05-08 15:41 ` Antonio Quartulli
2016-05-09 7:32 ` Marek Lindner
0 siblings, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2016-05-08 15:41 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking; +Cc: Marek Lindner
[-- Attachment #1: Type: text/plain, Size: 544 bytes --]
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,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: init ELP tweaking options only once
2016-05-08 15:41 ` Antonio Quartulli
@ 2016-05-09 7:32 ` Marek Lindner
2016-05-09 10:45 ` Antonio Quartulli
0 siblings, 1 reply; 6+ messages in thread
From: Marek Lindner @ 2016-05-09 7:32 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: init ELP tweaking options only once
2016-05-09 7:32 ` Marek Lindner
@ 2016-05-09 10:45 ` Antonio Quartulli
2016-05-09 13:10 ` Marek Lindner
0 siblings, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2016-05-09 10:45 UTC (permalink / raw)
To: Marek Lindner; +Cc: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]
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,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: init ELP tweaking options only once
2016-05-09 10:45 ` Antonio Quartulli
@ 2016-05-09 13:10 ` Marek Lindner
2016-05-09 14:20 ` Antonio Quartulli
0 siblings, 1 reply; 6+ messages in thread
From: Marek Lindner @ 2016-05-09 13:10 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: init ELP tweaking options only once
2016-05-09 13:10 ` Marek Lindner
@ 2016-05-09 14:20 ` Antonio Quartulli
0 siblings, 0 replies; 6+ messages in thread
From: Antonio Quartulli @ 2016-05-09 14:20 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking; +Cc: Marek Lindner
[-- Attachment #1: Type: text/plain, Size: 824 bytes --]
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,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-05-09 14:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-07 23:47 [B.A.T.M.A.N.] [PATCH] batman-adv: init ELP tweaking options only once Marek Lindner
2016-05-08 15:41 ` Antonio Quartulli
2016-05-09 7:32 ` Marek Lindner
2016-05-09 10:45 ` Antonio Quartulli
2016-05-09 13:10 ` Marek Lindner
2016-05-09 14:20 ` Antonio Quartulli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox