From 179d5a0e2652487da81eb8eb48173cadd47e113f Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Tue, 16 Nov 2021 21:11:26 +0000 Subject: [PATCH] bgpd: retain peer asn even with remove-private-AS In situations where remove-private-AS is configured for eBGP peers residing in a private ASN, the peer's ASN was not being retained in the AS-Path which can allow loops to occur. This was addressed in a prior commit but it only addressed cases where the "replace-AS" keyword was configured. This commit ensures we retain the peer's ASN when using "remove-private-AS" for eBGP peers in a private ASN regardless of other keywords. Setup: ========= router bgp 4200000002 neighbor enp1s0 interface v6only remote-as external neighbor enp6s0 interface v6only remote-as external ! address-family ipv4 unicast neighbor enp6s0 remove-private-AS exit-address-family ub18# show ip bgp sum | include 420000 BGP router identifier 100.64.0.111, local AS number 4200000002 vrf-id 0 <<<<< local asn 4200000002 ub20(enp1s0) 4 4200000001 22 22 0 0 0 00:00:57 1 1 ub20(enp6s0) 4 4200000001 21 22 0 0 0 00:00:57 0 1 <<<< peer asn 4200000001 ub18# show ip bgp | include 0.2 Default local pref 100, local AS 4200000002 *> 100.64.0.2/32 enp1s0 0 0 4200000001 4200000004 4200000005 4200000001 i Before ("remote-private-AS" only): ========= ub18# show ip bgp neighbors enp6s0 advertised-routes | include 100.64.0.2 *> 100.64.0.2/32 :: 0 i <<<<< empty as-path, no way to prevent loop After ("remote-private-AS" only): ========= ub18# show ip bgp neighbors enp6s0 advertised-routes | include 100.64.0.2 *> 100.64.0.2/32 :: 0 4200000001 4200000001 i <<<< retain peer's asn, breaks loop Ticket: 2857047 Signed-off-by: Trey Aspelund --- bgpd/bgp_aspath.c | 21 ++++++++++++++------- bgpd/bgp_route.c | 19 +++++++++++-------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index d3424b2957..192cd6ca82 100644 --- a/bgpd/bgp_aspath.c +++ b/bgpd/bgp_aspath.c @@ -1289,6 +1289,7 @@ struct aspath *aspath_remove_private_asns(struct aspath *aspath, as_t peer_asn) int i; int j; int public = 0; + int peer = 0; new = XCALLOC(MTYPE_AS_PATH, sizeof(struct aspath)); @@ -1297,14 +1298,17 @@ struct aspath *aspath_remove_private_asns(struct aspath *aspath, as_t peer_asn) last_new_seg = NULL; seg = aspath->segments; while (seg) { - public - = 0; + public = 0; + peer = 0; for (i = 0; i < seg->length; i++) { // ASN is public - if (!BGP_AS_IS_PRIVATE(seg->as[i])) { - public - ++; - } + if (!BGP_AS_IS_PRIVATE(seg->as[i])) + public++; + /* ASN matches peer's. + * Don't double-count if peer_asn is public. + */ + else if (seg->as[i] == peer_asn) + peer++; } // The entire segment is public so copy it @@ -1316,7 +1320,10 @@ struct aspath *aspath_remove_private_asns(struct aspath *aspath, as_t peer_asn) // there are public ASNs then come back and fill in only the // public ASNs. else { - new_seg = assegment_new(seg->type, public); + /* length needs to account for all retained ASNs + * (public or peer_asn), not just public + */ + new_seg = assegment_new(seg->type, (public + peer)); j = 0; for (i = 0; i < seg->length; i++) { // keep ASN if public or matches peer's ASN diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 309699fdf4..6f74de7d11 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1696,13 +1696,11 @@ static void bgp_peer_remove_private_as(struct bgp *bgp, afi_t afi, safi_t safi, attr->aspath = aspath_replace_private_asns( attr->aspath, bgp->as, peer->as); - // The entire aspath consists of private ASNs so create - // an empty aspath - else if (aspath_private_as_check(attr->aspath)) - attr->aspath = aspath_empty_get(); - - // There are some public and some private ASNs, remove - // the private ASNs + /* + * Even if the aspath consists of just private ASNs we + * need to walk the AS-Path to maintain all instances + * of the peer's ASN to break possible loops. + */ else attr->aspath = aspath_remove_private_asns( attr->aspath, peer->as); @@ -1718,7 +1716,12 @@ static void bgp_peer_remove_private_as(struct bgp *bgp, afi_t afi, safi_t safi, attr->aspath = aspath_replace_private_asns( attr->aspath, bgp->as, peer->as); else - attr->aspath = aspath_empty_get(); + /* + * Walk the aspath to retain any instances of + * the peer_asn + */ + attr->aspath = aspath_remove_private_asns( + attr->aspath, peer->as); } } } -- 2.39.5