diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 54d125a7dff4..e7ea0cf6d4b5 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1064,7 +1064,32 @@ int bgp_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new, } } - /* Tie-breaker - AIGP (Metric TLV) attribute */ + /* 3. Local route check. We prefer: + * - BGP_ROUTE_STATIC + * - BGP_ROUTE_AGGREGATE + * - BGP_ROUTE_REDISTRIBUTE + */ + new_origin = !(new->sub_type == BGP_ROUTE_NORMAL || new->sub_type == BGP_ROUTE_IMPORTED); + exist_origin = !(exist->sub_type == BGP_ROUTE_NORMAL || + exist->sub_type == BGP_ROUTE_IMPORTED); + + if (new_origin && !exist_origin) { + *reason = bgp_path_selection_local_route; + if (debug) + zlog_debug("%s: %s wins over %s due to preferred BGP_ROUTE type", pfx_buf, + new_buf, exist_buf); + return 1; + } + + if (!new_origin && exist_origin) { + *reason = bgp_path_selection_local_route; + if (debug) + zlog_debug("%s: %s loses to %s due to preferred BGP_ROUTE type", pfx_buf, + new_buf, exist_buf); + return 0; + } + + /* 3.5. Tie-breaker - AIGP (Metric TLV) attribute */ if (CHECK_FLAG(newattr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && CHECK_FLAG(existattr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && CHECK_FLAG(bgp->flags, BGP_FLAG_COMPARE_AIGP)) { @@ -1094,34 +1119,6 @@ int bgp_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new, } } - /* 3. Local route check. We prefer: - * - BGP_ROUTE_STATIC - * - BGP_ROUTE_AGGREGATE - * - BGP_ROUTE_REDISTRIBUTE - */ - new_origin = !(new->sub_type == BGP_ROUTE_NORMAL || - new->sub_type == BGP_ROUTE_IMPORTED); - exist_origin = !(exist->sub_type == BGP_ROUTE_NORMAL || - exist->sub_type == BGP_ROUTE_IMPORTED); - - if (new_origin && !exist_origin) { - *reason = bgp_path_selection_local_route; - if (debug) - zlog_debug( - "%s: %s wins over %s due to preferred BGP_ROUTE type", - pfx_buf, new_buf, exist_buf); - return 1; - } - - if (!new_origin && exist_origin) { - *reason = bgp_path_selection_local_route; - if (debug) - zlog_debug( - "%s: %s loses to %s due to preferred BGP_ROUTE type", - pfx_buf, new_buf, exist_buf); - return 0; - } - /* Here if these are imported routes then get ultimate pi for * path compare. */ diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index 4632c70d53c4..0f64f4bc1c96 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -160,16 +160,16 @@ bottom until one of the factors can be used. Prefer higher local preference routes to lower. +3. **Local route check** + + Prefer local routes (statics, aggregates, redistributed) to received routes. + If ``bgp bestpath aigp`` is enabled, and both paths that are compared have AIGP attribute, BGP uses AIGP tie-breaking unless both of the paths have the AIGP metric attribute. This means that the AIGP attribute is not evaluated during the best path selection process between two paths when one path does not have the AIGP attribute. -3. **Local route check** - - Prefer local routes (statics, aggregates, redistributed) to received routes. - 4. **AS path length check** Prefer shortest hop-count AS_PATHs. diff --git a/tests/topotests/bgp_aigp_rr/r1/bgpd.conf b/tests/topotests/bgp_aigp_rr/r1/bgpd.conf index 90d34bdf8382..4099a248f128 100644 --- a/tests/topotests/bgp_aigp_rr/r1/bgpd.conf +++ b/tests/topotests/bgp_aigp_rr/r1/bgpd.conf @@ -18,6 +18,7 @@ router bgp 65001 neighbor 10.0.0.4 timers connect 1 neighbor 10.0.0.4 route-reflector-client address-family ipv4 + network 10.0.1.2/32 route-map set-aigp neighbor 10.0.0.4 route-map set-nexthop out exit-address-family ! @@ -25,3 +26,7 @@ route-map set-nexthop permit 10 set ip next-hop peer-address exit ! +route-map set-aigp permit 10 + set aigp 50 + set weight 0 +! diff --git a/tests/topotests/bgp_aigp_rr/r2/bgpd.conf b/tests/topotests/bgp_aigp_rr/r2/bgpd.conf index 97059fdd333f..0159dc8e7c33 100644 --- a/tests/topotests/bgp_aigp_rr/r2/bgpd.conf +++ b/tests/topotests/bgp_aigp_rr/r2/bgpd.conf @@ -7,6 +7,7 @@ router bgp 65001 neighbor 10.0.0.1 timers connect 1 address-family ipv4 redistribute connected route-map connected-to-bgp + network 10.0.1.2/32 route-map set-aigp neighbor 10.0.0.1 next-hop-self exit-address-family ! @@ -16,3 +17,6 @@ route-map connected-to-bgp permit 10 match ip address prefix-list p22 set aigp 2 ! +route-map set-aigp permit 10 + set aigp 10 +! diff --git a/tests/topotests/bgp_aigp_rr/test_bgp_aigp_rr.py b/tests/topotests/bgp_aigp_rr/test_bgp_aigp_rr.py index 0079535da7ce..206e229b2ee6 100644 --- a/tests/topotests/bgp_aigp_rr/test_bgp_aigp_rr.py +++ b/tests/topotests/bgp_aigp_rr/test_bgp_aigp_rr.py @@ -101,6 +101,45 @@ def _bgp_check_aigp_metric(router, prefix, aigp): expected = {"paths": [{"aigpMetric": aigp, "valid": True}]} return topotest.json_cmp(output, expected) + def _bgp_check_aigp_bestpath(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast 10.0.1.2/32 json")) + expected = { + "prefix": "10.0.1.2/32", + "paths": [ + { + "aigpMetric": 50, + "valid": True, + "sourced": True, + "local": True, + "bestpath": {"overall": True, "selectionReason": "Local Route"}, + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "metric": 0, + "accessible": True, + "used": True, + } + ], + }, + { + "aigpMetric": 10, + "valid": True, + "nexthops": [ + { + "ip": "10.0.0.2", + "hostname": "r2", + "afi": "ipv4", + "metric": 10, + "accessible": True, + "used": True, + } + ], + }, + ], + } + return topotest.json_cmp(output, expected) # r2, 10.0.2.2/32 with aigp-metric 2 test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.2.2/32", 2) @@ -122,6 +161,11 @@ def _bgp_check_aigp_metric(router, prefix, aigp): _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) assert result is None, "aigp-metric for 10.0.2.2/32 is not 12" + # r1, check if the local route is favored over AIGP comparison + test_func = functools.partial(_bgp_check_aigp_bestpath) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assert result is None, "Local route is not favored over AIGP in best-path selection" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:]