[PATCH v2] Babel: Immediately update hello interval on interface reconfigure.
An interface reconfiguration may change both the hello and update intervals. An update interval change is immediately put into effect, while a hello interval change is not. This also updates the hello interval immediately (if the new interval is shorter than the old one), and sends a hello to notify peers of the change. Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> --- proto/babel/babel.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/proto/babel/babel.c b/proto/babel/babel.c index 8e104d6..15451ab 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1529,6 +1529,9 @@ babel_reconfigure_iface(struct babel_proto *p, struct babel_iface *ifa, struct b ifa->cf = new; + if (ifa->next_hello > (now + new->hello_interval)) + ifa->next_hello = now + (random() % new->hello_interval) + 1; + if (ifa->next_regular > (now + new->update_interval)) ifa->next_regular = now + (random() % new->update_interval) + 1; @@ -1538,8 +1541,10 @@ babel_reconfigure_iface(struct babel_proto *p, struct babel_iface *ifa, struct b if (new->check_link != old->check_link) babel_iface_update_state(ifa); - if (ifa->up) + if (ifa->up) { + babel_send_hello(ifa, 0) babel_iface_kick_timer(ifa); + } return 1; } -- 2.9.0
On Tue, Jul 19, 2016 at 06:34:44PM +0200, Toke Høiland-Jørgensen wrote:
@@ -1538,8 +1541,10 @@ babel_reconfigure_iface(struct babel_proto *p, struct babel_iface *ifa, struct b if (new->check_link != old->check_link) babel_iface_update_state(ifa);
- if (ifa->up) + if (ifa->up) { + babel_send_hello(ifa, 0) babel_iface_kick_timer(ifa); + }
Is there a reason to send Hello immediately? If hello_interval is increased, it is applied only after the already scheduled Hello is sent. So the condition from RFC ('it SHOULD NOT be increased, except immediately before sending a Hello packet.') is fulfilled. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On 20 July 2016 11:46:08 CEST, Ondrej Zajicek <santiago@crfreenet.org> wrote:
On Tue, Jul 19, 2016 at 06:34:44PM +0200, Toke Høiland-Jørgensen wrote:
@@ -1538,8 +1541,10 @@ babel_reconfigure_iface(struct babel_proto *p, struct babel_iface *ifa, struct b if (new->check_link != old->check_link) babel_iface_update_state(ifa);
- if (ifa->up) + if (ifa->up) { + babel_send_hello(ifa, 0) babel_iface_kick_timer(ifa); + }
Is there a reason to send Hello immediately? If hello_interval is increased, it is applied only after the already scheduled Hello is sent. So the condition from RFC ('it SHOULD NOT be increased, except immediately before sending a Hello packet.') is fulfilled.
Only consistency: in the current code, changes to the update interval are applied immediately, but changes to the hello interval are not. This could be confusing to a user that changes the config and expects it to take effect immediately (and indeed this was the feedback I got after the release of 1.6; the old thread is in the mailing list archives). But you're right that both behaviours are within spec :) -Toke
On Wed, Jul 20, 2016 at 11:51:28AM +0200, Toke Høiland-Jørgensen wrote:
On 20 July 2016 11:46:08 CEST, Ondrej Zajicek <santiago@crfreenet.org> wrote:
On Tue, Jul 19, 2016 at 06:34:44PM +0200, Toke Høiland-Jørgensen wrote:
@@ -1538,8 +1541,10 @@ babel_reconfigure_iface(struct babel_proto *p, struct babel_iface *ifa, struct b if (new->check_link != old->check_link) babel_iface_update_state(ifa);
- if (ifa->up) + if (ifa->up) { + babel_send_hello(ifa, 0) babel_iface_kick_timer(ifa); + }
Is there a reason to send Hello immediately? If hello_interval is increased, it is applied only after the already scheduled Hello is sent.
Only consistency: in the current code, changes to the update interval are applied immediately, but changes to the hello interval are not. This
Yes, i understand this. My question meant why also to send Hello immediately, if the interval was already changed in the patch by this: + if (ifa->next_hello > (now + new->hello_interval)) + ifa->next_hello = now + (random() % new->hello_interval) + 1; IMHO without the babel_send_hello(), you get more consistent behavior between Hellos and Updates, as both got rescheduled just by changing next_* variable, not immediately sent. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Ondrej Zajicek <santiago@crfreenet.org> writes:
On Wed, Jul 20, 2016 at 11:51:28AM +0200, Toke Høiland-Jørgensen wrote:
On 20 July 2016 11:46:08 CEST, Ondrej Zajicek <santiago@crfreenet.org> wrote:
On Tue, Jul 19, 2016 at 06:34:44PM +0200, Toke Høiland-Jørgensen wrote:
@@ -1538,8 +1541,10 @@ babel_reconfigure_iface(struct babel_proto *p, struct babel_iface *ifa, struct b if (new->check_link != old->check_link) babel_iface_update_state(ifa);
- if (ifa->up) + if (ifa->up) { + babel_send_hello(ifa, 0) babel_iface_kick_timer(ifa); + }
Is there a reason to send Hello immediately? If hello_interval is increased, it is applied only after the already scheduled Hello is sent.
Only consistency: in the current code, changes to the update interval are applied immediately, but changes to the hello interval are not. This
Yes, i understand this. My question meant why also to send Hello immediately, if the interval was already changed in the patch by this:
+ if (ifa->next_hello > (now + new->hello_interval)) + ifa->next_hello = now + (random() % new->hello_interval) + 1;
IMHO without the babel_send_hello(), you get more consistent behavior between Hellos and Updates, as both got rescheduled just by changing next_* variable, not immediately sent.
Yeah, you're right. My bad. Do you want me to send another patch, or can you just drop the second half when applying it? :) -Toke
On Wed, Jul 20, 2016 at 02:05:20PM +0200, Toke Høiland-Jørgensen wrote:
IMHO without the babel_send_hello(), you get more consistent behavior between Hellos and Updates, as both got rescheduled just by changing next_* variable, not immediately sent.
Yeah, you're right. My bad. Do you want me to send another patch, or can you just drop the second half when applying it? :)
I just drop the second half. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
participants (2)
-
Ondrej Zajicek -
Toke Høiland-Jørgensen