From 5a05dbeb133246431109f451b413d77c74bdb2fc Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 17:15:00 +0300 Subject: [PATCH 1/8] bgpd: Set MED using a helper bgp_attr_set_med() Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 7 ++----- bgpd/bgp_attr.h | 6 ++++++ bgpd/bgp_route.c | 7 +++---- bgpd/bgp_routemap.c | 3 +-- bgpd/bgp_updgrp_adv.c | 4 ++-- bgpd/rfapi/rfapi.c | 12 ++++-------- bgpd/rfapi/vnc_export_bgp.c | 22 ++++++++++++---------- 7 files changed, 30 insertions(+), 31 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 7aa9bbbb91b8..596d820f1b50 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1177,8 +1177,7 @@ struct attr *bgp_attr_aggregate_intern( SET_FLAG(attr.flag, ATTR_FLAG_BIT(BGP_ATTR_ORIGIN)); /* MED */ - attr.med = 0; - SET_FLAG(attr.flag, ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)); + bgp_attr_set_med(&attr, 0); /* AS path attribute. */ if (aspath) @@ -1933,9 +1932,7 @@ static enum bgp_attr_parse_ret bgp_attr_med(struct bgp_attr_parser_args *args) args->total); } - attr->med = stream_getl(peer->curr); - - SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)); + bgp_attr_set_med(attr, stream_getl(peer->curr)); return BGP_ATTR_PARSE_PROCEED; } diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 02ddf88b6242..c10a9dc90a74 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -608,6 +608,12 @@ static inline uint64_t bgp_aigp_metric_total(struct bgp_path_info *bpi) return aigp; } +static inline void bgp_attr_set_med(struct attr *attr, uint32_t med) +{ + attr->med = med; + SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)); +} + static inline struct cluster_list *bgp_attr_get_cluster(const struct attr *attr) { return attr->cluster1; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 0f196157c80c..a8ca887cdc12 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -6747,8 +6747,8 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p, bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); attr.nexthop = bgp_static->igpnexthop; - attr.med = bgp_static->igpmetric; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + + bgp_attr_set_med(&attr, bgp_static->igpmetric); if (afi == AFI_IP) attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; @@ -9015,9 +9015,8 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p, else UNSET_FLAG(attr.nh_flags, BGP_ATTR_NH_IF_OPERSTATE); - attr.med = metric; + bgp_attr_set_med(&attr, metric); attr.distance = distance; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); attr.tag = tag; if (metric) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 4900bb3ce351..0f8adea38db0 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -2222,8 +2222,7 @@ route_set_metric(void *rule, const struct prefix *prefix, void *object) if (path->attr->flag & ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)) med = path->attr->med; - path->attr->med = route_value_adjust(rv, med, path->peer); - path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + bgp_attr_set_med(path->attr, route_value_adjust(rv, med, path->peer)); return RMAP_OKAY; } diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index 250378af6845..1a66df59fc20 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -906,8 +906,8 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw) assert(attr.aspath); aspath = attr.aspath; - attr.med = 0; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + + bgp_attr_set_med(&attr, 0); if ((afi == AFI_IP6) || peer_cap_enhe(peer, afi, safi)) { /* IPv6 global nexthop must be included. */ diff --git a/bgpd/rfapi/rfapi.c b/bgpd/rfapi/rfapi.c index 23e3eb482308..61d154f1b436 100644 --- a/bgpd/rfapi/rfapi.c +++ b/bgpd/rfapi/rfapi.c @@ -667,10 +667,8 @@ void add_vnc_route(struct rfapi_descriptor *rfd, /* cookie, VPN UN addr, peer */ attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF); } - if (med) { - attr.med = *med; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); - } + if (med) + bgp_attr_set_med(&attr, *med); /* override default weight assigned by bgp_attr_default_set() */ attr.weight = rfd->peer ? rfd->peer->weight[afi][safi] : 0; @@ -863,10 +861,8 @@ void add_vnc_route(struct rfapi_descriptor *rfd, /* cookie, VPN UN addr, peer */ red = bgp_redist_lookup(bgp, afi, type, 0); - if (red && red->redist_metric_flag) { - attr.med = red->redist_metric; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); - } + if (red && red->redist_metric_flag) + bgp_attr_set_med(&attr, red->redist_metric); bn = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi, p, prd); diff --git a/bgpd/rfapi/vnc_export_bgp.c b/bgpd/rfapi/vnc_export_bgp.c index 4de2306609c9..c9a058ea44c0 100644 --- a/bgpd/rfapi/vnc_export_bgp.c +++ b/bgpd/rfapi/vnc_export_bgp.c @@ -96,15 +96,16 @@ static void encap_attr_export_ce(struct attr *new, struct attr *orig, * neighbor NEIGHBOR attribute-unchanged med */ if (!CHECK_FLAG(new->flag, BGP_ATTR_MULTI_EXIT_DISC)) { + uint32_t med = 255; + if (CHECK_FLAG(new->flag, BGP_ATTR_LOCAL_PREF)) { if (new->local_pref > 255) - new->med = 0; + med = 0; else - new->med = 255 - new->local_pref; - } else { - new->med = 255; /* shouldn't happen */ + med = 255 - new->local_pref; } - new->flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + + bgp_attr_set_med(new, med); } /* @@ -642,15 +643,16 @@ encap_attr_export(struct attr *new, struct attr *orig, * neighbor NEIGHBOR attribute-unchanged med */ if (!CHECK_FLAG(new->flag, BGP_ATTR_MULTI_EXIT_DISC)) { + uint32_t med = 255; + if (CHECK_FLAG(new->flag, BGP_ATTR_LOCAL_PREF)) { if (new->local_pref > 255) - new->med = 0; + med = 0; else - new->med = 255 - new->local_pref; - } else { - new->med = 255; /* shouldn't happen */ + med = 255 - new->local_pref; } - new->flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + + bgp_attr_set_med(new, med); } /* From f677fc8db3c0195dc1bc8d7cd9887cf4e7aa638e Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 20:57:49 +0300 Subject: [PATCH 2/8] bgpd: Implement `set metric igp` command Set metric automatically from the path info (IGP protocol). Signed-off-by: Donatas Abraitis --- bgpd/bgp_routemap.c | 26 +++++++++++++++++--------- doc/user/routemap.rst | 4 +++- lib/routemap_cli.c | 10 ++++++++-- lib/routemap_northbound.c | 21 +++++++++++++++++++++ yang/frr-route-map.yang | 8 ++++++++ 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 0f8adea38db0..0e6ba4ead877 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -125,6 +125,9 @@ o Local extensions #define RMAP_VALUE_ADD 1 #define RMAP_VALUE_SUB 2 +#define RMAP_VALUE_TYPE_RTT 1 +#define RMAP_VALUE_TYPE_IGP 2 + struct rmap_value { uint8_t action; uint8_t variable; @@ -140,14 +143,18 @@ static int route_value_match(struct rmap_value *rv, uint32_t value) } static uint32_t route_value_adjust(struct rmap_value *rv, uint32_t current, - struct peer *peer) + struct bgp_path_info *bpi) { uint32_t value; + struct peer *peer = bpi->peer; switch (rv->variable) { - case 1: + case RMAP_VALUE_TYPE_RTT: value = peer->rtt; break; + case RMAP_VALUE_TYPE_IGP: + value = bpi->extra ? bpi->extra->igpmetric : 0; + break; default: value = rv->value; break; @@ -187,11 +194,12 @@ static void *route_value_compile(const char *arg) larg = strtoul(arg, &endptr, 10); if (*arg == 0 || *endptr != 0 || errno || larg > UINT32_MAX) return NULL; + } else if (strmatch(arg, "rtt")) { + var = RMAP_VALUE_TYPE_RTT; + } else if (strmatch(arg, "igp")) { + var = RMAP_VALUE_TYPE_IGP; } else { - if (strcmp(arg, "rtt") == 0) - var = 1; - else - return NULL; + return NULL; } rv = XMALLOC(MTYPE_ROUTE_MAP_COMPILED, sizeof(struct rmap_value)); @@ -2145,7 +2153,7 @@ route_set_local_pref(void *rule, const struct prefix *prefix, void *object) locpref = path->attr->local_pref; path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF); - path->attr->local_pref = route_value_adjust(rv, locpref, path->peer); + path->attr->local_pref = route_value_adjust(rv, locpref, path); return RMAP_OKAY; } @@ -2172,7 +2180,7 @@ route_set_weight(void *rule, const struct prefix *prefix, void *object) path = object; /* Set weight value. */ - path->attr->weight = route_value_adjust(rv, 0, path->peer); + path->attr->weight = route_value_adjust(rv, 0, path); return RMAP_OKAY; } @@ -2222,7 +2230,7 @@ route_set_metric(void *rule, const struct prefix *prefix, void *object) if (path->attr->flag & ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)) med = path->attr->med; - bgp_attr_set_med(path->attr, route_value_adjust(rv, med, path->peer)); + bgp_attr_set_med(path->attr, route_value_adjust(rv, med, path)); return RMAP_OKAY; } diff --git a/doc/user/routemap.rst b/doc/user/routemap.rst index 9034af39c560..3ee5597f239a 100644 --- a/doc/user/routemap.rst +++ b/doc/user/routemap.rst @@ -305,13 +305,15 @@ Route Map Set Command Set the route's weight. -.. clicmd:: set metric <[+|-](1-4294967295)|rtt|+rtt|-rtt> +.. clicmd:: set metric <[+|-](1-4294967295)|rtt|+rtt|-rtt|igp> Set the route metric. When used with BGP, set the BGP attribute MED to a specific value. Use `+`/`-` to add or subtract the specified value to/from the existing/MED. Use `rtt` to set the MED to the round trip time or `+rtt`/`-rtt` to add/subtract the round trip time to/from the MED. + If ``igp`` is specified, then the actual value from the IGP protocol is used. + .. clicmd:: set min-metric <(0-4294967295)> Set the minimum metric for the route. diff --git a/lib/routemap_cli.c b/lib/routemap_cli.c index f64c3c2376a9..3e4893310e7e 100644 --- a/lib/routemap_cli.c +++ b/lib/routemap_cli.c @@ -922,13 +922,14 @@ DEFPY_YANG( DEFPY_YANG( set_metric, set_metric_cmd, - "set metric <(-4294967295-4294967295)$metric|rtt$rtt|+rtt$artt|-rtt$srtt>", + "set metric <(-4294967295-4294967295)$metric|rtt$rtt|+rtt$artt|-rtt$srtt|igp$igp>", SET_STR "Metric value for destination routing protocol\n" "Metric value (use +/- for additions or subtractions)\n" "Assign round trip time\n" "Add round trip time\n" - "Subtract round trip time\n") + "Subtract round trip time\n" + "Metric value from IGP protocol\n") { const char *xpath = "./set-action[action='frr-route-map:set-metric']"; char xpath_value[XPATH_MAXLEN]; @@ -939,6 +940,9 @@ DEFPY_YANG( snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/use-round-trip-time", xpath); snprintf(value, sizeof(value), "true"); + } else if (igp) { + snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/use-igp", xpath); + snprintf(value, sizeof(value), "true"); } else if (artt) { snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/add-round-trip-time", xpath); @@ -1148,6 +1152,8 @@ void route_map_action_show(struct vty *vty, const struct lyd_node *dnode, if (yang_dnode_get(dnode, "./rmap-set-action/use-round-trip-time")) { vty_out(vty, " set metric rtt\n"); + } else if (yang_dnode_get(dnode, "./rmap-set-action/use-igp")) { + vty_out(vty, " set metric igp\n"); } else if (yang_dnode_get( dnode, "./rmap-set-action/add-round-trip-time")) { diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c index 1bba4dad47a6..a868a80f392f 100644 --- a/lib/routemap_northbound.c +++ b/lib/routemap_northbound.c @@ -1213,6 +1213,20 @@ static int lib_route_map_entry_set_action_use_round_trip_time_destroy( return lib_route_map_entry_set_action_value_destroy(args); } +/* + * XPath: /frr-route-map:lib/route-map/entry/set-action/use-igp + */ +static int lib_route_map_entry_set_action_use_igp_modify(struct nb_cb_modify_args *args) +{ + return set_action_modify(args->event, args->dnode, args->resource, "igp", args->errmsg, + args->errmsg_len); +} + +static int lib_route_map_entry_set_action_use_igp_destroy(struct nb_cb_destroy_args *args) +{ + return lib_route_map_entry_set_action_value_destroy(args); +} + /* * XPath: /frr-route-map:lib/route-map/entry/set-action/add-round-trip-time */ @@ -1516,6 +1530,13 @@ const struct frr_yang_module_info frr_route_map_info = { .destroy = lib_route_map_entry_set_action_use_round_trip_time_destroy, } }, + { + .xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/use-igp", + .cbs = { + .modify = lib_route_map_entry_set_action_use_igp_modify, + .destroy = lib_route_map_entry_set_action_use_igp_destroy, + } + }, { .xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/add-round-trip-time", .cbs = { diff --git a/yang/frr-route-map.yang b/yang/frr-route-map.yang index c875a6ec7f05..b83047caee4f 100644 --- a/yang/frr-route-map.yang +++ b/yang/frr-route-map.yang @@ -355,6 +355,14 @@ module frr-route-map { "Subtract round trip time to metric"; } } + + case use-igp { + leaf use-igp { + type boolean; + description + "Use metric from IGP procotol"; + } + } } } From 8d39cfd613944be99671fe28a3d92c6b2cb716ab Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 21:36:13 +0300 Subject: [PATCH 3/8] tests: Check if MED can be derived from `set metric igp|aigp` Signed-off-by: Donatas Abraitis --- tests/topotests/bgp_aigp/r1/bgpd.conf | 9 +++++++ tests/topotests/bgp_aigp/r1/zebra.conf | 3 +++ tests/topotests/bgp_aigp/r8/bgpd.conf | 4 +++ tests/topotests/bgp_aigp/r8/zebra.conf | 4 +++ tests/topotests/bgp_aigp/test_bgp_aigp.py | 32 ++++++++++++++++++++++- 5 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 tests/topotests/bgp_aigp/r8/bgpd.conf create mode 100644 tests/topotests/bgp_aigp/r8/zebra.conf diff --git a/tests/topotests/bgp_aigp/r1/bgpd.conf b/tests/topotests/bgp_aigp/r1/bgpd.conf index 74a0215bc476..d99192421a39 100644 --- a/tests/topotests/bgp_aigp/r1/bgpd.conf +++ b/tests/topotests/bgp_aigp/r1/bgpd.conf @@ -9,4 +9,13 @@ router bgp 65001 neighbor 10.0.0.3 timers 1 3 neighbor 10.0.0.3 timers connect 1 neighbor 10.0.0.3 update-source lo + neighbor 192.168.18.8 remote-as external + neighbor 192.168.18.8 timers 1 3 + neighbor 192.168.18.8 timers connect 1 + address-family ipv4 + neighbor 192.168.18.8 route-map r8 out + exit-address-family ! +route-map r8 permit 10 + set metric igp +exit diff --git a/tests/topotests/bgp_aigp/r1/zebra.conf b/tests/topotests/bgp_aigp/r1/zebra.conf index 0ed22d37be8b..7ac4bb5de9a3 100644 --- a/tests/topotests/bgp_aigp/r1/zebra.conf +++ b/tests/topotests/bgp_aigp/r1/zebra.conf @@ -8,3 +8,6 @@ interface r1-eth0 interface r1-eth1 ip address 192.168.13.1/24 ! +interface r1-eth2 + ip address 192.168.18.1/24 +! diff --git a/tests/topotests/bgp_aigp/r8/bgpd.conf b/tests/topotests/bgp_aigp/r8/bgpd.conf new file mode 100644 index 000000000000..c50c3ce91a58 --- /dev/null +++ b/tests/topotests/bgp_aigp/r8/bgpd.conf @@ -0,0 +1,4 @@ +router bgp 65008 + no bgp ebgp-requires-policy + neighbor 192.168.18.1 remote-as external +! diff --git a/tests/topotests/bgp_aigp/r8/zebra.conf b/tests/topotests/bgp_aigp/r8/zebra.conf new file mode 100644 index 000000000000..f7545270b2c7 --- /dev/null +++ b/tests/topotests/bgp_aigp/r8/zebra.conf @@ -0,0 +1,4 @@ +! +interface r8-eth0 + ip address 192.168.18.8/24 +! diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py index 0d859adc53f4..92b54d0ae102 100644 --- a/tests/topotests/bgp_aigp/test_bgp_aigp.py +++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py @@ -15,6 +15,8 @@ and 10 appropriately. r1 receives routes with aigp-metric TLV 81, 91 and 82, 92 respectively. + +r1 advertises MED from IGP protocol (set metric igp) to r8. """ import os @@ -34,7 +36,7 @@ def build_topo(tgen): - for routern in range(1, 8): + for routern in range(1, 9): tgen.add_router("r{}".format(routern)) switch = tgen.add_switch("s1") @@ -65,6 +67,10 @@ def build_topo(tgen): switch.add_link(tgen.gears["r6"]) switch.add_link(tgen.gears["r7"]) + switch = tgen.add_switch("s8") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r8"]) + def setup_module(mod): tgen = Topogen(build_topo, mod.__name__) @@ -102,6 +108,7 @@ def test_bgp_aigp(): r3 = tgen.gears["r3"] r4 = tgen.gears["r4"] r5 = tgen.gears["r5"] + r8 = tgen.gears["r8"] def _bgp_converge(): output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast 10.0.0.71/32 json")) @@ -143,6 +150,11 @@ def _bgp_check_aigp_metric(router, prefix, aigp): expected = {"paths": [{"aigpMetric": aigp, "valid": True}]} return topotest.json_cmp(output, expected) + def _bgp_check_received_med(med): + output = json.loads(r8.vtysh_cmd("show bgp ipv4 unicast 10.0.0.71/32 json")) + expected = {"paths": [{"metric": med, "valid": True}]} + return topotest.json_cmp(output, expected) + def _bgp_check_aigp_metric_bestpath(): output = json.loads( r1.vtysh_cmd( @@ -252,6 +264,24 @@ def _bgp_check_aigp_metric_bestpath(): _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) assert result is None, "AIGP attribute is not considered in best-path selection" + # r8, check if MED is set to 20 (derived from `set metric igp`) + test_func = functools.partial(_bgp_check_received_med, 20) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "MED attribute value is not 20" + + r1.vtysh_cmd( + """ +configure terminal +route-map r8 permit 10 + set metric aigp +""" + ) + + # r8, check if MED is set to 111 (derived from `set metric aigp`) + test_func = functools.partial(_bgp_check_received_med, 111) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "MED attribute value is not 111" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] From e94f48498d3ec68bd7dec185d7658bdc583a0c32 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 21:47:40 +0300 Subject: [PATCH 4/8] bgpd: Implement `set metric aigp` command Same as `set metric igp`, but in this case accumulated IGP metric is being sent as MED attribute. Signed-off-by: Donatas Abraitis --- bgpd/bgp_routemap.c | 6 ++++++ lib/routemap_cli.c | 26 ++++++++++++-------------- lib/routemap_northbound.c | 21 +++++++++++++++++++++ yang/frr-route-map.yang | 8 ++++++++ 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 0e6ba4ead877..23e83f9bddac 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -127,6 +127,7 @@ o Local extensions #define RMAP_VALUE_TYPE_RTT 1 #define RMAP_VALUE_TYPE_IGP 2 +#define RMAP_VALUE_TYPE_AIGP 3 struct rmap_value { uint8_t action; @@ -155,6 +156,9 @@ static uint32_t route_value_adjust(struct rmap_value *rv, uint32_t current, case RMAP_VALUE_TYPE_IGP: value = bpi->extra ? bpi->extra->igpmetric : 0; break; + case RMAP_VALUE_TYPE_AIGP: + value = MIN(bpi->attr->aigp_metric, UINT32_MAX); + break; default: value = rv->value; break; @@ -198,6 +202,8 @@ static void *route_value_compile(const char *arg) var = RMAP_VALUE_TYPE_RTT; } else if (strmatch(arg, "igp")) { var = RMAP_VALUE_TYPE_IGP; + } else if (strmatch(arg, "aigp")) { + var = RMAP_VALUE_TYPE_AIGP; } else { return NULL; } diff --git a/lib/routemap_cli.c b/lib/routemap_cli.c index 3e4893310e7e..432805c8d51a 100644 --- a/lib/routemap_cli.c +++ b/lib/routemap_cli.c @@ -922,14 +922,15 @@ DEFPY_YANG( DEFPY_YANG( set_metric, set_metric_cmd, - "set metric <(-4294967295-4294967295)$metric|rtt$rtt|+rtt$artt|-rtt$srtt|igp$igp>", + "set metric <(-4294967295-4294967295)$metric|rtt$rtt|+rtt$artt|-rtt$srtt|igp$igp|aigp$aigp>", SET_STR "Metric value for destination routing protocol\n" "Metric value (use +/- for additions or subtractions)\n" "Assign round trip time\n" "Add round trip time\n" "Subtract round trip time\n" - "Metric value from IGP protocol\n") + "Metric value from IGP protocol\n" + "Metric value from AIGP (Accumulated IGP)\n") { const char *xpath = "./set-action[action='frr-route-map:set-metric']"; char xpath_value[XPATH_MAXLEN]; @@ -943,6 +944,9 @@ DEFPY_YANG( } else if (igp) { snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/use-igp", xpath); snprintf(value, sizeof(value), "true"); + } else if (aigp) { + snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/use-aigp", xpath); + snprintf(value, sizeof(value), "true"); } else if (artt) { snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/add-round-trip-time", xpath); @@ -1154,23 +1158,17 @@ void route_map_action_show(struct vty *vty, const struct lyd_node *dnode, vty_out(vty, " set metric rtt\n"); } else if (yang_dnode_get(dnode, "./rmap-set-action/use-igp")) { vty_out(vty, " set metric igp\n"); - } else if (yang_dnode_get( - dnode, - "./rmap-set-action/add-round-trip-time")) { + } else if (yang_dnode_get(dnode, "./rmap-set-action/use-aigp")) { + vty_out(vty, " set metric aigp\n"); + } else if (yang_dnode_get(dnode, "./rmap-set-action/add-round-trip-time")) { vty_out(vty, " set metric +rtt\n"); - } else if ( - yang_dnode_get( - dnode, - "./rmap-set-action/subtract-round-trip-time")) { + } else if (yang_dnode_get(dnode, "./rmap-set-action/subtract-round-trip-time")) { vty_out(vty, " set metric -rtt\n"); - } else if (yang_dnode_get(dnode, - "./rmap-set-action/add-metric")) { + } else if (yang_dnode_get(dnode, "./rmap-set-action/add-metric")) { vty_out(vty, " set metric +%s\n", yang_dnode_get_string( dnode, "./rmap-set-action/add-metric")); - } else if (yang_dnode_get( - dnode, - "./rmap-set-action/subtract-metric")) { + } else if (yang_dnode_get(dnode, "./rmap-set-action/subtract-metric")) { vty_out(vty, " set metric -%s\n", yang_dnode_get_string( dnode, diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c index a868a80f392f..0ee055e65304 100644 --- a/lib/routemap_northbound.c +++ b/lib/routemap_northbound.c @@ -1227,6 +1227,20 @@ static int lib_route_map_entry_set_action_use_igp_destroy(struct nb_cb_destroy_a return lib_route_map_entry_set_action_value_destroy(args); } +/* + * XPath: /frr-route-map:lib/route-map/entry/set-action/use-aigp + */ +static int lib_route_map_entry_set_action_use_aigp_modify(struct nb_cb_modify_args *args) +{ + return set_action_modify(args->event, args->dnode, args->resource, "aigp", args->errmsg, + args->errmsg_len); +} + +static int lib_route_map_entry_set_action_use_aigp_destroy(struct nb_cb_destroy_args *args) +{ + return lib_route_map_entry_set_action_value_destroy(args); +} + /* * XPath: /frr-route-map:lib/route-map/entry/set-action/add-round-trip-time */ @@ -1537,6 +1551,13 @@ const struct frr_yang_module_info frr_route_map_info = { .destroy = lib_route_map_entry_set_action_use_igp_destroy, } }, + { + .xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/use-aigp", + .cbs = { + .modify = lib_route_map_entry_set_action_use_aigp_modify, + .destroy = lib_route_map_entry_set_action_use_aigp_destroy, + } + }, { .xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/add-round-trip-time", .cbs = { diff --git a/yang/frr-route-map.yang b/yang/frr-route-map.yang index b83047caee4f..244c353a6357 100644 --- a/yang/frr-route-map.yang +++ b/yang/frr-route-map.yang @@ -363,6 +363,14 @@ module frr-route-map { "Use metric from IGP procotol"; } } + + case use-aigp { + leaf use-aigp { + type boolean; + description + "Use metric from AIGP (Accumulated IGP)"; + } + } } } From 3105ceaee9a6aab59d0c120fcf28f103bf84d0a3 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 21:48:23 +0300 Subject: [PATCH 5/8] tests: Check if `set metric aigp` works Signed-off-by: Donatas Abraitis --- tests/topotests/bgp_aigp/r1/bgpd.conf | 7 +++++ tests/topotests/bgp_aigp/test_bgp_aigp.py | 36 +++++++++++------------ 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/tests/topotests/bgp_aigp/r1/bgpd.conf b/tests/topotests/bgp_aigp/r1/bgpd.conf index d99192421a39..15621999c59f 100644 --- a/tests/topotests/bgp_aigp/r1/bgpd.conf +++ b/tests/topotests/bgp_aigp/r1/bgpd.conf @@ -16,6 +16,13 @@ router bgp 65001 neighbor 192.168.18.8 route-map r8 out exit-address-family ! +ip prefix-list p71 seq 5 permit 10.0.0.71/32 +ip prefix-list p72 seq 5 permit 10.0.0.72/32 +! route-map r8 permit 10 + match ip address prefix-list p71 set metric igp +route-map r8 permit 20 + match ip address prefix-list p72 + set metric aigp exit diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py index 92b54d0ae102..a59b71590b12 100644 --- a/tests/topotests/bgp_aigp/test_bgp_aigp.py +++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py @@ -17,6 +17,8 @@ r1 receives routes with aigp-metric TLV 81, 91 and 82, 92 respectively. r1 advertises MED from IGP protocol (set metric igp) to r8. + +r1 advertises MED from AIGP (set metric aigp) to r8. """ import os @@ -150,9 +152,16 @@ def _bgp_check_aigp_metric(router, prefix, aigp): expected = {"paths": [{"aigpMetric": aigp, "valid": True}]} return topotest.json_cmp(output, expected) - def _bgp_check_received_med(med): - output = json.loads(r8.vtysh_cmd("show bgp ipv4 unicast 10.0.0.71/32 json")) - expected = {"paths": [{"metric": med, "valid": True}]} + def _bgp_check_received_med(): + output = json.loads( + r8.vtysh_cmd("show bgp ipv4 unicast 10.0.0.64/28 longer-prefixes json") + ) + expected = { + "routes": { + "10.0.0.71/32": [{"valid": True, "metric": 20}], + "10.0.0.72/32": [{"valid": True, "metric": 112}], + } + } return topotest.json_cmp(output, expected) def _bgp_check_aigp_metric_bestpath(): @@ -264,23 +273,12 @@ def _bgp_check_aigp_metric_bestpath(): _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) assert result is None, "AIGP attribute is not considered in best-path selection" - # r8, check if MED is set to 20 (derived from `set metric igp`) - test_func = functools.partial(_bgp_check_received_med, 20) - _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert result is None, "MED attribute value is not 20" - - r1.vtysh_cmd( - """ -configure terminal -route-map r8 permit 10 - set metric aigp -""" - ) - - # r8, check if MED is set to 111 (derived from `set metric aigp`) - test_func = functools.partial(_bgp_check_received_med, 111) + # r8, check if MED is set derived from `set metric igp`, and `set metric aigp` + test_func = functools.partial(_bgp_check_received_med) _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert result is None, "MED attribute value is not 111" + assert ( + result is None + ), "MED attribute values are not derived from `set metric [a]igp`" if __name__ == "__main__": From 7cdece8c84bee4694e0679e402b01cd501669d99 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 21:53:19 +0300 Subject: [PATCH 6/8] doc: Add `set metric aigp` command Signed-off-by: Donatas Abraitis --- doc/user/routemap.rst | 5 ++++- tests/topotests/bgp_aigp/test_bgp_aigp.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/user/routemap.rst b/doc/user/routemap.rst index 3ee5597f239a..02a43bc980b8 100644 --- a/doc/user/routemap.rst +++ b/doc/user/routemap.rst @@ -305,7 +305,7 @@ Route Map Set Command Set the route's weight. -.. clicmd:: set metric <[+|-](1-4294967295)|rtt|+rtt|-rtt|igp> +.. clicmd:: set metric <[+|-](1-4294967295)|rtt|+rtt|-rtt|igp|aigp> Set the route metric. When used with BGP, set the BGP attribute MED to a specific value. Use `+`/`-` to add or subtract the specified value to/from @@ -314,6 +314,9 @@ Route Map Set Command If ``igp`` is specified, then the actual value from the IGP protocol is used. + If ``aigp`` is specified, then the actual value from the AIGP metric is used + (encoded as MED instead of AIGP attribute). + .. clicmd:: set min-metric <(0-4294967295)> Set the minimum metric for the route. diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py index a59b71590b12..5f253d2ef052 100644 --- a/tests/topotests/bgp_aigp/test_bgp_aigp.py +++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py @@ -158,8 +158,8 @@ def _bgp_check_received_med(): ) expected = { "routes": { - "10.0.0.71/32": [{"valid": True, "metric": 20}], - "10.0.0.72/32": [{"valid": True, "metric": 112}], + "10.0.0.71/32": [{"valid": True, "metric": 10}], + "10.0.0.72/32": [{"valid": True, "metric": 92}], } } return topotest.json_cmp(output, expected) From b924f033875729414d91b7efe5cd4d2003f28390 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 9 Oct 2024 18:20:48 +0300 Subject: [PATCH 7/8] bgpd: Re-announce the routes if the underlay IGP metric changes If the underlay IGP metric changes, we SHOULD re-announce the routes with the correct bpi->extra->igpmetric set. Without this patch if the IGP link cost (metric) changes, we never notice this and the peers do not have the updated metrics, which in turn causes incorrect best path selections on remote peers. Signed-off-by: Donatas Abraitis --- bgpd/bgp_route.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index a8ca887cdc12..93bfa62b7d12 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -3727,11 +3727,10 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, /* If there is a change of interest to peers, reannounce the * route. */ - if (CHECK_FLAG(old_select->flags, BGP_PATH_ATTR_CHANGED) - || CHECK_FLAG(old_select->flags, BGP_PATH_LINK_BW_CHG) - || CHECK_FLAG(dest->flags, BGP_NODE_LABEL_CHANGED)) { + if (CHECK_FLAG(old_select->flags, BGP_PATH_ATTR_CHANGED) || + CHECK_FLAG(dest->flags, BGP_NODE_LABEL_CHANGED) || + bgp_zebra_has_route_changed(old_select)) { group_announce_route(bgp, afi, safi, dest, new_select); - /* unicast routes must also be annouced to * labeled-unicast update-groups */ if (safi == SAFI_UNICAST) From a593f156d7711d778c76ab9b43dda6b90ed15712 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 9 Oct 2024 18:35:33 +0300 Subject: [PATCH 8/8] tests: Check if underlay IGP metric is reflected into BGP after cost changes Signed-off-by: Donatas Abraitis --- .../topotests/bgp_set_metric_igp/__init__.py | 0 .../topotests/bgp_set_metric_igp/r1/frr.conf | 13 ++ .../topotests/bgp_set_metric_igp/r2/frr.conf | 36 +++++ .../topotests/bgp_set_metric_igp/r3/frr.conf | 38 +++++ .../topotests/bgp_set_metric_igp/r4/frr.conf | 41 +++++ .../topotests/bgp_set_metric_igp/r5/frr.conf | 14 ++ .../test_bgp_set_metric_igp.py | 146 ++++++++++++++++++ 7 files changed, 288 insertions(+) create mode 100644 tests/topotests/bgp_set_metric_igp/__init__.py create mode 100644 tests/topotests/bgp_set_metric_igp/r1/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/r2/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/r3/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/r4/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/r5/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/test_bgp_set_metric_igp.py diff --git a/tests/topotests/bgp_set_metric_igp/__init__.py b/tests/topotests/bgp_set_metric_igp/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/topotests/bgp_set_metric_igp/r1/frr.conf b/tests/topotests/bgp_set_metric_igp/r1/frr.conf new file mode 100644 index 000000000000..018ea0269eda --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r1/frr.conf @@ -0,0 +1,13 @@ +! +int r1-eth0 + ip address 10.0.0.1/24 +! +int r1-eth1 + ip address 10.0.1.1/24 +! +router bgp 65001 + no bgp ebgp-requires-policy + no bgp network import-check + neighbor 10.0.0.2 remote-as external + neighbor 10.0.1.2 remote-as external +! diff --git a/tests/topotests/bgp_set_metric_igp/r2/frr.conf b/tests/topotests/bgp_set_metric_igp/r2/frr.conf new file mode 100644 index 000000000000..bce06c47f065 --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r2/frr.conf @@ -0,0 +1,36 @@ +! +int r2-eth0 + ip address 10.0.0.2/24 + ip router isis n2 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +int r2-eth1 + ip address 10.0.2.1/24 + ip router isis n2 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 10.0.0.1 remote-as external + neighbor 10.0.2.2 remote-as internal + address-family ipv4 unicast + neighbor 10.0.0.1 route-map igp out + exit-address-family +! +router isis n2 + is-type level-2-only + net 49.0001.0000.0000.0002.00 + lsp-mtu 1440 +exit +! +route-map igp permit 10 + set metric igp +exit diff --git a/tests/topotests/bgp_set_metric_igp/r3/frr.conf b/tests/topotests/bgp_set_metric_igp/r3/frr.conf new file mode 100644 index 000000000000..312f97d08a4a --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r3/frr.conf @@ -0,0 +1,38 @@ +! +int r3-eth0 + ip address 10.0.1.2/24 + ip router isis n3 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +int r3-eth1 + ip address 10.0.3.1/24 + ip router isis n3 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis metric level-1 10 + isis metric level-2 100 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 10.0.1.1 remote-as external + neighbor 10.0.3.2 remote-as internal + address-family ipv4 unicast + neighbor 10.0.1.1 route-map igp out + exit-address-family +! +router isis n3 + is-type level-2-only + net 49.0001.0000.0000.0003.00 + lsp-mtu 1440 +exit +! +route-map igp permit 10 + set metric igp +exit diff --git a/tests/topotests/bgp_set_metric_igp/r4/frr.conf b/tests/topotests/bgp_set_metric_igp/r4/frr.conf new file mode 100644 index 000000000000..04165b80b244 --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r4/frr.conf @@ -0,0 +1,41 @@ +! +int r4-eth0 + ip address 10.0.2.2/24 + ip router isis n4 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +int r4-eth1 + ip address 10.0.3.2/24 + ip router isis n4 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis metric level-1 10 + isis metric level-2 100 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +int r4-eth2 + ip address 10.0.4.1/24 + ip router isis n4 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 10.0.2.1 remote-as internal + neighbor 10.0.3.1 remote-as internal + neighbor 10.0.4.2 remote-as external +! +router isis n4 + is-type level-2-only + net 49.0001.0000.0000.0004.00 + lsp-mtu 1440 +exit diff --git a/tests/topotests/bgp_set_metric_igp/r5/frr.conf b/tests/topotests/bgp_set_metric_igp/r5/frr.conf new file mode 100644 index 000000000000..af23677b72b5 --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r5/frr.conf @@ -0,0 +1,14 @@ +! +int r5-eth0 + ip address 10.0.4.2/24 +! +router bgp 65005 + no bgp ebgp-requires-policy + no bgp network import-check + neighbor 10.0.4.1 remote-as external + neighbor 10.0.4.1 timers 1 3 + neighbor 10.0.4.1 timers connect 1 + address-family ipv4 unicast + network 10.5.5.5/32 + exit-address-family +! diff --git a/tests/topotests/bgp_set_metric_igp/test_bgp_set_metric_igp.py b/tests/topotests/bgp_set_metric_igp/test_bgp_set_metric_igp.py new file mode 100644 index 000000000000..8fbe817689d1 --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/test_bgp_set_metric_igp.py @@ -0,0 +1,146 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2024 by +# Donatas Abraitis +# + +import os +import sys +import json +import pytest +import functools + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, get_topogen + +pytestmark = [pytest.mark.bgpd] + + +def setup_module(mod): + topodef = { + "s1": ("r1", "r2"), + "s2": ("r1", "r3"), + "s3": ("r2", "r4"), + "s4": ("r3", "r4"), + "s5": ("r4", "r5"), + } + tgen = Topogen(topodef, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for _, (rname, router) in enumerate(router_list.items(), 1): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_set_metric_igp(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r2 = tgen.gears["r2"] + + def _bgp_converge(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast json")) + expected = { + "routes": { + "10.5.5.5/32": [ + { + "valid": True, + "bestpath": True, + "selectionReason": "MED", + "metric": 20, + "nexthops": [ + { + "ip": "10.0.0.2", + "hostname": "r2", + } + ], + }, + { + "valid": True, + "bestpath": None, + "metric": 110, + "nexthops": [ + { + "ip": "10.0.1.2", + "hostname": "r3", + } + ], + }, + ] + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_converge, + ) + _, result = topotest.run_and_expect(test_func, None, count=120, wait=5) + assert result is None, "10.5.5.5/32 best path is not via r2 (MED == 20)" + + r2.vtysh_cmd( + """ +configure terminal +interface r2-eth1 + isis metric level-2 6000 +""" + ) + + def _bgp_converge_after_isis_metric_changes(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast json")) + expected = { + "routes": { + "10.5.5.5/32": [ + { + "valid": True, + "bestpath": None, + "metric": 6010, + "nexthops": [ + { + "ip": "10.0.0.2", + "hostname": "r2", + } + ], + }, + { + "valid": True, + "bestpath": True, + "selectionReason": "MED", + "metric": 110, + "nexthops": [ + { + "ip": "10.0.1.2", + "hostname": "r3", + } + ], + }, + ] + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_converge_after_isis_metric_changes, + ) + _, result = topotest.run_and_expect(test_func, None, count=120, wait=5) + assert result is None, "10.5.5.5/32 best path is not via r3 (MED == 110)" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))