Commit feae132e authored by Ondřej Zajíček's avatar Ondřej Zajíček

Do not initialize route metrics in import_control hook

During route export, the receiving protocol often initialized route
metrics to default value in its import_control hook before export filter
was executed. This is inconsistent with the expectation that an export
filter would process the same route as one in the routing table and it
breaks setting these metrics before (e.g. for static routes directly in
static protocol).

The patch removes the initialization of route metrics in import_control
hook, the default values are already handled in rt_notify hook called
after export filters.

The patch also changed the behavior of OSPF to keep metrics when a route
is reannounced between OSPF instances (to be consistent with other
protocols) and the behavior when both ospf_metric1 and ospf_metric2
are specified (to have more expected behavior).
parent b24b7811
Pipeline #36488 passed with stages
in 8 minutes and 12 seconds
...@@ -1618,9 +1618,7 @@ regarded as empty bgppath/*clist for most purposes. ...@@ -1618,9 +1618,7 @@ regarded as empty bgppath/*clist for most purposes.
The optional attribute that can be used to specify a distance to the The optional attribute that can be used to specify a distance to the
network for routes that do not have a native protocol metric attribute network for routes that do not have a native protocol metric attribute
(like <cf/ospf_metric1/ for OSPF routes). It is used mainly by BGP to (like <cf/ospf_metric1/ for OSPF routes). It is used mainly by BGP to
compare internal distances to boundary routers (see below). It is also compare internal distances to boundary routers (see below).
used when the route is exported to OSPF as a default value for OSPF type
1 metric.
</descrip> </descrip>
<p>There also exist protocol-specific attributes which are described in the <p>There also exist protocol-specific attributes which are described in the
...@@ -3536,8 +3534,15 @@ protocol ospf [v2|v3] &lt;name&gt; { ...@@ -3536,8 +3534,15 @@ protocol ospf [v2|v3] &lt;name&gt; {
with internal <cf/metric/, a <cf/metric of type 2/ is always longer than any with internal <cf/metric/, a <cf/metric of type 2/ is always longer than any
<cf/metric of type 1/ or any <cf/internal metric/. <cf/Internal metric/ or <cf/metric of type 1/ or any <cf/internal metric/. <cf/Internal metric/ or
<cf/metric of type 1/ is stored in attribute <cf/ospf_metric1/, <cf/metric type <cf/metric of type 1/ is stored in attribute <cf/ospf_metric1/, <cf/metric type
2/ is stored in attribute <cf/ospf_metric2/. If you specify both metrics only 2/ is stored in attribute <cf/ospf_metric2/.
metric1 is used.
When both metrics are specified then <cf/metric of type 2/ is used. This is
relevant e.g. when a type 2 external route is propagated from one OSPF domain to
another and <cf/ospf_metric1/ is an internal distance to the original ASBR,
while <cf/ospf_metric2/ stores the type 2 metric. Note that in such cases if
<cf/ospf_metric1/ is non-zero then <cf/ospf_metric2/ is increased by one to
ensure monotonicity of metric, as internal distance is reset to zero when an
external route is announced.
<p>Each external route can also carry attribute <cf/ospf_tag/ which is a 32-bit <p>Each external route can also carry attribute <cf/ospf_tag/ which is a 32-bit
integer which is used when exporting routes to other protocols; otherwise, it integer which is used when exporting routes to other protocols; otherwise, it
......
...@@ -2087,20 +2087,14 @@ babel_prepare_attrs(struct linpool *pool, ea_list *next, uint metric, u64 router ...@@ -2087,20 +2087,14 @@ babel_prepare_attrs(struct linpool *pool, ea_list *next, uint metric, u64 router
static int static int
babel_import_control(struct proto *P, struct rte **new, struct ea_list **attrs, struct linpool *pool) babel_import_control(struct proto *P, struct rte **new, struct ea_list **attrs UNUSED, struct linpool *pool UNUSED)
{ {
struct babel_proto *p = (void *) P; rte *e = *new;
rte *rt = *new;
/* Reject our own unreachable routes */ /* Reject our own unreachable routes */
if ((rt->attrs->dest == RTD_UNREACHABLE) && (rt->attrs->src->proto == P)) if ((e->attrs->dest == RTD_UNREACHABLE) && (e->attrs->src->proto == P))
return -1; return -1;
/* Prepare attributes with initial values */
if (rt->attrs->source != RTS_BABEL)
*attrs = babel_prepare_attrs(pool, NULL, 0, p->router_id);
return 0; return 0;
} }
......
...@@ -446,34 +446,21 @@ ospf_disp(timer * timer) ...@@ -446,34 +446,21 @@ ospf_disp(timer * timer)
* import to the filters. * import to the filters.
*/ */
static int static int
ospf_import_control(struct proto *P, rte **new, ea_list **attrs, struct linpool *pool) ospf_import_control(struct proto *P, rte **new, ea_list **attrs UNUSED, struct linpool *pool UNUSED)
{ {
struct ospf_proto *p = (struct ospf_proto *) P; struct ospf_proto *p = (struct ospf_proto *) P;
struct ospf_area *oa = ospf_main_area(p); struct ospf_area *oa = ospf_main_area(p);
rte *e = *new; rte *e = *new;
/* Reject our own routes */
if (e->attrs->src->proto == P) if (e->attrs->src->proto == P)
return -1; /* Reject our own routes */ return -1;
/* Do not export routes to stub areas */
if (oa_is_stub(oa)) if (oa_is_stub(oa))
return -1; /* Do not export routes to stub areas */ return -1;
ea_list *ea = e->attrs->eattrs;
u32 m0 = ea_get_int(ea, EA_GEN_IGP_METRIC, LSINFINITY);
u32 m1 = MIN(m0, LSINFINITY);
u32 m2 = 10000;
u32 tag = 0;
/* Hack for setting attributes directly in static protocol */
if (e->attrs->source == RTS_STATIC)
{
m1 = ea_get_int(ea, EA_OSPF_METRIC1, m1);
m2 = ea_get_int(ea, EA_OSPF_METRIC2, 10000);
tag = ea_get_int(ea, EA_OSPF_TAG, 0);
}
*attrs = ospf_build_attrs(*attrs, pool, m1, m2, tag, 0); return 0;
return 0; /* Leave decision to the filters */
} }
static struct ea_list * static struct ea_list *
......
...@@ -1281,14 +1281,34 @@ ospf_rt_notify(struct proto *P, struct channel *ch UNUSED, net *n, rte *new, rte ...@@ -1281,14 +1281,34 @@ ospf_rt_notify(struct proto *P, struct channel *ch UNUSED, net *n, rte *new, rte
/* Get route attributes */ /* Get route attributes */
rta *a = new->attrs; rta *a = new->attrs;
u32 m1 = ea_get_int(ea, EA_OSPF_METRIC1, LSINFINITY); eattr *m1a = ea_find(ea, EA_OSPF_METRIC1);
u32 m2 = ea_get_int(ea, EA_OSPF_METRIC2, 10000); eattr *m2a = ea_find(ea, EA_OSPF_METRIC2);
int ebit = (m1 == LSINFINITY); uint m1 = m1a ? m1a->u.data : 0;
u32 metric = ebit ? m2 : m1; uint m2 = m2a ? m2a->u.data : 10000;
u32 tag = ea_get_int(ea, EA_OSPF_TAG, 0);
ip_addr fwd = IPA_NONE;
if (m1 > LSINFINITY)
{
log(L_WARN "%s: Invalid ospf_metric1 value %u for route %N",
p->p.name, m1, n->n.addr);
m1 = LSINFINITY;
}
if (m2 > LSINFINITY)
{
log(L_WARN "%s: Invalid ospf_metric2 value %u for route %N",
p->p.name, m2, n->n.addr);
m2 = LSINFINITY;
}
/* Ensure monotonicity of metric if both m1 and m2 are used */
if ((m1 > 0) && (m2 < LSINFINITY))
m2++;
uint ebit = m2a || !m1a;
uint metric = ebit ? m2 : m1;
uint tag = ea_get_int(ea, EA_OSPF_TAG, 0);
ip_addr fwd = IPA_NONE;
if ((a->dest == RTD_UNICAST) && use_gw_for_fwaddr(p, a->nh.gw, a->nh.iface)) if ((a->dest == RTD_UNICAST) && use_gw_for_fwaddr(p, a->nh.gw, a->nh.iface))
fwd = a->nh.gw; fwd = a->nh.gw;
......
...@@ -1020,16 +1020,6 @@ rip_prepare_attrs(struct linpool *pool, ea_list *next, u8 metric, u16 tag) ...@@ -1020,16 +1020,6 @@ rip_prepare_attrs(struct linpool *pool, ea_list *next, u8 metric, u16 tag)
return l; return l;
} }
static int
rip_import_control(struct proto *P UNUSED, struct rte **rt, struct ea_list **attrs, struct linpool *pool)
{
/* Prepare attributes with initial values */
if ((*rt)->attrs->source != RTS_RIP)
*attrs = rip_prepare_attrs(pool, *attrs, 1, 0);
return 0;
}
static void static void
rip_reload_routes(struct channel *C) rip_reload_routes(struct channel *C)
{ {
...@@ -1091,7 +1081,7 @@ rip_init(struct proto_config *CF) ...@@ -1091,7 +1081,7 @@ rip_init(struct proto_config *CF)
P->if_notify = rip_if_notify; P->if_notify = rip_if_notify;
P->rt_notify = rip_rt_notify; P->rt_notify = rip_rt_notify;
P->neigh_notify = rip_neigh_notify; P->neigh_notify = rip_neigh_notify;
P->import_control = rip_import_control; // P->import_control = rip_import_control;
P->reload_routes = rip_reload_routes; P->reload_routes = rip_reload_routes;
P->make_tmp_attrs = rip_make_tmp_attrs; P->make_tmp_attrs = rip_make_tmp_attrs;
P->store_tmp_attrs = rip_store_tmp_attrs; P->store_tmp_attrs = rip_store_tmp_attrs;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment