From e5a501c2edd1f3f18ac0ce4c5d415c97cbfd0cc6 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 14 Feb 2019 20:00:14 -0200 Subject: [PATCH 1/8] lib: consolidate nexthop-group deletion in a single place Reuse the nhgl_delete() function to avoid code duplication. Signed-off-by: Renato Westphal --- lib/nexthop_group.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 23ea96f75c..57609b648f 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -322,13 +322,7 @@ static void nexthop_group_unsave_nhop(struct nexthop_group_cmd *nhgc, return; list_delete_node(nhgc->nhg_list, node); - - if (nh->nhvrf_name) - XFREE(MTYPE_TMP, nh->nhvrf_name); - if (nh->intf) - XFREE(MTYPE_TMP, nh->intf); - - XFREE(MTYPE_TMP, nh); + nhgl_delete(nh); } static bool nexthop_group_parse_nexthop(struct nexthop *nhop, From b43bb64f82f599ec9fec0cb89bd2dd05cb54ab87 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 14 Feb 2019 20:00:14 -0200 Subject: [PATCH 2/8] lib: change how nexthop groups store nexthop addresses Use a pointer to a sockunion instead of a full sockunion in the nexthop_hold structure. This prepares the ground for the next commit, which will make nexthop addresses optional (in this commit we assume nh->addr will never be NULL, but this will change). Signed-off-by: Renato Westphal --- lib/nexthop_group.c | 16 +++++++++------- lib/nexthop_group.h | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 57609b648f..4c35d7c96c 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -192,7 +192,7 @@ static int nhgl_cmp(struct nexthop_hold *nh1, struct nexthop_hold *nh2) { int ret; - ret = sockunion_cmp(&nh1->addr, &nh2->addr); + ret = sockunion_cmp(nh1->addr, nh2->addr); if (ret) return ret; @@ -211,6 +211,8 @@ static void nhgl_delete(struct nexthop_hold *nh) if (nh->nhvrf_name) XFREE(MTYPE_TMP, nh->nhvrf_name); + sockunion_free(nh->addr); + XFREE(MTYPE_TMP, nh); } @@ -295,7 +297,7 @@ static void nexthop_group_save_nhop(struct nexthop_group_cmd *nhgc, if (intf) nh->intf = XSTRDUP(MTYPE_TMP, intf); - nh->addr = *addr; + nh->addr = sockunion_dup(addr); listnode_add_sort(nhgc->nhg_list, nh); } @@ -310,7 +312,7 @@ static void nexthop_group_unsave_nhop(struct nexthop_group_cmd *nhgc, for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_list, node, nh)) { if (nhgc_cmp_helper(nhvrf_name, nh->nhvrf_name) == 0 && - sockunion_cmp(addr, &nh->addr) == 0 && + sockunion_cmp(addr, nh->addr) == 0 && nhgc_cmp_helper(intf, nh->intf) == 0) break; } @@ -478,7 +480,7 @@ static void nexthop_group_write_nexthop_internal(struct vty *vty, vty_out(vty, "nexthop "); - vty_out(vty, "%s", sockunion2str(&nh->addr, buf, sizeof(buf))); + vty_out(vty, "%s", sockunion2str(nh->addr, buf, sizeof(buf))); if (nh->intf) vty_out(vty, " %s", nh->intf); @@ -522,7 +524,7 @@ void nexthop_group_enable_vrf(struct vrf *vrf) struct nexthop nhop; struct nexthop *nh; - if (!nexthop_group_parse_nexthop(&nhop, &nhh->addr, + if (!nexthop_group_parse_nexthop(&nhop, nhh->addr, nhh->intf, nhh->nhvrf_name)) continue; @@ -558,7 +560,7 @@ void nexthop_group_disable_vrf(struct vrf *vrf) struct nexthop nhop; struct nexthop *nh; - if (!nexthop_group_parse_nexthop(&nhop, &nhh->addr, + if (!nexthop_group_parse_nexthop(&nhop, nhh->addr, nhh->intf, nhh->nhvrf_name)) continue; @@ -596,7 +598,7 @@ void nexthop_group_interface_state_change(struct interface *ifp, struct nexthop nhop; if (!nexthop_group_parse_nexthop( - &nhop, &nhh->addr, nhh->intf, + &nhop, nhh->addr, nhh->intf, nhh->nhvrf_name)) continue; diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index b14cbb5b5c..c6e290eeea 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -68,7 +68,7 @@ void copy_nexthops(struct nexthop **tnh, struct nexthop *nh, struct nexthop_hold { char *nhvrf_name; - union sockunion addr; + union sockunion *addr; char *intf; }; From 1c869b64180e9a3d33e9bfec7113418231489522 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 14 Feb 2019 20:00:14 -0200 Subject: [PATCH 3/8] lib: add support for interface nexthops on nexthop groups This patch adds support to nexthops of type NEXTHOP_TYPE_IFINDEX to nexthop-groups. This should be especially useful when dealing with p2p interfaces like tunnels that don't have IP addresses assigned to them. NOTE: nh->addr can be NULL now, so we should always perform a null check before dereferencing this pointer. Signed-off-by: Renato Westphal --- lib/nexthop_group.c | 84 +++++++++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 4c35d7c96c..d8cabf62be 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -188,11 +188,25 @@ static int nhgc_cmp_helper(const char *a, const char *b) return strcmp(a, b); } +static int nhgc_addr_cmp_helper(const union sockunion *a, const union sockunion *b) +{ + if (!a && !b) + return 0; + + if (a && !b) + return -1; + + if (!a && b) + return 1; + + return sockunion_cmp(a, b); +} + static int nhgl_cmp(struct nexthop_hold *nh1, struct nexthop_hold *nh2) { int ret; - ret = sockunion_cmp(nh1->addr, nh2->addr); + ret = nhgc_addr_cmp_helper(nh1->addr, nh2->addr); if (ret) return ret; @@ -211,7 +225,8 @@ static void nhgl_delete(struct nexthop_hold *nh) if (nh->nhvrf_name) XFREE(MTYPE_TMP, nh->nhvrf_name); - sockunion_free(nh->addr); + if (nh->addr) + sockunion_free(nh->addr); XFREE(MTYPE_TMP, nh); } @@ -296,8 +311,8 @@ static void nexthop_group_save_nhop(struct nexthop_group_cmd *nhgc, nh->nhvrf_name = XSTRDUP(MTYPE_TMP, nhvrf_name); if (intf) nh->intf = XSTRDUP(MTYPE_TMP, intf); - - nh->addr = sockunion_dup(addr); + if (addr) + nh->addr = sockunion_dup(addr); listnode_add_sort(nhgc->nhg_list, nh); } @@ -312,7 +327,7 @@ static void nexthop_group_unsave_nhop(struct nexthop_group_cmd *nhgc, for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_list, node, nh)) { if (nhgc_cmp_helper(nhvrf_name, nh->nhvrf_name) == 0 && - sockunion_cmp(addr, nh->addr) == 0 && + nhgc_addr_cmp_helper(addr, nh->addr) == 0 && nhgc_cmp_helper(intf, nh->intf) == 0) break; } @@ -345,36 +360,45 @@ static bool nexthop_group_parse_nexthop(struct nexthop *nhop, nhop->vrf_id = vrf->vrf_id; - if (addr->sa.sa_family == AF_INET) { - nhop->gate.ipv4.s_addr = addr->sin.sin_addr.s_addr; - if (intf) { - nhop->type = NEXTHOP_TYPE_IPV4_IFINDEX; - nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id); - if (nhop->ifindex == IFINDEX_INTERNAL) - return false; - } else - nhop->type = NEXTHOP_TYPE_IPV4; - } else { - memcpy(&nhop->gate.ipv6, &addr->sin6.sin6_addr, 16); - if (intf) { - nhop->type = NEXTHOP_TYPE_IPV6_IFINDEX; - nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id); - if (nhop->ifindex == IFINDEX_INTERNAL) - return false; - } else - nhop->type = NEXTHOP_TYPE_IPV6; + if (intf) { + nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id); + if (nhop->ifindex == IFINDEX_INTERNAL) + return false; } + if (addr) { + if (addr->sa.sa_family == AF_INET) { + nhop->gate.ipv4.s_addr = addr->sin.sin_addr.s_addr; + if (intf) + nhop->type = NEXTHOP_TYPE_IPV4_IFINDEX; + else + nhop->type = NEXTHOP_TYPE_IPV4; + } else { + nhop->gate.ipv6 = addr->sin6.sin6_addr; + if (intf) + nhop->type = NEXTHOP_TYPE_IPV6_IFINDEX; + else + nhop->type = NEXTHOP_TYPE_IPV6; + } + } else + nhop->type = NEXTHOP_TYPE_IFINDEX; + return true; } DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, - "[no] nexthop $addr [INTERFACE]$intf [nexthop-vrf NAME$name]", + "[no] nexthop\ + <\ + $addr [INTERFACE$intf]\ + |INTERFACE$intf\ + >\ + [nexthop-vrf NAME$name]", NO_STR "Specify one of the nexthops in this ECMP group\n" "v4 Address\n" "v6 Address\n" "Interface to use\n" + "Interface to use\n" "If the nexthop is in a different vrf tell us\n" "The nexthop-vrf Name\n") { @@ -383,13 +407,6 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, struct nexthop *nh; bool legal; - /* - * This is impossible to happen as that the cli parser refuses - * to let you get here without an addr, but the SA system - * does not understand this intricacy - */ - assert(addr); - legal = nexthop_group_parse_nexthop(&nhop, addr, intf, name); if (nhop.type == NEXTHOP_TYPE_IPV6 @@ -478,9 +495,10 @@ static void nexthop_group_write_nexthop_internal(struct vty *vty, { char buf[100]; - vty_out(vty, "nexthop "); + vty_out(vty, "nexthop"); - vty_out(vty, "%s", sockunion2str(nh->addr, buf, sizeof(buf))); + if (nh->addr) + vty_out(vty, " %s", sockunion2str(nh->addr, buf, sizeof(buf))); if (nh->intf) vty_out(vty, " %s", nh->intf); From aafac994dc97702821563ed05d9513f802438a31 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 14 Feb 2019 20:00:14 -0200 Subject: [PATCH 4/8] pbrd: rename nh_afi variables to nh_type to better convey their meaning Signed-off-by: Renato Westphal --- pbrd/pbr_nht.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index 6103bd7db5..bc40caf1a0 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -50,7 +50,7 @@ static void pbr_nht_install_nexthop_group(struct pbr_nexthop_group_cache *pnhgc, static void pbr_nht_uninstall_nexthop_group(struct pbr_nexthop_group_cache *pnhgc, struct nexthop_group nhg, - enum nexthop_types_t nh_afi); + enum nexthop_types_t nh_type); /* * Nexthop refcount. @@ -274,7 +274,7 @@ void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, struct pbr_nexthop_group_cache *pnhgc; struct pbr_nexthop_cache pnhc_find = {}; struct pbr_nexthop_cache *pnhc; - enum nexthop_types_t nh_afi = nhop->type; + enum nexthop_types_t nh_type = nhop->type; /* find pnhgc by name */ strlcpy(pnhgc_find.name, nhgc->name, sizeof(pnhgc_find.name)); @@ -296,7 +296,7 @@ void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, if (pnhgc->nhh->count) pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg); else - pbr_nht_uninstall_nexthop_group(pnhgc, nhgc->nhg, nh_afi); + pbr_nht_uninstall_nexthop_group(pnhgc, nhgc->nhg, nh_type); pbr_map_check_nh_group_change(nhgc->name); } @@ -372,7 +372,7 @@ void pbr_nht_route_removed_for_table(uint32_t table_id) * - AFI_MAX on error */ static afi_t pbr_nht_which_afi(struct nexthop_group nhg, - enum nexthop_types_t nh_afi) + enum nexthop_types_t nh_type) { struct nexthop *nexthop; afi_t install_afi = AFI_MAX; @@ -380,14 +380,14 @@ static afi_t pbr_nht_which_afi(struct nexthop_group nhg, v6 = v4 = bh = false; - if (!nh_afi) { + if (!nh_type) { for (ALL_NEXTHOPS(nhg, nexthop)) { - nh_afi = nexthop->type; + nh_type = nexthop->type; break; } } - switch (nh_afi) { + switch (nh_type) { case NEXTHOP_TYPE_IFINDEX: break; case NEXTHOP_TYPE_IPV4: @@ -423,9 +423,9 @@ static void pbr_nht_install_nexthop_group(struct pbr_nexthop_group_cache *pnhgc, struct nexthop_group nhg) { afi_t install_afi; - enum nexthop_types_t nh_afi = 0; + enum nexthop_types_t nh_type = 0; - install_afi = pbr_nht_which_afi(nhg, nh_afi); + install_afi = pbr_nht_which_afi(nhg, nh_type); route_add(pnhgc, nhg, install_afi); } @@ -433,11 +433,11 @@ static void pbr_nht_install_nexthop_group(struct pbr_nexthop_group_cache *pnhgc, static void pbr_nht_uninstall_nexthop_group(struct pbr_nexthop_group_cache *pnhgc, struct nexthop_group nhg, - enum nexthop_types_t nh_afi) + enum nexthop_types_t nh_type) { afi_t install_afi; - install_afi = pbr_nht_which_afi(nhg, nh_afi); + install_afi = pbr_nht_which_afi(nhg, nh_type); pnhgc->installed = false; pnhgc->valid = false; @@ -526,7 +526,7 @@ void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms) struct listnode *node; struct pbr_map_interface *pmi; struct nexthop *nh; - enum nexthop_types_t nh_afi = 0; + enum nexthop_types_t nh_type = 0; if (pbrm->valid && pbrms->nhs_installed && pbrm->incoming->count) { for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, node, pmi)) @@ -542,13 +542,13 @@ void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms) pnhgc = hash_lookup(pbr_nhg_hash, &find); nh = pbrms->nhg->nexthop; - nh_afi = nh->type; + nh_type = nh->type; lup.nexthop = nh; pnhc = hash_lookup(pnhgc->nhh, &lup); pnhc->parent = NULL; hash_release(pnhgc->nhh, pnhc); pbr_nh_delete(&pnhc); - pbr_nht_uninstall_nexthop_group(pnhgc, *pbrms->nhg, nh_afi); + pbr_nht_uninstall_nexthop_group(pnhgc, *pbrms->nhg, nh_type); hash_release(pbr_nhg_hash, pnhgc); From 268c24ee9ee2510d6b4922053285254644609a0f Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 14 Feb 2019 20:00:15 -0200 Subject: [PATCH 5/8] pbrd: fix detection of inconsistent nexthop groups Commit ff9799c31 broke the detection of nexthop groups that contain both v4 and v6 nexthops. Move the switch statement back to the ALL_NEXTHOPS loop to fix this issue. Further, make pbr_nht_which_afi() return AFI_MAX only if all nexthops from the group are either NEXTHOP_TYPE_IFINDEX or NEXTHOP_TYPE_BLACKHOLE. Signed-off-by: Renato Westphal --- pbrd/pbr_nht.c | 54 +++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index bc40caf1a0..e196b4fe2c 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -378,33 +378,47 @@ static afi_t pbr_nht_which_afi(struct nexthop_group nhg, afi_t install_afi = AFI_MAX; bool v6, v4, bh; + if (nh_type) { + switch (nh_type) { + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: + return AFI_IP; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: + return AFI_IP6; + case NEXTHOP_TYPE_IFINDEX: + case NEXTHOP_TYPE_BLACKHOLE: + return AFI_MAX; + } + } + v6 = v4 = bh = false; - if (!nh_type) { - for (ALL_NEXTHOPS(nhg, nexthop)) { - nh_type = nexthop->type; + for (ALL_NEXTHOPS(nhg, nexthop)) { + nh_type = nexthop->type; + + switch (nh_type) { + case NEXTHOP_TYPE_IFINDEX: + break; + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: + v6 = true; + install_afi = AFI_IP; + break; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: + v4 = true; + install_afi = AFI_IP6; + break; + case NEXTHOP_TYPE_BLACKHOLE: + bh = true; break; } } - switch (nh_type) { - case NEXTHOP_TYPE_IFINDEX: - break; - case NEXTHOP_TYPE_IPV4: - case NEXTHOP_TYPE_IPV4_IFINDEX: - v6 = true; - install_afi = AFI_IP; - break; - case NEXTHOP_TYPE_IPV6: - case NEXTHOP_TYPE_IPV6_IFINDEX: - v4 = true; - install_afi = AFI_IP6; - break; - case NEXTHOP_TYPE_BLACKHOLE: - bh = true; + /* Interface and/or blackhole nexthops only. */ + if (!v4 && !v6) install_afi = AFI_MAX; - break; - } if (!bh && v6 && v4) DEBUGD(&pbr_dbg_nht, From a106a4087b47e180e32c66e139449bcf2660578f Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 14 Feb 2019 20:00:15 -0200 Subject: [PATCH 6/8] pbrd: add support for interface nexthops Now that nexthop groups can contain interface nexthops, make the necessary adjustments in pbrd to handle them appropriately. For normal IP nexthops, pbrd uses the NHT callbacks to validate these nexthops (i.e. check if they are reachable). NHT can't be used for interface nexthops though. To work around this issue, use the interface event callbacks from the zclient API to validate interface nexthops (an interface nexthop is valid only if the corresponding interface is up and running). Signed-off-by: Renato Westphal --- pbrd/pbr_nht.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++- pbrd/pbr_nht.h | 5 ++++ pbrd/pbr_zebra.c | 6 +++++ 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index e196b4fe2c..b269ce0cdf 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -157,7 +157,7 @@ static bool pbr_nh_hash_equal(const void *arg1, const void *arg2) switch (pbrnc1->nexthop->type) { case NEXTHOP_TYPE_IFINDEX: - return true; + return pbrnc1->nexthop->ifindex == pbrnc2->nexthop->ifindex; case NEXTHOP_TYPE_IPV4_IFINDEX: case NEXTHOP_TYPE_IPV4: return pbrnc1->nexthop->gate.ipv4.s_addr @@ -264,6 +264,14 @@ void pbr_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc, pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg); pbr_map_check_nh_group_change(nhgc->name); + + if (nhop->type == NEXTHOP_TYPE_IFINDEX) { + struct interface *ifp; + + ifp = if_lookup_by_index(nhop->ifindex, nhop->vrf_id); + if (ifp) + pbr_nht_nexthop_interface_update(ifp); + } } void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, @@ -667,6 +675,7 @@ bool pbr_nht_nexthop_group_valid(const char *name) struct pbr_nht_individual { struct zapi_route *nhr; + struct interface *ifp; uint32_t valid; }; @@ -730,6 +739,56 @@ void pbr_nht_nexthop_update(struct zapi_route *nhr) hash_iterate(pbr_nhg_hash, pbr_nht_nexthop_update_lookup, nhr); } +static void +pbr_nht_individual_nexthop_interface_update_lookup(struct hash_backet *b, + void *data) +{ + struct pbr_nexthop_cache *pnhc = b->data; + struct pbr_nht_individual *pnhi = data; + bool old_valid; + + old_valid = pnhc->valid; + + if (pnhc->nexthop->type == NEXTHOP_TYPE_IFINDEX + && pnhc->nexthop->ifindex == pnhi->ifp->ifindex) + pnhc->valid = !!if_is_up(pnhi->ifp); + + DEBUGD(&pbr_dbg_nht, "\tFound %s: old: %d new: %d", pnhi->ifp->name, + old_valid, pnhc->valid); + + if (pnhc->valid) + pnhi->valid += 1; +} + +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; + bool old_valid; + + old_valid = pnhgc->valid; + + pnhi.ifp = data; + pnhi.valid = 0; + hash_iterate(pnhgc->nhh, + pbr_nht_individual_nexthop_interface_update_lookup, &pnhi); + + /* + * If any of the specified nexthops are valid we are valid + */ + pnhgc->valid = !!pnhi.valid; + + if (old_valid != pnhgc->valid) + pbr_map_check_nh_group_change(pnhgc->name); +} + +void pbr_nht_nexthop_interface_update(struct interface *ifp) +{ + hash_iterate(pbr_nhg_hash, pbr_nht_nexthop_interface_update_lookup, + ifp); +} + static uint32_t pbr_nhg_hash_key(void *arg) { struct pbr_nexthop_group_cache *nhgc = diff --git a/pbrd/pbr_nht.h b/pbrd/pbr_nht.h index d37803fbe3..4ef41cede7 100644 --- a/pbrd/pbr_nht.h +++ b/pbrd/pbr_nht.h @@ -117,5 +117,10 @@ extern void pbr_nht_show_nexthop_group(struct vty *vty, const char *name); */ extern void pbr_nht_nexthop_update(struct zapi_route *nhr); +/* + * When we get a callback from zebra about an interface status update. + */ +extern void pbr_nht_nexthop_interface_update(struct interface *ifp); + extern void pbr_nht_init(void); #endif diff --git a/pbrd/pbr_zebra.c b/pbrd/pbr_zebra.c index 425bc04b4d..37209d4819 100644 --- a/pbrd/pbr_zebra.c +++ b/pbrd/pbr_zebra.c @@ -75,6 +75,8 @@ static int interface_add(int command, struct zclient *zclient, if (!ifp->info) pbr_if_new(ifp); + pbr_nht_nexthop_interface_update(ifp); + return 0; } @@ -144,6 +146,8 @@ static int interface_state_up(int command, struct zclient *zclient, DEBUGD(&pbr_dbg_zebra, "%s: %s is up", __PRETTY_FUNCTION__, ifp->name); + pbr_nht_nexthop_interface_update(ifp); + return 0; } @@ -157,6 +161,8 @@ static int interface_state_down(int command, struct zclient *zclient, DEBUGD(&pbr_dbg_zebra, "%s: %s is down", __PRETTY_FUNCTION__, ifp->name); + pbr_nht_nexthop_interface_update(ifp); + return 0; } From 9c0fd85360422b79e75e6ee5eeb1df5a9439e952 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 14 Feb 2019 20:00:15 -0200 Subject: [PATCH 7/8] pbrd: change the "set nexthop" command to accept interface nexthops In addition to nexthop groups, pbrd also supports the "set nexthop" command to specify the nexthop of a PBR map. This adds convenience when multiple nexthops aren't necessary. Change this command to support interface nexthops (without IP addresses) like nexthop groups do. At the end of the command, call pbr_nht_nexthop_interface_update() otherwise the interface nexthop won't be validated until we receive an interface up/down notification from zebra through the zapi protocol. Signed-off-by: Renato Westphal --- pbrd/pbr_vty.c | 82 +++++++++++++++++++++++++++----------------------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c index a4b87f99d9..0aedb98476 100644 --- a/pbrd/pbr_vty.c +++ b/pbrd/pbr_vty.c @@ -221,13 +221,19 @@ DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd, } DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, - "[no] set nexthop $addr [INTERFACE]$intf [nexthop-vrf NAME$name]", + "[no] set nexthop\ + <\ + $addr [INTERFACE$intf]\ + |INTERFACE$intf\ + >\ + [nexthop-vrf NAME$name]", NO_STR "Set for the PBR-MAP\n" "Specify one of the nexthops in this map\n" "v4 Address\n" "v6 Address\n" "Interface to use\n" + "Interface to use\n" "If the nexthop is in a different vrf tell us\n" "The nexthop-vrf Name\n") { @@ -255,45 +261,39 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, memset(&nhop, 0, sizeof(nhop)); nhop.vrf_id = vrf->vrf_id; - /* - * Make SA happy. CLIPPY is not going to give us a NULL - * addr. - */ - assert(addr); - if (addr->sa.sa_family == AF_INET) { - nhop.gate.ipv4.s_addr = addr->sin.sin_addr.s_addr; - if (intf) { - nhop.type = NEXTHOP_TYPE_IPV4_IFINDEX; - nhop.ifindex = ifname2ifindex(intf, vrf->vrf_id); - if (nhop.ifindex == IFINDEX_INTERNAL) { - vty_out(vty, - "Specified Intf %s does not exist in vrf: %s\n", - intf, vrf->name); - return CMD_WARNING_CONFIG_FAILED; - } - } else - nhop.type = NEXTHOP_TYPE_IPV4; - } else { - memcpy(&nhop.gate.ipv6, &addr->sin6.sin6_addr, 16); - if (intf) { - nhop.type = NEXTHOP_TYPE_IPV6_IFINDEX; - nhop.ifindex = ifname2ifindex(intf, vrf->vrf_id); - if (nhop.ifindex == IFINDEX_INTERNAL) { - vty_out(vty, - "Specified Intf %s does not exist in vrf: %s\n", - intf, vrf->name); - return CMD_WARNING_CONFIG_FAILED; - } - } else { - if (IN6_IS_ADDR_LINKLOCAL(&nhop.gate.ipv6)) { - vty_out(vty, - "Specified a v6 LL with no interface, rejecting\n"); - return CMD_WARNING_CONFIG_FAILED; - } - nhop.type = NEXTHOP_TYPE_IPV6; + if (intf) { + nhop.ifindex = ifname2ifindex(intf, vrf->vrf_id); + if (nhop.ifindex == IFINDEX_INTERNAL) { + vty_out(vty, + "Specified Intf %s does not exist in vrf: %s\n", + intf, vrf->name); + return CMD_WARNING_CONFIG_FAILED; } } + if (addr) { + if (addr->sa.sa_family == AF_INET) { + nhop.gate.ipv4.s_addr = addr->sin.sin_addr.s_addr; + if (intf) + nhop.type = NEXTHOP_TYPE_IPV4_IFINDEX; + else + nhop.type = NEXTHOP_TYPE_IPV4; + } else { + nhop.gate.ipv6 = addr->sin6.sin6_addr; + if (intf) + nhop.type = NEXTHOP_TYPE_IPV6_IFINDEX; + else { + if (IN6_IS_ADDR_LINKLOCAL(&nhop.gate.ipv6)) { + vty_out(vty, + "Specified a v6 LL with no interface, rejecting\n"); + return CMD_WARNING_CONFIG_FAILED; + } + nhop.type = NEXTHOP_TYPE_IPV6; + } + } + } else + nhop.type = NEXTHOP_TYPE_IFINDEX; + if (pbrms->nhg) nh = nexthop_exists(pbrms->nhg, &nhop); else { @@ -335,6 +335,14 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, pbr_map_check(pbrms); } + if (nhop.type == NEXTHOP_TYPE_IFINDEX) { + struct interface *ifp; + + ifp = if_lookup_by_index(nhop.ifindex, nhop.vrf_id); + if (ifp) + pbr_nht_nexthop_interface_update(ifp); + } + return CMD_SUCCESS; } From 7dce96f0e413e1c6c9d1d68196716e0343c14f6a Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 14 Feb 2019 20:00:15 -0200 Subject: [PATCH 8/8] lib, pbrd: fix indentation of a few commands When displaying the running configuration, we should use a single space to indent commands when necessary (and not two spaces). Signed-off-by: Renato Westphal --- lib/nexthop_group.c | 2 +- pbrd/pbr_vty.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index d8cabf62be..bf298c6dec 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -520,7 +520,7 @@ static int nexthop_group_write(struct vty *vty) vty_out(vty, "nexthop-group %s\n", nhgc->name); for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_list, node, nh)) { - vty_out(vty, " "); + vty_out(vty, " "); nexthop_group_write_nexthop_internal(vty, nh); } diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c index 0aedb98476..d1ce82190d 100644 --- a/pbrd/pbr_vty.c +++ b/pbrd/pbr_vty.c @@ -612,18 +612,18 @@ static int pbr_vty_map_config_write_sequence(struct vty *vty, vty_out(vty, "pbr-map %s seq %u\n", pbrm->name, pbrms->seqno); if (pbrms->src) - vty_out(vty, " match src-ip %s\n", + vty_out(vty, " match src-ip %s\n", prefix2str(pbrms->src, buff, sizeof(buff))); if (pbrms->dst) - vty_out(vty, " match dst-ip %s\n", + vty_out(vty, " match dst-ip %s\n", prefix2str(pbrms->dst, buff, sizeof(buff))); if (pbrms->nhgrp_name) - vty_out(vty, " set nexthop-group %s\n", pbrms->nhgrp_name); + vty_out(vty, " set nexthop-group %s\n", pbrms->nhgrp_name); if (pbrms->nhg) { - vty_out(vty, " set "); + vty_out(vty, " set "); nexthop_group_write_nexthop(vty, pbrms->nhg->nexthop); }