From a8474e4a464ade1d7dd942d82cda9e219329ebb4 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 11 Jan 2024 10:36:41 +0200 Subject: [PATCH 1/4] bgpd: Prefer routes over eBGP versus eBGP-OAD If at least one of the candidate routes was received via EBGP, remove from consideration all routes that were received via EBGP-OAD and IBGP. Signed-off-by: Donatas Abraitis --- bgpd/bgp_route.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 2899b94d2ffd..211eeeaf3f20 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -613,6 +613,8 @@ int bgp_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new, struct attr *newattr, *existattr; enum bgp_peer_sort new_sort; enum bgp_peer_sort exist_sort; + enum bgp_peer_sub_sort new_sub_sort; + enum bgp_peer_sub_sort exist_sub_sort; uint32_t new_pref; uint32_t exist_pref; uint32_t new_med; @@ -1147,26 +1149,34 @@ int bgp_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new, /* 7. Peer type check. */ new_sort = peer_new->sort; exist_sort = peer_exist->sort; + new_sub_sort = peer_new->sub_sort; + exist_sub_sort = peer_exist->sub_sort; - if (new_sort == BGP_PEER_EBGP - && (exist_sort == BGP_PEER_IBGP || exist_sort == BGP_PEER_CONFED)) { + if (new_sort == BGP_PEER_EBGP && + (exist_sort == BGP_PEER_IBGP || exist_sort == BGP_PEER_CONFED || + exist_sub_sort == BGP_PEER_EBGP_OAD)) { *reason = bgp_path_selection_peer; if (debug) - zlog_debug( - "%s: %s wins over %s due to eBGP peer > iBGP peer", - pfx_buf, new_buf, exist_buf); + zlog_debug("%s: %s wins over %s due to eBGP peer > %s peer", + pfx_buf, new_buf, exist_buf, + (exist_sub_sort == BGP_PEER_EBGP_OAD) + ? "eBGP-OAD" + : "iBGP"); if (!CHECK_FLAG(bgp->flags, BGP_FLAG_PEERTYPE_MULTIPATH_RELAX)) return 1; peer_sort_ret = 1; } - if (exist_sort == BGP_PEER_EBGP - && (new_sort == BGP_PEER_IBGP || new_sort == BGP_PEER_CONFED)) { + if (exist_sort == BGP_PEER_EBGP && + (new_sort == BGP_PEER_IBGP || new_sort == BGP_PEER_CONFED || + new_sub_sort == BGP_PEER_EBGP_OAD)) { *reason = bgp_path_selection_peer; if (debug) - zlog_debug( - "%s: %s loses to %s due to iBGP peer < eBGP peer", - pfx_buf, new_buf, exist_buf); + zlog_debug("%s: %s loses to %s due to %s peer < eBGP peer", + pfx_buf, new_buf, exist_buf, + (exist_sub_sort == BGP_PEER_EBGP_OAD) + ? "eBGP-OAD" + : "iBGP"); if (!CHECK_FLAG(bgp->flags, BGP_FLAG_PEERTYPE_MULTIPATH_RELAX)) return 0; peer_sort_ret = 0; From 584b031a4d0b7e20cd3835c8945451dcdac07998 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 11 Jan 2024 10:47:33 +0200 Subject: [PATCH 2/4] bgpd: Show external session sub-type (OAD) if exists ``` r1# sh ip bgp 10.10.10.10/32 BGP routing table entry for 10.10.10.10/32, version 1 Paths: (2 available, best #2, table default) Advertised to non peer-group peers: 192.168.1.2 192.168.1.4 65002 65003 192.168.1.2 from 192.168.1.2 (192.168.2.2) Origin incomplete, metric 123, localpref 123, valid, external (oad) Last update: Thu Jan 11 10:46:32 2024 65004 65005 192.168.1.4 from 192.168.1.4 (192.168.4.4) Origin incomplete, metric 123, localpref 123, valid, external, best (Peer Type) Last update: Thu Jan 11 10:46:30 2024 r1# ``` Signed-off-by: Donatas Abraitis --- bgpd/bgp_route.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 211eeeaf3f20..d6aa41bd36fa 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -10684,9 +10684,17 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn, } else { if (json_paths) json_object_string_add( - json_peer, "type", "external"); + json_peer, "type", + (path->peer->sub_sort == + BGP_PEER_EBGP_OAD) + ? "external (oad)" + : "external"); else - vty_out(vty, ", external"); + vty_out(vty, ", %s", + (path->peer->sub_sort == + BGP_PEER_EBGP_OAD) + ? "external (oad)" + : "external"); } } } else if (path->sub_type == BGP_ROUTE_AGGREGATE) { From 1c491dfbe646f885b0a570666da3ba1e062c2a6c Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 11 Jan 2024 10:34:05 +0200 Subject: [PATCH 3/4] tests: Check if the route over eBGP is preferred when eBGP-OAD is used If at least one of the candidate routes was received via EBGP, remove from consideration all routes that were received via EBGP-OAD and IBGP. Signed-off-by: Donatas Abraitis --- tests/topotests/bgp_oad/r1/frr.conf | 10 ++++++++++ tests/topotests/bgp_oad/r2/frr.conf | 8 ++++---- tests/topotests/bgp_oad/r3/frr.conf | 2 +- tests/topotests/bgp_oad/r4/frr.conf | 16 ++++++++++++++++ tests/topotests/bgp_oad/r5/frr.conf | 17 +++++++++++++++++ tests/topotests/bgp_oad/test_bgp_oad.py | 18 ++++++++++++++++-- 6 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 tests/topotests/bgp_oad/r4/frr.conf create mode 100644 tests/topotests/bgp_oad/r5/frr.conf diff --git a/tests/topotests/bgp_oad/r1/frr.conf b/tests/topotests/bgp_oad/r1/frr.conf index 471c5ef52ddf..39045ba648eb 100644 --- a/tests/topotests/bgp_oad/r1/frr.conf +++ b/tests/topotests/bgp_oad/r1/frr.conf @@ -8,4 +8,14 @@ router bgp 65001 neighbor 192.168.1.2 timers 1 3 neighbor 192.168.1.2 timers connect 1 neighbor 192.168.1.2 oad + neighbor 192.168.1.4 remote-as external + neighbor 192.168.1.4 timers 1 3 + neighbor 192.168.1.4 timers connect 1 + address-family ipv4 unicast + neighbor 192.168.1.4 route-map r4 in + exit-address-family ! +route-map r4 permit 10 + set local-preference 123 + set metric 123 +exit diff --git a/tests/topotests/bgp_oad/r2/frr.conf b/tests/topotests/bgp_oad/r2/frr.conf index c98b115896be..fdd23f32b48b 100644 --- a/tests/topotests/bgp_oad/r2/frr.conf +++ b/tests/topotests/bgp_oad/r2/frr.conf @@ -11,8 +11,8 @@ router bgp 65002 neighbor 192.168.1.1 timers 1 3 neighbor 192.168.1.1 timers connect 1 neighbor 192.168.1.1 oad - neighbor 192.168.2.1 remote-as external - neighbor 192.168.2.1 timers 1 3 - neighbor 192.168.2.1 timers connect 1 - neighbor 192.168.2.1 oad + neighbor 192.168.2.3 remote-as external + neighbor 192.168.2.3 timers 1 3 + neighbor 192.168.2.3 timers connect 1 + neighbor 192.168.2.3 oad ! diff --git a/tests/topotests/bgp_oad/r3/frr.conf b/tests/topotests/bgp_oad/r3/frr.conf index 875079a4f93d..02dd5adfe190 100644 --- a/tests/topotests/bgp_oad/r3/frr.conf +++ b/tests/topotests/bgp_oad/r3/frr.conf @@ -3,7 +3,7 @@ int lo ip address 10.10.10.10/32 ! int r3-eth0 - ip address 192.168.2.1/24 + ip address 192.168.2.3/24 ! router bgp 65003 no bgp ebgp-requires-policy diff --git a/tests/topotests/bgp_oad/r4/frr.conf b/tests/topotests/bgp_oad/r4/frr.conf new file mode 100644 index 000000000000..83dcf391b774 --- /dev/null +++ b/tests/topotests/bgp_oad/r4/frr.conf @@ -0,0 +1,16 @@ +! +int r4-eth0 + ip address 192.168.1.4/24 +! +int r4-eth1 + ip address 192.168.4.4/24 +! +router bgp 65004 + no bgp ebgp-requires-policy + neighbor 192.168.1.1 remote-as external + neighbor 192.168.1.1 timers 1 3 + neighbor 192.168.1.1 timers connect 1 + neighbor 192.168.4.5 remote-as external + neighbor 192.168.4.5 timers 1 3 + neighbor 192.168.4.5 timers connect 1 +! diff --git a/tests/topotests/bgp_oad/r5/frr.conf b/tests/topotests/bgp_oad/r5/frr.conf new file mode 100644 index 000000000000..f8e1609d15b0 --- /dev/null +++ b/tests/topotests/bgp_oad/r5/frr.conf @@ -0,0 +1,17 @@ +! +int lo + ip address 10.10.10.10/32 +! +int r5-eth0 + ip address 192.168.4.5/24 +! +router bgp 65005 + no bgp ebgp-requires-policy + neighbor 192.168.4.4 remote-as external + neighbor 192.168.4.4 timers 1 3 + neighbor 192.168.4.4 timers connect 1 + ! + address-family ipv4 unicast + redistribute connected + exit-address-family +! diff --git a/tests/topotests/bgp_oad/test_bgp_oad.py b/tests/topotests/bgp_oad/test_bgp_oad.py index a2e3eddc2610..b26c54835748 100644 --- a/tests/topotests/bgp_oad/test_bgp_oad.py +++ b/tests/topotests/bgp_oad/test_bgp_oad.py @@ -31,7 +31,7 @@ def setup_module(mod): - topodef = {"s1": ("r1", "r2"), "s2": ("r2", "r3")} + topodef = {"s1": ("r1", "r2", "r4"), "s2": ("r2", "r3"), "s3": ("r4", "r5")} tgen = Topogen(topodef, mod.__name__) tgen.start_topology() @@ -64,7 +64,21 @@ def _bgp_converge(): "aspath": {"string": "65002 65003"}, "metric": 123, "locPrf": 123, - } + "peer": { + "hostname": "r2", + "type": "external (oad)", + }, + }, + { + "aspath": {"string": "65004 65005"}, + "metric": 123, + "locPrf": 123, + "bestpath": {"selectionReason": "Peer Type"}, + "peer": { + "hostname": "r4", + "type": "external", + }, + }, ] } return topotest.json_cmp(output, expected) From a56beac98bf1c7660ad8ea3066b37e712302ba4b Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 11 Jan 2024 11:18:14 +0200 Subject: [PATCH 4/4] bgpd: Allow sending Origin Validation State extended community over EBGP-OAD https://datatracker.ietf.org/doc/html/draft-uttaro-idr-bgp-oad#section-3.13 Extended communities which are non-transitive across an AS boundary MAY be advertised over an EBGP-OAD session if allowed by explicit policy configuration. If allowed, all the members of the OAD SHOULD be configured to use the same criteria. For example, the Origin Validation State Extended Community, defined as non-transitive in [RFC8097], can be advertised to peers in the same OAD. Signed-off-by: Donatas Abraitis --- bgpd/bgp_route.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index d6aa41bd36fa..e1387b032181 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2661,8 +2661,12 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, /* If this is an iBGP, send Origin Validation State (OVS) * extended community (rfc8097). + * draft-uttaro-idr-bgp-oad states: + * For example, the Origin Validation State Extended Community, + * defined as non-transitive in [RFC8097], can be advertised to + * peers in the same OAD. */ - if (peer->sort == BGP_PEER_IBGP) { + if (peer->sort == BGP_PEER_IBGP || peer->sub_sort == BGP_PEER_EBGP_OAD) { enum rpki_states rpki_state = RPKI_NOT_BEING_USED; rpki_state = hook_call(bgp_rpki_prefix_status, peer, attr, p);