From ea7e52f4c45f3cafabdf5307ddca9670103fbc85 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 31 Oct 2024 08:21:22 +0100 Subject: [PATCH] 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 --- bgpd/bgp_bmp.c | 28 ++++++++++++------- .../bmp1vrf/bmp-update-post-policy-step1.json | 4 +-- .../bmp1vrf/bmp-update-pre-policy-step1.json | 4 +-- .../bmp-withdraw-post-policy-step1.json | 4 +-- .../bmp-withdraw-pre-policy-step1.json | 4 +-- 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 614bafe22db8..d74454269f8b 100644 --- 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 index 18f40b16c7d8..04e01623dfc0 100644 --- 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 index 61ef0eab86ac..760ee0409ab4 100644 --- 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 index c28ce851a2a4..f57b1a51cefb 100644 --- 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 index 976c7d47167e..a52308c7892e 100644 --- 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",