]> git.puffer.fish Git - matthieu/frr.git/commitdiff
bgpd, topotests: bmp, fix wrong peer type for vrf route messages
authorPhilippe Guibert <philippe.guibert@6wind.com>
Thu, 31 Oct 2024 08:18:56 +0000 (09:18 +0100)
committerPhilippe Guibert <philippe.guibert@6wind.com>
Wed, 11 Dec 2024 10:29:37 +0000 (11:29 +0100)
When running the bgp_bmp_2 vrf test, peer route messages from the pre
and post policy are received with a wrong peer type value

> {"peer_type": "global 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}

In addition to global instance peers, RFC7854 defines RD instance peers.
This value can be used for peers which are on a BGP VRF instance, for
example with an L3VPN setup.

When configuring a BGP VRF instance, the peer type should be seen as an
RD instance peer.

Fix this by modifying the BMP client:
- update the peer type for vrf mirror and monitoring messages
- modify bgp_bmp_2 vrf test to control the peer_type 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}

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
bgpd/bgp_bmp.c
tests/topotests/bgp_bmp/bmp1vrf/bmp-update-post-policy-step1.json
tests/topotests/bgp_bmp/bmp1vrf/bmp-update-pre-policy-step1.json
tests/topotests/bgp_bmp/bmp1vrf/bmp-withdraw-post-policy-step1.json
tests/topotests/bgp_bmp/bmp1vrf/bmp-withdraw-pre-policy-step1.json

index fbd6e8a1d14499646b62a4a4c08d59a39fb6c491..2b3c2193e717f55562ab7639fb884d695e4fd61c 100644 (file)
@@ -794,14 +794,17 @@ static void bmp_wrmirror_lost(struct bmp *bmp, struct pullwr *pullwr)
 {
        struct stream *s;
        struct timeval tv;
+       uint8_t peer_type_flag;
 
        gettimeofday(&tv, NULL);
 
+       peer_type_flag = bmp_get_peer_type_vrf(bmp->targets->bgp->vrf_id);
+
        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,
-                        BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &tv);
+       bmp_per_peer_hdr(s, bmp->targets->bgp, bmp->targets->bgp->peer_self, 0, peer_type_flag, 0,
+                        &tv);
 
        stream_putw(s, BMP_MIRROR_TLV_TYPE_INFO);
        stream_putw(s, 2);
@@ -818,6 +821,7 @@ static bool bmp_wrmirror(struct bmp *bmp, struct pullwr *pullwr)
        struct bmp_mirrorq *bmq;
        struct peer *peer;
        bool written = false;
+       uint8_t peer_type_flag;
 
        if (bmp->mirror_lost) {
                bmp_wrmirror_lost(bmp, pullwr);
@@ -835,12 +839,13 @@ static bool bmp_wrmirror(struct bmp *bmp, struct pullwr *pullwr)
                goto out;
        }
 
+       peer_type_flag = bmp_get_peer_type_vrf(bmp->targets->bgp->vrf_id);
+
        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,
-                        BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &bmq->tv);
+       bmp_per_peer_hdr(s, bmp->targets->bgp, peer, 0, peer_type_flag, 0, &bmq->tv);
 
        /* BMP Mirror TLV. */
        stream_putw(s, BMP_MIRROR_TLV_TYPE_BGP_MESSAGE);
@@ -1152,6 +1157,7 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr)
        uint8_t bpi_num_labels, adjin_num_labels;
        afi_t afi;
        safi_t safi;
