From 8babeb1add39ff1f63b8505bbdd75e75b8783a86 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 11 Sep 2019 14:22:55 -0400 Subject: pbrd: Handle GATEWAY_IFINDEX nht conflicts In pbrd we did not care if the nexthop interface nexthop tracking sent us back did not match the one specified with `nexthop [GATEWAY] [INTERFACE]`. This happened if the gateway was resolvable via a different interface and the inteface we specified in the config was unnumbered (no ipv4 address on it) since the default route gets forced onlink when it gets into zebra. This patch adds a check to not install the rule if the interface we got back was different from the specified. This patch also reworks the nexthop update path to make it a little more clear what its doing. Signed-off-by: Stephen Worley --- pbrd/pbr_nht.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 104 insertions(+), 16 deletions(-) (limited to 'pbrd/pbr_nht.c') diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index 57e8cf574..359f869c4 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -682,29 +682,119 @@ struct pbr_nht_individual { uint32_t valid; }; -static void pbr_nht_individual_nexthop_update_lookup(struct hash_bucket *b, - void *data) +static bool +pbr_nht_individual_nexthop_gw_update(struct pbr_nexthop_cache *pnhc, + const struct pbr_nht_individual *pnhi) { - struct pbr_nexthop_cache *pnhc = b->data; - struct pbr_nht_individual *pnhi = data; - char buf[PREFIX_STRLEN]; - bool old_valid; + bool is_valid = pnhc->valid; - old_valid = pnhc->valid; + if (!pnhi->nhr) /* It doesn't care about non-nexthop updates */ + goto done; switch (pnhi->nhr->prefix.family) { case AF_INET: if (pnhc->nexthop->gate.ipv4.s_addr - == pnhi->nhr->prefix.u.prefix4.s_addr) - pnhc->valid = !!pnhi->nhr->nexthop_num; + != pnhi->nhr->prefix.u.prefix4.s_addr) + goto done; /* Unrelated change */ break; case AF_INET6: if (memcmp(&pnhc->nexthop->gate.ipv6, &pnhi->nhr->prefix.u.prefix6, 16) - == 0) - pnhc->valid = !!pnhi->nhr->nexthop_num; + != 0) + goto done; /* Unrelated change */ + break; + } + + if (!pnhi->nhr->nexthop_num) { + is_valid = false; + goto done; + } + + if (pnhc->nexthop->type == NEXTHOP_TYPE_IPV4_IFINDEX + || pnhc->nexthop->type == NEXTHOP_TYPE_IPV4_IFINDEX) { + + /* GATEWAY_IFINDEX type shouldn't resolve to group */ + if (pnhi->nhr->nexthop_num > 1) { + is_valid = false; + goto done; + } + + /* If whatever we resolved to wasn't on the interface we + * specified. (i.e. not a connected route), its invalid. + */ + if (pnhi->nhr->nexthops[0].ifindex != pnhc->nexthop->ifindex) { + is_valid = false; + goto done; + } + } + + is_valid = true; + +done: + pnhc->valid = is_valid; + + return pnhc->valid; +} + +static bool pbr_nht_individual_nexthop_interface_update( + struct pbr_nexthop_cache *pnhc, const struct pbr_nht_individual *pnhi) +{ + bool is_valid = pnhc->valid; + + if (!pnhi->ifp) /* It doesn't care about non-interface updates */ + goto done; + + if (pnhc->nexthop->ifindex + != pnhi->ifp->ifindex) /* Un-related interface */ + goto done; + + is_valid = !!if_is_up(pnhi->ifp); + +done: + pnhc->valid = is_valid; + + return pnhc->valid; +} + +/* Given this update either from interface or nexthop tracking, re-validate this + * nexthop. + * + * If the update is un-related, the subroutines shoud just return their cached + * valid state. + */ +static void +pbr_nht_individual_nexthop_update(struct pbr_nexthop_cache *pnhc, + const struct pbr_nht_individual *pnhi) +{ + assert(pnhi->nhr || pnhi->ifp); /* Either nexthop or interface update */ + + switch (pnhc->nexthop->type) { + case NEXTHOP_TYPE_IFINDEX: + pbr_nht_individual_nexthop_interface_update(pnhc, pnhi); + break; + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV4_IFINDEX: + case NEXTHOP_TYPE_IPV6_IFINDEX: + pbr_nht_individual_nexthop_gw_update(pnhc, pnhi); + break; + case NEXTHOP_TYPE_BLACKHOLE: + pnhc->valid = true; break; } +} + +static void pbr_nht_individual_nexthop_update_lookup(struct hash_bucket *b, + void *data) +{ + struct pbr_nexthop_cache *pnhc = b->data; + struct pbr_nht_individual *pnhi = data; + char buf[PREFIX_STRLEN]; + bool old_valid; + + old_valid = pnhc->valid; + + pbr_nht_individual_nexthop_update(pnhc, pnhi); DEBUGD(&pbr_dbg_nht, "\tFound %s: old: %d new: %d", prefix2str(&pnhi->nhr->prefix, buf, sizeof(buf)), old_valid, @@ -736,7 +826,7 @@ pbr_nexthop_group_cache_to_nexthop_group(struct nexthop_group *nhg, static void pbr_nht_nexthop_update_lookup(struct hash_bucket *b, void *data) { struct pbr_nexthop_group_cache *pnhgc = b->data; - struct pbr_nht_individual pnhi; + struct pbr_nht_individual pnhi = {}; struct nexthop_group nhg = {}; bool old_valid; @@ -778,9 +868,7 @@ pbr_nht_individual_nexthop_interface_update_lookup(struct hash_backet *b, old_valid = pnhc->valid; - if (pnhc->nexthop->type == NEXTHOP_TYPE_IFINDEX - && pnhc->nexthop->ifindex == pnhi->ifp->ifindex) - pnhc->valid = !!if_is_up(pnhi->ifp); + pbr_nht_individual_nexthop_update(pnhc, pnhi); DEBUGD(&pbr_dbg_nht, "\tFound %s: old: %d new: %d", pnhi->ifp->name, old_valid, pnhc->valid); @@ -793,7 +881,7 @@ static void pbr_nht_nexthop_interface_update_lookup(struct hash_backet *b, void *data) { struct pbr_nexthop_group_cache *pnhgc = b->data; - struct pbr_nht_individual pnhi; + struct pbr_nht_individual pnhi = {}; bool old_valid; old_valid = pnhgc->valid; -- cgit v1.2.3