From: Ondrej Zajicek (work) Date: Thu, 24 May 2018 12:51:05 +0000 (+0200) Subject: Do not initialize route metrics in import_control hook X-Git-Tag: v2.0.3~90 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=feae132e0f9bdc62d2b90bf676d12241af8e794c;p=thirdparty%2Fbird.git 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). --- diff --git a/doc/bird.sgml b/doc/bird.sgml index ae308d4cd..b4d9ea4e9 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -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 network for routes that do not have a native protocol metric attribute (like

There also exist protocol-specific attributes which are described in the @@ -3536,8 +3534,15 @@ protocol ospf [v2|v3] <name> { with internal Each external route can also carry attribute attrs->dest == RTD_UNREACHABLE) && (rt->attrs->src->proto == P)) + if ((e->attrs->dest == RTD_UNREACHABLE) && (e->attrs->src->proto == P)) 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; } diff --git a/proto/ospf/ospf.c b/proto/ospf/ospf.c index df6c452e7..0a9efd195 100644 --- a/proto/ospf/ospf.c +++ b/proto/ospf/ospf.c @@ -446,34 +446,21 @@ ospf_disp(timer * timer) * import to the filters. */ 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_area *oa = ospf_main_area(p); rte *e = *new; + /* Reject our own routes */ 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)) - return -1; /* Do not export routes to stub areas */ - - 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); - } + return -1; - *attrs = ospf_build_attrs(*attrs, pool, m1, m2, tag, 0); - return 0; /* Leave decision to the filters */ + return 0; } static struct ea_list * diff --git a/proto/ospf/topology.c b/proto/ospf/topology.c index 717c82808..e909bbe99 100644 --- a/proto/ospf/topology.c +++ b/proto/ospf/topology.c @@ -1281,14 +1281,34 @@ ospf_rt_notify(struct proto *P, struct channel *ch UNUSED, net *n, rte *new, rte /* Get route attributes */ rta *a = new->attrs; - u32 m1 = ea_get_int(ea, EA_OSPF_METRIC1, LSINFINITY); - u32 m2 = ea_get_int(ea, EA_OSPF_METRIC2, 10000); - int ebit = (m1 == LSINFINITY); - u32 metric = ebit ? m2 : m1; - u32 tag = ea_get_int(ea, EA_OSPF_TAG, 0); - ip_addr fwd = IPA_NONE; + eattr *m1a = ea_find(ea, EA_OSPF_METRIC1); + eattr *m2a = ea_find(ea, EA_OSPF_METRIC2); + uint m1 = m1a ? m1a->u.data : 0; + uint m2 = m2a ? m2a->u.data : 10000; + 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)) fwd = a->nh.gw; diff --git a/proto/rip/rip.c b/proto/rip/rip.c index 85e37cea2..baf987374 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -1020,16 +1020,6 @@ rip_prepare_attrs(struct linpool *pool, ea_list *next, u8 metric, u16 tag) 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 rip_reload_routes(struct channel *C) { @@ -1091,7 +1081,7 @@ rip_init(struct proto_config *CF) P->if_notify = rip_if_notify; P->rt_notify = rip_rt_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->make_tmp_attrs = rip_make_tmp_attrs; P->store_tmp_attrs = rip_store_tmp_attrs;