The Babel seqno wraps around when reaching its maximum value (UINT16_MAX). When comparing seqnos, this has to be taken into account. Therefore, plain number comparisons do not work. In a previous attempt to fix the wrapping behavior, one particular comparison was missed. This causes the seqno of originated babel routes to not wrap, but remain at UINT16_MAX indefinitely, resulting in slow route propagation on topology changes. Make use of the previously introduced gt_mod64k macro to compare seqnos correctly. Fixes: 3e7e4a71868bc519aacc0eb785471b46fc345a5c Signed-off-by: Fabian Bläse <fabian@blaese.de> --- proto/babel/babel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/babel/babel.c b/proto/babel/babel.c index fe5c0599..b94b96d9 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1015,7 +1015,7 @@ babel_send_update_(struct babel_iface *ifa, btime changed, struct fib *rtable) /* Our own seqno might have changed, in which case we update the routes we originate. */ - if ((e->router_id == p->router_id) && (e->seqno < p->update_seqno)) + if ((e->router_id == p->router_id) && (gt_mod64k(p->update_seqno, e->seqno)) { e->seqno = p->update_seqno; e->updated = current_time(); -- 2.47.1
The Babel seqno wraps around when reaching its maximum value (UINT16_MAX). When comparing seqnos, this has to be taken into account. Therefore, plain number comparisons do not work. In a previous attempt to fix the wrapping behavior, one particular comparison was missed. This causes the seqno of originated babel routes to not wrap, but remain at UINT16_MAX indefinitely, resulting in slow route propagation on topology changes. Make use of the previously introduced gt_mod64k macro to compare seqnos correctly. Fixes: 3e7e4a71868bc519aacc0eb785471b46fc345a5c Signed-off-by: Fabian Bläse <fabian@blaese.de> --- v2: fix syntax error proto/babel/babel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/babel/babel.c b/proto/babel/babel.c index fe5c0599..209492f0 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1015,7 +1015,7 @@ babel_send_update_(struct babel_iface *ifa, btime changed, struct fib *rtable) /* Our own seqno might have changed, in which case we update the routes we originate. */ - if ((e->router_id == p->router_id) && (e->seqno < p->update_seqno)) + if ((e->router_id == p->router_id) && (gt_mod64k(p->update_seqno, e->seqno))) { e->seqno = p->update_seqno; e->updated = current_time(); -- 2.47.1
Fabian Bläse <fabian@blaese.de> writes:
The Babel seqno wraps around when reaching its maximum value (UINT16_MAX). When comparing seqnos, this has to be taken into account. Therefore, plain number comparisons do not work.
In a previous attempt to fix the wrapping behavior, one particular comparison was missed. This causes the seqno of originated babel routes to not wrap, but remain at UINT16_MAX indefinitely, resulting in slow route propagation on topology changes.
Make use of the previously introduced gt_mod64k macro to compare seqnos correctly.
Fixes: 3e7e4a71868bc519aacc0eb785471b46fc345a5c
Signed-off-by: Fabian Bläse <fabian@blaese.de>
--- v2: fix syntax error
proto/babel/babel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proto/babel/babel.c b/proto/babel/babel.c index fe5c0599..209492f0 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1015,7 +1015,7 @@ babel_send_update_(struct babel_iface *ifa, btime changed, struct fib *rtable)
/* Our own seqno might have changed, in which case we update the routes we originate. */ - if ((e->router_id == p->router_id) && (e->seqno < p->update_seqno)) + if ((e->router_id == p->router_id) && (gt_mod64k(p->update_seqno, e->seqno)))
I'd drop the extra parentheses around gt_mod64k(), but otherwise LGTM! Reviewed-by: Toke Høiland-Jørgensen <toke@toke.dk>
Thanks, merged. On Mon, Dec 09, 2024 at 10:51:28AM +0100, Toke Høiland-Jørgensen via Bird-users wrote:
Fabian Bläse <fabian@blaese.de> writes:
The Babel seqno wraps around when reaching its maximum value (UINT16_MAX). When comparing seqnos, this has to be taken into account. Therefore, plain number comparisons do not work.
In a previous attempt to fix the wrapping behavior, one particular comparison was missed. This causes the seqno of originated babel routes to not wrap, but remain at UINT16_MAX indefinitely, resulting in slow route propagation on topology changes.
Make use of the previously introduced gt_mod64k macro to compare seqnos correctly.
Fixes: 3e7e4a71868bc519aacc0eb785471b46fc345a5c
Signed-off-by: Fabian Bläse <fabian@blaese.de>
--- v2: fix syntax error
proto/babel/babel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proto/babel/babel.c b/proto/babel/babel.c index fe5c0599..209492f0 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1015,7 +1015,7 @@ babel_send_update_(struct babel_iface *ifa, btime changed, struct fib *rtable)
/* Our own seqno might have changed, in which case we update the routes we originate. */ - if ((e->router_id == p->router_id) && (e->seqno < p->update_seqno)) + if ((e->router_id == p->router_id) && (gt_mod64k(p->update_seqno, e->seqno)))
I'd drop the extra parentheses around gt_mod64k(), but otherwise LGTM!
Reviewed-by: Toke Høiland-Jørgensen <toke@toke.dk>
-- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) "To err is human -- to blame it on a computer is even more so."
participants (3)
-
Fabian Bläse -
Ondrej Zajicek -
Toke Høiland-Jørgensen