From c28b6ae527fc676950902c3aade38f703b91bb0d Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 7 Dec 2023 17:19:30 +0100 Subject: [PATCH 01/25] zebra: rework zebra_nhg_proto_add() parameters This small rework prepares next commit. Signed-off-by: Philippe Guibert --- zebra/zebra_nhg.c | 12 +++++++++--- zebra/zebra_nhg.h | 6 ++---- zebra/zebra_rib.c | 5 +---- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 9e3aa8bc5c8e..ff117d348cd2 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -3356,8 +3356,7 @@ bool zebra_nhg_proto_nexthops_only(void) } /* Add NHE from upper level proto */ -struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type, - uint16_t instance, uint32_t session, +struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, struct nexthop_group *nhg, afi_t afi) { struct nhg_hash_entry lookup; @@ -3365,7 +3364,14 @@ struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type, struct nhg_connected *rb_node_dep = NULL; struct nexthop *newhop; bool replace = false; - int ret = 0; + int ret = 0, type; + uint32_t id, session; + uint16_t instance; + + id = nhe->id; + type = nhe->type; + session = nhe->zapi_session; + instance = nhe->zapi_instance; if (!nhg->nexthop) { if (IS_ZEBRA_DEBUG_NHG) diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h index 4eddecb73d6e..ccdb34e8f5ba 100644 --- a/zebra/zebra_nhg.h +++ b/zebra/zebra_nhg.h @@ -322,10 +322,8 @@ zebra_nhg_rib_find_nhe(struct nhg_hash_entry *rt_nhe, afi_t rt_afi); * * Returns allocated NHE on success, otherwise NULL. */ -struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type, - uint16_t instance, uint32_t session, - struct nexthop_group *nhg, - afi_t afi); +struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, + struct nexthop_group *nhg, afi_t afi); /* * Del NHE. diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 4b5f81a3df51..6b03761a584d 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -2580,10 +2580,7 @@ static void process_subq_nhg(struct listnode *lnode) ZAPI_NHG_REMOVE_FAIL); } else { - newnhe = zebra_nhg_proto_add(nhe->id, nhe->type, - nhe->zapi_instance, - nhe->zapi_session, - &nhe->nhg, 0); + newnhe = zebra_nhg_proto_add(nhe, &nhe->nhg, 0); /* Report error to daemon via ZAPI */ if (newnhe == NULL) From 738de3faf030ac5202ab64a8c55fef3ed24eefac Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 8 Dec 2023 22:09:13 +0100 Subject: [PATCH 02/25] sharpd, zebra, lib, doc: add recursive support for protocol nhid In sharpd, configuring a nexthop-group with an IP nexthop that is not directly connected does not create a NHG context in zebra: > ubuntu2204(config)# interface loop1 > ubuntu2204(config-if)# ip address 192.0.2.1/24 > ubuntu2204(config-if)# exi > ubuntu2204(config)# ip route 192.168.0.0/24 192.0.2.100 > ubuntu2204(config)# nexthop-group ABCD > ubuntu2204(config-nh-group)# nexthop 192.168.0.100 > 2024/01/17 16:36:23 SHARP: [Y2G2F-ZTW6M] nhg_add: nhg 181818169 not sent: no valid nexthops > ubuntu2204(config-nh-group)# do show nexthop-group rib 181818169 > Nexthop Group ID: 181818169 does not exist Nexthops with an undefined interface index are neither handled in ZEBRA, nor in SHARPD. On the other hand, if we had created a route pointing to the same nexthop (by using ZEBRA_ROUTE_ADD zapi), the next-hop would have been installed, thanks to the ALLOW_RECURSION flag embedded in the zapi_route structure. Add the support for recursivity in nexthop groups by introducing a flags attribute in the nexthop group structure. Define the NEXTHOP_GROUP_ALLOW_RECURSION value, which will be used by ZEBRA to check for the next-hop group recursivity. This flag will be set when the 'allow-recursion' command will be used: > ubuntu2204(config)# nexthop-group ABCD > ubuntu2204(config-nh-group)# allow-recursion > ubuntu2204(config-nh-group)# nexthop 192.168.0.100 > 2024/01/17 16:57:44 SHARP: [JWRCN-N9K90] Installed nhg 181818168 > ubuntu2204(config-nh-group)# do show nexthop-group rib 181818168 > ID: 181818168 (sharp) > RefCnt: 1 > Uptime: 00:00:10 > VRF: default > Valid, Installed > Depends: (73) > via 192.168.0.100 (vrf default) (recursive), weight 1 > via 192.0.2.100, loop1 (vrf default), weight 1 This flag control is mandatory, as the recursivity may not be allowed by protocols like eBGP single-hop. Signed-off-by: Philippe Guibert --- bgpd/bgp_nhg.c | 2 +- doc/user/nexthop_groups.rst | 6 ++++++ lib/nexthop_group.c | 33 ++++++++++++++++++++++++++++++--- lib/nexthop_group.h | 20 ++++++++++++-------- lib/zclient.c | 2 ++ lib/zclient.h | 3 +++ pbrd/pbr_nht.c | 2 +- pbrd/pbr_nht.h | 3 ++- sharpd/sharp_nht.c | 8 +++++++- sharpd/sharp_zebra.c | 8 +++++--- zebra/zapi_msg.c | 3 +++ zebra/zebra_nhg.c | 23 +++++++++++++---------- 12 files changed, 85 insertions(+), 28 deletions(-) diff --git a/bgpd/bgp_nhg.c b/bgpd/bgp_nhg.c index bf0ba77444e3..de48a7a3ddea 100644 --- a/bgpd/bgp_nhg.c +++ b/bgpd/bgp_nhg.c @@ -26,7 +26,7 @@ static void bgp_nhg_add_cb(const char *name) { } -static void bgp_nhg_modify_cb(const struct nexthop_group_cmd *nhgc) +static void bgp_nhg_modify_cb(const struct nexthop_group_cmd *nhgc, bool reset) { } diff --git a/doc/user/nexthop_groups.rst b/doc/user/nexthop_groups.rst index 45f64eecb7e6..4667d0c7513b 100644 --- a/doc/user/nexthop_groups.rst +++ b/doc/user/nexthop_groups.rst @@ -27,3 +27,9 @@ listing of ECMP nexthops used to forward packets. will be automatically re-assigned. This cli command must be the first command entered currently. Additionally this command only works with linux 5.19 kernels or newer. + +.. clicmd:: allow-recursion + + Allow a next-hop to be resolved at zebra level. Instead of beig considered valid, + the next-hop is checked against route reachability at zebra level. This permits + sending an incomplete next-hop to zebra, whereas it was not authorised before. diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 3f408e0a71f9..b281a8fba456 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -35,7 +35,7 @@ struct nexthop_hold { struct nexthop_group_hooks { void (*new)(const char *name); - void (*modify)(const struct nexthop_group_cmd *nhgc); + void (*modify)(const struct nexthop_group_cmd *nhgc, bool reset); void (*add_nexthop)(const struct nexthop_group_cmd *nhg, const struct nexthop *nhop); void (*del_nexthop)(const struct nexthop_group_cmd *nhg, @@ -649,6 +649,28 @@ DEFPY(nexthop_group_backup, nexthop_group_backup_cmd, return CMD_SUCCESS; } +DEFPY(nexthop_group_allow_recursion, + nexthop_group_allow_recursion_cmd, + "[no] allow-recursion", + NO_STR + "Allow recursion for nexthops with no interface presence\n") +{ + VTY_DECLVAR_CONTEXT(nexthop_group_cmd, nhgc); + + if (!!no == !CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION)) + return CMD_SUCCESS; + + if (no) + UNSET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION); + else + SET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION); + + if (nhg_hooks.modify) + nhg_hooks.modify(nhgc, true); + + return CMD_SUCCESS; +} + DEFPY(no_nexthop_group_backup, no_nexthop_group_backup_cmd, "no backup-group [WORD$name]", NO_STR @@ -680,7 +702,7 @@ DEFPY(nexthop_group_resilience, nhgc->nhg.nhgr.unbalanced_timer = unbalanced_timer; if (nhg_hooks.modify) - nhg_hooks.modify(nhgc); + nhg_hooks.modify(nhgc, false); return CMD_SUCCESS; } @@ -1172,6 +1194,9 @@ static int nexthop_group_write(struct vty *vty) vty_out(vty, "nexthop-group %s\n", nhgc->name); + if (CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION)) + vty_out(vty, " allow-recursion\n"); + if (nhgc->nhg.nhgr.buckets) vty_out(vty, " resilient buckets %u idle-timer %u unbalanced-timer %u\n", @@ -1353,7 +1378,8 @@ static const struct cmd_variable_handler nhg_name_handlers[] = { {.completions = NULL}}; void nexthop_group_init(void (*new)(const char *name), - void (*modify)(const struct nexthop_group_cmd *nhgc), + void (*modify)(const struct nexthop_group_cmd *nhgc, + bool reset), void (*add_nexthop)(const struct nexthop_group_cmd *nhg, const struct nexthop *nhop), void (*del_nexthop)(const struct nexthop_group_cmd *nhg, @@ -1375,6 +1401,7 @@ void nexthop_group_init(void (*new)(const char *name), install_element(NH_GROUP_NODE, &nexthop_group_resilience_cmd); install_element(NH_GROUP_NODE, &no_nexthop_group_resilience_cmd); + install_element(NH_GROUP_NODE, &nexthop_group_allow_recursion_cmd); memset(&nhg_hooks, 0, sizeof(nhg_hooks)); diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index 822a35439cc0..83be3b4c503b 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -34,6 +34,10 @@ struct nexthop_group { struct nexthop *nexthop; struct nhg_resilience nhgr; + + /* nexthop group flags */ +#define NEXTHOP_GROUP_ALLOW_RECURSION (1 << 1) + uint8_t flags; }; struct nexthop_group *nexthop_group_new(void); @@ -113,14 +117,14 @@ DECLARE_QOBJ_TYPE(nexthop_group_cmd); * del_nexthop - A nexthop is deleted from the NHG * destroy - The NHG is deleted */ -void nexthop_group_init( - void (*create)(const char *name), - void (*modify)(const struct nexthop_group_cmd *nhgc), - void (*add_nexthop)(const struct nexthop_group_cmd *nhgc, - const struct nexthop *nhop), - void (*del_nexthop)(const struct nexthop_group_cmd *nhgc, - const struct nexthop *nhop), - void (*destroy)(const char *name)); +void nexthop_group_init(void (*create)(const char *name), + void (*modify)(const struct nexthop_group_cmd *nhgc, + bool reset), + void (*add_nexthop)(const struct nexthop_group_cmd *nhgc, + const struct nexthop *nhop), + void (*del_nexthop)(const struct nexthop_group_cmd *nhgc, + const struct nexthop *nhop), + void (*destroy)(const char *name)); void nexthop_group_enable_vrf(struct vrf *vrf); void nexthop_group_disable_vrf(struct vrf *vrf); diff --git a/lib/zclient.c b/lib/zclient.c index 2a7d2a8c5bc1..124241be7738 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -1173,6 +1173,8 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) stream_putl(s, api_nhg->resilience.idle_timer); stream_putl(s, api_nhg->resilience.unbalanced_timer); + stream_putc(s, api_nhg->flags); + if (cmd == ZEBRA_NHG_ADD) { /* Nexthops */ zapi_nexthop_group_sort(api_nhg->nexthops, diff --git a/lib/zclient.h b/lib/zclient.h index 8d7faeb223d5..0ca4b31c868e 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -486,6 +486,9 @@ struct zapi_nhg { uint16_t backup_nexthop_num; struct zapi_nexthop backup_nexthops[MULTIPATH_NUM]; + + /* nexthop group flags */ + uint8_t flags; }; /* diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index b9d1ca9bb7ab..21742af96055 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -216,7 +216,7 @@ void pbr_nhgroup_add_cb(const char *name) pbr_map_check_nh_group_change(name); } -void pbr_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc) +void pbr_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc, bool reset) { } diff --git a/pbrd/pbr_nht.h b/pbrd/pbr_nht.h index a702a57155fb..0189daaa12f9 100644 --- a/pbrd/pbr_nht.h +++ b/pbrd/pbr_nht.h @@ -83,7 +83,8 @@ extern void pbr_nht_set_rule_range(uint32_t low, uint32_t high); extern uint32_t pbr_nht_get_next_rule(uint32_t seqno); extern void pbr_nhgroup_add_cb(const char *name); -extern void pbr_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc); +extern void pbr_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc, + bool reset); extern void pbr_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhg, const struct nexthop *nhop); extern void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhg, diff --git a/sharpd/sharp_nht.c b/sharpd/sharp_nht.c index 6d64fcfb259b..862aa6c6e3aa 100644 --- a/sharpd/sharp_nht.c +++ b/sharpd/sharp_nht.c @@ -114,7 +114,8 @@ static void sharp_nhgroup_add_cb(const char *name) sharp_nhg_rb_add(&nhg_head, snhg); } -static void sharp_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc) +static void sharp_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc, + bool reset) { struct sharp_nhg lookup; struct sharp_nhg *snhg; @@ -126,6 +127,11 @@ static void sharp_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc) if (!nhgc->nhg.nexthop) return; + if (reset) { + /* nexthops must be removed before being re-added */ + nhg_del(snhg->id); + } + if (nhgc->backup_list_name[0]) bnhgc = nhgc_find(nhgc->backup_list_name); diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index cbfa0d1cccec..bfe3b6437c7e 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -539,6 +539,7 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, api_nhg.id = id; + api_nhg.flags = nhg->flags; api_nhg.resilience = nhg->nhgr; for (ALL_NEXTHOPS_PTR(nhg, nh)) { @@ -549,10 +550,11 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, break; } - /* Unresolved nexthops will lead to failure - only send - * nexthops that zebra will consider valid. + /* Unresolved nexthops will lead to failure, unless + * ALLOW_RECURSION flag is set */ - if (nh->ifindex == 0) + if (nh->ifindex == 0 && + !CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_ALLOW_RECURSION)) continue; api_nh = &api_nhg.nexthops[api_nhg.nexthop_num]; diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 761eafeb1375..1b3bcd1a2b8d 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1895,6 +1895,8 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) STREAM_GETL(s, api_nhg->resilience.idle_timer); STREAM_GETL(s, api_nhg->resilience.unbalanced_timer); + STREAM_GETC(s, api_nhg->flags); + /* Nexthops */ STREAM_GETW(s, api_nhg->nexthop_num); @@ -2013,6 +2015,7 @@ static void zread_nhg_add(ZAPI_HANDLER_ARGS) nhe = zebra_nhg_alloc(); nhe->id = api_nhg.id; nhe->type = api_nhg.proto; + nhe->nhg.flags = api_nhg.flags; nhe->zapi_instance = client->instance; nhe->zapi_session = client->session_id; diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index ff117d348cd2..d00bb247afdc 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -2346,8 +2346,10 @@ static int nexthop_active(struct nexthop *nexthop, struct nhg_hash_entry *nhe, * if specified) - i.e., we cannot have a nexthop NH1 is * resolved by a route NH1. The exception is if the route is a * host route. + * This control will not work by using nexthop groups, and will + * have to be handled at protocol level */ - if (prefix_same(&rn->p, top)) + if (top && prefix_same(&rn->p, top)) if (((afi == AFI_IP) && (rn->p.prefixlen != IPV4_MAX_BITLEN)) || ((afi == AFI_IP6) @@ -3365,7 +3367,7 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, struct nexthop *newhop; bool replace = false; int ret = 0, type; - uint32_t id, session; + uint32_t id, session, api_message = 0; uint16_t instance; id = nhe->id; @@ -3413,14 +3415,15 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, return NULL; } - if (!newhop->ifindex) { - if (IS_ZEBRA_DEBUG_NHG) - zlog_debug( - "%s: id %u, nexthop without ifindex is not supported", - __func__, id); - return NULL; - } - SET_FLAG(newhop->flags, NEXTHOP_FLAG_ACTIVE); + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_ALLOW_RECURSION)) + /* Tell zebra that the route may be recursively resolved */ + api_message = ZEBRA_FLAG_ALLOW_RECURSION; + + if (nexthop_active(newhop, nhe, NULL, type, api_message, NULL, + newhop->vrf_id)) + SET_FLAG(newhop->flags, NEXTHOP_FLAG_ACTIVE); + else + UNSET_FLAG(newhop->flags, NEXTHOP_FLAG_ACTIVE); } zebra_nhe_init(&lookup, afi, nhg->nexthop); From 9e4d0da74688d0a785f1e3f328ce08d5bd87a12e Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 8 Dec 2023 22:09:37 +0100 Subject: [PATCH 03/25] topotests: add nexthop-group allow-recursion test Add a nexthop group test that ensures that a recursive next-hop is resolved in zebra. Signed-off-by: Philippe Guibert --- .../all_protocol_startup/r1/ip_nht.ref | 8 +++++ .../all_protocol_startup/r1/ipv4_routes.ref | 1 + .../all_protocol_startup/r1/zebra.conf | 4 +++ .../test_all_protocol_startup.py | 36 +++++++++++++++++-- 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/tests/topotests/all_protocol_startup/r1/ip_nht.ref b/tests/topotests/all_protocol_startup/r1/ip_nht.ref index a2f3d3b0db4e..3bc2c7838d52 100644 --- a/tests/topotests/all_protocol_startup/r1/ip_nht.ref +++ b/tests/topotests/all_protocol_startup/r1/ip_nht.ref @@ -53,6 +53,10 @@ VRF default: 6.6.6.4 unresolved Client list: pbr(fd XX) +192.0.2.200 + resolved via static + via 192.168.0.208, r1-eth0 (vrf default), weight 1 + Client list: pbr(fd XX) 192.168.0.2 resolved via connected is directly connected, r1-eth0 (vrf default) @@ -61,6 +65,10 @@ VRF default: resolved via connected is directly connected, r1-eth0 (vrf default) Client list: static(fd XX) +192.168.0.208 + resolved via connected + is directly connected, r1-eth0 (vrf default) + Client list: static(fd XX) 192.168.7.10 resolved via connected is directly connected, r1-eth7 (vrf default) diff --git a/tests/topotests/all_protocol_startup/r1/ipv4_routes.ref b/tests/topotests/all_protocol_startup/r1/ipv4_routes.ref index a4a4aba3d181..c9c08dff7ec2 100644 --- a/tests/topotests/all_protocol_startup/r1/ipv4_routes.ref +++ b/tests/topotests/all_protocol_startup/r1/ipv4_routes.ref @@ -28,6 +28,7 @@ S>* 1.1.1.5/32 [1/0] is directly connected, r1-eth5, weight 1, XX:XX:XX S>* 1.1.1.6/32 [1/0] is directly connected, r1-eth6, weight 1, XX:XX:XX S>* 1.1.1.7/32 [1/0] is directly connected, r1-eth7, weight 1, XX:XX:XX S>* 1.1.1.8/32 [1/0] is directly connected, r1-eth8, weight 1, XX:XX:XX +S>* 192.0.2.0/24 [1/0] via 192.168.0.208, r1-eth0, weight 1, XX:XX:XX S>* 4.5.6.10/32 [1/0] via 192.168.0.2, r1-eth0, weight 1, XX:XX:XX S>* 4.5.6.11/32 [1/0] via 192.168.0.2, r1-eth0, weight 1, XX:XX:XX S>* 4.5.6.12/32 [1/0] is directly connected, r1-eth0, weight 1, XX:XX:XX diff --git a/tests/topotests/all_protocol_startup/r1/zebra.conf b/tests/topotests/all_protocol_startup/r1/zebra.conf index c5ef79630e21..0491d21a3a56 100644 --- a/tests/topotests/all_protocol_startup/r1/zebra.conf +++ b/tests/topotests/all_protocol_startup/r1/zebra.conf @@ -46,6 +46,10 @@ ip route 4.5.6.16/32 192.168.0.4 10 ip route 4.5.6.17/32 192.168.0.2 tag 9000 ip route 4.5.6.17/32 192.168.0.2 tag 10000 +! Create a static route to test recursive +! resolution +ip route 192.0.2.0/24 192.168.0.208 + ! interface r1-eth0 description to sw0 - no routing protocol diff --git a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py index e067cdb7634d..ffa095ce1132 100644 --- a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py +++ b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py @@ -397,7 +397,7 @@ def route_get_nhg_id(route_str): return nhg_id -def verify_nexthop_group(nhg_id, recursive=False, ecmp=0): +def verify_nexthop_group(nhg_id, recursive=False, ecmp=0, recursive_again=False): net = get_topogen().net count = 0 valid = None @@ -440,7 +440,7 @@ def verify_nexthop_group(nhg_id, recursive=False, ecmp=0): continue resolved_id = int(depends[0]) - verify_nexthop_group(resolved_id, False) + verify_nexthop_group(resolved_id, recursive=recursive_again) else: installed = re.search(r"Installed", output) if installed is None: @@ -613,6 +613,38 @@ def test_nexthop_groups(): % nhg_id ) + ## nexthop-group ALLOWRECURSION + ## create a static route, and use that static route to resolve the nexthop-group + tgen = get_topogen() + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group ALLOWRECURSION + allow-recursion + nexthop 192.0.2.200 + """ + ) + tgen.gears["r1"].vtysh_cmd( + "sharp install routes 9.9.9.9 nexthop-group ALLOWRECURSION 1" + ) + nhg_id = route_get_nhg_id("9.9.9.9/32") + verify_nexthop_group(nhg_id, recursive=True, ecmp=0, recursive_again=True) + + tgen.gears["r1"].vtysh_cmd("sharp remove routes 9.9.9.9 1") + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group ALLOWRECURSION + no allow-recursion + """ + ) + output = tgen.gears["r1"].vtysh_cmd("show nexthop-group rib %d" % nhg_id) + found = re.search(r"Time to Deletion", output) + assert found is not None, ( + "Route 9.9.9.9/32 with Nexthop Group ID=%d is not scheduled for removal" + % nhg_id + ) + ## Remove all NHG routes net["r1"].cmd('vtysh -c "sharp remove routes 2.2.2.1 1"') From dc0463eb7753030579e3aa7050b0b24f02bbae05 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Sun, 17 Dec 2023 22:10:58 +0100 Subject: [PATCH 04/25] sharpd, lib: add force-nexthop-config option to sharp in NHG In BGP, it is possible to push an eBGP route without using the ALLOW_RECURSION flag in the route flags option. In SHARP, it is possible to do the same with the 'sharp install route' command, but it is not possible to push a NHG that has not allow-recursion flag set. > sharp install routes 9.9.9.20 ... no-recurse > => pushed > nexthop-group A > nexthop 172.31.0.200 > => not pushed This is a design issue related to the SHARP daemon only. Change the design: - Extend the nexthop group hooks with a running-config call back - Add a vty command in SHARP only, under the nexthop-group node, that will permit to push any NHG. > nexthop-group A > force-nexthop-config > nexthop 172.31.0.200 > The nhg will be sent to ZEBRA, and will be attempted to be resolved. Signed-off-by: Philippe Guibert --- bgpd/bgp_nhg.c | 2 +- doc/user/nexthop_groups.rst | 5 ++++ lib/nexthop_group.c | 25 +++++++++++------ lib/nexthop_group.h | 18 +++++++------ pbrd/pbr_main.c | 3 ++- sharpd/sharp_nht.c | 54 ++++++++++++++++++++++++++++++++++--- sharpd/sharp_nht.h | 3 +++ sharpd/sharp_vty.c | 12 +++++++++ sharpd/sharp_zebra.c | 11 +++++--- sharpd/sharp_zebra.h | 3 ++- 10 files changed, 109 insertions(+), 27 deletions(-) diff --git a/bgpd/bgp_nhg.c b/bgpd/bgp_nhg.c index de48a7a3ddea..d0fa8e363bf8 100644 --- a/bgpd/bgp_nhg.c +++ b/bgpd/bgp_nhg.c @@ -55,7 +55,7 @@ static void bgp_nhg_zebra_init(void) bgp_nhg_start = zclient_get_nhg_start(ZEBRA_ROUTE_BGP); nexthop_group_init(bgp_nhg_add_cb, bgp_nhg_modify_cb, bgp_nhg_add_nexthop_cb, bgp_nhg_del_nexthop_cb, - bgp_nhg_del_cb); + bgp_nhg_del_cb, NULL); } void bgp_nhg_init(void) diff --git a/doc/user/nexthop_groups.rst b/doc/user/nexthop_groups.rst index 4667d0c7513b..24b130e092f0 100644 --- a/doc/user/nexthop_groups.rst +++ b/doc/user/nexthop_groups.rst @@ -33,3 +33,8 @@ listing of ECMP nexthops used to forward packets. Allow a next-hop to be resolved at zebra level. Instead of beig considered valid, the next-hop is checked against route reachability at zebra level. This permits sending an incomplete next-hop to zebra, whereas it was not authorised before. + +.. clicmd:: force-nexthop-config + + Allow a Nexthop Group to be configured in ZEBRA, even if the nexthop has neither + an interface configured and no `allow-recursion` set. diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index b281a8fba456..b5b3dca5a8dd 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -41,6 +41,8 @@ struct nexthop_group_hooks { void (*del_nexthop)(const struct nexthop_group_cmd *nhg, const struct nexthop *nhop); void (*delete)(const char *name); + int (*write_config)(struct vty *vty, + const struct nexthop_group_cmd *nhg); }; static struct nexthop_group_hooks nhg_hooks; @@ -1213,6 +1215,9 @@ static int nexthop_group_write(struct vty *vty) nexthop_group_write_nexthop_internal(vty, nh); } + if (nhg_hooks.write_config) + nhg_hooks.write_config(vty, nhgc); + vty_out(vty, "exit\n"); vty_out(vty, "!\n"); } @@ -1377,14 +1382,16 @@ static const struct cmd_variable_handler nhg_name_handlers[] = { {.tokenname = "NHGNAME", .completions = nhg_name_autocomplete}, {.completions = NULL}}; -void nexthop_group_init(void (*new)(const char *name), - void (*modify)(const struct nexthop_group_cmd *nhgc, - bool reset), - void (*add_nexthop)(const struct nexthop_group_cmd *nhg, - const struct nexthop *nhop), - void (*del_nexthop)(const struct nexthop_group_cmd *nhg, - const struct nexthop *nhop), - void (*delete)(const char *name)) +void nexthop_group_init( + void (*new)(const char *name), + void (*modify)(const struct nexthop_group_cmd *nhgc, bool reset), + void (*add_nexthop)(const struct nexthop_group_cmd *nhg, + const struct nexthop *nhop), + void (*del_nexthop)(const struct nexthop_group_cmd *nhg, + const struct nexthop *nhop), + void (*delete)(const char *name), + int (*write_config)(struct vty *vty, + const struct nexthop_group_cmd *nhgc)) { RB_INIT(nhgc_entry_head, &nhgc_entries); @@ -1415,4 +1422,6 @@ void nexthop_group_init(void (*new)(const char *name), nhg_hooks.del_nexthop = del_nexthop; if (delete) nhg_hooks.delete = delete; + if (write_config) + nhg_hooks.write_config = write_config; } diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index 83be3b4c503b..e37d94302d83 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -117,14 +117,16 @@ DECLARE_QOBJ_TYPE(nexthop_group_cmd); * del_nexthop - A nexthop is deleted from the NHG * destroy - The NHG is deleted */ -void nexthop_group_init(void (*create)(const char *name), - void (*modify)(const struct nexthop_group_cmd *nhgc, - bool reset), - void (*add_nexthop)(const struct nexthop_group_cmd *nhgc, - const struct nexthop *nhop), - void (*del_nexthop)(const struct nexthop_group_cmd *nhgc, - const struct nexthop *nhop), - void (*destroy)(const char *name)); +void nexthop_group_init( + void (*create)(const char *name), + void (*modify)(const struct nexthop_group_cmd *nhgc, bool reset), + void (*add_nexthop)(const struct nexthop_group_cmd *nhgc, + const struct nexthop *nhop), + void (*del_nexthop)(const struct nexthop_group_cmd *nhgc, + const struct nexthop *nhop), + void (*destroy)(const char *name), + int (*write_config)(struct vty *vty, + const struct nexthop_group_cmd *nhgc)); void nexthop_group_enable_vrf(struct vrf *vrf); void nexthop_group_disable_vrf(struct vrf *vrf); diff --git a/pbrd/pbr_main.c b/pbrd/pbr_main.c index dd4893bb78ad..eef45d75c741 100644 --- a/pbrd/pbr_main.c +++ b/pbrd/pbr_main.c @@ -151,7 +151,8 @@ int main(int argc, char **argv, char **envp) nexthop_group_init(pbr_nhgroup_add_cb, pbr_nhgroup_modify_cb, pbr_nhgroup_add_nexthop_cb, - pbr_nhgroup_del_nexthop_cb, pbr_nhgroup_delete_cb); + pbr_nhgroup_del_nexthop_cb, pbr_nhgroup_delete_cb, + NULL); /* * So we safely ignore these commands since diff --git a/sharpd/sharp_nht.c b/sharpd/sharp_nht.c index 862aa6c6e3aa..d3967b2d18b9 100644 --- a/sharpd/sharp_nht.c +++ b/sharpd/sharp_nht.c @@ -66,6 +66,7 @@ struct sharp_nhg { char name[NHG_NAME_LEN]; bool installed; + bool force_nhg_config; }; static uint32_t nhg_id; @@ -135,7 +136,8 @@ static void sharp_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc, if (nhgc->backup_list_name[0]) bnhgc = nhgc_find(nhgc->backup_list_name); - nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL)); + nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL), + snhg->force_nhg_config); } static void sharp_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc, @@ -151,7 +153,8 @@ static void sharp_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc, if (nhgc->backup_list_name[0]) bnhgc = nhgc_find(nhgc->backup_list_name); - nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL)); + nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL), + snhg->force_nhg_config); } static void sharp_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, @@ -167,7 +170,8 @@ static void sharp_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, if (nhgc->backup_list_name[0]) bnhgc = nhgc_find(nhgc->backup_list_name); - nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL)); + nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL), + snhg->force_nhg_config); } static void sharp_nhgroup_delete_cb(const char *name) @@ -186,6 +190,21 @@ static void sharp_nhgroup_delete_cb(const char *name) XFREE(MTYPE_NHG, snhg); } +static int sharp_nhgroup_write_config(struct vty *vty, + const struct nexthop_group_cmd *nhgc) +{ + struct sharp_nhg *snhg; + struct sharp_nhg lookup; + + strlcpy(lookup.name, nhgc->name, sizeof(lookup.name)); + snhg = sharp_nhg_rb_find(&nhg_head, &lookup); + if (!snhg) + return 0; + if (snhg->force_nhg_config) + vty_out(vty, " force-nexthop-config\n"); + return 1; +} + uint32_t sharp_nhgroup_get_id(const char *name) { struct sharp_nhg lookup; @@ -225,6 +244,33 @@ bool sharp_nhgroup_id_is_installed(uint32_t id) return snhg->installed; } +bool sharp_nhgroup_id_is_forced(uint32_t id) +{ + struct sharp_nhg *snhg; + + snhg = sharp_nhgroup_find_id(id); + if (!snhg) { + zlog_debug("%s: nhg %u not found", __func__, id); + return false; + } + return snhg->force_nhg_config; +} + +void sharp_nhgroup_force_nhg_config(struct nexthop_group_cmd *nhgc, bool force) +{ + struct sharp_nhg *snhg; + struct sharp_nhg lookup; + + strlcpy(lookup.name, nhgc->name, sizeof(lookup.name)); + snhg = sharp_nhg_rb_find(&nhg_head, &lookup); + if (!snhg) + return; + if (force == snhg->force_nhg_config) + return; + snhg->force_nhg_config = force; + sharp_nhgroup_modify_cb(nhgc, true); +} + void sharp_nhgroup_init(void) { sharp_nhg_rb_init(&nhg_head); @@ -233,5 +279,5 @@ void sharp_nhgroup_init(void) nexthop_group_init(sharp_nhgroup_add_cb, sharp_nhgroup_modify_cb, sharp_nhgroup_add_nexthop_cb, sharp_nhgroup_del_nexthop_cb, - sharp_nhgroup_delete_cb); + sharp_nhgroup_delete_cb, sharp_nhgroup_write_config); } diff --git a/sharpd/sharp_nht.h b/sharpd/sharp_nht.h index b27952ac511c..448b02b1d047 100644 --- a/sharpd/sharp_nht.h +++ b/sharpd/sharp_nht.h @@ -25,6 +25,9 @@ extern void sharp_nh_tracker_dump(struct vty *vty); extern uint32_t sharp_nhgroup_get_id(const char *name); extern void sharp_nhgroup_id_set_installed(uint32_t id, bool installed); extern bool sharp_nhgroup_id_is_installed(uint32_t id); +extern bool sharp_nhgroup_id_is_forced(uint32_t id); +extern void sharp_nhgroup_force_nhg_config(struct nexthop_group_cmd *nhgc, + bool force); extern void sharp_nhgroup_init(void); #endif diff --git a/sharpd/sharp_vty.c b/sharpd/sharp_vty.c index cf79bacc6b6e..5d72dbba0be7 100644 --- a/sharpd/sharp_vty.c +++ b/sharpd/sharp_vty.c @@ -1410,6 +1410,17 @@ DEFPY (tc_filter_rate, return CMD_SUCCESS; } +DEFPY(sharp_nhgroup_force_nh_config, sharp_nhgroup_force_nh_config_cmd, + "[no] force-nexthop-config", + NO_STR + "Force the nexthop configuration to ZEBRA, even if nexthop is not valid\n") +{ + VTY_DECLVAR_CONTEXT(nexthop_group_cmd, nhgc); + + sharp_nhgroup_force_nhg_config(nhgc, !no); + return CMD_SUCCESS; +} + void sharp_vty_init(void) { install_element(ENABLE_NODE, &install_routes_data_dump_cmd); @@ -1448,5 +1459,6 @@ void sharp_vty_init(void) install_element(ENABLE_NODE, &tc_filter_rate_cmd); + install_element(NH_GROUP_NODE, &sharp_nhgroup_force_nh_config_cmd); return; } diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index bfe3b6437c7e..a3767f88bed0 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -250,8 +250,10 @@ static bool route_add(const struct prefix *p, vrf_id_t vrf_id, uint8_t instance, SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP); /* Only send via ID if nhgroup has been successfully installed */ - if (nhgid && sharp_nhgroup_id_is_installed(nhgid)) { - zapi_route_set_nhg_id(&api, &nhgid); + if (nhgid && (sharp_nhgroup_id_is_installed(nhgid) || + sharp_nhgroup_id_is_forced(nhgid))) { + SET_FLAG(api.message, ZAPI_MESSAGE_NHG); + api.nhgid = nhgid; } else { for (ALL_NEXTHOPS_PTR(nhg, nh)) { /* Check if we set a VNI label */ @@ -530,7 +532,7 @@ void vrf_label_add(vrf_id_t vrf_id, afi_t afi, mpls_label_t label) } void nhg_add(uint32_t id, const struct nexthop_group *nhg, - const struct nexthop_group *backup_nhg) + const struct nexthop_group *backup_nhg, bool force_nhg_config) { struct zapi_nhg api_nhg = {}; struct zapi_nexthop *api_nh; @@ -554,7 +556,8 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, * ALLOW_RECURSION flag is set */ if (nh->ifindex == 0 && - !CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_ALLOW_RECURSION)) + !CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_ALLOW_RECURSION) && + !force_nhg_config) continue; api_nh = &api_nhg.nexthops[api_nhg.nexthop_num]; diff --git a/sharpd/sharp_zebra.h b/sharpd/sharp_zebra.h index 6314f862f57e..d67704db861d 100644 --- a/sharpd/sharp_zebra.h +++ b/sharpd/sharp_zebra.h @@ -16,7 +16,8 @@ int sharp_zclient_delete(uint32_t session_id); extern void vrf_label_add(vrf_id_t vrf_id, afi_t afi, mpls_label_t label); extern void nhg_add(uint32_t id, const struct nexthop_group *nhg, - const struct nexthop_group *backup_nhg); + const struct nexthop_group *backup_nhg, + bool force_nhg_config); extern void nhg_del(uint32_t id); extern void sharp_zebra_nexthop_watch(struct prefix *p, vrf_id_t vrf_id, bool import, bool watch, bool connected); From d5a61e0fe19bce10343113736db3cb0d65254630 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 15 Dec 2023 09:45:41 +0100 Subject: [PATCH 05/25] lib, zebra, doc: extend ibgp support to nhg in zebra When routes from eBGP peers are pushed in ZEBRA, and are not resolved over connected paths, an error message is displayed. This is because neither the ALLOW_RECURSION, nor the IBGP flag are set on the route. When attempting to do the same in SHARP, it appears that the NHG do not have this IBGP flag. > cli# debug zebra rib detail > [..] > 2023/12/17 21:09:43 ZEBRA: [NWWQT-S584X] nexthop_active: Route Type bgp has not turned on recursion 172.31.0.102/32 failed to match > 2023/12/17 21:09:43 ZEBRA: [PKF9S-7G8XM] EBGP: see "disable-ebgp-connected-route-check" or "disable-connected-check" Attempting to push NHGs to ZEBRA does not lead to the same error messages, for two reasons: - NHGs are not sent by SHARPD if allow-recursion flag is not set - the IBGP flag is not implemented in NHGs This commit includes two changes: - introduce the ZEBRA_NHG_FLAG in the nexthop_group structure The flag will be used by ZEBRA when attempting to check if the nexthops of the NHG are active or not. - add a vty command in nexthop group that maps over the IBGP flag. > ubuntu2204(config)# nexthop-group H > ubuntu2204(config-nh-group)# ibgp The below test shows that when turning on ibgp, the EBGP trace disappears: > ubuntu2204(config)# debu zebra nexthop de > ubuntu2204(config)# debu zebra rib de > ubuntu2204(config)# ip route 172.31.0.102/32 192.168.0.77 > ubuntu2204(config)# nexthop-group H > ubuntu2204(config-nh-group)# force-nexthop-config > ubuntu2204(config-nh-group)# nexthop 172.31.0.102 > [..] > 2023/12/17 22:09:43 ZEBRA: [NWWQT-S584X] nexthop_active: Route Type sharp has not turned on recursion 172.31.0.102/32 failed to match > 2023/12/17 22:09:43 ZEBRA: [PKF9S-7G8XM] EBGP: see "disable-ebgp-connected-route-check" or "disable-connected-check" > ubuntu2204(config-nh-group)# ibgp > 2023/12/17 22:14:57 ZEBRA: [NWWQT-S584X] nexthop_active: Route Type sharp has not turned on recursion 172.31.0.102/32 failed to match Signed-off-by: Philippe Guibert --- doc/user/nexthop_groups.rst | 5 +++++ lib/nexthop_group.c | 26 ++++++++++++++++++++++++++ lib/nexthop_group.h | 1 + zebra/zebra_nhg.c | 9 +++++++-- 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/doc/user/nexthop_groups.rst b/doc/user/nexthop_groups.rst index 24b130e092f0..4dd42d8b25c3 100644 --- a/doc/user/nexthop_groups.rst +++ b/doc/user/nexthop_groups.rst @@ -38,3 +38,8 @@ listing of ECMP nexthops used to forward packets. Allow a Nexthop Group to be configured in ZEBRA, even if the nexthop has neither an interface configured and no `allow-recursion` set. + +.. clicmd:: ibgp + + Allow a Nexthop Group to be considered as an IBGP nexthop. When used in ZEBRA, + if `allow-recursion` is not used, some debug information is given. diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index b5b3dca5a8dd..8b566efcc78e 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -673,6 +673,28 @@ DEFPY(nexthop_group_allow_recursion, return CMD_SUCCESS; } +DEFPY(nexthop_group_ibgp, + nexthop_group_ibgp_cmd, + "[no] ibgp", + NO_STR + "Declare the prefix to install from an IBGP peer\n") +{ + VTY_DECLVAR_CONTEXT(nexthop_group_cmd, nhgc); + + if (!!no == !CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_IBGP)) + return CMD_SUCCESS; + + if (no) + UNSET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_IBGP); + else + SET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_IBGP); + + if (nhg_hooks.modify) + nhg_hooks.modify(nhgc, true); + + return CMD_SUCCESS; +} + DEFPY(no_nexthop_group_backup, no_nexthop_group_backup_cmd, "no backup-group [WORD$name]", NO_STR @@ -1199,6 +1221,9 @@ static int nexthop_group_write(struct vty *vty) if (CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION)) vty_out(vty, " allow-recursion\n"); + if (CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_IBGP)) + vty_out(vty, " ibgp\n"); + if (nhgc->nhg.nhgr.buckets) vty_out(vty, " resilient buckets %u idle-timer %u unbalanced-timer %u\n", @@ -1409,6 +1434,7 @@ void nexthop_group_init( install_element(NH_GROUP_NODE, &nexthop_group_resilience_cmd); install_element(NH_GROUP_NODE, &no_nexthop_group_resilience_cmd); install_element(NH_GROUP_NODE, &nexthop_group_allow_recursion_cmd); + install_element(NH_GROUP_NODE, &nexthop_group_ibgp_cmd); memset(&nhg_hooks, 0, sizeof(nhg_hooks)); diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index e37d94302d83..73e6903afd5e 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -37,6 +37,7 @@ struct nexthop_group { /* nexthop group flags */ #define NEXTHOP_GROUP_ALLOW_RECURSION (1 << 1) +#define NEXTHOP_GROUP_IBGP (1 << 2) uint8_t flags; }; diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index d00bb247afdc..e68abe9202f4 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -2540,8 +2540,9 @@ static int nexthop_active(struct nexthop *nexthop, struct nhg_hash_entry *nhe, zlog_debug( " %s: Route Type %s has not turned on recursion %pRN failed to match", __func__, zebra_route_string(type), rn); - if (type == ZEBRA_ROUTE_BGP - && !CHECK_FLAG(flags, ZEBRA_FLAG_IBGP)) + if ((type == ZEBRA_ROUTE_BGP || + type == ZEBRA_ROUTE_SHARP) && + !CHECK_FLAG(flags, ZEBRA_FLAG_IBGP)) zlog_debug( " EBGP: see \"disable-ebgp-connected-route-check\" or \"disable-connected-check\""); } @@ -3419,6 +3420,10 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, /* Tell zebra that the route may be recursively resolved */ api_message = ZEBRA_FLAG_ALLOW_RECURSION; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_IBGP)) + /* Tell zebra that the prefix originates from an IBGP peer */ + SET_FLAG(api_message, ZEBRA_FLAG_IBGP); + if (nexthop_active(newhop, nhe, NULL, type, api_message, NULL, newhop->vrf_id)) SET_FLAG(newhop->flags, NEXTHOP_FLAG_ACTIVE); From f1b312f5ed4b65b089f8e010f9a4629f7a32d7af Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Sun, 17 Dec 2023 21:04:31 +0100 Subject: [PATCH 06/25] lib: fix copy srte_color from zapi_nexthop structure When passing from nexthop to zapi_nexthop, the srte color is copied. Do the same reversely. Fixes: 31f937fb43f4 ("lib, zebra: Add SR-TE policy infrastructure to zebra") Signed-off-by: Philippe Guibert --- lib/zclient.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/zclient.c b/lib/zclient.c index 124241be7738..d9db09939b65 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -2173,6 +2173,7 @@ int zapi_nexthop_from_nexthop(struct zapi_nexthop *znh, znh->weight = nh->weight; znh->ifindex = nh->ifindex; znh->gate = nh->gate; + znh->srte_color = nh->srte_color; if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_ONLINK)) SET_FLAG(znh->flags, ZAPI_NEXTHOP_FLAG_ONLINK); From b5f7f06db7be98877a093787caded5cf1a5f4c73 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 15 Dec 2023 17:10:24 +0100 Subject: [PATCH 07/25] *: add srte color support in nhg The nexthop group CLI can not configure color support. Add a new option under nexthop command to configure an srte color: > nexthop-group 1 > [..] > nexthop 192.0.2.100 color 100 Add srte color support for nhg: - Add NEXTHOP_GROUP_MESSAGE_COLOR support in NHG. - Add ZAPI_NEXTHOP_MESSAGE_SRTE support in ZAPI. - When checking for nexthop active, an SRTE policy with the (color, nexthop ip) tuple will be looked ip. Add a NHG sharp test for SRTE color: - a NHG matching an SRTE policy is configured - NHG sharp is extended to watch a given NHT color > sharp watch nexthop 172.31.0.200 color 1 - Upon NHT call back, the NHG is re-added, and updates the nexthop. Signed-off-by: Philippe Guibert --- doc/user/nexthop_groups.rst | 2 +- lib/nexthop_group.c | 74 +++++++++++++++---- lib/nexthop_group.h | 4 + lib/zclient.c | 9 ++- lib/zclient.h | 3 + sharpd/sharp_nht.h | 2 + sharpd/sharp_vty.c | 23 ++++-- sharpd/sharp_zebra.c | 49 +++++++++++- .../all_protocol_startup/r1/ip_nht.ref | 3 + .../all_protocol_startup/r1/pathd.conf | 12 +++ .../all_protocol_startup/r1/zebra.conf | 3 + .../test_all_protocol_startup.py | 40 ++++++++++ zebra/zapi_msg.c | 12 ++- 13 files changed, 207 insertions(+), 29 deletions(-) create mode 100644 tests/topotests/all_protocol_startup/r1/pathd.conf diff --git a/doc/user/nexthop_groups.rst b/doc/user/nexthop_groups.rst index 4dd42d8b25c3..d769abe8724e 100644 --- a/doc/user/nexthop_groups.rst +++ b/doc/user/nexthop_groups.rst @@ -12,7 +12,7 @@ listing of ECMP nexthops used to forward packets. sub-mode where you can specify individual nexthops. To exit this mode type exit or end as per normal conventions for leaving a sub-mode. -.. clicmd:: nexthop [A.B.C.D|X:X::X:XX] [interface [onlink]] [nexthop-vrf NAME] [label LABELS] +.. clicmd:: nexthop [A.B.C.D|X:X::X:XX] [interface [onlink]] [nexthop-vrf NAME] [label LABELS] [color (1-4294967295)] Create a v4 or v6 nexthop. All normal rules for creating nexthops that you are used to are allowed here. The syntax was intentionally kept the same as diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 8b566efcc78e..a14bd3916db9 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -31,6 +31,7 @@ struct nexthop_hold { vni_t vni; uint32_t weight; char *backup_str; + uint32_t color; }; struct nexthop_group_hooks { @@ -53,7 +54,7 @@ nexthop_group_cmd_compare(const struct nexthop_group_cmd *nhgc1, RB_GENERATE(nhgc_entry_head, nexthop_group_cmd, nhgc_entry, nexthop_group_cmd_compare) -static struct nhgc_entry_head nhgc_entries; +struct nhgc_entry_head nhgc_entries; static inline int nexthop_group_cmd_compare(const struct nexthop_group_cmd *nhgc1, @@ -695,6 +696,21 @@ DEFPY(nexthop_group_ibgp, return CMD_SUCCESS; } +static void nhgc_configure_color(struct nexthop_group_cmd *nhgc, bool enable) +{ + if (enable == + !!CHECK_FLAG(nhgc->nhg.message, NEXTHOP_GROUP_MESSAGE_SRTE)) + return; + + if (enable) + SET_FLAG(nhgc->nhg.message, NEXTHOP_GROUP_MESSAGE_SRTE); + else + UNSET_FLAG(nhgc->nhg.message, NEXTHOP_GROUP_MESSAGE_SRTE); + + if (nhg_hooks.modify) + nhg_hooks.modify(nhgc, true); +} + DEFPY(no_nexthop_group_backup, no_nexthop_group_backup_cmd, "no backup-group [WORD$name]", NO_STR @@ -752,12 +768,11 @@ DEFPY(no_nexthop_group_resilience, return CMD_SUCCESS; } -static void nexthop_group_save_nhop(struct nexthop_group_cmd *nhgc, - const char *nhvrf_name, - const union sockunion *addr, - const char *intf, bool onlink, - const char *labels, const uint32_t weight, - const char *backup_str) +static void +nexthop_group_save_nhop(struct nexthop_group_cmd *nhgc, const char *nhvrf_name, + const union sockunion *addr, const char *intf, + bool onlink, const char *labels, const uint32_t weight, + const char *backup_str, const uint32_t color) { struct nexthop_hold *nh; @@ -776,6 +791,8 @@ static void nexthop_group_save_nhop(struct nexthop_group_cmd *nhgc, nh->weight = weight; + nh->color = color; + if (backup_str) nh->backup_str = XSTRDUP(MTYPE_TMP, backup_str); @@ -824,8 +841,8 @@ static bool nexthop_group_parse_nexthop(struct nexthop *nhop, const union sockunion *addr, const char *intf, bool onlink, const char *name, const char *labels, - vni_t vni, int *lbl_ret, - uint32_t weight, const char *backup_str) + vni_t vni, int *lbl_ret, uint32_t weight, + const char *backup_str, uint32_t color) { int ret = 0; struct vrf *vrf; @@ -892,6 +909,7 @@ static bool nexthop_group_parse_nexthop(struct nexthop *nhop, } nhop->weight = weight; + nhop->srte_color = color; if (backup_str) { /* Parse backup indexes */ @@ -913,9 +931,11 @@ static bool nexthop_group_parse_nexthop(struct nexthop *nhop, static bool nexthop_group_parse_nhh(struct nexthop *nhop, const struct nexthop_hold *nhh) { - return (nexthop_group_parse_nexthop( - nhop, nhh->addr, nhh->intf, nhh->onlink, nhh->nhvrf_name, - nhh->labels, nhh->vni, NULL, nhh->weight, nhh->backup_str)); + return (nexthop_group_parse_nexthop(nhop, nhh->addr, nhh->intf, + nhh->onlink, nhh->nhvrf_name, + nhh->labels, nhh->vni, NULL, + nhh->weight, nhh->backup_str, + nhh->color)); } DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, @@ -930,6 +950,7 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, |vni (1-16777215) \ |weight (1-255) \ |backup-idx WORD \ + |color (1-4294967295) \ }]", NO_STR "Specify one of the nexthops in this ECMP group\n" @@ -947,7 +968,9 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, "Weight to be used by the nexthop for purposes of ECMP\n" "Weight value to be used\n" "Specify backup nexthop indexes in another group\n" - "One or more indexes in the range (0-254) separated by ','\n") + "One or more indexes in the range (0-254) separated by ','\n" + SRTE_STR + SRTE_COLOR_STR) { VTY_DECLVAR_CONTEXT(nexthop_group_cmd, nhgc); struct nexthop nhop; @@ -956,7 +979,9 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, bool legal; int num; uint8_t backups[NEXTHOP_MAX_BACKUPS]; - bool yes = !no; + bool yes = !no, color_config = false; + struct nexthop_hold *nhh; + struct listnode *node; /* Pre-parse backup string to validate */ if (backup_idx) { @@ -969,7 +994,7 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, legal = nexthop_group_parse_nexthop(&nhop, addr, intf, !!onlink, vrf_name, label, vni, &lbl_ret, - weight, backup_idx); + weight, backup_idx, color); if (nhop.type == NEXTHOP_TYPE_IPV6 && IN6_IS_ADDR_LINKLOCAL(&nhop.gate.ipv6)) { @@ -1032,10 +1057,18 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, /* Save config always */ nexthop_group_save_nhop(nhgc, vrf_name, addr, intf, !!onlink, - label, weight, backup_idx); + label, weight, backup_idx, color); + if (color) + nhgc_configure_color(nhgc, true); if (legal && nhg_hooks.add_nexthop) nhg_hooks.add_nexthop(nhgc, nh); + } else { + for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_list, node, nhh)) { + if (nhh->color) + color_config = true; + } + nhgc_configure_color(nhgc, color_config); } return CMD_SUCCESS; @@ -1108,6 +1141,9 @@ void nexthop_group_write_nexthop(struct vty *vty, const struct nexthop *nh) if (nh->weight) vty_out(vty, " weight %u", nh->weight); + if (nh->srte_color) + vty_out(vty, " color %u", nh->srte_color); + if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_HAS_BACKUP)) { vty_out(vty, " backup-idx %d", nh->backup_idx[0]); @@ -1165,6 +1201,9 @@ void nexthop_group_json_nexthop(json_object *j, const struct nexthop *nh) if (nh->weight) json_object_int_add(j, "weight", nh->weight); + if (nh->srte_color) + json_object_int_add(j, "color", nh->srte_color); + if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_HAS_BACKUP)) { json_backups = json_object_new_array(); for (i = 0; i < nh->backup_num; i++) @@ -1202,6 +1241,9 @@ static void nexthop_group_write_nexthop_internal(struct vty *vty, if (nh->weight) vty_out(vty, " weight %u", nh->weight); + if (nh->color) + vty_out(vty, " color %u", nh->color); + if (nh->backup_str) vty_out(vty, " backup-idx %s", nh->backup_str); diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index 73e6903afd5e..dba2f98376ed 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -39,6 +39,9 @@ struct nexthop_group { #define NEXTHOP_GROUP_ALLOW_RECURSION (1 << 1) #define NEXTHOP_GROUP_IBGP (1 << 2) uint8_t flags; + /* nexthop group messages */ +#define NEXTHOP_GROUP_MESSAGE_SRTE (1 << 1) + uint8_t message; }; struct nexthop_group *nexthop_group_new(void); @@ -104,6 +107,7 @@ RB_HEAD(nhgc_entry_head, nexthp_group_cmd); RB_PROTOTYPE(nhgc_entry_head, nexthop_group_cmd, nhgc_entry, nexthop_group_cmd_compare) DECLARE_QOBJ_TYPE(nexthop_group_cmd); +extern struct nhgc_entry_head nhgc_entries; /* * Initialize nexthop_groups. If you are interested in when diff --git a/lib/zclient.c b/lib/zclient.c index d9db09939b65..d67e07631e4b 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -1148,6 +1148,7 @@ int zapi_srv6_locator_decode(struct stream *s, struct srv6_locator *l) static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) { int i; + uint32_t api_message = 0; if (cmd != ZEBRA_NHG_DEL && cmd != ZEBRA_NHG_ADD) { flog_err(EC_LIB_ZAPI_ENCODE, @@ -1174,6 +1175,9 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) stream_putl(s, api_nhg->resilience.unbalanced_timer); stream_putc(s, api_nhg->flags); + stream_putc(s, api_nhg->message); + if (CHECK_FLAG(api_nhg->message, ZAPI_NEXTHOP_MESSAGE_SRTE)) + SET_FLAG(api_message, ZAPI_MESSAGE_SRTE); if (cmd == ZEBRA_NHG_ADD) { /* Nexthops */ @@ -1183,14 +1187,15 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) stream_putw(s, api_nhg->nexthop_num); for (i = 0; i < api_nhg->nexthop_num; i++) - zapi_nexthop_encode(s, &api_nhg->nexthops[i], 0, 0); + zapi_nexthop_encode(s, &api_nhg->nexthops[i], 0, + api_message); /* Backup nexthops */ stream_putw(s, api_nhg->backup_nexthop_num); for (i = 0; i < api_nhg->backup_nexthop_num; i++) zapi_nexthop_encode(s, &api_nhg->backup_nexthops[i], 0, - 0); + api_message); } stream_putw_at(s, 0, stream_get_endp(s)); diff --git a/lib/zclient.h b/lib/zclient.h index 0ca4b31c868e..14b63e04b80a 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -489,6 +489,9 @@ struct zapi_nhg { /* nexthop group flags */ uint8_t flags; + /* nexthop group message : definition from nexthop_group.h */ +#define ZAPI_NEXTHOP_MESSAGE_SRTE NEXTHOP_GROUP_MESSAGE_SRTE + uint8_t message; }; /* diff --git a/sharpd/sharp_nht.h b/sharpd/sharp_nht.h index 448b02b1d047..8d7eadc34fea 100644 --- a/sharpd/sharp_nht.h +++ b/sharpd/sharp_nht.h @@ -15,6 +15,8 @@ struct sharp_nh_tracker { uint32_t nhop_num; uint32_t updates; + + uint32_t color; }; extern struct sharp_nh_tracker *sharp_nh_tracker_get(struct prefix *p); diff --git a/sharpd/sharp_vty.c b/sharpd/sharp_vty.c index 5d72dbba0be7..4a10486316cf 100644 --- a/sharpd/sharp_vty.c +++ b/sharpd/sharp_vty.c @@ -55,7 +55,7 @@ DEFPY(watch_redistribute, watch_redistribute_cmd, } DEFPY(watch_nexthop_v6, watch_nexthop_v6_cmd, - "sharp watch [vrf NAME$vrf_name] [connected$connected]", + "sharp watch [vrf NAME$vrf_name] [connected$connected] [color (1-4294967295)]", "Sharp routing Protocol\n" "Watch for changes\n" "The vrf we would like to watch if non-default\n" @@ -64,11 +64,14 @@ DEFPY(watch_nexthop_v6, watch_nexthop_v6_cmd, "The v6 nexthop to signal for watching\n" "Watch for import check changes\n" "The v6 prefix to signal for watching\n" - "Should the route be connected\n") + "Should the route be connected\n" + SRTE_STR + SRTE_COLOR_STR) { struct vrf *vrf; struct prefix p; bool type_import; + struct sharp_nh_tracker *nht; if (!vrf_name) vrf_name = VRF_DEFAULT_NAME; @@ -91,7 +94,10 @@ DEFPY(watch_nexthop_v6, watch_nexthop_v6_cmd, prefix_copy(&p, inhop); } - sharp_nh_tracker_get(&p); + nht = sharp_nh_tracker_get(&p); + if (color) + nht->color = color; + sharp_zebra_nexthop_watch(&p, vrf->vrf_id, type_import, true, !!connected); @@ -99,7 +105,7 @@ DEFPY(watch_nexthop_v6, watch_nexthop_v6_cmd, } DEFPY(watch_nexthop_v4, watch_nexthop_v4_cmd, - "sharp watch [vrf NAME$vrf_name] [connected$connected]", + "sharp watch [vrf NAME$vrf_name] [connected$connected] [color (1-4294967295)]", "Sharp routing Protocol\n" "Watch for changes\n" "The vrf we would like to watch if non-default\n" @@ -108,11 +114,14 @@ DEFPY(watch_nexthop_v4, watch_nexthop_v4_cmd, "The v4 address to signal for watching\n" "Watch for import check changes\n" "The v4 prefix for import check to watch\n" - "Should the route be connected\n") + "Should the route be connected\n" + SRTE_STR + SRTE_COLOR_STR) { struct vrf *vrf; struct prefix p; bool type_import; + struct sharp_nh_tracker *nht; if (!vrf_name) vrf_name = VRF_DEFAULT_NAME; @@ -136,7 +145,9 @@ DEFPY(watch_nexthop_v4, watch_nexthop_v4_cmd, prefix_copy(&p, inhop); } - sharp_nh_tracker_get(&p); + nht = sharp_nh_tracker_get(&p); + if (color) + nht->color = color; sharp_zebra_nexthop_watch(&p, vrf->vrf_id, type_import, true, !!connected); diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index a3767f88bed0..ed863431bbdb 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -542,6 +542,7 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, api_nhg.id = id; api_nhg.flags = nhg->flags; + api_nhg.message = nhg->message; api_nhg.resilience = nhg->nhgr; for (ALL_NEXTHOPS_PTR(nhg, nh)) { @@ -682,15 +683,59 @@ static void sharp_nexthop_update(struct vrf *vrf, struct prefix *matched, struct zapi_route *nhr) { struct sharp_nh_tracker *nht; + struct nexthop_group_cmd *nhgc; + struct nexthop_group *nhg; + struct nexthop *nexthop; + uint32_t nhg_id; zlog_debug("Received update for %pFX actual match: %pFX metric: %u", matched, &nhr->prefix, nhr->metric); nht = sharp_nh_tracker_get(matched); - nht->nhop_num = nhr->nexthop_num; - nht->updates++; + + if ((nht->color && nhr->srte_color == nht->color) || (!nht->color)) { + nht->nhop_num = nhr->nexthop_num; + nht->updates++; + } sharp_debug_nexthops(nhr); + + if (nht->color) { + /* check any nhg with same match, color */ + RB_FOREACH (nhgc, nhgc_entry_head, &nhgc_entries) { + nhg_id = sharp_nhgroup_get_id(nhgc->name); + if (!nhg_id) + continue; + + nhg = &nhgc->nhg; + if (!CHECK_FLAG(nhg->message, + NEXTHOP_GROUP_MESSAGE_SRTE)) + continue; + + nexthop = nhg->nexthop; + while (nexthop) { + if (nexthop->srte_color != nht->color) { + nexthop = nexthop->next; + continue; + } + if (matched->family == AF_INET && + (nexthop->type == NEXTHOP_TYPE_IPV4_IFINDEX || + nexthop->type == NEXTHOP_TYPE_IPV4) && + IPV4_ADDR_SAME(&matched->u.prefix4, + &nexthop->gate.ipv4)) { + nhg_add(nhg_id, nhg, NULL); + } else if (matched->family == AF_INET6 && + (nexthop->type == + NEXTHOP_TYPE_IPV6_IFINDEX || + nexthop->type == NEXTHOP_TYPE_IPV6) && + IPV6_ADDR_SAME(&matched->u.prefix6, + &nexthop->gate.ipv6)) { + nhg_add(nhg_id, nhg, NULL); + } + nexthop = nexthop->next; + } + } + } } static int sharp_redistribute_route(ZAPI_CALLBACK_ARGS) diff --git a/tests/topotests/all_protocol_startup/r1/ip_nht.ref b/tests/topotests/all_protocol_startup/r1/ip_nht.ref index 3bc2c7838d52..157df34bcaa0 100644 --- a/tests/topotests/all_protocol_startup/r1/ip_nht.ref +++ b/tests/topotests/all_protocol_startup/r1/ip_nht.ref @@ -53,6 +53,9 @@ VRF default: 6.6.6.4 unresolved Client list: pbr(fd XX) +172.31.0.200 + unresolved + Client list: pbr(fd XX) sharp(fd XX) 192.0.2.200 resolved via static via 192.168.0.208, r1-eth0 (vrf default), weight 1 diff --git a/tests/topotests/all_protocol_startup/r1/pathd.conf b/tests/topotests/all_protocol_startup/r1/pathd.conf new file mode 100644 index 000000000000..6fb037066941 --- /dev/null +++ b/tests/topotests/all_protocol_startup/r1/pathd.conf @@ -0,0 +1,12 @@ +segment-routing + traffic-eng + segment-list seg1 + index 10 mpls label 400 + index 20 mpls label 600 + exit + policy color 1 endpoint 172.31.0.200 + name steer_color1 + candidate-path preference 10 name to210 explicit segment-list seg1 + exit + exit +exit diff --git a/tests/topotests/all_protocol_startup/r1/zebra.conf b/tests/topotests/all_protocol_startup/r1/zebra.conf index 0491d21a3a56..ba101491a3f2 100644 --- a/tests/topotests/all_protocol_startup/r1/zebra.conf +++ b/tests/topotests/all_protocol_startup/r1/zebra.conf @@ -50,6 +50,9 @@ ip route 4.5.6.17/32 192.168.0.2 tag 10000 ! resolution ip route 192.0.2.0/24 192.168.0.208 +# Create mpls lsp entry to resolve TE +# seg1 segment-list needs to resolve label 400 +mpls lsp 400 192.168.0.209 500 ! interface r1-eth0 description to sw0 - no routing protocol diff --git a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py index ffa095ce1132..e134c1a2b955 100644 --- a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py +++ b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py @@ -105,6 +105,7 @@ def setup_module(module): net["r%s" % i].loadConf("nhrpd", "%s/r%s/nhrpd.conf" % (thisDir, i)) net["r%s" % i].loadConf("babeld", "%s/r%s/babeld.conf" % (thisDir, i)) net["r%s" % i].loadConf("pbrd", "%s/r%s/pbrd.conf" % (thisDir, i)) + net["r%s" % i].loadConf("pathd", "%s/r%s/pathd.conf" % (thisDir, i)) tgen.gears["r%s" % i].start() # For debugging after starting FRR daemons, uncomment the next line @@ -645,6 +646,45 @@ def test_nexthop_groups(): % nhg_id ) + # Create colored static route + ## create a colored static route, and expect that the nexthop is steered by pathd + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group TESTSRTE + allow-recursion + nexthop 172.31.0.200 color 1 + """ + ) + tgen.gears["r1"].vtysh_cmd( + "sharp install routes 10.10.10.10 nexthop-group TESTSRTE 1" + ) + tgen.gears["r1"].vtysh_cmd("sharp watch nexthop 172.31.0.200 color 1") + nhg_id = route_get_nhg_id("10.10.10.10/32") + verify_nexthop_group(nhg_id, recursive=True, ecmp=0, recursive_again=True) + + output = tgen.gears["r1"].vtysh_cmd("show nexthop-group rib %d json" % nhg_id) + joutput = json.loads(output) + + # Use the json output and collect the nhg id from it + jnexthops = joutput[str(nhg_id)]["nexthops"] + for jnexthop in jnexthops: + if "recursive" in jnexthop.keys(): + assert jnexthop["srteColor"] == 1, "nexthop-group is not colored" + assert ( + jnexthop["ip"] == "172.31.0.200" + ), "nexthop-group 172.31.0.200 address not found" + if "labels" in jnexthop.keys(): + assert ( + jnexthop["ip"] == "192.168.0.209" + ), "nexthop-group recursive nexthop is not 192.168.0.209" + assert jnexthop["labels"] == [ + 400, + 600, + ], "nexthop-group recursive label is not 400/600" + + tgen.gears["r1"].vtysh_cmd("sharp remove routes 10.10.10.10 1") + ## Remove all NHG routes net["r1"].cmd('vtysh -c "sharp remove routes 2.2.2.1 1"') diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 1b3bcd1a2b8d..16a312ddcaf4 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1645,6 +1645,9 @@ static struct nexthop *nexthop_from_zapi(const struct zapi_nexthop *api_nh, if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_WEIGHT)) nexthop->weight = api_nh->weight; + /* save srte color */ + nexthop->srte_color = api_nh->srte_color; + if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP)) { /* Validate count */ if (api_nh->backup_num > NEXTHOP_MAX_BACKUPS) { @@ -1884,6 +1887,7 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) { uint16_t i; struct zapi_nexthop *znh; + uint32_t api_message = 0; STREAM_GETW(s, api_nhg->proto); STREAM_GETL(s, api_nhg->id); @@ -1896,6 +1900,10 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) STREAM_GETL(s, api_nhg->resilience.unbalanced_timer); STREAM_GETC(s, api_nhg->flags); + STREAM_GETC(s, api_nhg->message); + + if (CHECK_FLAG(api_nhg->message, ZAPI_NEXTHOP_MESSAGE_SRTE)) + SET_FLAG(api_message, ZAPI_MESSAGE_SRTE); /* Nexthops */ STREAM_GETW(s, api_nhg->nexthop_num); @@ -1912,7 +1920,7 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) for (i = 0; i < api_nhg->nexthop_num; i++) { znh = &((api_nhg->nexthops)[i]); - if (zapi_nexthop_decode(s, znh, 0, 0) != 0) { + if (zapi_nexthop_decode(s, znh, 0, api_message) != 0) { flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, "%s: Nexthop creation failed", __func__); return -1; @@ -1928,7 +1936,7 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) for (i = 0; i < api_nhg->backup_nexthop_num; i++) { znh = &((api_nhg->backup_nexthops)[i]); - if (zapi_nexthop_decode(s, znh, 0, 0) != 0) { + if (zapi_nexthop_decode(s, znh, 0, api_message) != 0) { flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, "%s: Backup Nexthop creation failed", __func__); From 9ea4a6cb0a149a2c44329b3da51836258c1b0ea4 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 23 Nov 2023 10:25:08 +0100 Subject: [PATCH 08/25] zebra, lib, doc: add protocol-controlled command to nhg Today, when a protocol creates a NHG, then ZEBRA automatically creates a NHG dependency with this NHG. Creating dynamic NHGs at ZEBRA layer goes against the concept of a protocol daemon that wants to entirely control its NHG. The below example considers the sharpd created nexthop as a NHG with id 179687502, while the nexthop information is stored in a zebra created nexthop 6462. > ubuntu2204(config)# nexthop-group 10 > 2023/11/23 10:27:34 SHARP: [Q5NBA-GN1BG] NHG ID assigned: 179687502 > ubuntu2204(config-nh-group)# nexthop 192.0.2.200 loop10 > [..] > ID: 179687502 (sharp) > RefCnt: 1 > Uptime: 00:00:17 > VRF: default > Valid, Installed > Depends: (6462) > via 192.0.2.200, loop1 (vrf default), weight 1 > ID: 6462 (sharp) > RefCnt: 2 > Uptime: 00:00:17 > VRF: default > Valid, Installed > Interface Index: 2964 > via 192.0.2.200, loop1 (vrf default), weight 1 > Dependents: (179687502) > > # ip nexthop ls > [..] > id 179687502 group 6462 proto 194 > id 6462 via 192.0.2.200 dev loop1 scope link proto 194 Add a CLI option to NHG that request zebra to not move the nexthop information in a separate NHG. > ubuntu2204(config-nh-group)# protocol-controlled > ubuntu2204(config-nh-group)# nexthop 192.0.2.200 loop10 > ubuntu2204(config-nh-group)# do show nexthop-group rib sharpd > ID: 179687504 (sharp) > RefCnt: 1 > Uptime: 00:00:04 > VRF: default > Valid, Installed > via 192.0.2.200, loop1 (vrf default), weight 1 > > # ip nexthop ls > [..] > id 179687504 via 192.0.2.200 dev loop1 scope link proto 194 Add associate tests for that in topotests. Signed-off-by: Philippe Guibert --- doc/user/nexthop_groups.rst | 6 +++ lib/nexthop_group.c | 35 +++++++++++++++ lib/nexthop_group.h | 1 + .../all_protocol_startup/r1/ip_nht.ref | 4 ++ .../test_all_protocol_startup.py | 43 +++++++++++++++++++ zebra/zebra_nhg.c | 11 ++++- 6 files changed, 99 insertions(+), 1 deletion(-) diff --git a/doc/user/nexthop_groups.rst b/doc/user/nexthop_groups.rst index d769abe8724e..afdf13d606ed 100644 --- a/doc/user/nexthop_groups.rst +++ b/doc/user/nexthop_groups.rst @@ -43,3 +43,9 @@ listing of ECMP nexthops used to forward packets. Allow a Nexthop Group to be considered as an IBGP nexthop. When used in ZEBRA, if `allow-recursion` is not used, some debug information is given. + +.. clicmd:: protocol-controlled + + Allow a Nexthop Group to be completely controlled at the protocol level, by preventing + ZEBRA from creating a dependent NHG at ZEBRA Level. This option is useful when + protocol considers Nexthop Groups with multiple paths. diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index a14bd3916db9..59f802facd1e 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -264,6 +264,7 @@ void nexthop_group_copy(struct nexthop_group *to, const struct nexthop_group *from) { to->nhgr = from->nhgr; + to->flags = from->flags; /* Copy everything, including recursive info */ copy_nexthops(&to->nexthop, from->nexthop, NULL); } @@ -667,6 +668,35 @@ DEFPY(nexthop_group_allow_recursion, UNSET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION); else SET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION); + if (nhg_hooks.modify) + nhg_hooks.modify(nhgc, true); + + return CMD_SUCCESS; +} + +DEFPY(nexthop_group_protocol_controlled, + nexthop_group_protocol_controlled_cmd, + "[no] protocol-controlled", + NO_STR + "Let the protocol daemon control the Nexthop Group, instead of Zebra\n") +{ + VTY_DECLVAR_CONTEXT(nexthop_group_cmd, nhgc); + + if (!!no == + !CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_PROTOCOL_CONTROLLED)) + return CMD_SUCCESS; + + /* if already nexthops, forbid */ + if (listcount(nhgc->nhg_list)) { + vty_out(vty, + "%% configured next-hops can not be modified with 'protocol-controlled'\n"); + return CMD_WARNING_CONFIG_FAILED; + } + + if (no) + UNSET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_PROTOCOL_CONTROLLED); + else + SET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_PROTOCOL_CONTROLLED); if (nhg_hooks.modify) nhg_hooks.modify(nhgc, true); @@ -1266,6 +1296,10 @@ static int nexthop_group_write(struct vty *vty) if (CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_IBGP)) vty_out(vty, " ibgp\n"); + if (CHECK_FLAG(nhgc->nhg.flags, + NEXTHOP_GROUP_PROTOCOL_CONTROLLED)) + vty_out(vty, " protocol-controlled\n"); + if (nhgc->nhg.nhgr.buckets) vty_out(vty, " resilient buckets %u idle-timer %u unbalanced-timer %u\n", @@ -1471,6 +1505,7 @@ void nexthop_group_init( install_default(NH_GROUP_NODE); install_element(NH_GROUP_NODE, &nexthop_group_backup_cmd); install_element(NH_GROUP_NODE, &no_nexthop_group_backup_cmd); + install_element(NH_GROUP_NODE, &nexthop_group_protocol_controlled_cmd); install_element(NH_GROUP_NODE, &ecmp_nexthops_cmd); install_element(NH_GROUP_NODE, &nexthop_group_resilience_cmd); diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index dba2f98376ed..cba449b54ff0 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -38,6 +38,7 @@ struct nexthop_group { /* nexthop group flags */ #define NEXTHOP_GROUP_ALLOW_RECURSION (1 << 1) #define NEXTHOP_GROUP_IBGP (1 << 2) +#define NEXTHOP_GROUP_PROTOCOL_CONTROLLED (1 << 3) uint8_t flags; /* nexthop group messages */ #define NEXTHOP_GROUP_MESSAGE_SRTE (1 << 1) diff --git a/tests/topotests/all_protocol_startup/r1/ip_nht.ref b/tests/topotests/all_protocol_startup/r1/ip_nht.ref index 157df34bcaa0..1dceb3ca76ac 100644 --- a/tests/topotests/all_protocol_startup/r1/ip_nht.ref +++ b/tests/topotests/all_protocol_startup/r1/ip_nht.ref @@ -68,6 +68,10 @@ VRF default: resolved via connected is directly connected, r1-eth0 (vrf default) Client list: static(fd XX) +192.168.0.200 + resolved via connected + is directly connected, r1-eth0 (vrf default) + Client list: pbr(fd XX) 192.168.0.208 resolved via connected is directly connected, r1-eth0 (vrf default) diff --git a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py index e134c1a2b955..615d2f392ce1 100644 --- a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py +++ b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py @@ -685,6 +685,48 @@ def test_nexthop_groups(): tgen.gears["r1"].vtysh_cmd("sharp remove routes 10.10.10.10 1") + ## protocol-controlled + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group CONTROL + protocol-controlled + nexthop 192.168.0.200 r1-eth0 + """ + ) + tgen.gears["r1"].vtysh_cmd("sharp install routes 7.7.7.7 nexthop-group CONTROL 1") + verify_route_nexthop_group("7.7.7.7/32") + + nhg_id = route_get_nhg_id("7.7.7.7/32") + output = tgen.gears["r1"].vtysh_cmd("show nexthop-group rib %d" % nhg_id) + valid = re.search(r"Valid", output) + assert valid is not None, ( + "Route 7.7.7.7/32 with Nexthop Group ID=%d is not active" % nhg_id + ) + valid = re.search(r"Depends:", output) + assert valid is None, ( + "Route 7.7.7.7/32 with Nexthop Group ID=%d has a dependency" % nhg_id + ) + + ## disable protocol-controlled + ## modifying the value switches the nexthop from nexthop to nhgroup or vice-versa + ## Following error can be seen: "Extended Error: Can not replace a nexthop with a nexthop group." + ## not supported today, as sharpd has no way to know if the nhgroup is really removed + ## the nhgroups are removed from a configurable timer (zebra nexthop-group keep XX) + + tgen.gears["r1"].vtysh_cmd("sharp remove routes 7.7.7.7 1") + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group CONTROL + no protocol-controlled + """ + ) + valid = re.search(r"configured next-hops can not be modified with ", output) + assert ( + valid is not None + ), "nexthop-group CONTROL 'protocol-controlled' command should not be modified" + ## Remove all NHG routes net["r1"].cmd('vtysh -c "sharp remove routes 2.2.2.1 1"') @@ -696,6 +738,7 @@ def test_nexthop_groups(): net["r1"].cmd('vtysh -c "sharp remove routes 5.5.5.1 1"') net["r1"].cmd('vtysh -c "sharp remove routes 6.6.6.1 4"') net["r1"].cmd('vtysh -c "c t" -c "no ip route 6.6.6.0/24 1.1.1.1"') + net["r1"].cmd('vtysh -c "sharp remove routes 7.7.7.7 1"') def test_rip_status(): diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index e68abe9202f4..52a985934294 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -779,7 +779,7 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ recursive = true; } } else { - /* Proto-owned are groups by default */ + /* Proto-owned are groups by default, unless PROTOCOL_CONTROLLED group flg is set */ /* List of nexthops */ for (nh = newnhe->nhg.nexthop; nh; nh = nh->next) { if (IS_ZEBRA_DEBUG_NHG_DETAIL) @@ -789,6 +789,10 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ NEXTHOP_FLAG_RECURSIVE) ? "(R)" : ""); + if (CHECK_FLAG(newnhe->nhg.flags, + NEXTHOP_GROUP_PROTOCOL_CONTROLLED)) + continue; + depends_find_add(&newnhe->nhg_depends, nh, afi, newnhe->type, from_dplane); } @@ -837,6 +841,10 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ NEXTHOP_FLAG_RECURSIVE) ? "(R)" : ""); + if (CHECK_FLAG(backup_nhe->nhg.flags, + NEXTHOP_GROUP_PROTOCOL_CONTROLLED)) + continue; + depends_find_add(&backup_nhe->nhg_depends, nh, afi, backup_nhe->type, from_dplane); } @@ -3436,6 +3444,7 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, lookup.nhg.nhgr = nhg->nhgr; lookup.id = id; lookup.type = type; + lookup.nhg.flags = nhg->flags; old = zebra_nhg_lookup_id(id); From 2e4736a0d7e240bd0edf2cbf0dfd4eb8a4cefab4 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Wed, 30 Aug 2023 11:43:18 +0200 Subject: [PATCH 09/25] *: move [backup_]nexthop from zapi_nhg into nhg_nexthop sub-structure The zapi nexthop group structure is unable to reference other nexthop groups. Actually, only nexthop IP addresses can be associated to a nexthop group via the ZAPI interface. This first commit is a preliminary commit that prepares the ZAPI interface in order to host a second type of nexthop group, by moving the current zapi_nexthop definition into a specific sub-structure. This commit does not change the functionality. Signed-off-by: Philippe Guibert --- bgpd/bgp_evpn_mh.c | 9 +++++---- lib/zclient.c | 20 ++++++++++---------- lib/zclient.h | 15 ++++++++++----- sharpd/sharp_zebra.c | 19 +++++++++++-------- zebra/zapi_msg.c | 36 ++++++++++++++++++++---------------- 5 files changed, 56 insertions(+), 43 deletions(-) diff --git a/bgpd/bgp_evpn_mh.c b/bgpd/bgp_evpn_mh.c index d88c52d1f6d9..6f9e377b7306 100644 --- a/bgpd/bgp_evpn_mh.c +++ b/bgpd/bgp_evpn_mh.c @@ -2825,7 +2825,7 @@ static void bgp_evpn_l3nhg_zebra_add_v4_or_v6(struct bgp_evpn_es_vrf *es_vrf, continue; /* Don't overrun the zapi buffer. */ - if (api_nhg.nexthop_num == MULTIPATH_NUM) + if (api_nhg.nhg_nexthop.nexthop_num == MULTIPATH_NUM) break; /* overwrite the gw */ @@ -2836,10 +2836,11 @@ static void bgp_evpn_l3nhg_zebra_add_v4_or_v6(struct bgp_evpn_es_vrf *es_vrf, es_vtep->vtep_ip); /* convert to zapi format */ - api_nh = &api_nhg.nexthops[api_nhg.nexthop_num]; + api_nh = &api_nhg.nhg_nexthop + .nexthops[api_nhg.nhg_nexthop.nexthop_num]; zapi_nexthop_from_nexthop(api_nh, &nh); - ++api_nhg.nexthop_num; + ++api_nhg.nhg_nexthop.nexthop_num; if (BGP_DEBUG(evpn_mh, EVPN_MH_ES)) zlog_debug("nhg %u vtep %pI4 l3-svi %d", api_nhg.id, &es_vtep->vtep_ip, @@ -2848,7 +2849,7 @@ static void bgp_evpn_l3nhg_zebra_add_v4_or_v6(struct bgp_evpn_es_vrf *es_vrf, frrtrace(3, frr_bgp, evpn_mh_nh_zsend, nhg_id, es_vtep, es_vrf); } - if (!api_nhg.nexthop_num) + if (!api_nhg.nhg_nexthop.nexthop_num) return; zclient_nhg_send(zclient, ZEBRA_NHG_ADD, &api_nhg); diff --git a/lib/zclient.c b/lib/zclient.c index d67e07631e4b..f8e04a42ac87 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -1157,8 +1157,8 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) return -1; } - if (api_nhg->nexthop_num >= MULTIPATH_NUM || - api_nhg->backup_nexthop_num >= MULTIPATH_NUM) { + if (api_nhg->nhg_nexthop.nexthop_num >= MULTIPATH_NUM || + api_nhg->nhg_nexthop.backup_nexthop_num >= MULTIPATH_NUM) { flog_err(EC_LIB_ZAPI_ENCODE, "%s: zapi NHG encode with invalid input", __func__); return -1; @@ -1181,20 +1181,20 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) if (cmd == ZEBRA_NHG_ADD) { /* Nexthops */ - zapi_nexthop_group_sort(api_nhg->nexthops, - api_nhg->nexthop_num); + zapi_nexthop_group_sort(api_nhg->nhg_nexthop.nexthops, + api_nhg->nhg_nexthop.nexthop_num); - stream_putw(s, api_nhg->nexthop_num); + stream_putw(s, api_nhg->nhg_nexthop.nexthop_num); - for (i = 0; i < api_nhg->nexthop_num; i++) - zapi_nexthop_encode(s, &api_nhg->nexthops[i], 0, + for (i = 0; i < api_nhg->nhg_nexthop.nexthop_num; i++) + zapi_nexthop_encode(s, &api_nhg->nhg_nexthop.nexthops[i], 0, api_message); /* Backup nexthops */ - stream_putw(s, api_nhg->backup_nexthop_num); + stream_putw(s, api_nhg->nhg_nexthop.backup_nexthop_num); - for (i = 0; i < api_nhg->backup_nexthop_num; i++) - zapi_nexthop_encode(s, &api_nhg->backup_nexthops[i], 0, + for (i = 0; i < api_nhg->nhg_nexthop.backup_nexthop_num; i++) + zapi_nexthop_encode(s, &api_nhg->nhg_nexthop.backup_nexthops[i], 0, api_message); } diff --git a/lib/zclient.h b/lib/zclient.h index 14b63e04b80a..1198c725cdd0 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -475,17 +475,22 @@ struct zapi_nexthop { /* * ZAPI Nexthop Group. For use with protocol creation of nexthop groups. */ +struct zapi_nhg_nexthop { + uint16_t nexthop_num; + struct zapi_nexthop nexthops[MULTIPATH_NUM]; + + uint16_t backup_nexthop_num; + struct zapi_nexthop backup_nexthops[MULTIPATH_NUM]; +}; + + struct zapi_nhg { uint16_t proto; uint32_t id; struct nhg_resilience resilience; - uint16_t nexthop_num; - struct zapi_nexthop nexthops[MULTIPATH_NUM]; - - uint16_t backup_nexthop_num; - struct zapi_nexthop backup_nexthops[MULTIPATH_NUM]; + struct zapi_nhg_nexthop nhg_nexthop; /* nexthop group flags */ uint8_t flags; diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index ed863431bbdb..8cc3457450b2 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -546,7 +546,7 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, api_nhg.resilience = nhg->nhgr; for (ALL_NEXTHOPS_PTR(nhg, nh)) { - if (api_nhg.nexthop_num >= MULTIPATH_NUM) { + if (api_nhg.nhg_nexthop.nexthop_num >= MULTIPATH_NUM) { zlog_warn( "%s: number of nexthops greater than max multipath size, truncating", __func__); @@ -561,13 +561,14 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, !force_nhg_config) continue; - api_nh = &api_nhg.nexthops[api_nhg.nexthop_num]; + api_nh = &api_nhg.nhg_nexthop + .nexthops[api_nhg.nhg_nexthop.nexthop_num]; zapi_nexthop_from_nexthop(api_nh, nh); - api_nhg.nexthop_num++; + api_nhg.nhg_nexthop.nexthop_num++; } - if (api_nhg.nexthop_num == 0) { + if (api_nhg.nhg_nexthop.nexthop_num == 0) { if (sharp_nhgroup_id_is_installed(id)) { zlog_debug("%s: nhg %u: no nexthops, deleting nexthop group", __func__, id); @@ -582,7 +583,8 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, if (backup_nhg) { for (ALL_NEXTHOPS_PTR(backup_nhg, nh)) { - if (api_nhg.backup_nexthop_num >= MULTIPATH_NUM) { + if (api_nhg.nhg_nexthop.backup_nexthop_num >= + MULTIPATH_NUM) { zlog_warn( "%s: number of backup nexthops greater than max multipath size, truncating", __func__); @@ -602,11 +604,12 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, break; } - api_nh = &api_nhg.backup_nexthops - [api_nhg.backup_nexthop_num]; + api_nh = + &api_nhg.nhg_nexthop.backup_nexthops + [api_nhg.nhg_nexthop.backup_nexthop_num]; zapi_backup_nexthop_from_nexthop(api_nh, nh); - api_nhg.backup_nexthop_num++; + api_nhg.nhg_nexthop.backup_nexthop_num++; } } diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 16a312ddcaf4..dc5c5c2f301b 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1906,19 +1906,20 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) SET_FLAG(api_message, ZAPI_MESSAGE_SRTE); /* Nexthops */ - STREAM_GETW(s, api_nhg->nexthop_num); + STREAM_GETW(s, api_nhg->nhg_nexthop.nexthop_num); - if (zserv_nexthop_num_warn(__func__, NULL, api_nhg->nexthop_num)) + if (zserv_nexthop_num_warn(__func__, NULL, + api_nhg->nhg_nexthop.nexthop_num)) return -1; - if (api_nhg->nexthop_num <= 0) { + if (api_nhg->nhg_nexthop.nexthop_num <= 0) { flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, "%s: No nexthops sent", __func__); return -1; } - for (i = 0; i < api_nhg->nexthop_num; i++) { - znh = &((api_nhg->nexthops)[i]); + for (i = 0; i < api_nhg->nhg_nexthop.nexthop_num; i++) { + znh = &((api_nhg->nhg_nexthop.nexthops)[i]); if (zapi_nexthop_decode(s, znh, 0, api_message) != 0) { flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, @@ -1928,13 +1929,14 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) } /* Backup Nexthops */ - STREAM_GETW(s, api_nhg->backup_nexthop_num); + STREAM_GETW(s, api_nhg->nhg_nexthop.backup_nexthop_num); - if (zserv_nexthop_num_warn(__func__, NULL, api_nhg->backup_nexthop_num)) + if (zserv_nexthop_num_warn(__func__, NULL, + api_nhg->nhg_nexthop.backup_nexthop_num)) return -1; - for (i = 0; i < api_nhg->backup_nexthop_num; i++) { - znh = &((api_nhg->backup_nexthops)[i]); + for (i = 0; i < api_nhg->nhg_nexthop.backup_nexthop_num; i++) { + znh = &((api_nhg->nhg_nexthop.backup_nexthops)[i]); if (zapi_nexthop_decode(s, znh, 0, api_message) != 0) { flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, @@ -2002,13 +2004,15 @@ static void zread_nhg_add(ZAPI_HANDLER_ARGS) return; } - if ((!zapi_read_nexthops(client, NULL, api_nhg.nexthops, 0, 0, - api_nhg.nexthop_num, - api_nhg.backup_nexthop_num, &nhg, NULL)) - || (!zapi_read_nexthops(client, NULL, api_nhg.backup_nexthops, 0, 0, - api_nhg.backup_nexthop_num, - api_nhg.backup_nexthop_num, NULL, &bnhg))) { - + if ((!zapi_read_nexthops(client, NULL, api_nhg.nhg_nexthop.nexthops, 0, + 0, api_nhg.nhg_nexthop.nexthop_num, + api_nhg.nhg_nexthop.backup_nexthop_num, &nhg, + NULL)) || + (!zapi_read_nexthops(client, NULL, + api_nhg.nhg_nexthop.backup_nexthops, 0, 0, + api_nhg.nhg_nexthop.backup_nexthop_num, + api_nhg.nhg_nexthop.backup_nexthop_num, NULL, + &bnhg))) { flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, "%s: Nexthop Group Creation failed", __func__); From 129a217c9b70c1f9099035956f1e92394fc21493 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Wed, 30 Aug 2023 19:29:32 +0200 Subject: [PATCH 10/25] *: add nexthop_group group ids support, part 1 Today, protocol daemons can not create ECMP next-hops by using multiple NHGs. The below configuration shows an ECMP NHG. > nexthop-group ABCD > nexthop 192.168.0.100 loop1 > nexthop 192.168.0.105 loop1 > In the 'multiple NHG' case, 2 NHGs would be used for each nexthop, and the 3rd ony would group the two first, similar to what is done with iproute2: > ip nexthop add id 210 via 192.168.0.100 dev loop1 > ip nexthop add id 211 via 192.168.0.105 dev loop1 > ip nexthop add id 212 group 210/211 Extend the nexthop group structure to support a list of nexthop groups. Expose a vty API inside the nexthop group node, to host a new command to configure a group name. That group name points to another nexthop group. > nexthop-group ABCD > group ECMP1 > group ECMP2 > exit > nexthop-group ECMP1 > nexthop 192.168.0.100 loop1 > exit > nexthop-group ECMP2 > nexthop 192.168.0.105 loop1 > exit This commit only focuses to changes in the ZAPI interface, and on the sharpd daemon. The handling of group ids on zebra is stopped in 'zebra_nhg_proto_add()' function. Signed-off-by: Philippe Guibert --- doc/user/nexthop_groups.rst | 8 + lib/nexthop_group.c | 286 ++++++++++++++++++++++++++++++++---- lib/nexthop_group.h | 30 +++- lib/zclient.c | 23 ++- lib/zclient.h | 15 +- pbrd/pbr_nht.c | 9 +- sharpd/sharp_nht.c | 204 +++++++++++++++++++++++-- sharpd/sharp_nht.h | 2 + sharpd/sharp_vty.c | 6 +- sharpd/sharp_zebra.c | 71 +++++++-- sharpd/sharp_zebra.h | 2 +- zebra/zapi_msg.c | 132 +++++++++++++++-- zebra/zebra_nhg.c | 6 + 13 files changed, 717 insertions(+), 77 deletions(-) diff --git a/doc/user/nexthop_groups.rst b/doc/user/nexthop_groups.rst index afdf13d606ed..a61acd57e2fd 100644 --- a/doc/user/nexthop_groups.rst +++ b/doc/user/nexthop_groups.rst @@ -49,3 +49,11 @@ listing of ECMP nexthops used to forward packets. Allow a Nexthop Group to be completely controlled at the protocol level, by preventing ZEBRA from creating a dependent NHG at ZEBRA Level. This option is useful when protocol considers Nexthop Groups with multiple paths. + +.. clicmd:: group NAME + + Append a Nexthop Group group dependency in the current nexthop group. When used with + the `protocol-controlled` option, the protocol daemon using it will ensure that the + dependent group is configured at the protocol level, and installed at ZEBRA level, + before installing the parent Nexthop Group. This option is very useful to consider + nexthop groups having multiple paths. diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 59f802facd1e..88ffa5ca318c 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -18,6 +18,7 @@ #include "lib/nexthop_group_clippy.c" DEFINE_MTYPE_STATIC(LIB, NEXTHOP_GROUP, "Nexthop Group"); +DEFINE_MTYPE_STATIC(LIB, NH_GROUP_ID, "Nexthop Group Id"); /* * Internal struct used to hold nhg config strings @@ -37,10 +38,10 @@ struct nexthop_hold { struct nexthop_group_hooks { void (*new)(const char *name); void (*modify)(const struct nexthop_group_cmd *nhgc, bool reset); - void (*add_nexthop)(const struct nexthop_group_cmd *nhg, - const struct nexthop *nhop); - void (*del_nexthop)(const struct nexthop_group_cmd *nhg, - const struct nexthop *nhop); + void (*add_nexthop_or_group)(const struct nexthop_group_cmd *nhg, + const struct nexthop *nhop); + void (*del_nexthop_or_group)(const struct nexthop_group_cmd *nhg, + const struct nexthop *nhop); void (*delete)(const char *name); int (*write_config)(struct vty *vty, const struct nexthop_group_cmd *nhg); @@ -275,7 +276,10 @@ void nexthop_group_delete(struct nexthop_group **nhg) if ((*nhg) == NULL) return; - if ((*nhg)->nexthop) + if (CHECK_FLAG((*nhg)->flags, NEXTHOP_GROUP_TYPE_GROUP) && + ((*nhg)->group)) + nexthop_group_ids_free((*nhg)->group); + else if ((*nhg)->nexthop) nexthops_free((*nhg)->nexthop); XFREE(MTYPE_NEXTHOP_GROUP, *nhg); @@ -490,8 +494,8 @@ static void nhgc_delete_nexthops(struct nexthop_group_cmd *nhgc) struct nexthop *next = nexthop_next(nexthop); _nexthop_del(&nhgc->nhg, nexthop); - if (nhg_hooks.del_nexthop) - nhg_hooks.del_nexthop(nhgc, nexthop); + if (nhg_hooks.del_nexthop_or_group) + nhg_hooks.del_nexthop_or_group(nhgc, nexthop); nexthop_free(nexthop); @@ -573,6 +577,16 @@ static void nhgl_delete(struct nexthop_hold *nh) XFREE(MTYPE_TMP, nh); } +static int nhgl_group_cmp(char *group1, char *group2) +{ + return nhgc_cmp_helper(group1, group2); +} + +static void nhgl_group_delete(char *group1) +{ + XFREE(MTYPE_TMP, group1); +} + static struct nexthop_group_cmd *nhgc_get(const char *name) { struct nexthop_group_cmd *nhgc; @@ -589,6 +603,11 @@ static struct nexthop_group_cmd *nhgc_get(const char *name) nhgc->nhg_list->cmp = (int (*)(void *, void *))nhgl_cmp; nhgc->nhg_list->del = (void (*)(void *))nhgl_delete; + nhgc->nhg_group_list = list_new(); + nhgc->nhg_group_list->cmp = + (int (*)(void *, void *))nhgl_group_cmp; + nhgc->nhg_group_list->del = (void (*)(void *))nhgl_group_delete; + if (nhg_hooks.new) nhg_hooks.new(name); } @@ -606,11 +625,45 @@ static void nhgc_delete(struct nexthop_group_cmd *nhgc) RB_REMOVE(nhgc_entry_head, &nhgc_entries, nhgc); list_delete(&nhgc->nhg_list); + list_delete(&nhgc->nhg_group_list); QOBJ_UNREG(nhgc); XFREE(MTYPE_TMP, nhgc); } +/* remove group configuration + * return true if found, false if not + */ +static bool nexthop_group_unsave_group(struct nexthop_group_cmd *nhgc, + const char *group) +{ + char *groupname; + struct listnode *node; + bool found = false; + + for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_group_list, node, groupname)) { + if (nhgc_cmp_helper(group, groupname) == 0) { + found = true; + break; + } + } + + list_delete_node(nhgc->nhg_group_list, node); + nhgl_group_delete(groupname); + return found; +} + +static void nexthop_group_save_group(struct nexthop_group_cmd *nhgc, + const char *group) +{ + char *nhgroup; + + nhgroup = XSTRDUP(MTYPE_TMP, group); + + if (!listnode_add_sort_nodup(nhgc->nhg_group_list, nhgroup)) + XFREE(MTYPE_TMP, nhgroup); +} + DEFINE_QOBJ_TYPE(nexthop_group_cmd); DEFUN_NOSH(nexthop_group, nexthop_group_cmd, "nexthop-group NHGNAME", @@ -704,6 +757,37 @@ DEFPY(nexthop_group_protocol_controlled, return CMD_SUCCESS; } +DEFPY(nexthop_group_id, nexthop_group_id_cmd, "[no$no] group NHGNAME$name", + NO_STR "Specify a group name containing nexthops\n" + "The name of the group\n") +{ + VTY_DECLVAR_CONTEXT(nexthop_group_cmd, nhgc); + + /* if already nexthops, forbid */ + if (listcount(nhgc->nhg_list)) { + vty_out(vty, "%% nexthop group ids nexthop group nexthop\n"); + return CMD_WARNING_CONFIG_FAILED; + } + + if (no) { + if (nexthop_group_unsave_group(nhgc, name)) { + if (!listcount(nhgc->nhg_group_list)) + SET_FLAG(nhgc->nhg.flags, + NEXTHOP_GROUP_TYPE_GROUP); + if (nhg_hooks.del_nexthop_or_group) + nhg_hooks.del_nexthop_or_group(nhgc, NULL); + } + } else { + nexthop_group_save_group(nhgc, name); + SET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP); + + if (nhg_hooks.add_nexthop_or_group) + nhg_hooks.add_nexthop_or_group(nhgc, NULL); + } + + return CMD_SUCCESS; +} + DEFPY(nexthop_group_ibgp, nexthop_group_ibgp_cmd, "[no] ibgp", @@ -1013,6 +1097,10 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, struct nexthop_hold *nhh; struct listnode *node; + if (CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) { + vty_out(vty, "%% nexthop not possible on nexthop group ids\n"); + return CMD_WARNING_CONFIG_FAILED; + } /* Pre-parse backup string to validate */ if (backup_idx) { lbl_ret = nexthop_str2backups(backup_idx, &num, backups); @@ -1069,8 +1157,8 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, if (nh) { nexthop_unlink(&nhgc->nhg, nh); - if (nhg_hooks.del_nexthop) - nhg_hooks.del_nexthop(nhgc, nh); + if (nhg_hooks.del_nexthop_or_group) + nhg_hooks.del_nexthop_or_group(nhgc, nh); nexthop_free(nh); } @@ -1091,8 +1179,8 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, if (color) nhgc_configure_color(nhgc, true); - if (legal && nhg_hooks.add_nexthop) - nhg_hooks.add_nexthop(nhgc, nh); + if (legal && nhg_hooks.add_nexthop_or_group) + nhg_hooks.add_nexthop_or_group(nhgc, nh); } else { for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_list, node, nhh)) { if (nhh->color) @@ -1284,6 +1372,7 @@ static int nexthop_group_write(struct vty *vty) { struct nexthop_group_cmd *nhgc; struct nexthop_hold *nh; + char *group; RB_FOREACH (nhgc, nhgc_entry_head, &nhgc_entries) { struct listnode *node; @@ -1316,6 +1405,9 @@ static int nexthop_group_write(struct vty *vty) nexthop_group_write_nexthop_internal(vty, nh); } + for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_group_list, node, group)) + vty_out(vty, " group %s\n", group); + if (nhg_hooks.write_config) nhg_hooks.write_config(vty, nhgc); @@ -1354,8 +1446,8 @@ void nexthop_group_enable_vrf(struct vrf *vrf) memcpy(nh, &nhop, sizeof(nhop)); _nexthop_add(&nhgc->nhg.nexthop, nh); - if (nhg_hooks.add_nexthop) - nhg_hooks.add_nexthop(nhgc, nh); + if (nhg_hooks.add_nexthop_or_group) + nhg_hooks.add_nexthop_or_group(nhgc, nh); } } } @@ -1364,10 +1456,18 @@ void nexthop_group_disable_vrf(struct vrf *vrf) { struct nexthop_group_cmd *nhgc; struct nexthop_hold *nhh; + char *groupname; RB_FOREACH (nhgc, nhgc_entry_head, &nhgc_entries) { struct listnode *node, *nnode; + for (ALL_LIST_ELEMENTS(nhgc->nhg_group_list, node, nnode, + groupname)) { + if (nhg_hooks.del_nexthop_or_group) + nhg_hooks.del_nexthop_or_group(nhgc, NULL); + XFREE(MTYPE_TMP, groupname); + list_delete_node(nhgc->nhg_group_list, node); + } for (ALL_LIST_ELEMENTS(nhgc->nhg_list, node, nnode, nhh)) { struct nexthop nhop; struct nexthop *nh; @@ -1385,8 +1485,8 @@ void nexthop_group_disable_vrf(struct vrf *vrf) _nexthop_del(&nhgc->nhg, nh); - if (nhg_hooks.del_nexthop) - nhg_hooks.del_nexthop(nhgc, nh); + if (nhg_hooks.del_nexthop_or_group) + nhg_hooks.del_nexthop_or_group(nhgc, nh); nexthop_free(nh); @@ -1397,6 +1497,26 @@ void nexthop_group_disable_vrf(struct vrf *vrf) } } +void nexthop_group_dependent_group_match( + const char *nhgc_name, + void (*cb_func)(const struct nexthop_group_cmd *nhgc)) +{ + struct nexthop_group_cmd *nhgc_tmp; + struct listnode *node; + char *groupname; + + RB_FOREACH (nhgc_tmp, nhgc_entry_head, &nhgc_entries) { + if (!CHECK_FLAG(nhgc_tmp->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + continue; + for (ALL_LIST_ELEMENTS_RO(nhgc_tmp->nhg_group_list, node, + groupname)) { + if (strmatch(groupname, nhgc_name) && cb_func) { + (*cb_func)(nhgc_tmp); + break; + } + } + } +} void nexthop_group_interface_state_change(struct interface *ifp, ifindex_t oldifindex) { @@ -1437,8 +1557,8 @@ void nexthop_group_interface_state_change(struct interface *ifp, memcpy(nh, &nhop, sizeof(nhop)); _nexthop_add(&nhgc->nhg.nexthop, nh); - if (nhg_hooks.add_nexthop) - nhg_hooks.add_nexthop(nhgc, nh); + if (nhg_hooks.add_nexthop_or_group) + nhg_hooks.add_nexthop_or_group(nhgc, nh); } } else { struct nexthop *next_nh; @@ -1461,8 +1581,8 @@ void nexthop_group_interface_state_change(struct interface *ifp, _nexthop_del(&nhgc->nhg, nh); - if (nhg_hooks.del_nexthop) - nhg_hooks.del_nexthop(nhgc, nh); + if (nhg_hooks.del_nexthop_or_group) + nhg_hooks.del_nexthop_or_group(nhgc, nh); nexthop_free(nh); } @@ -1486,10 +1606,10 @@ static const struct cmd_variable_handler nhg_name_handlers[] = { void nexthop_group_init( void (*new)(const char *name), void (*modify)(const struct nexthop_group_cmd *nhgc, bool reset), - void (*add_nexthop)(const struct nexthop_group_cmd *nhg, - const struct nexthop *nhop), - void (*del_nexthop)(const struct nexthop_group_cmd *nhg, - const struct nexthop *nhop), + void (*add_nexthop_or_group)(const struct nexthop_group_cmd *nhg, + const struct nexthop *nhop), + void (*del_nexthop_or_group)(const struct nexthop_group_cmd *nhg, + const struct nexthop *nhop), void (*delete)(const char *name), int (*write_config)(struct vty *vty, const struct nexthop_group_cmd *nhgc)) @@ -1504,6 +1624,7 @@ void nexthop_group_init( install_default(NH_GROUP_NODE); install_element(NH_GROUP_NODE, &nexthop_group_backup_cmd); + install_element(NH_GROUP_NODE, &nexthop_group_id_cmd); install_element(NH_GROUP_NODE, &no_nexthop_group_backup_cmd); install_element(NH_GROUP_NODE, &nexthop_group_protocol_controlled_cmd); install_element(NH_GROUP_NODE, &ecmp_nexthops_cmd); @@ -1519,12 +1640,123 @@ void nexthop_group_init( nhg_hooks.new = new; if (modify) nhg_hooks.modify = modify; - if (add_nexthop) - nhg_hooks.add_nexthop = add_nexthop; - if (del_nexthop) - nhg_hooks.del_nexthop = del_nexthop; + if (add_nexthop_or_group) + nhg_hooks.add_nexthop_or_group = add_nexthop_or_group; + if (del_nexthop_or_group) + nhg_hooks.del_nexthop_or_group = del_nexthop_or_group; if (delete) nhg_hooks.delete = delete; if (write_config) nhg_hooks.write_config = write_config; } + +/* allocate if group not already available in the list */ +static struct nexthop_group_id * +nexthop_group_id_allocate(struct nexthop_group *nhg, uint32_t nhgid) +{ + struct nexthop_group_id *nhid; + + for (nhid = nhg->group; nhid; nhid = nhid->next) { + if (nhid->id_grp == nhgid) + break; + } + if (nhid) /* already present */ + return NULL; + + nhid = XCALLOC(MTYPE_NH_GROUP_ID, sizeof(struct nexthop_group_id)); + nhid->id_grp = nhgid; + + return nhid; +} + +static void nexthop_group_id_insert(struct nexthop_group_id **nhg, + struct nexthop_group_id *nhid) +{ + struct nexthop_group_id *nhid_tmp; + + for (nhid_tmp = *nhg; nhid_tmp && nhid_tmp->next; + nhid_tmp = nhid_tmp->next) + ; + if (nhid_tmp) + nhid_tmp->next = nhid; + else + *nhg = nhid; + nhid->prev = nhid_tmp; +} + +static void nexthop_group_id_insert_sorted(struct nexthop_group *nhg, + struct nexthop_group_id *nhid) +{ + struct nexthop_group_id *nhid_tmp, *nhid_prev = NULL; + + if (!nhg->group || nhid->id_grp < nhg->group->id_grp) { + /* first group in this nexthop group, + * or the first entry has a bigger group: insert it + */ + nhid->next = nhg->group; + nhg->group = nhid; + nhid->prev = NULL; + return; + } + + /* insert the new group in the list of groups by increasing number */ + for (nhid_tmp = nhg->group; nhid_tmp; nhid_tmp = nhid_tmp->next) { + nhid_prev = nhid_tmp; + if (nhid->id_grp < nhid_tmp->id_grp) { + nhid->next = nhid_tmp; + nhid->prev = nhid_tmp->prev; + nhid_tmp->prev = nhid; + break; + } + } + /* tail */ + if (!nhid_tmp) { + nhid_prev->next = nhid; + nhid->prev = nhid_prev; + nhid->next = NULL; + } +} + +/* group ids are sorted by increasing number + * this sorting is necessary in order to have shared nexthops + * like it has been done on nexthop lists + */ +void nexthop_group_id_add_sorted(struct nexthop_group *nhg, uint32_t nhgid) +{ + struct nexthop_group_id *nhid; + + nhid = nexthop_group_id_allocate(nhg, nhgid); + if (!nhid) + return; + + nexthop_group_id_insert_sorted(nhg, nhid); +} + +void nexthop_group_id_add(struct nexthop_group *nhg, uint32_t nhgid) +{ + struct nexthop_group_id *nhid; + + nhid = nexthop_group_id_allocate(nhg, nhgid); + if (!nhid) + return; + + /* append at the end the new group id */ + nexthop_group_id_insert(&nhg->group, nhid); +} + +void nexthop_group_ids_free(struct nexthop_group_id *nhid) +{ + struct nexthop_group_id *nhid_tmp = NULL, *nhid_tmp_to_free; + + if (!nhid) + return; + nhid_tmp = nhid; + do { + nhid_tmp_to_free = nhid_tmp; + nhid_tmp = nhid_tmp->next; + if (nhid_tmp) + nhid_tmp->prev = NULL; + nhid_tmp_to_free->next = NULL; + XFREE(MTYPE_NH_GROUP_ID, nhid_tmp_to_free); + } while (nhid_tmp != NULL); +} diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index cba449b54ff0..86dadba34e25 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -22,6 +22,12 @@ struct nhg_resilience { uint64_t unbalanced_time; }; +struct nexthop_group_id { + uint32_t id_grp; + struct nexthop_group_id *next; + struct nexthop_group_id *prev; +}; + /* * What is a nexthop group? * @@ -31,7 +37,10 @@ struct nhg_resilience { * This module provides a proper abstraction to this idea. */ struct nexthop_group { - struct nexthop *nexthop; + union { + struct nexthop *nexthop; + struct nexthop_group_id *group; + }; struct nhg_resilience nhgr; @@ -39,6 +48,7 @@ struct nexthop_group { #define NEXTHOP_GROUP_ALLOW_RECURSION (1 << 1) #define NEXTHOP_GROUP_IBGP (1 << 2) #define NEXTHOP_GROUP_PROTOCOL_CONTROLLED (1 << 3) +#define NEXTHOP_GROUP_TYPE_GROUP (1 << 4) uint8_t flags; /* nexthop group messages */ #define NEXTHOP_GROUP_MESSAGE_SRTE (1 << 1) @@ -102,6 +112,8 @@ struct nexthop_group_cmd { struct list *nhg_list; + struct list *nhg_group_list; + QOBJ_FIELDS; }; RB_HEAD(nhgc_entry_head, nexthp_group_cmd); @@ -126,10 +138,10 @@ extern struct nhgc_entry_head nhgc_entries; void nexthop_group_init( void (*create)(const char *name), void (*modify)(const struct nexthop_group_cmd *nhgc, bool reset), - void (*add_nexthop)(const struct nexthop_group_cmd *nhgc, - const struct nexthop *nhop), - void (*del_nexthop)(const struct nexthop_group_cmd *nhgc, - const struct nexthop *nhop), + void (*add_nexthop_or_group)(const struct nexthop_group_cmd *nhgc, + const struct nexthop *nhop), + void (*del_nexthop_or_group)(const struct nexthop_group_cmd *nhgc, + const struct nexthop *nhop), void (*destroy)(const char *name), int (*write_config)(struct vty *vty, const struct nexthop_group_cmd *nhgc)); @@ -138,7 +150,9 @@ void nexthop_group_enable_vrf(struct vrf *vrf); void nexthop_group_disable_vrf(struct vrf *vrf); void nexthop_group_interface_state_change(struct interface *ifp, ifindex_t oldifindex); - +void nexthop_group_dependent_group_match( + const char *nhgc_name, + void (*cb_func)(const struct nexthop_group_cmd *nhgc)); extern struct nexthop *nexthop_exists(const struct nexthop_group *nhg, const struct nexthop *nh); /* This assumes ordered */ @@ -167,6 +181,10 @@ nexthop_group_active_nexthop_num(const struct nexthop_group *nhg); extern bool nexthop_group_has_label(const struct nexthop_group *nhg); +extern void nexthop_group_id_add(struct nexthop_group *nhg, uint32_t nhgid); +extern void nexthop_group_id_add_sorted(struct nexthop_group *nhg, + uint32_t nhgid); +extern void nexthop_group_ids_free(struct nexthop_group_id *nhid); #ifdef __cplusplus } #endif diff --git a/lib/zclient.c b/lib/zclient.c index f8e04a42ac87..744b5b48f3bc 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -1157,8 +1157,15 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) return -1; } - if (api_nhg->nhg_nexthop.nexthop_num >= MULTIPATH_NUM || - api_nhg->nhg_nexthop.backup_nexthop_num >= MULTIPATH_NUM) { + if (!CHECK_FLAG(api_nhg->flags, ZAPI_NEXTHOP_GROUP_TYPE_GROUP) && + (api_nhg->nhg_nexthop.nexthop_num >= MULTIPATH_NUM || + api_nhg->nhg_nexthop.backup_nexthop_num >= MULTIPATH_NUM)) { + flog_err(EC_LIB_ZAPI_ENCODE, + "%s: zapi NHG encode with invalid input", __func__); + return -1; + } else if (CHECK_FLAG(api_nhg->flags, ZAPI_NEXTHOP_GROUP_TYPE_GROUP) && + (api_nhg->nhg_grp.nh_grp_count >= MULTIPATH_NUM || + api_nhg->nhg_grp.backup_nh_grp_count >= MULTIPATH_NUM)) { flog_err(EC_LIB_ZAPI_ENCODE, "%s: zapi NHG encode with invalid input", __func__); return -1; @@ -1179,7 +1186,8 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) if (CHECK_FLAG(api_nhg->message, ZAPI_NEXTHOP_MESSAGE_SRTE)) SET_FLAG(api_message, ZAPI_MESSAGE_SRTE); - if (cmd == ZEBRA_NHG_ADD) { + if (cmd == ZEBRA_NHG_ADD && + !CHECK_FLAG(api_nhg->flags, ZAPI_NEXTHOP_GROUP_TYPE_GROUP)) { /* Nexthops */ zapi_nexthop_group_sort(api_nhg->nhg_nexthop.nexthops, api_nhg->nhg_nexthop.nexthop_num); @@ -1196,6 +1204,15 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) for (i = 0; i < api_nhg->nhg_nexthop.backup_nexthop_num; i++) zapi_nexthop_encode(s, &api_nhg->nhg_nexthop.backup_nexthops[i], 0, api_message); + } else if (cmd == ZEBRA_NHG_ADD && + CHECK_FLAG(api_nhg->flags, ZAPI_NEXTHOP_GROUP_TYPE_GROUP)) { + stream_putw(s, api_nhg->nhg_grp.nh_grp_count); + for (i = 0; i < api_nhg->nhg_grp.nh_grp_count; i++) + stream_putl(s, api_nhg->nhg_grp.id_grp[i]); + + stream_putw(s, api_nhg->nhg_grp.backup_nh_grp_count); + for (i = 0; i < api_nhg->nhg_grp.backup_nh_grp_count; i++) + stream_putl(s, api_nhg->nhg_grp.backup_id_grp[i]); } stream_putw_at(s, 0, stream_get_endp(s)); diff --git a/lib/zclient.h b/lib/zclient.h index 1198c725cdd0..e0f5c3174798 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -483,6 +483,13 @@ struct zapi_nhg_nexthop { struct zapi_nexthop backup_nexthops[MULTIPATH_NUM]; }; +struct zapi_nhg_grp { + uint16_t nh_grp_count; + uint32_t id_grp[MULTIPATH_NUM]; + + uint16_t backup_nh_grp_count; + uint32_t backup_id_grp[MULTIPATH_NUM]; +}; struct zapi_nhg { uint16_t proto; @@ -490,9 +497,13 @@ struct zapi_nhg { struct nhg_resilience resilience; - struct zapi_nhg_nexthop nhg_nexthop; + union { + struct zapi_nhg_nexthop nhg_nexthop; + struct zapi_nhg_grp nhg_grp; + }; - /* nexthop group flags */ + /* nexthop group flags : definition from nexthop_group.h */ +#define ZAPI_NEXTHOP_GROUP_TYPE_GROUP NEXTHOP_GROUP_TYPE_GROUP uint8_t flags; /* nexthop group message : definition from nexthop_group.h */ #define ZAPI_NEXTHOP_MESSAGE_SRTE NEXTHOP_GROUP_MESSAGE_SRTE diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index 21742af96055..523f3f49bad8 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -245,6 +245,8 @@ void pbr_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc, /* No nhgc but range not exhausted? Then alloc it */ pnhgc = hash_get(pbr_nhg_hash, &pnhgc_find, pbr_nhgc_alloc); } + if (!nhop) + return; /* create & insert new pnhc into pnhgc->nhh */ pnhc_find.nexthop = *nhop; @@ -281,7 +283,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_type = nhop->type; + enum nexthop_types_t nh_type; /* find pnhgc by name */ strlcpy(pnhgc_find.name, nhgc->name, sizeof(pnhgc_find.name)); @@ -294,6 +296,9 @@ void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, if (!pnhgc) return; + if (!nhop) + return; + /* delete pnhc from pnhgc->nhh */ pnhc_find.nexthop = *nhop; pnhc = hash_release(pnhgc->nhh, &pnhc_find); @@ -307,6 +312,8 @@ void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, __func__, debugstr, nhgc->name); } + nh_type = nhop->type; + if (pnhgc->nhh->count) pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg); else diff --git a/sharpd/sharp_nht.c b/sharpd/sharp_nht.c index d3967b2d18b9..739797d99d9f 100644 --- a/sharpd/sharp_nht.c +++ b/sharpd/sharp_nht.c @@ -67,6 +67,7 @@ struct sharp_nhg { bool installed; bool force_nhg_config; + bool to_be_removed; }; static uint32_t nhg_id; @@ -136,10 +137,105 @@ static void sharp_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc, if (nhgc->backup_list_name[0]) bnhgc = nhgc_find(nhgc->backup_list_name); - nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL), + nhg_add(snhg->id, nhgc, (bnhgc ? &bnhgc->nhg : NULL), snhg->force_nhg_config); } +static void +sharp_nhgroup_dependent_add_nexthop_cb(const struct nexthop_group_cmd *nhgc) +{ + struct listnode *node; + struct sharp_nhg lookup; + char *groupname; + struct sharp_nhg *snhg, *snhg_tmp; + uint32_t id, nh_num = 0; + + if (!CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + return; + + strlcpy(lookup.name, nhgc->name, sizeof(nhgc->name)); + snhg = sharp_nhg_rb_find(&nhg_head, &lookup); + if (!snhg || !snhg->id) + return; + id = snhg->id; + + for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_group_list, node, groupname)) { + strlcpy(lookup.name, groupname, sizeof(lookup.name)); + snhg_tmp = sharp_nhg_rb_find(&nhg_head, &lookup); + if (!snhg_tmp) { + zlog_debug("%s() : nhg %s, group %s not found", + __func__, nhgc->name, groupname); + continue; + } + if (!snhg_tmp->id) { + zlog_debug("%s() : nhg %s, group %s has no valid id %p", + __func__, nhgc->name, groupname, snhg_tmp); + continue; + } + if (!sharp_nhgroup_id_is_installed(snhg_tmp->id)) { + zlog_debug("%s() : nhg %s, group %s not insalled (%u)", + __func__, nhgc->name, groupname, + snhg_tmp->id); + continue; + } + + if (sharp_nhgroup_id_is_being_removed(snhg_tmp->id)) + continue; + + /* assumption a dependent next-hop has only 1 next-hop */ + nh_num++; + } + if (nh_num) + nhg_add(id, nhgc, NULL, snhg->force_nhg_config); +} + +static void +sharp_nhgroup_dependent_del_nexthop_cb(const struct nexthop_group_cmd *nhgc) +{ + struct listnode *node; + struct sharp_nhg lookup; + char *groupname; + struct sharp_nhg *snhg; + int nh_num = 0, id = 0; + + if (!CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + return; + + for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_group_list, node, groupname)) { + strlcpy(lookup.name, groupname, sizeof(lookup.name)); + snhg = sharp_nhg_rb_find(&nhg_head, &lookup); + if (!snhg) { + zlog_debug("%s() : nhg %s, group %s not found", + __func__, nhgc->name, groupname); + continue; + } + if (!snhg->id) { + zlog_debug("%s() : nhg %s, group %s has no valid id %p", + __func__, nhgc->name, groupname, snhg); + continue; + } + + if (sharp_nhgroup_id_is_being_removed(snhg->id)) + continue; + + /* assumption a dependent next-hop has only 1 next-hop */ + nh_num++; + } + strlcpy(lookup.name, nhgc->name, sizeof(lookup.name)); + snhg = sharp_nhg_rb_find(&nhg_head, &lookup); + if (snhg) + id = snhg->id; + if (nh_num) { + zlog_debug("%s() : nhg %s, id %u needs update, now has %u groups", + __func__, nhgc->name, id, nh_num); + nhg_add(snhg->id, nhgc, NULL, snhg->force_nhg_config); + } else if (sharp_nhgroup_id_is_installed(snhg->id)) { + zlog_debug("%s() : nhg %s, id %u needs delete, no valid nh_num", + __func__, nhgc->name, id); + nhg_del(snhg->id); + } +} + static void sharp_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc, const struct nexthop *nhop) { @@ -149,12 +245,29 @@ static void sharp_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc, strlcpy(lookup.name, nhgc->name, sizeof(lookup.name)); snhg = sharp_nhg_rb_find(&nhg_head, &lookup); + if (!snhg) { + zlog_debug("%s() : nexthop %s not found", __func__, snhg->name); + return; + } + if (!snhg->id) { + zlog_debug("%s() : nexthop %s has no valid id %p", __func__, + snhg->name, snhg); + return; + } + if (!CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) { + if (nhgc->backup_list_name[0]) + bnhgc = nhgc_find(nhgc->backup_list_name); + nhg_add(snhg->id, nhgc, (bnhgc ? &bnhgc->nhg : NULL), + snhg->force_nhg_config); + } - if (nhgc->backup_list_name[0]) - bnhgc = nhgc_find(nhgc->backup_list_name); - - nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL), - snhg->force_nhg_config); + /* lookup dependent nexthops */ + if (nhop) { + nexthop_group_dependent_group_match(nhgc->name, + sharp_nhgroup_dependent_add_nexthop_cb); + return; + } + sharp_nhgroup_dependent_add_nexthop_cb(nhgc); } static void sharp_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, @@ -163,15 +276,54 @@ static void sharp_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc, struct sharp_nhg lookup; struct sharp_nhg *snhg; struct nexthop_group_cmd *bnhgc = NULL; + struct nexthop *nh = NULL; + int nh_num = 0; + + if (!CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) { + strlcpy(lookup.name, nhgc->name, sizeof(lookup.name)); + snhg = sharp_nhg_rb_find(&nhg_head, &lookup); + if (nhgc->backup_list_name[0]) + bnhgc = nhgc_find(nhgc->backup_list_name); + + for (ALL_NEXTHOPS_PTR(&nhgc->nhg, nh)) { + if (nh_num >= MULTIPATH_NUM) { + zlog_warn("%s: number of nexthops greater than max multipath size, truncating", + __func__); + break; + } + + /* Unresolved nexthops will lead to failure - only send + * nexthops that zebra will consider valid. + */ + if (nh->ifindex == 0) + continue; + + nh_num++; + } + if (nh_num == 0 && sharp_nhgroup_id_is_installed(snhg->id)) { + /* before deleting, notify other users */ + snhg->to_be_removed = true; + nexthop_group_dependent_group_match( + nhgc->name, + sharp_nhgroup_dependent_del_nexthop_cb); + zlog_debug("%s: nhg %s, id %u: no nexthops, deleting nexthop group", + __func__, nhgc->name, snhg->id); + nhg_del(snhg->id); + snhg->to_be_removed = false; + return; + } + + nhg_add(snhg->id, nhgc, (bnhgc ? &bnhgc->nhg : NULL), + snhg->force_nhg_config); + } - strlcpy(lookup.name, nhgc->name, sizeof(lookup.name)); - snhg = sharp_nhg_rb_find(&nhg_head, &lookup); - - if (nhgc->backup_list_name[0]) - bnhgc = nhgc_find(nhgc->backup_list_name); - - nhg_add(snhg->id, &nhgc->nhg, (bnhgc ? &bnhgc->nhg : NULL), - snhg->force_nhg_config); + /* lookup dependent nexthops */ + if (nhop) { + nexthop_group_dependent_group_match(nhgc->name, + sharp_nhgroup_dependent_del_nexthop_cb); + return; + } + sharp_nhgroup_dependent_del_nexthop_cb(nhgc); } static void sharp_nhgroup_delete_cb(const char *name) @@ -218,6 +370,18 @@ uint32_t sharp_nhgroup_get_id(const char *name) return snhg->id; } +void sharp_nhgroup_dependent_trigger_add_nexthop(uint32_t id) +{ + struct sharp_nhg *snhg; + + snhg = sharp_nhgroup_find_id(id); + if (!snhg) + return; + /* lookup dependent nexthops */ + nexthop_group_dependent_group_match(snhg->name, + sharp_nhgroup_dependent_add_nexthop_cb); +} + void sharp_nhgroup_id_set_installed(uint32_t id, bool installed) { struct sharp_nhg *snhg; @@ -256,6 +420,18 @@ bool sharp_nhgroup_id_is_forced(uint32_t id) return snhg->force_nhg_config; } +bool sharp_nhgroup_id_is_being_removed(uint32_t id) +{ + struct sharp_nhg *snhg; + + snhg = sharp_nhgroup_find_id(id); + if (!snhg) { + zlog_debug("%s: nhg %u not found", __func__, id); + return false; + } + return snhg->to_be_removed; +} + void sharp_nhgroup_force_nhg_config(struct nexthop_group_cmd *nhgc, bool force) { struct sharp_nhg *snhg; diff --git a/sharpd/sharp_nht.h b/sharpd/sharp_nht.h index 8d7eadc34fea..d3e10fb8434a 100644 --- a/sharpd/sharp_nht.h +++ b/sharpd/sharp_nht.h @@ -30,6 +30,8 @@ extern bool sharp_nhgroup_id_is_installed(uint32_t id); extern bool sharp_nhgroup_id_is_forced(uint32_t id); extern void sharp_nhgroup_force_nhg_config(struct nexthop_group_cmd *nhgc, bool force); +extern bool sharp_nhgroup_id_is_being_removed(uint32_t id); +extern void sharp_nhgroup_dependent_trigger_add_nexthop(uint32_t id); extern void sharp_nhgroup_init(void); #endif diff --git a/sharpd/sharp_vty.c b/sharpd/sharp_vty.c index 4a10486316cf..bf50c5b9f812 100644 --- a/sharpd/sharp_vty.c +++ b/sharpd/sharp_vty.c @@ -276,7 +276,11 @@ DEFPY (install_routes, nhgid = sharp_nhgroup_get_id(nexthop_group); sg.r.nhgid = nhgid; - sg.r.nhop_group.nexthop = nhgc->nhg.nexthop; + if (CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + SET_FLAG(sg.r.nhop_group.flags, + NEXTHOP_GROUP_TYPE_GROUP); + else + sg.r.nhop_group.nexthop = nhgc->nhg.nexthop; /* Use group's backup nexthop info if present */ if (nhgc->backup_list_name[0]) { diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index 8cc3457450b2..352ec21952fd 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -247,7 +247,6 @@ static bool route_add(const struct prefix *p, vrf_id_t vrf_id, uint8_t instance, memcpy(&api.prefix, p, sizeof(*p)); api.flags = flags; - SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP); /* Only send via ID if nhgroup has been successfully installed */ if (nhgid && (sharp_nhgroup_id_is_installed(nhgid) || @@ -255,6 +254,7 @@ static bool route_add(const struct prefix *p, vrf_id_t vrf_id, uint8_t instance, SET_FLAG(api.message, ZAPI_MESSAGE_NHG); api.nhgid = nhgid; } else { + SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP); for (ALL_NEXTHOPS_PTR(nhg, nh)) { /* Check if we set a VNI label */ if (nh->nh_label && @@ -531,13 +531,18 @@ void vrf_label_add(vrf_id_t vrf_id, afi_t afi, mpls_label_t label) zclient_send_vrf_label(zclient, vrf_id, afi, label, ZEBRA_LSP_SHARP); } -void nhg_add(uint32_t id, const struct nexthop_group *nhg, +void nhg_add(uint32_t id, const struct nexthop_group_cmd *nhgc, const struct nexthop_group *backup_nhg, bool force_nhg_config) { struct zapi_nhg api_nhg = {}; struct zapi_nexthop *api_nh; struct nexthop *nh; bool is_valid = true; + int count_grp_id = 0; + const struct nexthop_group *nhg = &nhgc->nhg; + uint32_t group_id; + char *groupname; + struct listnode *node; api_nhg.id = id; @@ -545,6 +550,45 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, api_nhg.message = nhg->message; api_nhg.resilience = nhg->nhgr; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + if (listcount(nhgc->nhg_group_list) >= MULTIPATH_NUM) { + zlog_warn("%s: %s, number of nexthops groups greater than max multipath size, truncating", + __func__, nhgc->name); + is_valid = false; + goto done; + } + SET_FLAG(api_nhg.flags, ZAPI_NEXTHOP_GROUP_TYPE_GROUP); + count_grp_id = 0; + for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_group_list, node, + groupname)) { + group_id = sharp_nhgroup_get_id(groupname); + if (group_id == 0) { + zlog_warn("%s: nhg %s, group %s has no identifier", + __func__, nhgc->name, groupname); + continue; + } + + if (sharp_nhgroup_id_is_being_removed(group_id)) + continue; + + api_nhg.nhg_grp.id_grp[count_grp_id++] = group_id; + } + api_nhg.nhg_grp.nh_grp_count = count_grp_id; + if (api_nhg.nhg_grp.nh_grp_count == 0) { + if (sharp_nhgroup_id_is_installed(id)) { + zlog_debug("%s: nhg %u: no groups, deleting nexthop group", + __func__, id); + zclient_nhg_send(zclient, ZEBRA_NHG_DEL, + &api_nhg); + return; + } + zlog_debug("%s: nhg %u not sent: no valid nexthops", + __func__, id); + return; + } + goto done; + } + for (ALL_NEXTHOPS_PTR(nhg, nh)) { if (api_nhg.nhg_nexthop.nexthop_num >= MULTIPATH_NUM) { zlog_warn( @@ -569,12 +613,7 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg, } if (api_nhg.nhg_nexthop.nexthop_num == 0) { - if (sharp_nhgroup_id_is_installed(id)) { - zlog_debug("%s: nhg %u: no nexthops, deleting nexthop group", __func__, - id); - zclient_nhg_send(zclient, ZEBRA_NHG_DEL, &api_nhg); - return; - } + /* assumption that dependent nhg are removed before when id is installed */ zlog_debug("%s: nhg %u not sent: no valid nexthops", __func__, id); is_valid = false; @@ -711,6 +750,9 @@ static void sharp_nexthop_update(struct vrf *vrf, struct prefix *matched, continue; nhg = &nhgc->nhg; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) + continue; + if (!CHECK_FLAG(nhg->message, NEXTHOP_GROUP_MESSAGE_SRTE)) continue; @@ -726,14 +768,18 @@ static void sharp_nexthop_update(struct vrf *vrf, struct prefix *matched, nexthop->type == NEXTHOP_TYPE_IPV4) && IPV4_ADDR_SAME(&matched->u.prefix4, &nexthop->gate.ipv4)) { - nhg_add(nhg_id, nhg, NULL); + nhg_add(nhg_id, nhgc, NULL, + sharp_nhgroup_id_is_forced( + nhg_id)); } else if (matched->family == AF_INET6 && (nexthop->type == NEXTHOP_TYPE_IPV6_IFINDEX || nexthop->type == NEXTHOP_TYPE_IPV6) && IPV6_ADDR_SAME(&matched->u.prefix6, &nexthop->gate.ipv6)) { - nhg_add(nhg_id, nhg, NULL); + nhg_add(nhg_id, nhgc, NULL, + sharp_nhgroup_id_is_forced( + nhg_id)); } nexthop = nexthop->next; } @@ -979,8 +1025,11 @@ static int nhg_notify_owner(ZAPI_CALLBACK_ARGS) switch (note) { case ZAPI_NHG_INSTALLED: - sharp_nhgroup_id_set_installed(id, true); zlog_debug("Installed nhg %u", id); + if (!sharp_nhgroup_id_is_installed(id)) { + sharp_nhgroup_id_set_installed(id, true); + sharp_nhgroup_dependent_trigger_add_nexthop(id); + } break; case ZAPI_NHG_FAIL_INSTALL: zlog_debug("Failed install of nhg %u", id); diff --git a/sharpd/sharp_zebra.h b/sharpd/sharp_zebra.h index d67704db861d..52c7ac39bab9 100644 --- a/sharpd/sharp_zebra.h +++ b/sharpd/sharp_zebra.h @@ -15,7 +15,7 @@ int sharp_zclient_create(uint32_t session_id); int sharp_zclient_delete(uint32_t session_id); extern void vrf_label_add(vrf_id_t vrf_id, afi_t afi, mpls_label_t label); -extern void nhg_add(uint32_t id, const struct nexthop_group *nhg, +extern void nhg_add(uint32_t id, const struct nexthop_group_cmd *nhgc, const struct nexthop_group *backup_nhg, bool force_nhg_config); extern void nhg_del(uint32_t id); diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index dc5c5c2f301b..c4954d3f0af6 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1683,6 +1683,66 @@ static struct nexthop *nexthop_from_zapi(const struct zapi_nexthop *api_nh, return nexthop; } +static bool zapi_read_nexthop_groups(struct zserv *client, + uint16_t nexthopgroup_num, uint32_t *id_grp, + struct nexthop_group **png, + struct nhg_backup_info **pbnhg) +{ + struct nexthop_group *ng = NULL; + struct nhg_backup_info *bnhg = NULL; + uint16_t i; + uint32_t nhgid; + + assert(!(png && pbnhg)); + + if (png) { + ng = nexthop_group_new(); + SET_FLAG(ng->flags, NEXTHOP_GROUP_TYPE_GROUP); + } + if (pbnhg && nexthopgroup_num > 0) { + if (IS_ZEBRA_DEBUG_RECV) + zlog_debug("%s: adding %d backup nexthop groups", + __func__, nexthopgroup_num); + + bnhg = zebra_nhg_backup_alloc(); + SET_FLAG(bnhg->nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP); + } + for (i = 0; i < nexthopgroup_num; i++) { + nhgid = *(id_grp + i); + if (!nhgid) { + flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, + "%s: Nexthops Groups Specified: %u(%u) but we failed to properly create one", + __func__, nexthopgroup_num, i); + if (ng) + nexthop_group_delete(&ng); + if (bnhg) + zebra_nhg_backup_free(&bnhg); + return false; + } + if (IS_ZEBRA_DEBUG_RECV) + zlog_debug("%s: nhgroup=%u)", __func__, nhgid); + + if (ng) + nexthop_group_id_add_sorted(ng, nhgid); + if (bnhg) + /* Note that the order of the backup groups is + * significant, so we don't sort this list as we do the + * primary group, we just append. + */ + nexthop_group_id_add(&bnhg->nhe->nhg, nhgid); + } + + + /* succesfully read, set caller pointers now */ + if (png) + *png = ng; + + if (pbnhg) + *pbnhg = bnhg; + + return true; +} + static bool zapi_read_nexthops(struct zserv *client, struct prefix *p, struct zapi_nexthop *nhops, uint32_t flags, uint32_t message, uint16_t nexthop_num, @@ -1905,6 +1965,34 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg) if (CHECK_FLAG(api_nhg->message, ZAPI_NEXTHOP_MESSAGE_SRTE)) SET_FLAG(api_message, ZAPI_MESSAGE_SRTE); + if (CHECK_FLAG(api_nhg->flags, ZAPI_NEXTHOP_GROUP_TYPE_GROUP)) { + STREAM_GETW(s, api_nhg->nhg_grp.nh_grp_count); + + if (zserv_nexthop_num_warn(__func__, NULL, + api_nhg->nhg_grp.nh_grp_count)) + return -1; + + if (api_nhg->nhg_grp.nh_grp_count <= 0) { + flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, + "%s: No nexthops Groups sent", __func__); + return -1; + } + + for (i = 0; i < api_nhg->nhg_grp.nh_grp_count; i++) + STREAM_GETL(s, api_nhg->nhg_grp.id_grp[i]); + + STREAM_GETW(s, api_nhg->nhg_grp.backup_nh_grp_count); + + if (zserv_nexthop_num_warn(__func__, NULL, + api_nhg->nhg_grp.backup_nh_grp_count)) + return -1; + + for (i = 0; i < api_nhg->nhg_grp.backup_nh_grp_count; i++) + STREAM_GETL(s, api_nhg->nhg_grp.backup_id_grp[i]); + + return 0; + } + /* Nexthops */ STREAM_GETW(s, api_nhg->nhg_nexthop.nexthop_num); @@ -2004,15 +2092,32 @@ static void zread_nhg_add(ZAPI_HANDLER_ARGS) return; } - if ((!zapi_read_nexthops(client, NULL, api_nhg.nhg_nexthop.nexthops, 0, - 0, api_nhg.nhg_nexthop.nexthop_num, - api_nhg.nhg_nexthop.backup_nexthop_num, &nhg, - NULL)) || - (!zapi_read_nexthops(client, NULL, - api_nhg.nhg_nexthop.backup_nexthops, 0, 0, - api_nhg.nhg_nexthop.backup_nexthop_num, - api_nhg.nhg_nexthop.backup_nexthop_num, NULL, - &bnhg))) { + if (CHECK_FLAG(api_nhg.flags, ZAPI_NEXTHOP_GROUP_TYPE_GROUP) && + (!zapi_read_nexthop_groups(client, api_nhg.nhg_grp.nh_grp_count, + &api_nhg.nhg_grp.id_grp[0], &nhg, NULL) || + !zapi_read_nexthop_groups(client, + api_nhg.nhg_grp.backup_nh_grp_count, + &api_nhg.nhg_grp.backup_id_grp[0], NULL, + &bnhg))) { + flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, + "%s: Nexthop Group Creation failed", __func__); + + /* Free any local allocations */ + nexthop_group_delete(&nhg); + zebra_nhg_backup_free(&bnhg); + + return; + } + if (!CHECK_FLAG(api_nhg.flags, ZAPI_NEXTHOP_GROUP_TYPE_GROUP) && + ((!zapi_read_nexthops(client, NULL, api_nhg.nhg_nexthop.nexthops, 0, + 0, api_nhg.nhg_nexthop.nexthop_num, + api_nhg.nhg_nexthop.backup_nexthop_num, &nhg, + NULL)) || + (!zapi_read_nexthops(client, NULL, + api_nhg.nhg_nexthop.backup_nexthops, 0, 0, + api_nhg.nhg_nexthop.backup_nexthop_num, + api_nhg.nhg_nexthop.backup_nexthop_num, NULL, + &bnhg)))) { flog_warn(EC_ZEBRA_NEXTHOP_CREATION_FAILED, "%s: Nexthop Group Creation failed", __func__); @@ -2032,8 +2137,13 @@ static void zread_nhg_add(ZAPI_HANDLER_ARGS) nhe->zapi_session = client->session_id; /* Take over the list(s) of nexthops */ - nhe->nhg.nexthop = nhg->nexthop; - nhg->nexthop = NULL; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + nhe->nhg.group = nhg->group; + nhg->group = NULL; + } else { + nhe->nhg.nexthop = nhg->nexthop; + nhg->nexthop = NULL; + } nhe->nhg.nhgr = api_nhg.resilience; diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 52a985934294..2488766cf132 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -3384,6 +3384,12 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, session = nhe->zapi_session; instance = nhe->zapi_instance; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + if (IS_ZEBRA_DEBUG_NHG) + zlog_debug("%s: id %u, nexthop group ids not supported", + __func__, id); + return NULL; + } if (!nhg->nexthop) { if (IS_ZEBRA_DEBUG_NHG) zlog_debug("%s: id %u, no nexthops passed to add", From efd100129bc620f46bbfcbbeb907025f3df3911f Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 24 Nov 2023 09:55:32 +0100 Subject: [PATCH 11/25] zebra: factorise 'show nexthop-group rib' nexthops Reduce the code size by reusing the same code to display nexthops and backup nexthops. Signed-off-by: Philippe Guibert --- zebra/zebra_vty.c | 144 +++++++++++++++++++++------------------------- 1 file changed, 67 insertions(+), 77 deletions(-) diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index b778b395085d..69bedcbe010c 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -1184,10 +1184,68 @@ DEFUN (ip_nht_default_route, return CMD_SUCCESS; } +static void +show_nexthop_group_out_nexthop(struct vty *vty, struct nexthop_group *nhg, + bool display_backup_info, + struct json_object *json_nexthop_array) +{ + struct nexthop *nexthop = NULL; + json_object *json_nexthops = NULL; + + for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { + if (json_nexthop_array) { + json_nexthops = json_object_new_object(); + show_nexthop_json_helper(json_nexthops, nexthop, NULL); + } else { + if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) + vty_out(vty, " "); + else + /* Make recursive nexthops a bit more clear */ + vty_out(vty, " "); + show_route_nexthop_helper(vty, NULL, nexthop); + } + + if (display_backup_info) { + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { + if (json_nexthop_array) + json_object_int_add(json_nexthops, + "backup", + nexthop->backup_idx[0]); + else + vty_out(vty, " [backup %d]", + nexthop->backup_idx[0]); + } + + if (!json_nexthop_array) + vty_out(vty, "\n"); + else + json_object_array_add(json_nexthop_array, + json_nexthops); + + continue; + } + + if (!json_nexthop_array) { + /* TODO -- print more useful backup info */ + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { + int i; + + vty_out(vty, "[backup"); + for (i = 0; i < nexthop->backup_num; i++) + vty_out(vty, " %d", + nexthop->backup_idx[i]); + vty_out(vty, "]"); + } + vty_out(vty, "\n"); + } else { + json_object_array_add(json_nexthop_array, json_nexthops); + } + } +} + static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe, json_object *json_nhe_hdr) { - struct nexthop *nexthop = NULL; struct nhg_connected *rb_node_dep = NULL; struct nexthop_group *backup_nhg; char up_str[MONOTIME_STRLEN]; @@ -1195,10 +1253,8 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe, json_object *json_dependants = NULL; json_object *json_depends = NULL; json_object *json_nexthop_array = NULL; - json_object *json_nexthops = NULL; json_object *json = NULL; json_object *json_backup_nexthop_array = NULL; - json_object *json_backup_nexthops = NULL; uptime2str(nhe->uptime, up_str, sizeof(up_str)); @@ -1284,58 +1340,12 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe, json_nexthop_array = json_object_new_array(); - for (ALL_NEXTHOPS(nhe->nhg, nexthop)) { - if (json_nexthop_array) { - json_nexthops = json_object_new_object(); - show_nexthop_json_helper(json_nexthops, nexthop, NULL); - } else { - if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) - vty_out(vty, " "); - else - /* Make recursive nexthops a bit more clear */ - vty_out(vty, " "); - show_route_nexthop_helper(vty, NULL, nexthop); - } - - if (nhe->backup_info == NULL || nhe->backup_info->nhe == NULL) { - if (CHECK_FLAG(nexthop->flags, - NEXTHOP_FLAG_HAS_BACKUP)) { - if (json) - json_object_int_add( - json_nexthops, "backup", - nexthop->backup_idx[0]); - else - vty_out(vty, " [backup %d]", - nexthop->backup_idx[0]); - } - - if (!json) - vty_out(vty, "\n"); - else - json_object_array_add(json_nexthop_array, - json_nexthops); - - continue; - } - - if (!json) { - /* TODO -- print more useful backup info */ - if (CHECK_FLAG(nexthop->flags, - NEXTHOP_FLAG_HAS_BACKUP)) { - int i; - - vty_out(vty, "[backup"); - for (i = 0; i < nexthop->backup_num; i++) - vty_out(vty, " %d", - nexthop->backup_idx[i]); - vty_out(vty, "]"); - } - vty_out(vty, "\n"); - } else { - json_object_array_add(json_nexthop_array, - json_nexthops); - } - } + if (nhe->backup_info == NULL || nhe->backup_info->nhe == NULL) + show_nexthop_group_out_nexthop(vty, &nhe->nhg, true, + json_nexthop_array); + else + show_nexthop_group_out_nexthop(vty, &nhe->nhg, false, + json_nexthop_array); if (json) json_object_object_add(json, "nexthops", json_nexthop_array); @@ -1348,28 +1358,8 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe, else vty_out(vty, " Backups:\n"); - for (ALL_NEXTHOPS_PTR(backup_nhg, nexthop)) { - if (json_backup_nexthop_array) { - json_backup_nexthops = json_object_new_object(); - show_nexthop_json_helper(json_backup_nexthops, - nexthop, NULL); - json_object_array_add(json_backup_nexthop_array, - json_backup_nexthops); - } else { - - if (!CHECK_FLAG(nexthop->flags, - NEXTHOP_FLAG_RECURSIVE)) - vty_out(vty, " "); - else - /* Make recursive nexthops a bit more - * clear - */ - vty_out(vty, " "); - show_route_nexthop_helper(vty, NULL, nexthop); - vty_out(vty, "\n"); - } - } - + show_nexthop_group_out_nexthop(vty, backup_nhg, false, + json_backup_nexthop_array); if (json) json_object_object_add(json, "backupNexthops", json_backup_nexthop_array); From 755460b39803d7ef55277b91597416fe6b680471 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 27 Nov 2023 09:43:13 +0100 Subject: [PATCH 12/25] zebra: move early rib_evpn_route_[add/delete]() function in early_ function Move the rib_evpn_route_add() and rib_evpn_route_delete() functions in a separate place. Signed-off-by: Philippe Guibert --- zebra/zebra_rib.c | 70 ++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 40 deletions(-) diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 6b03761a584d..d7a8e169de32 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -2694,6 +2694,33 @@ static void early_route_memory_free(struct zebra_early_route *ere) XFREE(MTYPE_WQ_WRAPPER, ere); } +static void +zebra_rib_queue_early_evpn_route_handle(struct zebra_early_route *ere, + struct nexthop_group *nhg, bool add) +{ + struct route_entry *re = ere->re; + struct ipaddr vtep_ip = {}; + struct nexthop *tmp_nh; + + for (ALL_NEXTHOPS_PTR(nhg, tmp_nh)) { + if (!CHECK_FLAG(tmp_nh->flags, NEXTHOP_FLAG_EVPN)) + continue; + if (ere->afi == AFI_IP) { + vtep_ip.ipa_type = IPADDR_V4; + vtep_ip.ipaddr_v4 = tmp_nh->gate.ipv4; + } else { + vtep_ip.ipa_type = IPADDR_V6; + vtep_ip.ipaddr_v6 = tmp_nh->gate.ipv6; + } + if (add) + zebra_rib_queue_evpn_route_add(re->vrf_id, &tmp_nh->rmac, + &vtep_ip, &ere->p); + else + zebra_rib_queue_evpn_route_del(re->vrf_id, &vtep_ip, + &ere->p); + } +} + static void process_subq_early_route_add(struct zebra_early_route *ere) { struct route_entry *re = ere->re; @@ -2736,8 +2763,6 @@ static void process_subq_early_route_add(struct zebra_early_route *ere) return; } } else { - struct nexthop *tmp_nh; - /* Lookup nhe from route information */ nhe = zebra_nhg_rib_find_nhe(ere->re_nhe, ere->afi); if (!nhe) { @@ -2755,22 +2780,7 @@ static void process_subq_early_route_add(struct zebra_early_route *ere) early_route_memory_free(ere); return; } - for (ALL_NEXTHOPS(nhe->nhg, tmp_nh)) { - if (CHECK_FLAG(tmp_nh->flags, NEXTHOP_FLAG_EVPN)) { - struct ipaddr vtep_ip = {}; - - if (ere->afi == AFI_IP) { - vtep_ip.ipa_type = IPADDR_V4; - vtep_ip.ipaddr_v4 = tmp_nh->gate.ipv4; - } else { - vtep_ip.ipa_type = IPADDR_V6; - vtep_ip.ipaddr_v6 = tmp_nh->gate.ipv6; - } - zebra_rib_queue_evpn_route_add( - re->vrf_id, &tmp_nh->rmac, &vtep_ip, - &ere->p); - } - } + zebra_rib_queue_early_evpn_route_handle(ere, &nhe->nhg, true); } /* @@ -3086,8 +3096,6 @@ static void process_subq_early_route_delete(struct zebra_early_route *ere) } if (same) { - struct nexthop *tmp_nh; - if (ere->fromkernel && CHECK_FLAG(ere->re->flags, ZEBRA_FLAG_SELFROUTE) && !zrouter.allow_delete) { @@ -3102,26 +3110,8 @@ static void process_subq_early_route_delete(struct zebra_early_route *ere) * EVPN - the nexthop (and associated MAC) need to be * uninstalled if no more refs. */ - for (ALL_NEXTHOPS(re->nhe->nhg, tmp_nh)) { - struct ipaddr vtep_ip; - - if (CHECK_FLAG(tmp_nh->flags, NEXTHOP_FLAG_EVPN)) { - memset(&vtep_ip, 0, sizeof(struct ipaddr)); - if (ere->afi == AFI_IP) { - vtep_ip.ipa_type = IPADDR_V4; - memcpy(&(vtep_ip.ipaddr_v4), - &(tmp_nh->gate.ipv4), - sizeof(struct in_addr)); - } else { - vtep_ip.ipa_type = IPADDR_V6; - memcpy(&(vtep_ip.ipaddr_v6), - &(tmp_nh->gate.ipv6), - sizeof(struct in6_addr)); - } - zebra_rib_queue_evpn_route_del( - re->vrf_id, &vtep_ip, &ere->p); - } - } + zebra_rib_queue_early_evpn_route_handle(ere, &re->nhe->nhg, + false); /* Notify dplane if system route changes */ if (RIB_SYSTEM_ROUTE(re)) From 757aaa14fe6879d35de971aa4c215bdad01f33e9 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 27 Nov 2023 10:24:35 +0100 Subject: [PATCH 13/25] zebra: move route_entry_dump_nh() into route_entry_dump_nhg() Factorise the nexthop display handling in a separate function. Signed-off-by: Philippe Guibert --- zebra/zebra_rib.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index d7a8e169de32..f49bb35fed62 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -4173,6 +4173,16 @@ static void _route_entry_dump_nh(const struct route_entry *re, } +static void _route_entry_dump_nhg(const struct route_entry *re, + const char *straddr, + const struct nexthop_group *nhg) +{ + struct nexthop *nhop; + + for (ALL_NEXTHOPS_PTR(nhg, nhop)) + _route_entry_dump_nh(re, straddr, nhop); +} + /* This function dumps the contents of a given RE entry into * standard debug log. Calling function name and IP prefix in * question are passed as 1st and 2nd arguments. @@ -4187,7 +4197,6 @@ void _route_entry_dump(const char *func, union prefixconstptr pp, char srcaddr[PREFIX_STRLEN]; char flags_buf[128]; char status_buf[128]; - struct nexthop *nexthop; struct vrf *vrf = vrf_lookup_by_id(re->vrf_id); struct nexthop_group *nhg; @@ -4213,15 +4222,13 @@ void _route_entry_dump(const char *func, union prefixconstptr pp, nexthop_group_active_nexthop_num(&(re->nhe->nhg))); /* Dump nexthops */ - for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) - _route_entry_dump_nh(re, straddr, nexthop); + _route_entry_dump_nhg(re, straddr, &re->nhe->nhg); if (zebra_nhg_get_backup_nhg(re->nhe)) { zlog_debug("%s: backup nexthops:", straddr); nhg = zebra_nhg_get_backup_nhg(re->nhe); - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) - _route_entry_dump_nh(re, straddr, nexthop); + _route_entry_dump_nhg(re, straddr, nhg); } zlog_debug("%s: dump complete", straddr); From 48fbc9c059931de0d9cd93f5db1e54f12ccdb531 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 5 Dec 2023 14:35:15 +0100 Subject: [PATCH 14/25] lib, zebra: add nhg group ids support, part 2, add nhg ref When a nexthop group with dependencies is attached to a route entry, the real nexthop flags are not easily reachable for read or write operations. In order to do so, at the route install, or at the nexthop group update, for each nexthop dependency, use an internal nhg pointer to remember the used nexthop group. The pointer is refreshed at nexthop group replacement. Signed-off-by: Philippe Guibert --- lib/nexthop_group.h | 1 + zebra/zebra_errors.c | 9 +++++++++ zebra/zebra_errors.h | 1 + zebra/zebra_nhg.c | 8 +------- zebra/zebra_rib.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 7 deletions(-) diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index 86dadba34e25..341d33d626d8 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -24,6 +24,7 @@ struct nhg_resilience { struct nexthop_group_id { uint32_t id_grp; + struct nexthop_group *nhg; struct nexthop_group_id *next; struct nexthop_group_id *prev; }; diff --git a/zebra/zebra_errors.c b/zebra/zebra_errors.c index 09b369e23eaf..1cb8b645dc8e 100644 --- a/zebra/zebra_errors.c +++ b/zebra/zebra_errors.c @@ -787,6 +787,15 @@ static struct log_ref ferr_zebra_err[] = { .suggestion = "Wait for Zebra to reattempt update.", }, + { + .code = EC_ZEBRA_NHG_MISSING_DEPENDENCIES, + .title = + "Zebra failed to update protocol nhg dependencies of a group nhg", + .description = + "Zebra made an attempt to update internal nhg dependencies, but failed.", + .suggestion = + "Ensure that the protocol nexthop group has been sent in correct order.", + }, { .code = END_FERR, } diff --git a/zebra/zebra_errors.h b/zebra/zebra_errors.h index 3ac654bda5c3..de39d1b639b5 100644 --- a/zebra/zebra_errors.h +++ b/zebra/zebra_errors.h @@ -124,6 +124,7 @@ enum zebra_log_refs { EC_ZEBRA_GRE_SET_UPDATE, EC_ZEBRA_SRV6M_UNRELEASED_LOCATOR_CHUNK, EC_ZEBRA_INTF_UPDATE_FAILURE, + EC_ZEBRA_NHG_MISSING_DEPENDENCIES, }; void zebra_error_init(void); diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 2488766cf132..45976b949b1f 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -3384,13 +3384,7 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, session = nhe->zapi_session; instance = nhe->zapi_instance; - if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { - if (IS_ZEBRA_DEBUG_NHG) - zlog_debug("%s: id %u, nexthop group ids not supported", - __func__, id); - return NULL; - } - if (!nhg->nexthop) { + if (!CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP) && !nhg->nexthop) { if (IS_ZEBRA_DEBUG_NHG) zlog_debug("%s: id %u, no nexthops passed to add", __func__, id); diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index f49bb35fed62..34667a7a5b70 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -68,6 +68,7 @@ DEFINE_HOOK(rib_update, (struct route_node * rn, const char *reason), (rn, reason)); DEFINE_HOOK(rib_shutdown, (struct route_node * rn), (rn)); +static bool zebra_rib_queue_early_update_nhe(struct nexthop_group *nhg); /* * Meta Q's specific names @@ -466,6 +467,17 @@ int rib_handle_nhg_replace(struct nhg_hash_entry *old_entry, zlog_debug("%s: replacing routes nhe (%u) OLD %p NEW %p", __func__, new_entry->id, new_entry, old_entry); + /* update dependent nhes if nhe is a group + * failure means a dependent nhe could not be resolved + * cancel the route change operation + */ + if (!zebra_rib_queue_early_update_nhe(&new_entry->nhg)) { + flog_err(EC_ZEBRA_NHG_MISSING_DEPENDENCIES, + "Zebra failed to find the dependencies for nexthop hash entry for id=%u", + new_entry->id); + return ret; + } + /* We have to do them ALL */ RB_FOREACH (zrt, zebra_router_table_head, &zrouter.tables) { for (rn = route_top(zrt->table); rn; @@ -2694,6 +2706,28 @@ static void early_route_memory_free(struct zebra_early_route *ere) XFREE(MTYPE_WQ_WRAPPER, ere); } +static bool zebra_rib_queue_early_update_nhe(struct nexthop_group *nhg) +{ + struct nexthop_group_id *nhgid; + struct nhg_hash_entry *nhe_tmp; + bool ret; + + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + nhe_tmp = zebra_nhg_lookup_id(nhgid->id_grp); + if (nhe_tmp) { + nhgid->nhg = &nhe_tmp->nhg; + ret = zebra_rib_queue_early_update_nhe( + &(nhe_tmp->nhg)); + if (!ret) + return false; + } else + return false; + } + } + return true; +} + static void zebra_rib_queue_early_evpn_route_handle(struct zebra_early_route *ere, struct nexthop_group *nhg, bool add) @@ -2782,6 +2816,17 @@ static void process_subq_early_route_add(struct zebra_early_route *ere) } zebra_rib_queue_early_evpn_route_handle(ere, &nhe->nhg, true); } + /* update dependent nhes if nhe is a group + * failure means a dependent nhe could not be resolved + * cancel the route add operation then + */ + if (!zebra_rib_queue_early_update_nhe(&nhe->nhg)) { + flog_err(EC_ZEBRA_NHG_MISSING_DEPENDENCIES, + "Zebra failed to find the dependencies for nexthop hash entry for id=%u in a route entry %pFX", + re->nhe_id, &ere->p); + early_route_memory_free(ere); + return; + } /* * Attach the re to the nhe's nexthop group. From 9fa19cc01ccd2cc2a0e0c6707a694cad4b9fac4a Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 23 Nov 2023 15:46:55 +0100 Subject: [PATCH 15/25] zebra: add nexthop group ids support, part 3 The zebra nexthop group support is not adapted to create dependencies between group when a request comes from the routing protocols. Add those extra changes in the zebra nexthop group library for that, a stub is temporarily added in dataplane_ctx_init. The changes ignore for now the zebra dataplane handling. Signed-off-by: Philippe Guibert --- lib/nexthop_group.c | 44 ++++++++- lib/nexthop_group.h | 2 + zebra/zebra_dplane.c | 13 ++- zebra/zebra_nhg.c | 212 ++++++++++++++++++++++++++++++++++--------- zebra/zebra_rib.c | 20 ++++ zebra/zebra_vty.c | 15 +-- 6 files changed, 253 insertions(+), 53 deletions(-) diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 88ffa5ca318c..bcffefbcf06d 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -49,6 +49,9 @@ struct nexthop_group_hooks { static struct nexthop_group_hooks nhg_hooks; +static void copy_nexthop_groups(struct nexthop_group *to, + struct nexthop_group_id *nhg); + static inline int nexthop_group_cmd_compare(const struct nexthop_group_cmd *nhgc1, const struct nexthop_group_cmd *nhgc2); @@ -78,7 +81,15 @@ uint8_t nexthop_group_nexthop_num(const struct nexthop_group *nhg) { struct nexthop *nhop; uint8_t num = 0; + struct nexthop_group_id *nhgid; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + num += nexthop_group_nexthop_num(nhgid->nhg); + } + return num; + } for (ALL_NEXTHOPS_PTR(nhg, nhop)) num++; @@ -101,7 +112,16 @@ uint8_t nexthop_group_active_nexthop_num(const struct nexthop_group *nhg) { struct nexthop *nhop; uint8_t num = 0; + struct nexthop_group_id *nhgid; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + num += nexthop_group_active_nexthop_num( + nhgid->nhg); + } + return num; + } for (ALL_NEXTHOPS_PTR(nhg, nhop)) { if (CHECK_FLAG(nhop->flags, NEXTHOP_FLAG_ACTIVE)) num++; @@ -267,7 +287,11 @@ void nexthop_group_copy(struct nexthop_group *to, to->nhgr = from->nhgr; to->flags = from->flags; /* Copy everything, including recursive info */ - copy_nexthops(&to->nexthop, from->nexthop, NULL); + if (CHECK_FLAG(to->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + to->group = NULL; + copy_nexthop_groups(to, from->group); + } else + copy_nexthops(&to->nexthop, from->nexthop, NULL); } void nexthop_group_delete(struct nexthop_group **nhg) @@ -1684,8 +1708,8 @@ static void nexthop_group_id_insert(struct nexthop_group_id **nhg, nhid->prev = nhid_tmp; } -static void nexthop_group_id_insert_sorted(struct nexthop_group *nhg, - struct nexthop_group_id *nhid) +void nexthop_group_id_insert_sorted(struct nexthop_group *nhg, + struct nexthop_group_id *nhid) { struct nexthop_group_id *nhid_tmp, *nhid_prev = NULL; @@ -1716,7 +1740,6 @@ static void nexthop_group_id_insert_sorted(struct nexthop_group *nhg, nhid->next = NULL; } } - /* group ids are sorted by increasing number * this sorting is necessary in order to have shared nexthops * like it has been done on nexthop lists @@ -1760,3 +1783,16 @@ void nexthop_group_ids_free(struct nexthop_group_id *nhid) XFREE(MTYPE_NH_GROUP_ID, nhid_tmp_to_free); } while (nhid_tmp != NULL); } + +static void copy_nexthop_groups(struct nexthop_group *to, + struct nexthop_group_id *nhg) +{ + struct nexthop_group_id *nhid; + const struct nexthop_group_id *nhg1; + + for (nhg1 = nhg; nhg1; nhg1 = nhg1->next) { + nhid = nexthop_group_id_allocate(to, nhg1->id_grp); + if (nhid) + nexthop_group_id_insert(&to->group, nhid); + } +} diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index 341d33d626d8..72a2d0807411 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -186,6 +186,8 @@ extern void nexthop_group_id_add(struct nexthop_group *nhg, uint32_t nhgid); extern void nexthop_group_id_add_sorted(struct nexthop_group *nhg, uint32_t nhgid); extern void nexthop_group_ids_free(struct nexthop_group_id *nhid); +extern void nexthop_group_id_insert_sorted(struct nexthop_group *nhg, + struct nexthop_group_id *nhid); #ifdef __cplusplus } #endif diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 99693a587403..a12e9175dabd 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -750,11 +750,18 @@ static void dplane_ctx_free_internal(struct zebra_dplane_ctx *ctx) case DPLANE_OP_NH_INSTALL: case DPLANE_OP_NH_UPDATE: case DPLANE_OP_NH_DELETE: { - if (ctx->u.rinfo.nhe.ng.nexthop) { + if (!CHECK_FLAG(ctx->u.rinfo.nhe.ng.flags, + NEXTHOP_GROUP_TYPE_GROUP) && + ctx->u.rinfo.nhe.ng.nexthop) { /* This deals with recursive nexthops too */ nexthops_free(ctx->u.rinfo.nhe.ng.nexthop); ctx->u.rinfo.nhe.ng.nexthop = NULL; + } else if (CHECK_FLAG(ctx->u.rinfo.nhe.ng.flags, + NEXTHOP_GROUP_TYPE_GROUP) && + ctx->u.rinfo.nhe.ng.group) { + nexthop_group_ids_free(ctx->u.rinfo.nhe.ng.group); + ctx->u.rinfo.nhe.ng.group = NULL; } break; } @@ -3428,6 +3435,10 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, return dplane_ctx_route_init_basic(ctx, op, NULL, NULL, NULL, AFI_UNSPEC, SAFI_UNSPEC); + /* routes attached to nexthop groups are not possible for now */ + if (CHECK_FLAG(re->nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + return ret; + /* * Let's grab the data from the route_node * so that we can call a helper function diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 45976b949b1f..827c7f648ee5 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -59,6 +59,9 @@ static void depends_add(struct nhg_connected_tree_head *head, static struct nhg_hash_entry * depends_find_add(struct nhg_connected_tree_head *head, struct nexthop *nh, afi_t afi, int type, bool from_dplane); +static bool depends_find_add_group(struct nhg_connected_tree_head *head, + struct nexthop_group_id *nhg, afi_t afi, + int type, bool from_dplane); static struct nhg_hash_entry * depends_find_id_add(struct nhg_connected_tree_head *head, uint32_t id); static void depends_decrement_free(struct nhg_connected_tree_head *head); @@ -435,6 +438,9 @@ static void *zebra_nhg_hash_alloc(void *arg) nhe = zebra_nhe_copy(copy, copy->id); + if (CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + return nhe; + /* Mark duplicate nexthops in a group at creation time. */ nexthop_group_mark_duplicates(&(nhe->nhg)); @@ -689,6 +695,7 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ bool recursive = false; struct nhg_hash_entry *newnhe, *backup_nhe; struct nexthop *nh = NULL; + struct nexthop_group_id *nhgroup = NULL; if (IS_ZEBRA_DEBUG_NHG_DETAIL) zlog_debug( @@ -702,8 +709,18 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ else (*nhe) = hash_lookup(zrouter.nhgs, lookup); - if (IS_ZEBRA_DEBUG_NHG_DETAIL) - zlog_debug("%s: lookup => %p (%pNG)", __func__, *nhe, *nhe); + if (IS_ZEBRA_DEBUG_NHG_DETAIL) { + if ((*nhe) && + !CHECK_FLAG((*nhe)->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + zlog_debug("%s: lookup => %p (%pNG)", __func__, *nhe, + *nhe); + else if (*nhe && CHECK_FLAG((*nhe)->nhg.flags, + NEXTHOP_GROUP_TYPE_GROUP)) + zlog_debug("%s: lookup => %p (group %u)", __func__, + *nhe, (*nhe)->nhg.group->id_grp); + else + zlog_debug("%s: lookup => %p", __func__, *nhe); + } /* If we found an existing object, we're done */ if (*nhe) @@ -765,7 +782,21 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ */ zebra_nhg_depends_init(newnhe); + if (CHECK_FLAG(newnhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) { + SET_FLAG(newnhe->flags, NEXTHOP_GROUP_VALID); + for (nhgroup = newnhe->nhg.group; nhgroup; + nhgroup = nhgroup->next) + if (depends_find_add_group(&newnhe->nhg_depends, + nhgroup, afi, newnhe->type, + from_dplane) == false) + UNSET_FLAG(newnhe->flags, NEXTHOP_GROUP_VALID); + /* Attach dependent backpointers to singletons */ + zebra_nhg_connect_depends(newnhe, &newnhe->nhg_depends); + goto done; + } nh = newnhe->nhg.nexthop; + if (!nh) + goto done; if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_ACTIVE)) SET_FLAG(newnhe->flags, NEXTHOP_GROUP_VALID); @@ -880,13 +911,17 @@ static bool zebra_nhg_find(struct nhg_hash_entry **nhe, uint32_t id, /* Use a temporary nhe and call into the superset/common code */ lookup.id = id; lookup.type = type ? type : ZEBRA_ROUTE_NHG; - lookup.nhg = *nhg; + if (nhg) + lookup.nhg = *nhg; lookup.vrf_id = vrf_id; - if (nhg_depends || lookup.nhg.nexthop->next) { + if (nhg_depends || + (nhg && (CHECK_FLAG(lookup.nhg.flags, NEXTHOP_GROUP_TYPE_GROUP) || + lookup.nhg.nexthop->next))) { /* Groups can have all vrfs and AF's in them */ lookup.afi = AFI_UNSPEC; - } else { + } else if (nhg && + !CHECK_FLAG(lookup.nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) { switch (lookup.nhg.nexthop->type) { case (NEXTHOP_TYPE_IFINDEX): case (NEXTHOP_TYPE_BLACKHOLE): @@ -1467,6 +1502,29 @@ static void depends_add(struct nhg_connected_tree_head *head, zebra_nhg_increment_ref(depend); } +static bool depends_find_add_group(struct nhg_connected_tree_head *head, + struct nexthop_group_id *nhid, afi_t afi, + int type, bool from_dplane) +{ + struct nhg_hash_entry *nhe = NULL; + + zebra_nhg_find(&nhe, nhid->id_grp, NULL, NULL, VRF_DEFAULT, afi, type, + from_dplane); + if (nhe == NULL) { + if (IS_ZEBRA_DEBUG_NHG_DETAIL) + zlog_debug("%s: nhgrp %u => error", __func__, + nhid->id_grp); + return false; + } + + if (IS_ZEBRA_DEBUG_NHG_DETAIL) + zlog_debug("%s: nh %u => OK", __func__, nhid->id_grp); + + if (nhe) + depends_add(head, nhe); + return true; +} + static struct nhg_hash_entry * depends_find_add(struct nhg_connected_tree_head *head, struct nexthop *nh, afi_t afi, int type, bool from_dplane) @@ -1510,7 +1568,7 @@ struct nhg_hash_entry *zebra_nhg_rib_find(uint32_t id, afi_t rt_afi, int type) { struct nhg_hash_entry *nhe = NULL; - vrf_id_t vrf_id; + vrf_id_t vrf_id = VRF_DEFAULT; /* * CLANG SA is complaining that nexthop may be NULL @@ -1539,19 +1597,35 @@ zebra_nhg_rib_find_nhe(struct nhg_hash_entry *rt_nhe, afi_t rt_afi) return NULL; } - if (!rt_nhe->nhg.nexthop) { + if (!CHECK_FLAG(rt_nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP) && + !rt_nhe->nhg.nexthop) { flog_err(EC_ZEBRA_TABLE_LOOKUP_FAILED, "No nexthop passed to %s", __func__); return NULL; + } else if (CHECK_FLAG(rt_nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP) && + !rt_nhe->nhg.group) { + flog_err(EC_ZEBRA_TABLE_LOOKUP_FAILED, + "No nexthop group passed to %s", __func__); + return NULL; } - if (IS_ZEBRA_DEBUG_NHG_DETAIL) - zlog_debug("%s: rt_nhe %p (%pNG)", __func__, rt_nhe, rt_nhe); - + if (IS_ZEBRA_DEBUG_NHG_DETAIL) { + if (CHECK_FLAG(rt_nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + zlog_debug("%s: rt_nhe %p (group %u)", __func__, rt_nhe, + rt_nhe->nhg.group->id_grp); + else + zlog_debug("%s: rt_nhe %p (%pNG)", __func__, rt_nhe, + rt_nhe); + } zebra_nhe_find(&nhe, rt_nhe, NULL, rt_afi, false); - if (IS_ZEBRA_DEBUG_NHG_DETAIL) - zlog_debug("%s: => nhe %p (%pNG)", __func__, nhe, nhe); + if (IS_ZEBRA_DEBUG_NHG_DETAIL) { + if (nhe && !CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + zlog_debug("%s: => nhe %p (%pNG)", __func__, nhe, nhe); + else if (nhe && nhe->nhg.group) + zlog_debug("%s: nhe %p (group %u)", __func__, nhe, + nhe->nhg.group->id_grp); + } return nhe; } @@ -1620,7 +1694,10 @@ nhg_backup_copy(const struct nhg_backup_info *orig) static void zebra_nhg_free_members(struct nhg_hash_entry *nhe) { - nexthops_free(nhe->nhg.nexthop); + if (CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + nexthop_group_ids_free(nhe->nhg.group); + else + nexthops_free(nhe->nhg.nexthop); zebra_nhg_backup_free(&nhe->backup_info); @@ -1633,14 +1710,18 @@ static void zebra_nhg_free_members(struct nhg_hash_entry *nhe) void zebra_nhg_free(struct nhg_hash_entry *nhe) { if (IS_ZEBRA_DEBUG_NHG_DETAIL) { - /* Group or singleton? */ - if (nhe->nhg.nexthop && nhe->nhg.nexthop->next) - zlog_debug("%s: nhe %p (%pNG), refcnt %d", __func__, - nhe, nhe, nhe->refcnt); - else - zlog_debug("%s: nhe %p (%pNG), refcnt %d, NH %pNHv", - __func__, nhe, nhe, nhe->refcnt, - nhe->nhg.nexthop); + if (!CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) { + /* Group or singleton? */ + if (nhe->nhg.nexthop && nhe->nhg.nexthop->next) + zlog_debug("%s: nhe %p (%pNG), refcnt %d", + __func__, nhe, nhe, nhe->refcnt); + else + zlog_debug("%s: nhe %p (%pNG), refcnt %d, NH %pNHv", + __func__, nhe, nhe, nhe->refcnt, + nhe->nhg.nexthop); + } else if (nhe->nhg.group) + zlog_debug("%s: nhe %p (group %u), refcnt %d", __func__, + nhe, nhe->nhg.group->id_grp, nhe->refcnt); } EVENT_OFF(nhe->timer); @@ -1658,19 +1739,27 @@ void zebra_nhg_hash_free(void *p) struct nhg_hash_entry *nhe = p; if (IS_ZEBRA_DEBUG_NHG_DETAIL) { - /* Group or singleton? */ - if (nhe->nhg.nexthop && nhe->nhg.nexthop->next) - zlog_debug("%s: nhe %p (%u), refcnt %d", __func__, nhe, - nhe->id, nhe->refcnt); - else - zlog_debug("%s: nhe %p (%pNG), refcnt %d, NH %pNHv", - __func__, nhe, nhe, nhe->refcnt, - nhe->nhg.nexthop); + if (!CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) { + /* Group or singleton? */ + if (nhe->nhg.nexthop && nhe->nhg.nexthop->next) + zlog_debug("%s: nhe %p (%u), refcnt %d", + __func__, nhe, nhe->id, nhe->refcnt); + else + zlog_debug("%s: nhe %p (%pNG), refcnt %d, NH %pNHv", + __func__, nhe, nhe, nhe->refcnt, + nhe->nhg.nexthop); + } else if (nhe->nhg.group) + zlog_debug("%s: nhe %p (group %u), refcnt %d", __func__, + nhe, nhe->nhg.group->id_grp, nhe->refcnt); } EVENT_OFF(nhe->timer); - nexthops_free(nhe->nhg.nexthop); + if (CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP) && + nhe->nhg.group) + nexthop_group_ids_free(nhe->nhg.group); + else + nexthops_free(nhe->nhg.nexthop); XFREE(MTYPE_NHG, nhe); } @@ -1711,8 +1800,13 @@ static void zebra_nhg_timer(struct event *thread) { struct nhg_hash_entry *nhe = EVENT_ARG(thread); - if (IS_ZEBRA_DEBUG_NHG_DETAIL) - zlog_debug("Nexthop Timer for nhe: %pNG", nhe); + if (IS_ZEBRA_DEBUG_NHG_DETAIL) { + if (!CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + zlog_debug("Nexthop Timer for nhe: %pNG", nhe); + else if (nhe->nhg.group) + zlog_debug("Nexthop Timer for nhe: %p (group %u)", nhe, + nhe->nhg.group->id_grp); + } if (nhe->refcnt == 1) zebra_nhg_decrement_ref(nhe); @@ -1720,10 +1814,15 @@ static void zebra_nhg_timer(struct event *thread) void zebra_nhg_decrement_ref(struct nhg_hash_entry *nhe) { - if (IS_ZEBRA_DEBUG_NHG_DETAIL) - zlog_debug("%s: nhe %p (%pNG) %d => %d", __func__, nhe, nhe, - nhe->refcnt, nhe->refcnt - 1); - + if (IS_ZEBRA_DEBUG_NHG_DETAIL) { + if (!CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + zlog_debug("%s: nhe %p (%pNG) %d => %d", __func__, nhe, + nhe, nhe->refcnt, nhe->refcnt - 1); + else if (nhe->nhg.group) + zlog_debug("%s: nhe %p (group %u) %d => %d", __func__, + nhe, nhe->nhg.group->id_grp, nhe->refcnt, + nhe->refcnt - 1); + } nhe->refcnt--; if (!zebra_router_in_shutdown() && nhe->refcnt <= 0 && @@ -1745,9 +1844,15 @@ void zebra_nhg_decrement_ref(struct nhg_hash_entry *nhe) void zebra_nhg_increment_ref(struct nhg_hash_entry *nhe) { - if (IS_ZEBRA_DEBUG_NHG_DETAIL) - zlog_debug("%s: nhe %p (%pNG) %d => %d", __func__, nhe, nhe, - nhe->refcnt, nhe->refcnt + 1); + if (IS_ZEBRA_DEBUG_NHG_DETAIL) { + if (!CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) + zlog_debug("%s: nhe %p (%pNG) %d => %d", __func__, nhe, + nhe, nhe->refcnt, nhe->refcnt + 1); + else if (nhe->nhg.group) + zlog_debug("%s: nhe %p (group %u) %d => %d", __func__, + nhe, nhe->nhg.group->id_grp, nhe->refcnt, + nhe->refcnt + 1); + } nhe->refcnt++; @@ -2750,7 +2855,8 @@ static bool zebra_nhg_set_valid_if_active(struct nhg_hash_entry *nhe) } /* should be fully resolved singleton at this point */ - if (CHECK_FLAG(nhe->nhg.nexthop->flags, NEXTHOP_FLAG_ACTIVE)) + if (!CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP) && + nhe->nhg.nexthop && CHECK_FLAG(nhe->nhg.nexthop->flags, NEXTHOP_FLAG_ACTIVE)) valid = true; done: @@ -2889,9 +2995,19 @@ static uint32_t proto_nhg_nexthop_active_update(struct nexthop_group *nhg) { struct nexthop *nh; uint32_t curr_active = 0; + struct nexthop_group_id *nhgid; /* Assume all active for now */ + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + curr_active += proto_nhg_nexthop_active_update( + nhgid->nhg); + } + return curr_active; + } + /* get nexthop associated */ for (nh = nhg->nexthop; nh; nh = nh->next) { SET_FLAG(nh->flags, NEXTHOP_FLAG_ACTIVE); curr_active++; @@ -3390,8 +3506,20 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, __func__, id); return NULL; } + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP) && !nhg->group) { + if (IS_ZEBRA_DEBUG_NHG) + zlog_debug("%s: id %u, no nexthops groups passed to add", + __func__, id); + return NULL; + } - + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + memset(&lookup, 0, sizeof(struct nhg_hash_entry)); + zebra_nhe_init(&lookup, afi, NULL); + SET_FLAG(lookup.nhg.flags, NEXTHOP_GROUP_TYPE_GROUP); + lookup.nhg.group = nhg->group; + goto done; + } /* Set nexthop list as active, since they wont go through rib * processing. * @@ -3441,6 +3569,8 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe, zebra_nhe_init(&lookup, afi, nhg->nexthop); lookup.nhg.nexthop = nhg->nexthop; + +done: lookup.nhg.nhgr = nhg->nhgr; lookup.id = id; lookup.type = type; diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 34667a7a5b70..caa7938e7d63 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -2735,7 +2735,19 @@ zebra_rib_queue_early_evpn_route_handle(struct zebra_early_route *ere, struct route_entry *re = ere->re; struct ipaddr vtep_ip = {}; struct nexthop *tmp_nh; + struct nexthop_group_id *nhgid; + struct nhg_hash_entry *nhe_tmp; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + nhe_tmp = zebra_nhg_lookup_id(nhgid->id_grp); + if (nhe_tmp) + zebra_rib_queue_early_evpn_route_handle(ere, + &(nhe_tmp->nhg), + add); + } + return; + } for (ALL_NEXTHOPS_PTR(nhg, tmp_nh)) { if (!CHECK_FLAG(tmp_nh->flags, NEXTHOP_FLAG_EVPN)) continue; @@ -4223,7 +4235,15 @@ static void _route_entry_dump_nhg(const struct route_entry *re, const struct nexthop_group *nhg) { struct nexthop *nhop; + struct nexthop_group_id *nhgid; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + _route_entry_dump_nhg(re, straddr, nhgid->nhg); + } + return; + } for (ALL_NEXTHOPS_PTR(nhg, nhop)) _route_entry_dump_nh(re, straddr, nhop); } diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 69bedcbe010c..42c8736b6422 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -1340,13 +1340,14 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe, json_nexthop_array = json_object_new_array(); - if (nhe->backup_info == NULL || nhe->backup_info->nhe == NULL) - show_nexthop_group_out_nexthop(vty, &nhe->nhg, true, - json_nexthop_array); - else - show_nexthop_group_out_nexthop(vty, &nhe->nhg, false, - json_nexthop_array); - + if (!CHECK_FLAG(nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) { + if (nhe->backup_info == NULL || nhe->backup_info->nhe == NULL) + show_nexthop_group_out_nexthop(vty, &nhe->nhg, true, + json_nexthop_array); + else + show_nexthop_group_out_nexthop(vty, &nhe->nhg, false, + json_nexthop_array); + } if (json) json_object_object_add(json, "nexthops", json_nexthop_array); From 9e88ee04431cbcc65218eefcbba37903710e0993 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 24 Nov 2023 18:16:08 +0100 Subject: [PATCH 16/25] zebra: add nexthop group ids support, part 4, netlink Add support to install routes attached to a nexthop group which is dependent of other nexthop groups: - Disable the src option. - The dataplane stub is removed Signed-off-by: Philippe Guibert --- zebra/rt_netlink.c | 32 ++++++++++++++++++++------------ zebra/zebra_dplane.c | 26 +++++++++++++++++--------- zebra/zebra_rib.c | 3 +++ 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index 98bb890eb682..0ef118a0db99 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -2319,7 +2319,8 @@ ssize_t netlink_route_multipath_msg_encode(int cmd, struct zebra_dplane_ctx *ctx * 1. Kernel nexthops don't suport unreachable/prohibit route types. * 2. Blackhole kernel nexthops are deleted when loopback is down. */ - nexthop = dplane_ctx_get_ng(ctx)->nexthop; + if (!CHECK_FLAG(dplane_ctx_get_ng(ctx)->flags, NEXTHOP_GROUP_TYPE_GROUP)) + nexthop = dplane_ctx_get_ng(ctx)->nexthop; if (nexthop) { if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) nexthop = nexthop->resolved; @@ -2355,11 +2356,15 @@ ssize_t netlink_route_multipath_msg_encode(int cmd, struct zebra_dplane_ctx *ctx return 0; /* Have to determine src still */ - for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop)) { - if (setsrc) - break; + if (!CHECK_FLAG(dplane_ctx_get_ng(ctx)->flags, + NEXTHOP_GROUP_TYPE_GROUP)) { + for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop)) { + if (setsrc) + break; - setsrc = nexthop_set_src(nexthop, p->family, &src); + setsrc = nexthop_set_src(nexthop, p->family, + &src); + } } if (setsrc) { @@ -2381,13 +2386,16 @@ ssize_t netlink_route_multipath_msg_encode(int cmd, struct zebra_dplane_ctx *ctx * or multipath case. */ nexthop_num = 0; - for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop)) { - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) - continue; - if (!NEXTHOP_IS_ACTIVE(nexthop->flags)) - continue; + if (!CHECK_FLAG(dplane_ctx_get_ng(ctx)->flags, + NEXTHOP_GROUP_TYPE_GROUP)) { + for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop)) { + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) + continue; + if (!NEXTHOP_IS_ACTIVE(nexthop->flags)) + continue; - nexthop_num++; + nexthop_num++; + } } /* Singlepath case. */ @@ -2445,7 +2453,7 @@ ssize_t netlink_route_multipath_msg_encode(int cmd, struct zebra_dplane_ctx *ctx return 0; } } - } else { /* Multipath case */ + } else if (nexthop_num > 1) { /* Multipath case */ struct rtattr *nest; const union g_addr *src1 = NULL; diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index a12e9175dabd..450f988c20ce 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -711,11 +711,18 @@ static void dplane_ctx_free_internal(struct zebra_dplane_ctx *ctx) case DPLANE_OP_ROUTE_NOTIFY: /* Free allocated nexthops */ - if (ctx->u.rinfo.zd_ng.nexthop) { + if (!CHECK_FLAG(ctx->u.rinfo.zd_ng.flags, + NEXTHOP_GROUP_TYPE_GROUP) && + ctx->u.rinfo.zd_ng.nexthop) { /* This deals with recursive nexthops too */ nexthops_free(ctx->u.rinfo.zd_ng.nexthop); ctx->u.rinfo.zd_ng.nexthop = NULL; + } else if (CHECK_FLAG(ctx->u.rinfo.zd_ng.flags, + NEXTHOP_GROUP_TYPE_GROUP) && + ctx->u.rinfo.zd_ng.group) { + nexthop_group_ids_free(ctx->u.rinfo.zd_ng.group); + ctx->u.rinfo.zd_ng.group = NULL; } /* Free backup info also (if present) */ @@ -3435,10 +3442,6 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, return dplane_ctx_route_init_basic(ctx, op, NULL, NULL, NULL, AFI_UNSPEC, SAFI_UNSPEC); - /* routes attached to nexthop groups are not possible for now */ - if (CHECK_FLAG(re->nhe->nhg.flags, NEXTHOP_GROUP_TYPE_GROUP)) - return ret; - /* * Let's grab the data from the route_node * so that we can call a helper function @@ -3454,8 +3457,7 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, return ret; /* Copy nexthops; recursive info is included too */ - copy_nexthops(&(ctx->u.rinfo.zd_ng.nexthop), - re->nhe->nhg.nexthop, NULL); + nexthop_group_copy(&ctx->u.rinfo.zd_ng, &re->nhe->nhg); ctx->u.rinfo.zd_nhg_id = re->nhe->id; /* Copy backup nexthop info, if present */ @@ -3464,6 +3466,9 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, re->nhe->backup_info->nhe->nhg.nexthop, NULL); } + if (CHECK_FLAG(ctx->u.rinfo.zd_ng.flags, NEXTHOP_GROUP_TYPE_GROUP)) + goto done; + /* * Ensure that the dplane nexthops' flags are clear and copy * encapsulation information. @@ -3504,6 +3509,7 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, } } +done: /* Don't need some info when capturing a system notification */ if (op == DPLANE_OP_SYS_ROUTE_ADD || op == DPLANE_OP_SYS_ROUTE_DELETE) { @@ -6710,8 +6716,10 @@ static void kernel_dplane_handle_result(struct zebra_dplane_ctx *ctx) atomic_fetch_add_explicit(&zdplane_info.dg_route_errors, 1, memory_order_relaxed); - if ((dplane_ctx_get_op(ctx) != DPLANE_OP_ROUTE_DELETE) - && (res == ZEBRA_DPLANE_REQUEST_SUCCESS)) { + if ((dplane_ctx_get_op(ctx) != DPLANE_OP_ROUTE_DELETE) && + (res == ZEBRA_DPLANE_REQUEST_SUCCESS) && + !CHECK_FLAG(dplane_ctx_get_ng(ctx)->flags, + NEXTHOP_GROUP_TYPE_GROUP)) { struct nexthop *nexthop; /* Update installed nexthops to signal which have been diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index caa7938e7d63..fbbb6940d3a5 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -1653,6 +1653,9 @@ static bool rib_update_nhg_from_ctx(struct nexthop_group *re_nhg, bool matched_p = true; struct nexthop *nexthop, *ctx_nexthop; + if (CHECK_FLAG(ctx_nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) + return matched_p; + /* Get the first `installed` one to check against. * If the dataplane doesn't set these to be what was actually installed, * it will just be whatever was in re->nhe->nhg? From 05c235b8e0cd41b9127865fcbf3c16febcd1ac49 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 24 Nov 2023 18:08:42 +0100 Subject: [PATCH 17/25] zebra: rework show_nexthop_group_out_nexthop() separate function Create a separate function to handle separately the nexthop structure. On the next commit, when a nexthop group is used, the nexthop of each nexthop group dependency is used. Signed-off-by: Philippe Guibert --- zebra/zebra_vty.c | 98 +++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 45 deletions(-) diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 42c8736b6422..0a2d86ea09c4 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -1184,62 +1184,70 @@ DEFUN (ip_nht_default_route, return CMD_SUCCESS; } -static void -show_nexthop_group_out_nexthop(struct vty *vty, struct nexthop_group *nhg, - bool display_backup_info, - struct json_object *json_nexthop_array) +static void show_nexthop_group_out_nexthop_specific( + struct vty *vty, struct json_object *json_nexthop_array, + int display_backup_info, struct nexthop *nexthop) { - struct nexthop *nexthop = NULL; json_object *json_nexthops = NULL; - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { - if (json_nexthop_array) { - json_nexthops = json_object_new_object(); - show_nexthop_json_helper(json_nexthops, nexthop, NULL); - } else { - if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) - vty_out(vty, " "); + if (json_nexthop_array) { + json_nexthops = json_object_new_object(); + show_nexthop_json_helper(json_nexthops, nexthop, NULL); + } else { + if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) + vty_out(vty, " "); + else + /* Make recursive nexthops a bit more clear */ + vty_out(vty, " "); + show_route_nexthop_helper(vty, NULL, nexthop); + } + + if (display_backup_info) { + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { + if (json_nexthop_array) + json_object_int_add(json_nexthops, "backup", + nexthop->backup_idx[0]); else - /* Make recursive nexthops a bit more clear */ - vty_out(vty, " "); - show_route_nexthop_helper(vty, NULL, nexthop); + vty_out(vty, " [backup %d]", + nexthop->backup_idx[0]); } - if (display_backup_info) { - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { - if (json_nexthop_array) - json_object_int_add(json_nexthops, - "backup", - nexthop->backup_idx[0]); - else - vty_out(vty, " [backup %d]", - nexthop->backup_idx[0]); - } + if (!json_nexthop_array) + vty_out(vty, "\n"); + else + json_object_array_add(json_nexthop_array, json_nexthops); - if (!json_nexthop_array) - vty_out(vty, "\n"); - else - json_object_array_add(json_nexthop_array, - json_nexthops); + return; + } - continue; + if (!json_nexthop_array) { + /* TODO -- print more useful backup info */ + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { + int i; + + vty_out(vty, "[backup"); + for (i = 0; i < nexthop->backup_num; i++) + vty_out(vty, " %d", nexthop->backup_idx[i]); + vty_out(vty, "]"); } + vty_out(vty, "\n"); + } else { + json_object_array_add(json_nexthop_array, json_nexthops); + } +} - if (!json_nexthop_array) { - /* TODO -- print more useful backup info */ - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { - int i; +static void +show_nexthop_group_out_nexthop(struct vty *vty, struct nexthop_group *nhg, + bool display_backup_info, + struct json_object *json_nexthop_array) +{ + struct nexthop *nexthop = NULL; - vty_out(vty, "[backup"); - for (i = 0; i < nexthop->backup_num; i++) - vty_out(vty, " %d", - nexthop->backup_idx[i]); - vty_out(vty, "]"); - } - vty_out(vty, "\n"); - } else { - json_object_array_add(json_nexthop_array, json_nexthops); - } + + for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { + show_nexthop_group_out_nexthop_specific(vty, json_nexthop_array, + display_backup_info, + nexthop); } } From d8d4c0f8e3fb02f0a3de0bc3ed16cc97dcd66f8e Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 27 Nov 2023 15:17:02 +0100 Subject: [PATCH 18/25] zebra: rework 'vty_show_ip_route()' function The show_route_nexthop_group_helper() and the show_nexthop_group_json_helper() helper functions are introduced to separate nexthop handling from the remaining 'show ip route' command. Signed-off-by: Philippe Guibert --- zebra/zebra_vty.c | 147 +++++++++++++++++++++++++++------------------- 1 file changed, 85 insertions(+), 62 deletions(-) diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 0a2d86ea09c4..8f30f4bc1024 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -520,12 +520,29 @@ static void uptime2str(time_t uptime, char *buf, size_t bufsize) frrtime_to_interval(cur, buf, bufsize); } +static void show_nexthop_detail_group_helper(struct vty *vty, + struct route_entry *re, + struct nexthop_group *nhg) +{ + struct nexthop *nexthop; + + for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { + /* Use helper to format each nexthop */ + show_nexthop_detail_helper(vty, re, nexthop, + false /*not backup*/); + vty_out(vty, "\n"); + + /* Include backup(s), if present */ + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) + show_nh_backup_helper(vty, re, nexthop); + } +} + /* New RIB. Detailed information for IPv4 route. */ static void vty_show_ip_route_detail(struct vty *vty, struct route_node *rn, int mcast, bool use_fib, bool show_ng) { struct route_entry *re; - struct nexthop *nexthop; char buf[SRCDEST2STR_BUFFER]; struct zebra_vrf *zvrf; rib_dest_t *dest; @@ -587,31 +604,79 @@ static void vty_show_ip_route_detail(struct vty *vty, struct route_node *rn, re->nhe_installed_id); } - for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) { - /* Use helper to format each nexthop */ - show_nexthop_detail_helper(vty, re, nexthop, - false /*not backup*/); - vty_out(vty, "\n"); - - /* Include backup(s), if present */ - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) - show_nh_backup_helper(vty, re, nexthop); - } + show_nexthop_detail_group_helper(vty, re, &re->nhe->nhg); zebra_show_ip_route_opaque(vty, re, NULL); vty_out(vty, "\n"); } } +static void show_route_nexthop_helper_specific( + struct vty *vty, struct route_entry *re, const struct nexthop *nexthop, + bool *first_p, bool is_fib, bool nhg_from_backup, int len, char *up_str) +{ + if (*first_p) { + *first_p = false; + } else if (nhg_from_backup) { + vty_out(vty, " b%c%*c", + re_status_output_char(re, nexthop, is_fib), + len - 3 + (2 * nexthop_level(nexthop)), ' '); + } else { + vty_out(vty, " %c%*c", + re_status_output_char(re, nexthop, is_fib), + len - 3 + (2 * nexthop_level(nexthop)), ' '); + } + + show_route_nexthop_helper(vty, re, nexthop); + vty_out(vty, ", %s\n", up_str); +} + +static void show_route_nexthop_group_helper(struct vty *vty, + struct route_entry *re, + const struct nexthop_group *nhg, + bool is_fib, int len) +{ + const struct nexthop *nexthop; + bool star_p = false; + + for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { + if (is_fib) + star_p = CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); + + /* TODO -- it'd be nice to be able to include + * the entire list of backups, *and* include the + * real installation state. + */ + vty_out(vty, " b%c %*c", (star_p ? '*' : ' '), + len - 3 + (2 * nexthop_level(nexthop)), ' '); + show_route_nexthop_helper(vty, re, nexthop); + vty_out(vty, "\n"); + } +} + +static void show_nexthop_group_json_helper(json_object *json_nexthops, + const struct nexthop_group *nhg, + struct route_entry *re) +{ + const struct nexthop *nexthop; + json_object *json_nexthop = NULL; + + for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { + json_nexthop = json_object_new_object(); + show_nexthop_json_helper(json_nexthop, nexthop, re); + + json_object_array_add(json_nexthops, json_nexthop); + } +} + static void vty_show_ip_route(struct vty *vty, struct route_node *rn, struct route_entry *re, json_object *json, bool is_fib, bool show_ng) { - const struct nexthop *nexthop; + struct nexthop *nexthop; int len = 0; char buf[SRCDEST2STR_BUFFER]; json_object *json_nexthops = NULL; - json_object *json_nexthop = NULL; json_object *json_route = NULL; const rib_dest_t *dest = rib_dest_from_rnode(rn); const struct nexthop_group *nhg; @@ -700,14 +765,7 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn, json_object_string_add(json_route, "uptime", up_str); - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { - json_nexthop = json_object_new_object(); - show_nexthop_json_helper(json_nexthop, - nexthop, re); - - json_object_array_add(json_nexthops, - json_nexthop); - } + show_nexthop_group_json_helper(json_nexthops, nhg, re); json_object_object_add(json_route, "nexthops", json_nexthops); @@ -720,14 +778,7 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn, if (nhg && nhg->nexthop) { json_nexthops = json_object_new_array(); - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { - json_nexthop = json_object_new_object(); - - show_nexthop_json_helper(json_nexthop, - nexthop, re); - json_object_array_add(json_nexthops, - json_nexthop); - } + show_nexthop_group_json_helper(json_nexthops, nhg, re); json_object_object_add(json_route, "backupNexthops", json_nexthops); @@ -776,22 +827,10 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn, len += vty_out(vty, " (%u)", re->nhe_id); /* Nexthop information. */ - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { - if (first_p) { - first_p = false; - } else if (nhg_from_backup) { - vty_out(vty, " b%c%*c", - re_status_output_char(re, nexthop, is_fib), - len - 3 + (2 * nexthop_level(nexthop)), ' '); - } else { - vty_out(vty, " %c%*c", - re_status_output_char(re, nexthop, is_fib), - len - 3 + (2 * nexthop_level(nexthop)), ' '); - } - - show_route_nexthop_helper(vty, re, nexthop); - vty_out(vty, ", %s\n", up_str); - } + for (ALL_NEXTHOPS_PTR(nhg, nexthop)) + show_route_nexthop_helper_specific(vty, re, nexthop, &first_p, + is_fib, nhg_from_backup, len, + up_str); /* If we only had backup nexthops, we're done */ if (nhg_from_backup) @@ -807,23 +846,7 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn, return; /* Print backup info */ - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { - bool star_p = false; - - if (is_fib) - star_p = CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); - - /* TODO -- it'd be nice to be able to include - * the entire list of backups, *and* include the - * real installation state. - */ - vty_out(vty, " b%c %*c", - (star_p ? '*' : ' '), - len - 3 + (2 * nexthop_level(nexthop)), ' '); - show_route_nexthop_helper(vty, re, nexthop); - vty_out(vty, "\n"); - } - + show_route_nexthop_group_helper(vty, re, nhg, is_fib, len); } static void vty_show_ip_route_detail_json(struct vty *vty, From ad826b76155a1ddc0ad9a32d7ea286e64d3f6fec Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 24 Nov 2023 18:27:37 +0100 Subject: [PATCH 19/25] zebra: add nexthop group ids support, part 5, show The following commands can not be used when using nhg groups, forged by the protocol: - 'show nexthop-group rib' - 'show ip route' Add the support for the 2 commands Signed-off-by: Philippe Guibert --- zebra/zebra_vty.c | 90 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 78 insertions(+), 12 deletions(-) diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 8f30f4bc1024..6f1139fbd794 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -525,6 +525,16 @@ static void show_nexthop_detail_group_helper(struct vty *vty, struct nexthop_group *nhg) { struct nexthop *nexthop; + struct nexthop_group_id *nhgid; + + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + show_nexthop_detail_group_helper(vty, re, + nhgid->nhg); + } + return; + } for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { /* Use helper to format each nexthop */ @@ -638,7 +648,17 @@ static void show_route_nexthop_group_helper(struct vty *vty, { const struct nexthop *nexthop; bool star_p = false; - + struct nexthop_group_id *nhgid; + + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + show_route_nexthop_group_helper(vty, re, + nhgid->nhg, + is_fib, len); + } + return; + } for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { if (is_fib) star_p = CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); @@ -660,7 +680,16 @@ static void show_nexthop_group_json_helper(json_object *json_nexthops, { const struct nexthop *nexthop; json_object *json_nexthop = NULL; + struct nexthop_group_id *nhgid; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + show_nexthop_group_json_helper(json_nexthops, + nhgid->nhg, re); + } + return; + } for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { json_nexthop = json_object_new_object(); show_nexthop_json_helper(json_nexthop, nexthop, re); @@ -669,11 +698,37 @@ static void show_nexthop_group_json_helper(json_object *json_nexthops, } } +static void show_route_nexthop_group_helper_specific( + struct vty *vty, struct route_entry *re, const struct nexthop_group *nhg, + bool *first_p, bool is_fib, bool nhg_from_backup, int len, char *up_str) +{ + struct nexthop_group_id *nhgid; + const struct nexthop *nexthop; + + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + show_route_nexthop_group_helper_specific(vty, re, + nhgid->nhg, + first_p, + is_fib, + nhg_from_backup, + len, + up_str); + } + return; + } + /* Nexthop information. */ + for (ALL_NEXTHOPS_PTR(nhg, nexthop)) + show_route_nexthop_helper_specific(vty, re, nexthop, first_p, + is_fib, nhg_from_backup, len, + up_str); +} + static void vty_show_ip_route(struct vty *vty, struct route_node *rn, struct route_entry *re, json_object *json, bool is_fib, bool show_ng) { - struct nexthop *nexthop; int len = 0; char buf[SRCDEST2STR_BUFFER]; json_object *json_nexthops = NULL; @@ -826,11 +881,9 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn, if (show_ng) len += vty_out(vty, " (%u)", re->nhe_id); - /* Nexthop information. */ - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) - show_route_nexthop_helper_specific(vty, re, nexthop, &first_p, - is_fib, nhg_from_backup, len, - up_str); + /* Nexthop Group information. */ + show_route_nexthop_group_helper_specific(vty, re, nhg, &first_p, is_fib, + nhg_from_backup, len, up_str); /* If we only had backup nexthops, we're done */ if (nhg_from_backup) @@ -1265,12 +1318,25 @@ show_nexthop_group_out_nexthop(struct vty *vty, struct nexthop_group *nhg, struct json_object *json_nexthop_array) { struct nexthop *nexthop = NULL; + struct nexthop_group_id *group = NULL; - - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { - show_nexthop_group_out_nexthop_specific(vty, json_nexthop_array, - display_backup_info, - nexthop); + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (group = nhg->group; group; group = group->next) { + if (!group->nhg) + continue; + for (ALL_NEXTHOPS_PTR(group->nhg, nexthop)) { + show_nexthop_group_out_nexthop_specific( + vty, json_nexthop_array, + display_backup_info, nexthop); + } + } + } else { + for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { + show_nexthop_group_out_nexthop_specific(vty, + json_nexthop_array, + display_backup_info, + nexthop); + } } } From 1393cfdc2bf1139ce1db264a62fda16565fe8cfe Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 27 Nov 2023 17:08:51 +0100 Subject: [PATCH 20/25] zebra: rework no nexthop available test Some tests may check if there is a valid nexthop in a nexthop-group. Generalize the check by accounting for the number of nexthops in a nexthop group. Signed-off-by: Philippe Guibert --- zebra/zebra_vty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 6f1139fbd794..d302593c3d32 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -260,7 +260,7 @@ static void show_nh_backup_helper(struct vty *vty, /* Double-check that there _is_ a backup */ if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP) || re->nhe->backup_info == NULL || re->nhe->backup_info->nhe == NULL || - re->nhe->backup_info->nhe->nhg.nexthop == NULL) + nexthop_group_nexthop_num(&re->nhe->backup_info->nhe->nhg) == 0) return; /* Locate the backup nexthop(s) */ @@ -848,7 +848,7 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn, * and there are no installed primary nexthops, see if there are any * backup nexthops and start with those. */ - if (is_fib && nhg->nexthop == NULL) { + if (is_fib && nexthop_group_nexthop_num(nhg) == 0) { nhg = rib_get_fib_backup_nhg(re); nhg_from_backup = true; } From 32eea724bba419c80ba12bbc832e7e8491c314c9 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 28 Nov 2023 14:56:08 +0100 Subject: [PATCH 21/25] zebra: rework mpls uninstall based on nexthop group Create a mpls_ftn_uninstall_nhg() function that handles the nexthop parsing for each route entry. This parsing is common to primary and backup nexthops. Signed-off-by: Philippe Guibert --- zebra/zebra_mpls.c | 47 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c index 15e36acda8c3..b0f620b5269a 100644 --- a/zebra/zebra_mpls.c +++ b/zebra/zebra_mpls.c @@ -3357,6 +3357,23 @@ static void mpls_lsp_uninstall_all_type(struct hash_bucket *bucket, void *ctxt) mpls_lsp_uninstall_all(lsp_table, lsp, args->type); } +static void mpls_ftn_uninstall_nhg(struct route_entry *re, + struct nexthop_group *nhg, + enum lsp_types_t lsp_type, bool *update) +{ + struct nexthop *nexthop; + + for (nexthop = nhg->nexthop; nexthop; nexthop = nexthop->next) { + if (nexthop->nh_label_type != lsp_type) + continue; + + nexthop_del_labels(nexthop); + SET_FLAG(re->status, ROUTE_ENTRY_CHANGED); + SET_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED); + *update = true; + } +} + /* * Uninstall all FEC-To-NHLFE (FTN) bindings of the given address-family and * LSP type. @@ -3367,7 +3384,6 @@ static void mpls_ftn_uninstall_all(struct zebra_vrf *zvrf, struct route_table *table; struct route_node *rn; struct route_entry *re; - struct nexthop *nexthop; struct nexthop_group *nhg; bool update; @@ -3385,34 +3401,13 @@ static void mpls_ftn_uninstall_all(struct zebra_vrf *zvrf, new_nhe = zebra_nhe_copy(re->nhe, 0); nhg = &new_nhe->nhg; - for (nexthop = nhg->nexthop; nexthop; - nexthop = nexthop->next) { - if (nexthop->nh_label_type != lsp_type) - continue; - - nexthop_del_labels(nexthop); - SET_FLAG(re->status, ROUTE_ENTRY_CHANGED); - SET_FLAG(re->status, - ROUTE_ENTRY_LABELS_CHANGED); - update = true; - } + mpls_ftn_uninstall_nhg(re, nhg, lsp_type, &update); /* Check for backup info and update that also */ nhg = zebra_nhg_get_backup_nhg(new_nhe); - if (nhg != NULL) { - for (nexthop = nhg->nexthop; nexthop; - nexthop = nexthop->next) { - if (nexthop->nh_label_type != lsp_type) - continue; - - nexthop_del_labels(nexthop); - SET_FLAG(re->status, - ROUTE_ENTRY_CHANGED); - SET_FLAG(re->status, - ROUTE_ENTRY_LABELS_CHANGED); - update = true; - } - } + if (nhg != NULL) + mpls_ftn_uninstall_nhg(re, nhg, lsp_type, + &update); if (CHECK_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED)) mpls_zebra_nhe_update(re, afi, new_nhe); From 9d8b551d5d1179085acc31d1b7ebd740d476fdf3 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 28 Nov 2023 15:12:37 +0100 Subject: [PATCH 22/25] zebra: add support for 'mpls uninstall' of nexthop group dependencies The nexthop group dependencies may also have an NHLFE entry. Signed-off-by: Philippe Guibert --- zebra/zebra_mpls.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c index b0f620b5269a..3c1ab2acd538 100644 --- a/zebra/zebra_mpls.c +++ b/zebra/zebra_mpls.c @@ -3362,7 +3362,16 @@ static void mpls_ftn_uninstall_nhg(struct route_entry *re, enum lsp_types_t lsp_type, bool *update) { struct nexthop *nexthop; + struct nexthop_group_id *nhgid; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + mpls_ftn_uninstall_nhg(re, nhgid->nhg, lsp_type, + update); + } + return; + } for (nexthop = nhg->nexthop; nexthop; nexthop = nexthop->next) { if (nexthop->nh_label_type != lsp_type) continue; From aab3273bb19eae26c4adc5a7a9e73634460608bc Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 28 Nov 2023 17:01:51 +0100 Subject: [PATCH 23/25] zebra: rework zsend_redistribute_route() function with nhg Create a second function zsend_redistribute_route_nhg() to handle nexthop group. Signed-off-by: Philippe Guibert --- zebra/zapi_msg.c | 68 +++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index c4954d3f0af6..d357c33e08ce 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -509,13 +509,48 @@ int zsend_interface_update(int cmd, struct zserv *client, struct interface *ifp) return zserv_send_message(client, s); } +static int zsend_redistribute_route_nhg(const struct nexthop_group *nhg, + struct zapi_route *api) +{ + struct nexthop *nexthop; + struct zapi_nexthop *api_nh; + int count = 0; + + for (nexthop = nhg->nexthop; nexthop; nexthop = nexthop->next) { + if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) + continue; + + api_nh = &(api->nexthops[count]); + api_nh->vrf_id = nexthop->vrf_id; + api_nh->type = nexthop->type; + api_nh->weight = nexthop->weight; + switch (nexthop->type) { + case NEXTHOP_TYPE_BLACKHOLE: + api_nh->bh_type = nexthop->bh_type; + break; + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: + api_nh->gate.ipv4 = nexthop->gate.ipv4; + api_nh->ifindex = nexthop->ifindex; + break; + case NEXTHOP_TYPE_IFINDEX: + api_nh->ifindex = nexthop->ifindex; + break; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: + api_nh->gate.ipv6 = nexthop->gate.ipv6; + api_nh->ifindex = nexthop->ifindex; + } + count++; + } + return count; +} + int zsend_redistribute_route(int cmd, struct zserv *client, const struct route_node *rn, const struct route_entry *re, bool is_table_direct) { struct zapi_route api; - struct zapi_nexthop *api_nh; - struct nexthop *nexthop; const struct prefix *p, *src_p; uint8_t count = 0; afi_t afi; @@ -561,34 +596,7 @@ int zsend_redistribute_route(int cmd, struct zserv *client, memcpy(&api.src_prefix, src_p, sizeof(api.src_prefix)); } - for (nexthop = re->nhe->nhg.nexthop; - nexthop; nexthop = nexthop->next) { - if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) - continue; - - api_nh = &api.nexthops[count]; - api_nh->vrf_id = nexthop->vrf_id; - api_nh->type = nexthop->type; - api_nh->weight = nexthop->weight; - switch (nexthop->type) { - case NEXTHOP_TYPE_BLACKHOLE: - api_nh->bh_type = nexthop->bh_type; - break; - case NEXTHOP_TYPE_IPV4: - case NEXTHOP_TYPE_IPV4_IFINDEX: - api_nh->gate.ipv4 = nexthop->gate.ipv4; - api_nh->ifindex = nexthop->ifindex; - break; - case NEXTHOP_TYPE_IFINDEX: - api_nh->ifindex = nexthop->ifindex; - break; - case NEXTHOP_TYPE_IPV6: - case NEXTHOP_TYPE_IPV6_IFINDEX: - api_nh->gate.ipv6 = nexthop->gate.ipv6; - api_nh->ifindex = nexthop->ifindex; - } - count++; - } + count = zsend_redistribute_route_nhg(&re->nhe->nhg, &api); /* Nexthops. */ if (count) { From f3c50c09d1f0f34ff6250b7601502c1da29786cf Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 28 Nov 2023 17:03:58 +0100 Subject: [PATCH 24/25] zebra: add support for redistributed routes with nexthop groups Signed-off-by: Philippe Guibert --- zebra/zapi_msg.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index d357c33e08ce..ae9b0ec7333a 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -515,7 +515,16 @@ static int zsend_redistribute_route_nhg(const struct nexthop_group *nhg, struct nexthop *nexthop; struct zapi_nexthop *api_nh; int count = 0; + struct nexthop_group_id *nhgid; + if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_TYPE_GROUP)) { + for (nhgid = nhg->group; nhgid; nhgid = nhgid->next) { + if (nhgid->nhg) + count += zsend_redistribute_route_nhg(nhgid->nhg, + api); + } + return count; + } for (nexthop = nhg->nexthop; nexthop; nexthop = nexthop->next) { if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) continue; From 27d4fdc59f0a5f2a520d8326b12f1877d0578ade Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 28 Nov 2023 15:06:55 +0100 Subject: [PATCH 25/25] topotests: all_protocol_startup, add nexthop-group test with nhg group An ECMP configuration is tested with nexthop group. Signed-off-by: Philippe Guibert --- .../all_protocol_startup/r1/ip_nht.ref | 12 ++++ .../test_all_protocol_startup.py | 58 ++++++++++++++++++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/tests/topotests/all_protocol_startup/r1/ip_nht.ref b/tests/topotests/all_protocol_startup/r1/ip_nht.ref index 1dceb3ca76ac..224080942bc9 100644 --- a/tests/topotests/all_protocol_startup/r1/ip_nht.ref +++ b/tests/topotests/all_protocol_startup/r1/ip_nht.ref @@ -72,6 +72,18 @@ VRF default: resolved via connected is directly connected, r1-eth0 (vrf default) Client list: pbr(fd XX) +192.168.0.202 + resolved via connected + is directly connected, r1-eth0 (vrf default) + Client list: pbr(fd XX) +192.168.0.205 + resolved via connected + is directly connected, r1-eth0 (vrf default) + Client list: pbr(fd XX) +192.168.0.207 + resolved via connected + is directly connected, r1-eth0 (vrf default) + Client list: pbr(fd XX) 192.168.0.208 resolved via connected is directly connected, r1-eth0 (vrf default) diff --git a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py index 615d2f392ce1..d6f5e6f4f360 100644 --- a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py +++ b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py @@ -715,7 +715,7 @@ def test_nexthop_groups(): ## the nhgroups are removed from a configurable timer (zebra nexthop-group keep XX) tgen.gears["r1"].vtysh_cmd("sharp remove routes 7.7.7.7 1") - tgen.gears["r1"].vtysh_cmd( + output = tgen.gears["r1"].vtysh_cmd( """ configure terminal nexthop-group CONTROL @@ -727,6 +727,61 @@ def test_nexthop_groups(): valid is not None ), "nexthop-group CONTROL 'protocol-controlled' command should not be modified" + ## multiple nexthop-group dependencies + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group GROUP1 + group ECMP1 + group ECMP2 + """ + ) + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group ECMP1 + protocol-controlled + nexthop 192.168.0.202 r1-eth0 + """ + ) + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group ECMP2 + protocol-controlled + nexthop 192.168.0.205 r1-eth0 + """ + ) + tgen.gears["r1"].vtysh_cmd("sharp install routes 8.8.8.8 nexthop-group GROUP1 1") + verify_route_nexthop_group("8.8.8.8/32", ecmp=2) + + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group GROUP1 + no group ECMP2 + """ + ) + verify_route_nexthop_group("8.8.8.8/32", ecmp=1) + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group ECMP3 + protocol-controlled + nexthop 192.168.0.207 r1-eth0 + """ + ) + tgen.gears["r1"].vtysh_cmd( + """ + configure terminal + nexthop-group GROUP1 + group ECMP3 + group ECMP2 + nexthop 192.168.0.207 r1-eth0 + """ + ) + verify_route_nexthop_group("8.8.8.8/32", ecmp=3) + ## Remove all NHG routes net["r1"].cmd('vtysh -c "sharp remove routes 2.2.2.1 1"') @@ -739,6 +794,7 @@ def test_nexthop_groups(): net["r1"].cmd('vtysh -c "sharp remove routes 6.6.6.1 4"') net["r1"].cmd('vtysh -c "c t" -c "no ip route 6.6.6.0/24 1.1.1.1"') net["r1"].cmd('vtysh -c "sharp remove routes 7.7.7.7 1"') + net["r1"].cmd('vtysh -c "sharp remove routes 8.8.8.8 1"') def test_rip_status():