+       uint8_t peer_type_flag;
 
        if (bmp->syncafi == AFI_MAX) {
                FOREACH_AFI_SAFI (afi, safi) {
@@ -1194,6 +1200,8 @@ afibreak:
        struct bgp_path_info *bpi = NULL, *bpiter;
        struct bgp_adj_in *adjin = NULL, *adjiter;
 
+       peer_type_flag = bmp_get_peer_type_vrf(bmp->targets->bgp->vrf_id);
+
        if ((afi == AFI_L2VPN && safi == SAFI_EVPN) ||
            (safi == SAFI_MPLS_VPN)) {
                /* initialize syncrdpos to the first
@@ -1248,10 +1256,8 @@ afibreak:
                                                bmp->remote, afi2str(afi),
                                                safi2str(safi));
 
-                               bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L,
-                                       BMP_PEER_TYPE_GLOBAL_INSTANCE);
-                               bmp_eor(bmp, afi, safi, 0,
-                                       BMP_PEER_TYPE_GLOBAL_INSTANCE);
+                               bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L, peer_type_flag);
+                               bmp_eor(bmp, afi, safi, 0, peer_type_flag);
                                bmp_eor(bmp, afi, safi, 0,
                                        BMP_PEER_TYPE_LOC_RIB_INSTANCE);
 
@@ -1335,19 +1341,20 @@ afibreak:
                            bpi_num_labels);
        }
 
+       if (bpi)
+               peer_type_flag = bmp_get_peer_type(bpi->peer);
+
        if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_VALID) &&
            CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_POSTPOLICY))
-               bmp_monitor(bmp, bpi->peer, BMP_PEER_FLAG_L,
-                           BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, bpi->attr,
+               bmp_monitor(bmp, bpi->peer, BMP_PEER_FLAG_L, peer_type_flag, bn_p, prd, bpi->attr,
                            afi, safi, bpi->uptime,
-                           bpi_num_labels ? bpi->extra->labels->label : NULL,
-                           bpi_num_labels);
+                           bpi_num_labels ? bpi->extra->labels->label : NULL, bpi_num_labels);
 
        if (adjin) {
                adjin_num_labels = adjin->labels ? adjin->labels->num_labels : 0;
-               bmp_monitor(bmp, adjin->peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd,
-                           adjin->attr, afi, safi, adjin->uptime,
-                           adjin_num_labels ? &adjin->labels->label[0] : NULL, adjin_num_labels);
+               bmp_monitor(bmp, adjin->peer, 0, peer_type_flag, bn_p, prd, adjin->attr, afi, safi,
+                           adjin->uptime, adjin_num_labels ? &adjin->labels->label[0] : NULL,
+                           adjin_num_labels);
        }
 
        if (bn)
@@ -1486,6 +1493,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr)
        struct bgp_dest *bn = NULL;
        bool written = false;
        uint8_t bpi_num_labels, adjin_num_labels;
+       uint8_t peer_type_flag;
 
        bqe = bmp_pull(bmp);
        if (!bqe)
@@ -1526,6 +1534,8 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr)
        bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi,
                                  &bqe->p, prd);
 
+       peer_type_flag = bmp_get_peer_type(peer);
+
        if (CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_POSTPOLICY)) {
                struct bgp_path_info *bpi;
 
@@ -1539,12 +1549,9 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr)
 
                bpi_num_labels = BGP_PATH_INFO_NUM_LABELS(bpi);
 
-               bmp_monitor(bmp, peer, BMP_PEER_FLAG_L,
-                           BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd,
-                           bpi ? bpi->attr : NULL, afi, safi,
-                           bpi ? bpi->uptime : monotime(NULL),
-                           bpi_num_labels ? bpi->extra->labels->label : NULL,
-                           bpi_num_labels);
+               bmp_monitor(bmp, peer, BMP_PEER_FLAG_L, peer_type_flag, &bqe->p, prd,
+                           bpi ? bpi->attr : NULL, afi, safi, bpi ? bpi->uptime : monotime(NULL),
+                           bpi_num_labels ? bpi->extra->labels->label : NULL, bpi_num_labels);
                written = true;
        }
 
@@ -1557,9 +1564,8 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr)
                                break;
                }
                adjin_num_labels = adjin && adjin->labels ? adjin->labels->num_labels : 0;
-               bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd,
-                           adjin ? adjin->attr : NULL, afi, safi,
-                           adjin ? adjin->uptime : monotime(NULL),
+               bmp_monitor(bmp, peer, 0, peer_type_flag, &bqe->p, prd, adjin ? adjin->attr : NULL,
+                           afi, safi, adjin ? adjin->uptime : monotime(NULL),
                            adjin_num_labels ? &adjin->labels->label[0] : NULL, adjin_num_labels);
                written = true;
        }
index d5d9d651826579ad9dad99dc467de6e90babf1d1..18f40b16c7d8d95690299365b2e91ca84120e269 100644 (file)
@@ -12,7 +12,7 @@
                 "peer_bgp_id": "192.168.0.2",
                 "peer_distinguisher": "0:0",
                 "peer_ip": "192.168.0.2",
-                "peer_type": "global instance",
+                "peer_type": "route distinguisher instance",
                 "policy": "post-policy"
             },
             "2111::1111/128": {
@@ -27,7 +27,7 @@
                 "peer_bgp_id": "192.168.0.2",
                 "peer_distinguisher": "0:0",
                 "peer_ip": "192:168::2",
-                "peer_type": "global instance",
+                "peer_type": "route distinguisher instance",
                 "policy": "post-policy",
                 "safi": 1
             }
index e11badc040a936e0972657e4fbd2a5b41a26189e..61ef0eab86acd04e28537da944f7b8bfa0b7a9b4 100644 (file)
@@ -12,7 +12,7 @@
                 "peer_bgp_id": "192.168.0.2",
                 "peer_distinguisher": "0:0",
                 "peer_ip": "192.168.0.2",
-                "peer_type": "global instance",
+                "peer_type": "route distinguisher instance",
                 "policy": "pre-policy"
             },
             "2111::1111/128": {
@@ -27,7 +27,7 @@
                 "peer_bgp_id": "192.168.0.2",
                 "peer_distinguisher": "0:0",
                 "peer_ip": "192:168::2",
-                "peer_type": "global instance",
+                "peer_type": "route distinguisher instance",
                 "policy": "pre-policy",
                 "safi": 1
             }
index de84307a4e6be462a021da618b8db63c1948e847..c28ce851a2a43341d80ad15f04eea7e0330d19a4 100644 (file)
@@ -9,7 +9,7 @@
                 "peer_bgp_id": "192.168.0.2",
                 "peer_distinguisher": "0:0",
                 "peer_ip": "192.168.0.2",
-                "peer_type": "global instance",
+                "peer_type": "route distinguisher instance",
                 "policy": "post-policy"
             },
             "2111::1111/128": {
@@ -21,7 +21,7 @@
                 "peer_bgp_id": "192.168.0.2",
                 "peer_distinguisher": "0:0",
                 "peer_ip": "192:168::2",
-                "peer_type": "global instance",
+                "peer_type": "route distinguisher instance",
                 "policy": "post-policy",
                 "safi": 1
             }
index 1c34498b7a38a00ee5f5aaef443bea16b03dd0dd..976c7d47167e627c1c49cb07633dd5c4db007402 100644 (file)
@@ -9,7 +9,7 @@
                 "peer_bgp_id": "192.168.0.2",
                 "peer_distinguisher": "0:0",
                 "peer_ip": "192.168.0.2",
-                "peer_type": "global instance",
+                "peer_type": "route distinguisher instance",
                 "policy": "pre-policy"
             },
             "2111::1111/128": {
@@ -21,7 +21,7 @@
                 "peer_bgp_id": "192.168.0.2",
                 "peer_distinguisher": "0:0",
                 "peer_ip": "192:168::2",
-                "peer_type": "global instance",
+                "peer_type": "route distinguisher instance",
                 "policy": "pre-policy",
                 "safi": 1
             }