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 <taspelund@nvidia.com>

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index d3424b29578601b8066c05a760fb128a4f4fcaea..192cd6ca82fc14ca02bce215b70e7686b99c3612 100644 (file)
--- 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 309699fdf489c47369d2ffea027df5c847982564..6f74de7d118f7778c982074f8870928366cd4a17 100644 (file)
--- 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);
                }
        }
 }