Skip to content

Commit

Permalink
Merge pull request FRRouting#15128 from opensourcerouting/fix/bgp_oad…
Browse files Browse the repository at this point in the history 
…_ECOMMUNITY_ORIGIN_VALIDATION_STATE

bgpd: Recent EBGP-OAD improvements
  • Loading branch information
@donaldsharp
donaldsharp authored Jan 11, 2024
2 parents 67e8ef2 + a56beac commit 86cbd58
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 20 deletions.
48 changes: 35 additions & 13 deletions bgpd/bgp_route.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2651,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);
Expand Down Expand Up @@ -10674,9 +10688,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) {
Expand Down
10 changes: 10 additions & 0 deletions tests/topotests/bgp_oad/r1/frr.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 4 additions & 4 deletions tests/topotests/bgp_oad/r2/frr.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
!
2 changes: 1 addition & 1 deletion tests/topotests/bgp_oad/r3/frr.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions tests/topotests/bgp_oad/r4/frr.conf
View file
Original file line number Diff line number Diff line change
@@ -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
!
17 changes: 17 additions & 0 deletions tests/topotests/bgp_oad/r5/frr.conf
View file
Original file line number Diff line number Diff line change
@@ -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
!
18 changes: 16 additions & 2 deletions tests/topotests/bgp_oad/test_bgp_oad.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 86cbd58

Please sign in to comment.