bgpd, topotests: bmp, fix wrong peer distinguisher for vrf route events

When running the bgp_bmp_2 vrf test, peer route messages from the pre and post policy are received with a wrong peer distinguisher value. > {"peer_type": "route distinguisher instance", "policy": "pre-policy", "ipv6": false, > "peer_ip": "192.168.0.2", "peer_distinguisher": "0:0", "peer_asn": 65502, > "peer_bgp_id": "192.168.0.2", "timestamp": "2024-10-31 08:19:58.111963", > "bmp_log_type": "update", "origin": "IGP", "as_path": "65501 65502", > "bgp_nexthop": "192.168.0.2", "ip_prefix": "172.31.0.15/32", "seq": 15} RFC7854 mentions in 4.2 that if the peer is a "RD Instance Peer", it is set to the route distinguisher of the particular instance the peer belongs to. Fix this by modifying the BMP client: - update the peer distinguisher value by unlocking the filling of the peer distinguisher in the function. This change impacts monitoring messages. - add the peer distinguisher computation for mirror messages - modify the bgp_bmp_2 vrf test, update the peer_distinguisher value > {"peer_type": "route distinguisher instance", "policy": "pre-policy", "ipv6": false, > "peer_ip": "192.168.0.2", "peer_distinguisher": "444:1", "peer_asn": 65502, > "peer_bgp_id": "192.168.0.2", "timestamp": "2024-10-31 08:19:58.111963", > "bmp_log_type": "update", "origin": "IGP", "as_path": "65501 65502", > "bgp_nexthop": "192.168.0.2", "ip_prefix": "172.31.0.15/32", "seq": 15} Signed-off-by: Philippe Guibert <[email protected]>
FRRouting · Dec 3, 2024 · a745a29 · a745a29
1 parent a2bd2a6
commit a745a29
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 18 deletions.
diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c
@@ -285,13 +285,6 @@ static inline int bmp_get_peer_distinguisher(struct bgp *bgp, afi_t afi, uint8_t
 	if (peer_type == BMP_PEER_TYPE_LOCAL_INSTANCE || bgp->vrf_id == VRF_UNKNOWN)
 		return 1;
 
-	/* remove this check when the other peer types get correct peer dist.
-	 *(RFC7854) impl.
-	 * for now, always return no error and 0 peer distinguisher as before
-	 */
-	if (peer_type != BMP_PEER_TYPE_LOC_RIB_INSTANCE)
-		return (*result_ref = 0);
-
 	/* vrf default => ok, distinguisher 0 */
 	if (bgp->inst_type == VRF_DEFAULT)
 		return (*result_ref = 0);
@@ -791,16 +784,23 @@ static void bmp_wrmirror_lost(struct bmp *bmp, struct pullwr *pullwr)
 	struct stream *s;
 	struct timeval tv;
 	uint8_t peer_type_flag;
+	uint64_t peer_distinguisher = 0;
 
 	gettimeofday(&tv, NULL);
 
 	peer_type_flag = bmp_get_peer_type_vrf(bmp->targets->bgp->vrf_id);
 
+	if (bmp_get_peer_distinguisher(bmp->targets->bgp, AFI_UNSPEC, peer_type_flag,
+				       &peer_distinguisher)) {
+		zlog_warn("skipping bmp message for reason: can't get peer distinguisher");
+		return;
+	}
+
 	s = stream_new(BGP_MAX_PACKET_SIZE);
 
 	bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_ROUTE_MIRRORING);
-	bmp_per_peer_hdr(s, bmp->targets->bgp, bmp->targets->bgp->peer_self, 0, peer_type_flag, 0,
-			 &tv);
+	bmp_per_peer_hdr(s, bmp->targets->bgp, bmp->targets->bgp->peer_self, 0, peer_type_flag,
+			 peer_distinguisher, &tv);
 
 	stream_putw(s, BMP_MIRROR_TLV_TYPE_INFO);
 	stream_putw(s, 2);
@@ -818,6 +818,7 @@ static bool bmp_wrmirror(struct bmp *bmp, struct pullwr *pullwr)
 	struct peer *peer;
 	bool written = false;
 	uint8_t peer_type_flag;
+	uint64_t peer_distinguisher = 0;
 
 	if (bmp->mirror_lost) {
 		bmp_wrmirror_lost(bmp, pullwr);
@@ -837,11 +838,18 @@ static bool bmp_wrmirror(struct bmp *bmp, struct pullwr *pullwr)
 
 	peer_type_flag = bmp_get_peer_type_vrf(bmp->targets->bgp->vrf_id);
 
+	if (bmp_get_peer_distinguisher(peer->bgp, AFI_UNSPEC, peer_type_flag, &peer_distinguisher)) {
+		zlog_warn("skipping bmp message for peer %s: can't get peer distinguisher",
+			  peer->host);
+		goto out;
+	}
+
 	struct stream *s;
 	s = stream_new(BGP_MAX_PACKET_SIZE);
 
 	bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_ROUTE_MIRRORING);
