static default route not always pushed to kernel (2.0.4)
Hi! There are cases where bird does not push a static default route to the kernel. One way to see this is as follows; A default route from the kernel bird> show route for 0.0.0.0 Table master4: 0.0.0.0/0 unicast [kernel1 13:46:07.958] (215) via 10.210.137.1 on eth1 Add two static default routes bird> show static 0.0.0.0/0 via 10.210.137.1 0.0.0.0/0 via 192.168.120.1 (dormant) bird> The static default route has higher preference, hece I expected it to be pushed to the kernel. But it is not... bird> show route for 0.0.0.0 Table master4: 0.0.0.0/0 unicast [kernel1 13:46:07.958] (215) via 10.210.137.1 on eth1 bird> Next remove both static routes, configure, add one of them back again and both are shown. bird> show route for 0.0.0.0 Table master4: 0.0.0.0/0 unicast [static1 14:13:16.762] * (255) via 10.210.137.1 on eth1 unicast [kernel1 13:46:07.958] (215) via 10.210.137.1 on eth1 bird> Thanks, Kenth
On Mon, Apr 29, 2019 at 12:14:56PM +0000, Kenth Eriksson wrote:
Hi!
There are cases where bird does not push a static default route to the kernel. One way to see this is as follows;
Next remove both static routes, configure, add one of them back again and both are shown.
bird> show route for 0.0.0.0 Table master4: 0.0.0.0/0 unicast [static1 14:13:16.762] * (255) via 10.210.137.1 on eth1 unicast [kernel1 13:46:07.958] (215) via 10.210.137.1 on eth1
Hi Yes, BIRD on Linux intentionally avoids replacing any existing non-BIRD (alien) routes in kernel tables. The reason is: Consider route A from BIRD and alien route B, both for the same network. A is preferred in BIRD and therefore pushed to kernel, where it replaces route B. Therefore, route B is overwritten and disappears. Then route A is for some reason removed, but route B no longer exists, so it cannot be restored and we end with no route. There are two ways how to fix that: one way is that kernel would remember all alien routes it learned, even when they were replaced by BIRD route. This would solve the problem above, but has other issues (e.g. the original source of alien route may want update or remove it, but it is no longer in kernel table). The approach we use is that we expect to have dedicated kernel metric value (by default 32) that is not used by alien routes (as kernel table keeps multiple routes with different kernel metric). This works well with IPv6, where default kernel metric used by alien routes is higher (256 or 1024) but in IPv4 the default kernel metric is 0 (most preferred), so we cannot override such routes without replacing them. The answer is to create alien routes with higher kernel metric (easy when created with 'ip' tool, perhaps harder in other cases). It would be great if there existed sysctl option for default IPv4 route metric. -- 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 Mon, 2019-04-29 at 14:46 +0200, Ondrej Zajicek wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Mon, Apr 29, 2019 at 12:14:56PM +0000, Kenth Eriksson wrote:
Hi!
There are cases where bird does not push a static default route to the kernel. One way to see this is as follows;
Next remove both static routes, configure, add one of them back again and both are shown.
bird> show route for 0.0.0.0 Table master4: 0.0.0.0/0 unicast [static1 14:13:16.762] * (255) via 10.210.137.1 on eth1 unicast [kernel1 13:46:07.958] (215) via 10.210.137.1 on eth1
Hi
Yes, BIRD on Linux intentionally avoids replacing any existing non- BIRD (alien) routes in kernel tables.
The reason is: Consider route A from BIRD and alien route B, both for the same network. A is preferred in BIRD and therefore pushed to kernel, where it replaces route B. Therefore, route B is overwritten and disappears. Then route A is for some reason removed, but route B no longer exists, so it cannot be restored and we end with no route.
There are two ways how to fix that: one way is that kernel would remember all alien routes it learned, even when they were replaced by BIRD route. This would solve the problem above, but has other issues (e.g. the original source of alien route may want update or remove it, but it is no longer in kernel table).
The approach we use is that we expect to have dedicated kernel metric value (by default 32) that is not used by alien routes (as kernel table keeps multiple routes with different kernel metric). This works well with IPv6, where default kernel metric used by alien routes is higher (256 or 1024) but in IPv4 the default kernel metric is 0 (most preferred), so we cannot override such routes without replacing them. The answer is to create alien routes with higher kernel metric (easy when created with 'ip' tool, perhaps harder in other cases). It would be great if there existed sysctl option for default IPv4 route metric.
There is no overwrite involed here. The default route in the kernel here has metric 100. As you said, bird pushes with metric 32 so it should push again. kenth ~ # ip route show default via 10.210.137.1 dev eth1 proto bird metric 32 default via 10.210.137.1 dev eth1 metric 100
-- 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 Mon, Apr 29, 2019 at 01:01:13PM +0000, Kenth Eriksson wrote:
'ip' tool, perhaps harder in other cases). It would be great if there existed sysctl option for default IPv4 route metric.
There is no overwrite involed here. The default route in the kernel here has metric 100. As you said, bird pushes with metric 32 so it should push again.
Well, i misread your mail. If i understand it correctly, the issue is not that routes are not pushed into kernel, but that they are not pushed into master table. I guess you have two routes for the same network in one static protocol. That is not valid case, although we are missing error checks for that configuration. You could either use two separate static protocol instances, or you could use this patch: https://gitlab.labs.nic.cz/labs/bird/commit/9861dba5230da539e6ce7d2b6baa4f26... That allows multiple routes for the same network with different preferences (with explicit preference parameter) in one static protocol instance, e.g.: route 0.0.0.0/0 via 10.210.137.1 { preference 220; }; route 0.0.0.0/0 via 192.168.120.1 { preference 230; }; -- 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 Mon, 2019-04-29 at 15:42 +0200, Ondrej Zajicek wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Mon, Apr 29, 2019 at 01:01:13PM +0000, Kenth Eriksson wrote:
'ip' tool, perhaps harder in other cases). It would be great if there existed sysctl option for default IPv4 route metric.
There is no overwrite involed here. The default route in the kernel here has metric 100. As you said, bird pushes with metric 32 so it should push again.
Well, i misread your mail. If i understand it correctly, the issue is not that routes are not pushed into kernel, but that they are not pushed into master table. I guess you have two routes for the same network in one static protocol. That is not valid case, although we are missing error checks for that configuration.
You could either use two separate static protocol instances, or you could use this patch:
That patch did not work for me, still same issue. I had to add DISTANCE to the CF_KEYWORDS to make it compile. But is it anything else that is required? I thought two prefixes differing only on metric should be perfectly valid? The kernel accepts that, so why would bird not?
That allows multiple routes for the same network with different preferences (with explicit preference parameter) in one static protocol instance, e.g.:
route 0.0.0.0/0 via 10.210.137.1 { preference 220; }; route 0.0.0.0/0 via 192.168.120.1 { preference 230; };
-- 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 Mon, Apr 29, 2019 at 03:46:09PM +0000, Kenth Eriksson wrote:
On Mon, 2019-04-29 at 15:42 +0200, Ondrej Zajicek wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Mon, Apr 29, 2019 at 01:01:13PM +0000, Kenth Eriksson wrote:
'ip' tool, perhaps harder in other cases). It would be great if there existed sysctl option for default IPv4 route metric.
There is no overwrite involed here. The default route in the kernel here has metric 100. As you said, bird pushes with metric 32 so it should push again.
Well, i misread your mail. If i understand it correctly, the issue is not that routes are not pushed into kernel, but that they are not pushed into master table. I guess you have two routes for the same network in one static protocol. That is not valid case, although we are missing error checks for that configuration.
You could either use two separate static protocol instances, or you could use this patch:
That patch did not work for me, still same issue. I had to add DISTANCE to the CF_KEYWORDS to make it compile. But is it anything else that is required?
Hi The patch should be more-or-less all that is required, but it is not fully ready for master branch, so it has a bit awkward behavior. Namely it requires that preference is specified without '=', like in example i provided:
route 0.0.0.0/0 via 10.210.137.1 { preference 220; }; route 0.0.0.0/0 via 192.168.120.1 { preference 230; };
If you used the same config like without patch (i.e. with 'preference = XXX'), it would not work.
I thought two prefixes differing only on metric should be perfectly valid? The kernel accepts that, so why would bird not?
Mainly implementation reasons. In BIRD, preference is generally not part of key, therefore route update for one network could replace a route for route with different preference (which makes sense with filter-based dynamic preference). Traditionally BIRD supported in one routing table one route per network and protocol instance. With BGP ADD_PATH extension the key was extended with arbitrary u32 distinguisher, which in BGP case is the path ID value from BGP ADD_PATH, but most other protocols does not use that. The patch above uses preference as distinguisher in case of static routes. -- 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 Tue, 2019-04-30 at 13:24 +0200, Ondrej Zajicek wrote:
On Mon, Apr 29, 2019 at 03:46:09PM +0000, Kenth Eriksson wrote:
On Mon, 2019-04-29 at 15:42 +0200, Ondrej Zajicek wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Mon, Apr 29, 2019 at 01:01:13PM +0000, Kenth Eriksson wrote:
'ip' tool, perhaps harder in other cases). It would be great if there existed sysctl option for default IPv4 route metric.
There is no overwrite involed here. The default route in the kernel here has metric 100. As you said, bird pushes with metric 32 so it should push again.
Well, i misread your mail. If i understand it correctly, the issue is not that routes are not pushed into kernel, but that they are not pushed into master table. I guess you have two routes for the same network in one static protocol. That is not valid case, although we are missing error checks for that configuration.
You could either use two separate static protocol instances, or you could use this patch:
That patch did not work for me, still same issue. I had to add DISTANCE to the CF_KEYWORDS to make it compile. But is it anything else that is required?
Hi
The patch should be more-or-less all that is required, but it is not fully ready for master branch, so it has a bit awkward behavior. Namely it requires that preference is specified without '=', like in example i provided:
route 0.0.0.0/0 via 10.210.137.1 { preference 220; }; route 0.0.0.0/0 via 192.168.120.1 { preference 230; };
If you used the same config like without patch (i.e. with 'preference = XXX'), it would not work.
I have attached three patches can you review them? It is your first commit, then a patch to make it compile and a patch to fix the syntax to be compliant with the filter syntax. Can this be upstreamed so you will support this use-case in upcoming releases?
I thought two prefixes differing only on metric should be perfectly valid? The kernel accepts that, so why would bird not?
Mainly implementation reasons. In BIRD, preference is generally not part of key, therefore route update for one network could replace a route for route with different preference (which makes sense with filter-based dynamic preference).
Traditionally BIRD supported in one routing table one route per network and protocol instance. With BGP ADD_PATH extension the key was extended with arbitrary u32 distinguisher, which in BGP case is the path ID value from BGP ADD_PATH, but most other protocols does not use that. The patch above uses preference as distinguisher in case of static routes.
From: "Ondrej Zajicek (work)" <santiago@crfreenet.org> Signed-off-by: Kenth Eriksson <kenth.eriksson@infinera.com> --- proto/static/config.Y | 2 ++ proto/static/static.c | 27 ++++++++++++++++++++------- proto/static/static.h | 1 + 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/proto/static/config.Y b/proto/static/config.Y index d7a02961..32eba0fe 100644 --- a/proto/static/config.Y +++ b/proto/static/config.Y @@ -135,6 +135,8 @@ stat_route: stat_route_item: cmd { *this_srt_last_cmd = $1; this_srt_last_cmd = &($1->next); } + | PREFERENCE expr ';' { this_srt->preference = $2; check_u16($2); } + | DISTANCE expr ';' { this_srt->preference = $2; check_u16($2); } ; stat_route_opts: diff --git a/proto/static/static.c b/proto/static/static.c index 75a74ad0..feb8e427 100644 --- a/proto/static/static.c +++ b/proto/static/static.c @@ -53,7 +53,7 @@ static void static_announce_rte(struct static_proto *p, struct static_route *r) { rta *a = allocz(RTA_MAX_SIZE); - a->src = p->p.main_source; + a->src = rt_get_source(&p->p, r->preference); a->source = RTS_STATIC; a->scope = SCOPE_UNIVERSE; a->dest = r->dest; @@ -101,11 +101,13 @@ static_announce_rte(struct static_proto *p, struct static_route *r) /* We skip rta_lookup() here */ rte *e = rte_get_temp(a); e->pflags = 0; + e->pref = r->preference; if (r->cmds) f_eval_rte(r->cmds, &e, static_lp); - rte_update(&p->p, r->net, e); + e->pref = r->preference; /* Avoid preference from filter */ + rte_update2(p->p.main_channel, r->net, e, a->src); r->state = SRS_CLEAN; if (r->cmds) @@ -117,7 +119,7 @@ withdraw: if (r->state == SRS_DOWN) return; - rte_update(&p->p, r->net, NULL); + rte_update2(p->p.main_channel, r->net, NULL, a->src); r->state = SRS_DOWN; } @@ -249,7 +251,7 @@ static void static_remove_rte(struct static_proto *p, struct static_route *r) { if (r->state) - rte_update(&p->p, r->net, NULL); + rte_update2(p->p.main_channel, r->net, NULL, rt_get_source(&p->p, r->preference)); static_reset_rte(p, r); } @@ -379,8 +381,13 @@ static_postconfig(struct proto_config *CF) cc->table : cf->c.global->def_tables[NET_IP6]; WALK_LIST(r, cf->routes) + { if (r->net && (r->net->type != CF->net_type)) cf_error("Route %N incompatible with channel type", r->net); + + if (!r->preference) + r->preference = cc->preference; + } } static struct proto * @@ -485,11 +492,17 @@ static_dump(struct proto *P) #define IGP_TABLE(cf, sym) ((cf)->igp_table_##sym ? (cf)->igp_table_##sym ->table : NULL ) +static inline int srt_equal(struct static_route *a, struct static_route *b) +{ return net_equal(a->net, b->net) && (a->preference == b->preference); } + +static inline int srt_compare(struct static_route *a, struct static_route *b) +{ return net_compare(a->net, b->net) ?: uint_cmp(a->preference, b->preference); } + static inline int static_cmp_rte(const void *X, const void *Y) { struct static_route *x = *(void **)X, *y = *(void **)Y; - return net_compare(x->net, y->net); + return srt_compare(x, y); } static int @@ -518,7 +531,7 @@ static_reconfigure(struct proto *P, struct proto_config *CF) /* Reconfigure initial matching sequence */ for (or = HEAD(o->routes), nr = HEAD(n->routes); - NODE_VALID(or) && NODE_VALID(nr) && net_equal(or->net, nr->net); + NODE_VALID(or) && NODE_VALID(nr) && srt_equal(or, nr); or = NODE_NEXT(or), nr = NODE_NEXT(nr)) static_reconfigure_rte(p, or, nr); @@ -549,7 +562,7 @@ static_reconfigure(struct proto *P, struct proto_config *CF) while ((orpos < ornum) && (nrpos < nrnum)) { - int x = net_compare(orbuf[orpos]->net, nrbuf[nrpos]->net); + int x = srt_compare(orbuf[orpos], nrbuf[nrpos]); if (x < 0) static_remove_rte(p, orbuf[orpos++]); else if (x > 0) diff --git a/proto/static/static.h b/proto/static/static.h index a3c30b87..ec9dffb3 100644 --- a/proto/static/static.h +++ b/proto/static/static.h @@ -40,6 +40,7 @@ struct static_route { struct static_route *mp_head; /* First nexthop of this route */ struct static_route *mp_next; /* Nexthops for multipath routes */ struct f_inst *cmds; /* List of commands for setting attributes */ + u16 preference; /* Route preference */ byte dest; /* Destination type (RTD_*) */ byte state; /* State of route announcement (SRS_*) */ byte active; /* Next hop is active (nbr/iface/BFD available) */ -- 2.21.0
Signed-off-by: Kenth Eriksson <kenth.eriksson@infinera.com> --- nest/config.Y | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nest/config.Y b/nest/config.Y index aef5ed46..aee52b97 100644 --- a/nest/config.Y +++ b/nest/config.Y @@ -64,7 +64,7 @@ proto_postconfig(void) CF_DECLS -CF_KEYWORDS(ROUTER, ID, PROTOCOL, TEMPLATE, PREFERENCE, DISABLED, DEBUG, ALL, OFF, DIRECT) +CF_KEYWORDS(ROUTER, ID, PROTOCOL, TEMPLATE, PREFERENCE, DISTANCE, DISABLED, DEBUG, ALL, OFF, DIRECT) CF_KEYWORDS(INTERFACE, IMPORT, EXPORT, FILTER, NONE, VRF, TABLE, STATES, ROUTES, FILTERS) CF_KEYWORDS(IPV4, IPV6, VPN4, VPN6, ROA4, ROA6, FLOW4, FLOW6, SADR, MPLS) CF_KEYWORDS(RECEIVE, LIMIT, ACTION, WARN, BLOCK, RESTART, DISABLE, KEEP, FILTERED) -- 2.21.0
Setting preference value for filters and static routes is done using the following syntax: preference = value. Signed-off-by: Kenth Eriksson <kenth.eriksson@infinera.com> --- proto/static/config.Y | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proto/static/config.Y b/proto/static/config.Y index 32eba0fe..0526a8cd 100644 --- a/proto/static/config.Y +++ b/proto/static/config.Y @@ -135,8 +135,8 @@ stat_route: stat_route_item: cmd { *this_srt_last_cmd = $1; this_srt_last_cmd = &($1->next); } - | PREFERENCE expr ';' { this_srt->preference = $2; check_u16($2); } - | DISTANCE expr ';' { this_srt->preference = $2; check_u16($2); } + | PREFERENCE '=' expr ';' { this_srt->preference = $3; check_u16($3); } + | DISTANCE '=' expr ';' { this_srt->preference = $3; check_u16($3); } ; stat_route_opts: -- 2.21.0
On Thu, 2019-05-02 at 11:29 +0200, Kenth Eriksson wrote:
> From: "Ondrej Zajicek (work)" <santiago@crfreenet.org>
>
> Signed-off-by: Kenth Eriksson <kenth.eriksson@infinera.com>
> ---
> proto/static/config.Y | 2 ++
> proto/static/static.c | 27 ++++++++++++++++++++-------
> proto/static/static.h | 1 +
> 3 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/proto/static/config.Y b/proto/static/config.Y
> index d7a02961..32eba0fe 100644
> --- a/proto/static/config.Y
> +++ b/proto/static/config.Y
> @@ -135,6 +135,8 @@ stat_route:
>
> stat_route_item:
> cmd { *this_srt_last_cmd = $1; this_srt_last_cmd = &($1->next); }
> + | PREFERENCE expr ';' { this_srt->preference = $2; check_u16($2); }
> + | DISTANCE expr ';' { this_srt->preference = $2; check_u16($2); }
> ;
>
> stat_route_opts:
> diff --git a/proto/static/static.c b/proto/static/static.c
> index 75a74ad0..feb8e427 100644
> --- a/proto/static/static.c
> +++ b/proto/static/static.c
> @@ -53,7 +53,7 @@ static void
> static_announce_rte(struct static_proto *p, struct static_route *r)
> {
> rta *a = allocz(RTA_MAX_SIZE);
> - a->src = p->p.main_source;
> + a->src = rt_get_source(&p->p, r->preference);
> a->source = RTS_STATIC;
> a->scope = SCOPE_UNIVERSE;
> a->dest = r->dest;
> @@ -101,11 +101,13 @@ static_announce_rte(struct static_proto *p,
> struct static_route *r)
> /* We skip rta_lookup() here */
> rte *e = rte_get_temp(a);
> e->pflags = 0;
> + e->pref = r->preference;
>
> if (r->cmds)
> f_eval_rte(r->cmds, &e, static_lp);
>
> - rte_update(&p->p, r->net, e);
> + e->pref = r->preference; /* Avoid preference from filter */
> + rte_update2(p->p.main_channel, r->net, e, a->src);
> r->state = SRS_CLEAN;
>
> if (r->cmds)
> @@ -117,7 +119,7 @@ withdraw:
> if (r->state == SRS_DOWN)
> return;
>
> - rte_update(&p->p, r->net, NULL);
> + rte_update2(p->p.main_channel, r->net, NULL, a->src);
> r->state = SRS_DOWN;
> }
>
> @@ -249,7 +251,7 @@ static void
> static_remove_rte(struct static_proto *p, struct static_route *r)
> {
> if (r->state)
> - rte_update(&p->p, r->net, NULL);
> + rte_update2(p->p.main_channel, r->net, NULL, rt_get_source(&p-
> >p, r->preference));
>
> static_reset_rte(p, r);
> }
> @@ -379,8 +381,13 @@ static_postconfig(struct proto_config *CF)
> cc->table : cf->c.global->def_tables[NET_IP6];
>
> WALK_LIST(r, cf->routes)
> + {
> if (r->net && (r->net->type != CF->net_type))
> cf_error("Route %N incompatible with channel type", r->net);
> +
> + if (!r->preference)
> + r->preference = cc->preference;
> + }
> }
>
> static struct proto *
> @@ -485,11 +492,17 @@ static_dump(struct proto *P)
>
> #define IGP_TABLE(cf, sym) ((cf)->igp_table_##sym ? (cf)-
> >igp_table_##sym ->table : NULL )
>
> +static inline int srt_equal(struct static_route *a, struct
> static_route *b)
> +{ return net_equal(a->net, b->net) && (a->preference == b-
> >preference); }
> +
> +static inline int srt_compare(struct static_route *a, struct
> static_route *b)
> +{ return net_compare(a->net, b->net) ?: uint_cmp(a->preference, b-
> >preference); }
> +
> static inline int
> static_cmp_rte(const void *X, const void *Y)
> {
> struct static_route *x = *(void **)X, *y = *(void **)Y;
> - return net_compare(x->net, y->net);
> + return srt_compare(x, y);
> }
>
> static int
> @@ -518,7 +531,7 @@ static_reconfigure(struct proto *P, struct
> proto_config *CF)
>
> /* Reconfigure initial matching sequence */
> for (or = HEAD(o->routes), nr = HEAD(n->routes);
> - NODE_VALID(or) && NODE_VALID(nr) && net_equal(or->net, nr-
> >net);
> + NODE_VALID(or) && NODE_VALID(nr) && srt_equal(or, nr);
> or = NODE_NEXT(or), nr = NODE_NEXT(nr))
> static_reconfigure_rte(p, or, nr);
>
> @@ -549,7 +562,7 @@ static_reconfigure(struct proto *P, struct
> proto_config *CF)
>
> while ((orpos < ornum) && (nrpos < nrnum))
> {
> - int x = net_compare(orbuf[orpos]->net, nrbuf[nrpos]->net);
> + int x = srt_compare(orbuf[orpos], nrbuf[nrpos]);
> if (x < 0)
> static_remove_rte(p, orbuf[orpos++]);
> else if (x > 0)
> diff --git a/proto/static/static.h b/proto/static/static.h
> index a3c30b87..ec9dffb3 100644
> --- a/proto/static/static.h
> +++ b/proto/static/static.h
> @@ -40,6 +40,7 @@ struct static_route {
> struct static_route *mp_head; /* First nexthop of
> this route */
> struct static_route *mp_next; /* Nexthops for
> multipath routes */
> struct f_inst *cmds; /* List of commands for
> setting attributes */
> + u16 preference; /* Route preference */
> byte dest; /* Destination type (RTD_*) */
> byte state; /* State of route
> announcement (SRS_*) */
> byte active; /* Next hop is active
> (nbr/iface/BFD available) */
Ondrej, any updates on this patch set? Will you merge it into master?
Thanks,
Kenth
On Mon, 2019-04-29 at 13:01 +0000, Kenth Eriksson wrote:
On Mon, 2019-04-29 at 14:46 +0200, Ondrej Zajicek wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Mon, Apr 29, 2019 at 12:14:56PM +0000, Kenth Eriksson wrote:
Hi!
There are cases where bird does not push a static default route to the kernel. One way to see this is as follows;
Next remove both static routes, configure, add one of them back again and both are shown.
bird> show route for 0.0.0.0 Table master4: 0.0.0.0/0 unicast [static1 14:13:16.762] * (255) via 10.210.137.1 on eth1 unicast [kernel1 13:46:07.958] (215) via 10.210.137.1 on eth1
Hi
Yes, BIRD on Linux intentionally avoids replacing any existing non- BIRD (alien) routes in kernel tables.
The reason is: Consider route A from BIRD and alien route B, both for the same network. A is preferred in BIRD and therefore pushed to kernel, where it replaces route B. Therefore, route B is overwritten and disappears. Then route A is for some reason removed, but route B no longer exists, so it cannot be restored and we end with no route.
There are two ways how to fix that: one way is that kernel would remember all alien routes it learned, even when they were replaced by BIRD route. This would solve the problem above, but has other issues (e.g. the original source of alien route may want update or remove it, but it is no longer in kernel table).
The approach we use is that we expect to have dedicated kernel metric value (by default 32) that is not used by alien routes (as kernel table keeps multiple routes with different kernel metric). This works well with IPv6, where default kernel metric used by alien routes is higher (256 or 1024) but in IPv4 the default kernel metric is 0 (most preferred), so we cannot override such routes without replacing them. The answer is to create alien routes with higher kernel metric (easy when created with 'ip' tool, perhaps harder in other cases). It would be great if there existed sysctl option for default IPv4 route metric.
There is no overwrite involed here. The default route in the kernel here has metric 100. As you said, bird pushes with metric 32 so it should push again.
kenth ~ # ip route show default via 10.210.137.1 dev eth1 proto bird metric 32 default via 10.210.137.1 dev eth1 metric 100
There is an even simpler way to reproduce this bug; Add two static default routes that differs only on metric (no alien kernel route this time) route 0.0.0.0/0 via 10.210.137.1 {preference=255;}; route 0.0.0.0/0 via 10.210.137.1 {preference=254;}; bird> show static 0.0.0.0/0 via 10.210.137.1 0.0.0.0/0 via 10.210.137.1 bird> Next note how it select the route with lower preference value. That is not correct according to the docs. bird> show route for 0.0.0.0 Table master4: 0.0.0.0/0 unicast [static1 15:49:16.285] * (254) via 10.210.137.1 on eth1 bird> Remove one of the static routes. Now the route disappears from the RIB and it is also removed from the FIB. bird> show static 0.0.0.0/0 via 10.210.137.1 bird> show route for 0.0.0.0 Network not found bird>
-- 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 (3)
-
Kenth Eriksson -
Kenth Eriksson
-
Ondrej Zajicek