Merge pull request #17037 from louis-6wind/fix-ipv6-ll-nexthop

bgpd: fix ipv6 nexthop-local unchanged
FRRouting · Oct 11, 2024 · 813a9f6 · 813a9f6
2 parents 997f710 + 6dc4d95
commit 813a9f6
Show file tree

Hide file tree

Showing 31 changed files with 2,874 additions and 15 deletions.
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
@@ -2155,6 +2155,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 	bool nh_reset = false;
 	uint64_t cum_bw;
 	mpls_label_t label;
+	bool global_and_ll = false;
 
 	if (DISABLE_BGP_ANNOUNCE)
 		return false;
@@ -2465,33 +2466,37 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 	 * we do not announce LL address as `::`.
 	 */
 	if (NEXTHOP_IS_V6) {
-		attr->mp_nexthop_len = BGP_ATTR_NHLEN_IPV6_GLOBAL;
-		if ((CHECK_FLAG(peer->af_flags[afi][safi],
-				PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED) &&
-		     IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_local)) ||
-		    (!reflect && !transparent &&
-		     IN6_IS_ADDR_LINKLOCAL(&peer->nexthop.v6_local) &&
-		     peer->shared_network &&
-		     ((from == bgp->peer_self && peer->sort == BGP_PEER_EBGP) ||
-		      (from == bgp->peer_self && peer->sort != BGP_PEER_EBGP) ||
-		      (from != bgp->peer_self &&
-		       IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_local) &&
-		       peer->sort == BGP_PEER_EBGP)))) {
+		if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED)) {
+			/* nexthop local unchanged: only include the link-local nexthop if it
+			 * was already present.
+			 */
+			if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_local))
+				global_and_ll = true;
+		} else if (!reflect && !transparent &&
+			   IN6_IS_ADDR_LINKLOCAL(&peer->nexthop.v6_local) && peer->shared_network &&
+			   (from == bgp->peer_self || peer->sort == BGP_PEER_EBGP))
+			global_and_ll = true;
+
+		if (global_and_ll) {
 			if (safi == SAFI_MPLS_VPN)
 				attr->mp_nexthop_len =
 					BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL;
 			else
 				attr->mp_nexthop_len =
 					BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL;
-		}
+		} else
+			attr->mp_nexthop_len = BGP_ATTR_NHLEN_IPV6_GLOBAL;
 
 		/* Clear off link-local nexthop in source, whenever it is not
 		 * needed to
 		 * ensure more prefixes share the same attribute for
 		 * announcement.
 		 */
-		if (!(CHECK_FLAG(peer->af_flags[afi][safi],
-				 PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED)))
+		if (!(CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED)) ||
+		    !IPV6_ADDR_SAME(&peer->nexthop.v6_global, &from->nexthop.v6_global))
+			/* Reset if "nexthop-local unchanged" is not set or originating and destination peer
+			 * does not share the same subnet.
+			 */
 			memset(&attr->mp_nexthop_local, 0, IPV6_MAX_BYTELEN);
 	}
 

diff --git a/tests/topotests/bgp_nexthop_ipv6/exabgp.env b/tests/topotests/bgp_nexthop_ipv6/exabgp.env
@@ -0,0 +1,53 @@
+[exabgp.api]
+encoder = text
+highres = false
+respawn = false
+socket = ''
+
+[exabgp.bgp]
+openwait = 60
+
+[exabgp.cache]
+attributes = true
+nexthops = true
+
+[exabgp.daemon]
+daemonize = true
+pid = '/var/run/exabgp/exabgp.pid'
+user = 'exabgp'
+##daemonize = false
+
+[exabgp.log]
+all = false
+configuration = true
+daemon = true
+destination = '/var/log/exabgp.log'
+enable = true
+level = INFO
+message = false
+network = true
+packets = false
+parser = false
+processes = true
+reactor = true
+rib = false
+routes = false
+short = false
+timers = false
+
+[exabgp.pdb]
+enable = false
+
+[exabgp.profile]
+enable = false
+file = ''
+
+[exabgp.reactor]
+speed = 1.0
+
+[exabgp.tcp]
+acl = false
+bind = ''
+delay = 0
+once = false
+port = 179
diff --git a/tests/topotests/bgp_nexthop_ipv6/r1/bgpd.conf b/tests/topotests/bgp_nexthop_ipv6/r1/bgpd.conf
@@ -0,0 +1,22 @@
+router bgp 65000
+ no bgp ebgp-requires-policy
+ neighbor fd00:0:2::9 remote-as internal
+ neighbor fd00:0:2::9 timers 3 10
+ address-family ipv4 unicast
+  redistribute connected route-map RMAP4
+ !
+ address-family ipv6 unicast
+  redistribute connected route-map RMAP6
+  neighbor fd00:0:2::9 activate
+
+ip prefix-list RANGE4 seq 10 permit 172.16.0.0/16 le 24
+ip prefix-list RANGE4 seq 20 permit 10.0.0.0/8 ge 32
+
+ipv6 prefix-list RANGE6 seq 10 permit fd00:100::0/64
+ipv6 prefix-list RANGE6 seq 20 permit 2001:db8::0/64 ge 128
+
+route-map RMAP4 permit 10
+ match ip address prefix-list RANGE4
+!
+route-map RMAP6 permit 10
+ match ipv6 address prefix-list RANGE6
diff --git a/tests/topotests/bgp_nexthop_ipv6/r1/show_bgp_ipv6_step1.json b/tests/topotests/bgp_nexthop_ipv6/r1/show_bgp_ipv6_step1.json
@@ -0,0 +1,162 @@
+{
+  "routerId": "10.1.1.1",
+  "localAS": 65000,
+  "routes": {
+    "2001:db8::1/128": [
+      {
+        "nexthops": [
+          {
+            "ip": "::",
+            "hostname": "r1",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "2001:db8::2/128": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:2::2",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "2001:db8::3/128": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:2::3",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "2001:db8::4/128": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:2::4",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "2001:db8::5/128": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:3::5",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "2001:db8::6/128": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:4::6",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "fd00:100::/64": [
+      {
+        "nexthops": [
+          {
+            "ip": "::",
+            "hostname": "r1",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "fd00:200::/64": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:2::2",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "fd00:300::/64": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:2::3",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "fd00:400::/64": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:2::4",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "fd00:500::/64": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:3::5",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ],
+    "fd00:600::/64": [
+      {
+        "nexthops": [
+          {
+            "ip": "fd00:0:4::6",
+            "hostname": "rr",
+            "afi": "ipv6",
+            "scope": "global",
+            "used": true
+          }
+        ]
+      }
+    ]
+  }
+}