-	bmp_per_peer_hdr(s, bmp->targets->bgp, peer, 0, peer_type_flag, 0, &bmq->tv);
+	bmp_per_peer_hdr(s, bmp->targets->bgp, peer, 0, peer_type_flag, peer_distinguisher,
+			 &bmq->tv);
 
 	/* BMP Mirror TLV. */
 	stream_putw(s, BMP_MIRROR_TLV_TYPE_BGP_MESSAGE);

diff --git a/tests/topotests/bgp_bmp/bmp1vrf/bmp-update-post-policy-step1.json b/tests/topotests/bgp_bmp/bmp1vrf/bmp-update-post-policy-step1.json
@@ -10,7 +10,7 @@
                 "origin": "IGP",
                 "peer_asn": 65502,
                 "peer_bgp_id": "192.168.0.2",
-                "peer_distinguisher": "0:0",
+                "peer_distinguisher": "444:1",
                 "peer_ip": "192.168.0.2",
                 "peer_type": "route distinguisher instance",
                 "policy": "post-policy"
@@ -25,7 +25,7 @@
                 "origin": "IGP",
                 "peer_asn": 65502,
                 "peer_bgp_id": "192.168.0.2",
-                "peer_distinguisher": "0:0",
+                "peer_distinguisher": "555:1",
                 "peer_ip": "192:168::2",
                 "peer_type": "route distinguisher instance",
                 "policy": "post-policy",

diff --git a/tests/topotests/bgp_bmp/bmp1vrf/bmp-update-pre-policy-step1.json b/tests/topotests/bgp_bmp/bmp1vrf/bmp-update-pre-policy-step1.json
@@ -10,7 +10,7 @@
                 "origin": "IGP",
                 "peer_asn": 65502,
                 "peer_bgp_id": "192.168.0.2",
-                "peer_distinguisher": "0:0",
+                "peer_distinguisher": "444:1",
                 "peer_ip": "192.168.0.2",
                 "peer_type": "route distinguisher instance",
                 "policy": "pre-policy"
@@ -25,7 +25,7 @@
                 "origin": "IGP",
                 "peer_asn": 65502,
                 "peer_bgp_id": "192.168.0.2",
-                "peer_distinguisher": "0:0",
+                "peer_distinguisher": "555:1",
                 "peer_ip": "192:168::2",
                 "peer_type": "route distinguisher instance",
                 "policy": "pre-policy",

diff --git a/tests/topotests/bgp_bmp/bmp1vrf/bmp-withdraw-post-policy-step1.json b/tests/topotests/bgp_bmp/bmp1vrf/bmp-withdraw-post-policy-step1.json
@@ -7,7 +7,7 @@
                 "ipv6": false,
                 "peer_asn": 65502,
                 "peer_bgp_id": "192.168.0.2",
-                "peer_distinguisher": "0:0",
+                "peer_distinguisher": "444:1",
                 "peer_ip": "192.168.0.2",
                 "peer_type": "route distinguisher instance",
                 "policy": "post-policy"
@@ -19,7 +19,7 @@
                 "ipv6": true,
                 "peer_asn": 65502,
                 "peer_bgp_id": "192.168.0.2",
-                "peer_distinguisher": "0:0",
+                "peer_distinguisher": "555:1",
                 "peer_ip": "192:168::2",
                 "peer_type": "route distinguisher instance",
                 "policy": "post-policy",

diff --git a/tests/topotests/bgp_bmp/bmp1vrf/bmp-withdraw-pre-policy-step1.json b/tests/topotests/bgp_bmp/bmp1vrf/bmp-withdraw-pre-policy-step1.json
@@ -7,7 +7,7 @@
                 "ipv6": false,
                 "peer_asn": 65502,
                 "peer_bgp_id": "192.168.0.2",
-                "peer_distinguisher": "0:0",
+                "peer_distinguisher": "444:1",
                 "peer_ip": "192.168.0.2",
                 "peer_type": "route distinguisher instance",
                 "policy": "pre-policy"
@@ -19,7 +19,7 @@
                 "ipv6": true,
                 "peer_asn": 65502,
                 "peer_bgp_id": "192.168.0.2",
-                "peer_distinguisher": "0:0",
+                "peer_distinguisher": "555:1",
                 "peer_ip": "192:168::2",
                 "peer_type": "route distinguisher instance",
                 "policy": "pre-policy",