Merge pull request #15448 from louis-6wind/bmp-labels

bgpd: export labels into BMP

8 files changed, 249 insertions, 105 deletions

diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c
index 1de48a072a..5fcb8c5645 100644
--- a/bgpd/bgp_bmp.c
+++ b/bgpd/bgp_bmp.c
@@ -910,7 +910,8 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags,
 
 static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
 				 struct peer *peer, struct attr *attr,
-				 afi_t afi, safi_t safi)
+				 afi_t afi, safi_t safi, mpls_label_t *label,
+				 uint32_t num_labels)
 {
 	struct bpacket_attr_vec_arr vecarr;
 	struct stream *s;
@@ -946,8 +947,8 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
 
 		mpattrlen_pos = bgp_packet_mpattr_start(s, peer, afi, safi,
 				&vecarr, attr);
-		bgp_packet_mpattr_prefix(s, afi, safi, p, prd, NULL, 0, 0, 0,
-					 attr);
+		bgp_packet_mpattr_prefix(s, afi, safi, p, prd, label,
+					 num_labels, 0, 0, attr);
 		bgp_packet_mpattr_end(s, mpattrlen_pos);
 		total_attr_len += stream_get_endp(s) - p1;
 	}
@@ -1002,7 +1003,8 @@ static struct stream *bmp_withdraw(const struct prefix *p,
 static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags,
 			uint8_t peer_type_flag, const struct prefix *p,
 			struct prefix_rd *prd, struct attr *attr, afi_t afi,
-			safi_t safi, time_t uptime)
+			safi_t safi, time_t uptime, mpls_label_t *label,
+			uint32_t num_labels)
 {
 	struct stream *hdr, *msg;
 	struct timeval tv = { .tv_sec = uptime, .tv_usec = 0 };
@@ -1019,7 +1021,8 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags,
 
 	monotime_to_realtime(&tv, &uptime_real);
 	if (attr)
-		msg = bmp_update(p, prd, peer, attr, afi, safi);
+		msg = bmp_update(p, prd, peer, attr, afi, safi, label,
+				 num_labels);
 	else
 		msg = bmp_withdraw(p, prd, afi, safi);
 
@@ -1219,18 +1222,24 @@ afibreak:
 		bmp_monitor(bmp, bpi->peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE,
 			    bn_p, prd, bpi->attr, afi, safi,
 			    bpi && bpi->extra ? bpi->extra->bgp_rib_uptime
-					      : (time_t)(-1L));
+					      : (time_t)(-1L),
+			    bpi->extra ? bpi->extra->label : NULL,
+			    bpi->extra ? bpi->extra->num_labels : 0);
 	}
 
 	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,
-			    afi, safi, bpi->uptime);
+			    afi, safi, bpi->uptime,
+			    bpi->extra ? bpi->extra->label : NULL,
+			    bpi->extra ? bpi->extra->num_labels : 0);
 
 	if (adjin)
+		/* TODO: set label here when adjin supports labels */
 		bmp_monitor(bmp, adjin->peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE,
-			    bn_p, prd, adjin->attr, afi, safi, adjin->uptime);
+			    bn_p, prd, adjin->attr, afi, safi, adjin->uptime,
+			    NULL, 0);
 
 	if (bn)
 		bgp_dest_unlock_node(bn);
@@ -1343,7 +1352,9 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr)
 	bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, &bqe->p, prd,
 		    bpi ? bpi->attr : NULL, afi, safi,
 		    bpi && bpi->extra ? bpi->extra->bgp_rib_uptime
-				      : (time_t)(-1L));
+				      : (time_t)(-1L),
+		    (bpi && bpi->extra) ? bpi->extra->label : NULL,
+		    (bpi && bpi->extra) ? bpi->extra->num_labels : 0);
 	written = true;
 
 out:
@@ -1416,7 +1427,9 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr)
 		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 ? bpi->uptime : monotime(NULL),
+			    (bpi && bpi->extra) ? bpi->extra->label : NULL,
+			    (bpi && bpi->extra) ? bpi->extra->num_labels : 0);
 		written = true;
 	}
 
@@ -1428,9 +1441,10 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr)
 			if (adjin->peer == peer)
 				break;
 		}
+		/* TODO: set label here when adjin supports labels */
 		bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE,
 			    &bqe->p, prd, adjin ? adjin->attr : NULL, afi, safi,
-			    adjin ? adjin->uptime : monotime(NULL));
+			    adjin ? adjin->uptime : monotime(NULL), NULL, 0);
 		written = true;
 	}
 
diff --git a/tests/topotests/bgp_bmp/r1/bgpd.conf b/tests/topotests/bgp_bmp/r1/bgpd.conf
index 69acf6e750..24505de4a8 100644
--- a/tests/topotests/bgp_bmp/r1/bgpd.conf
+++ b/tests/topotests/bgp_bmp/r1/bgpd.conf
@@ -6,9 +6,17 @@ router bgp 65501
  neighbor 192:168::2 remote-as 65502
 !
  bmp targets bmp1
-  bmp connect 192.0.178.10 port 1789 min-retry 100 max-retry 10000
+  bmp connect 192.0.2.10 port 1789 min-retry 100 max-retry 10000
  exit
 !
+ address-family ipv4 vpn
+  neighbor 192.168.0.2 activate
+  neighbor 192.168.0.2 soft-reconfiguration inbound
+ exit-address-family
+ address-family ipv6 vpn
+  neighbor 192:168::2 activate
+  neighbor 192:168::2 soft-reconfiguration inbound
+ exit-address-family
  address-family ipv4 unicast
   neighbor 192.168.0.2 activate
   neighbor 192.168.0.2 soft-reconfiguration inbound
@@ -20,3 +28,21 @@ router bgp 65501
   neighbor 192:168::2 soft-reconfiguration inbound
  exit-address-family
 !
+router bgp 65502 vrf vrf1
+ bgp router_id 192.168.0.1
+ bgp log-neighbor-changes
+ address-family ipv4 unicast
+  label vpn export 101
+  rd vpn export 444:1
+  rt vpn both 52:100
+  export vpn
+  import vpn
+  exit-address-family
+ address-family ipv6 unicast
+  label vpn export 103
+  rd vpn export 555:1
+  rt vpn both 54:200
+  export vpn
+  import vpn
+  exit-address-family
+exit
diff --git a/tests/topotests/bgp_bmp/r1/zebra.conf b/tests/topotests/bgp_bmp/r1/zebra.conf
index 6a25a6f4c2..0b523c9e18 100644
--- a/tests/topotests/bgp_bmp/r1/zebra.conf
+++ b/tests/topotests/bgp_bmp/r1/zebra.conf
@@ -1,5 +1,5 @@
 interface r1-eth0
- ip address 192.0.178.1/24
+ ip address 192.0.2.1/24
 !
 interface r1-eth1
  ip address 192.168.0.1/24
diff --git a/tests/topotests/bgp_bmp/r2/bgpd.conf b/tests/topotests/bgp_bmp/r2/bgpd.conf
index 7c8255a175..40e2cd5bbc 100644
--- a/tests/topotests/bgp_bmp/r2/bgpd.conf
+++ b/tests/topotests/bgp_bmp/r2/bgpd.conf
@@ -12,8 +12,35 @@ router bgp 65502
   redistribute connected
  exit-address-family
 !
+ address-family ipv4 vpn
+  neighbor 192.168.0.1 activate
+ exit-address-family
+!
+ address-family ipv6 vpn
+  neighbor 192:168::1 activate
+ exit-address-family
+!
  address-family ipv6 unicast
   neighbor 192:168::1 activate
   redistribute connected
  exit-address-family
 !
+router bgp 65502 vrf vrf1
+ bgp router-id 192.168.0.2
+ bgp log-neighbor-changes
+ no bgp network import-check
+ address-family ipv4 unicast
+  label vpn export 102
+  rd vpn export 444:2
+  rt vpn both 52:100
+  export vpn
+  import vpn
+ exit-address-family
+ address-family ipv6 unicast
+  label vpn export 105
+  rd vpn export 555:2
+  rt vpn both 54:200
+  export vpn
+  import vpn
+ exit-address-family
+exit
diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp.py b/tests/topotests/bgp_bmp/test_bgp_bmp.py
index 250f1cb90d..02de805068 100644
--- a/tests/topotests/bgp_bmp/test_bgp_bmp.py
+++ b/tests/topotests/bgp_bmp/test_bgp_bmp.py
@@ -55,7 +55,7 @@ LOC_RIB = "loc-rib"
 def build_topo(tgen):
     tgen.add_router("r1")
     tgen.add_router("r2")
-    tgen.add_bmp_server("bmp1", ip="192.0.178.10", defaultRoute="via 192.0.178.1")
+    tgen.add_bmp_server("bmp1", ip="192.0.2.10", defaultRoute="via 192.0.2.1")
 
     switch = tgen.add_switch("s1")
     switch.add_link(tgen.gears["r1"])
@@ -81,8 +81,8 @@ def setup_module(mod):
     tgen.start_router()
 
     logger.info("starting BMP servers")
-    for _, server in tgen.get_bmp_servers().items():
-        server.start()
+    for bmp_name, server in tgen.get_bmp_servers().items():
+        server.start(log_file=os.path.join(tgen.logdir, bmp_name, "bmp.log"))
 
 
 def teardown_module(_mod):
@@ -105,7 +105,9 @@ def get_bmp_messages():
     """
     messages = []
     tgen = get_topogen()
-    text_output = tgen.gears["bmp1"].run("cat /var/log/bmp.log")
+    text_output = tgen.gears["bmp1"].run(
+        "cat {}".format(os.path.join(tgen.logdir, "bmp1", "bmp.log"))
+    )
 
     for m in text_output.splitlines():
         # some output in the bash can break the message decoding
@@ -121,7 +123,7 @@ def get_bmp_messages():
     return messages
 
 
-def check_for_prefixes(expected_prefixes, bmp_log_type, policy):
+def check_for_prefixes(expected_prefixes, bmp_log_type, policy, labels=None):
     """
     Check for the presence of the given prefixes in the BMP server logs with
     the given message type and the set policy.
@@ -140,6 +142,12 @@ def check_for_prefixes(expected_prefixes, bmp_log_type, policy):
         and "bmp_log_type" in m.keys()
         and m["bmp_log_type"] == bmp_log_type
         and m["policy"] == policy
+        and (
+            labels is None
+            or (
+                m["ip_prefix"] in labels.keys() and m["label"] == labels[m["ip_prefix"]]
+            )
+        )
     ]
 
     # check for prefixes
@@ -174,7 +182,7 @@ def configure_prefixes(tgen, node, asn, safi, prefixes, vrf=None, update=True):
     Configure the bgp prefixes.
     """
     withdraw = "no " if not update else ""
-    vrf = " vrf {}" if vrf else ""
+    vrf = " vrf {}".format(vrf) if vrf else ""
     for p in prefixes:
         ip = ip_network(p)
         cmd = [
@@ -216,6 +224,45 @@ def unicast_prefixes(policy):
     assert success, "Checking the withdrawed prefixes has been failed !."
 
 
+def vpn_prefixes(policy):
+    """
+    Setup the BMP  monitor policy, Add and withdraw ipv4/v6 prefixes.
+    Check if the previous actions are logged in the BMP server with the right
+    message type and the right policy.
+    """
+    tgen = get_topogen()
+    set_bmp_policy(tgen, "r1", 65501, "bmp1", "vpn", policy)
+
+    prefixes = ["172.31.10.1/32", "2001::2222/128"]
+
+    if policy == PRE_POLICY:
+        # labels are not yet supported in adj-RIB-in. Do not test for the moment
+        labels = None
+    else:
+        # "label vpn export" value in r2/bgpd.conf
+        labels = {
+            "172.31.10.1/32": 102,
+            "2001::2222/128": 105,
+        }
+
+    # add prefixes
+    configure_prefixes(tgen, "r2", 65502, "unicast", prefixes, vrf="vrf1")
+
+    logger.info("checking for updated prefixes")
+    # check
+    test_func = partial(check_for_prefixes, prefixes, "update", policy, labels=labels)
+    success, _ = topotest.run_and_expect(test_func, True, wait=0.5)
+    assert success, "Checking the updated prefixes has been failed !."
+
+    # withdraw prefixes
+    configure_prefixes(tgen, "r2", 65502, "unicast", prefixes, vrf="vrf1", update=False)
+    logger.info("checking for withdrawed prefixes")
+    # check
+    test_func = partial(check_for_prefixes, prefixes, "withdraw", policy)
+    success, _ = topotest.run_and_expect(test_func, True, wait=0.5)
+    assert success, "Checking the withdrawed prefixes has been failed !."
+
+
 def test_bmp_server_logging():
     """
     Assert the logging of the bmp server.
@@ -223,7 +270,9 @@ def test_bmp_server_logging():
 
     def check_for_log_file():
         tgen = get_topogen()
-        output = tgen.gears["bmp1"].run("ls /var/log/")
+        output = tgen.gears["bmp1"].run(
+            "ls {}".format(os.path.join(tgen.logdir, "bmp1"))
+        )
         if "bmp.log" not in output:
             return False
         return True
@@ -244,6 +293,16 @@ def test_bmp_bgp_unicast():
     unicast_prefixes(LOC_RIB)
 
 
+def test_bmp_bgp_vpn():
+    # check for the prefixes in the BMP server logging file
+    logger.info("***** VPN prefixes pre-policy logging *****")
+    vpn_prefixes(PRE_POLICY)
+    logger.info("***** VPN prefixes post-policy logging *****")
+    vpn_prefixes(POST_POLICY)
+    logger.info("***** VPN prefixes loc-rib logging *****")
+    vpn_prefixes(LOC_RIB)
+
+
 if __name__ == "__main__":
     args = ["-s"] + sys.argv[1:]
     sys.exit(pytest.main(args))
diff --git a/tests/topotests/lib/bmp_collector/bmp.py b/tests/topotests/lib/bmp_collector/bmp.py
index 57f642aa0e..237decdd5e 100644
--- a/tests/topotests/lib/bmp_collector/bmp.py
+++ b/tests/topotests/lib/bmp_collector/bmp.py
@@ -33,30 +33,33 @@ IS_FILTERED = 1 << 7
 if not os.path.exists(LOG_DIR):
     os.makedirs(LOG_DIR)
 
+
 def bin2str_ipaddress(ip_bytes, is_ipv6=False):
     if is_ipv6:
         return str(ipaddress.IPv6Address(ip_bytes))
     return str(ipaddress.IPv4Address(ip_bytes[-4:]))
 
-def log2file(logs):
+
+def log2file(logs, log_file):
     """
     XXX: extract the useful information and save it in a flat dictionnary
     """
-    with open(LOG_FILE, 'a') as f:
+    with open(log_file, "a") as f:
         f.write(json.dumps(logs) + "\n")
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 class BMPCodes:
     """
     XXX: complete the list, provide RFCs.
     """
+
     VERSION = 0x3
 
     BMP_MSG_TYPE_ROUTE_MONITORING = 0x00
     BMP_MSG_TYPE_STATISTICS_REPORT = 0x01
-    BMP_MSG_TYPE_PEER_DOWN_NOTIFICATION =  0x02
-    BMP_MSG_TYPE_PEER_UP_NOTIFICATION =  0x03
+    BMP_MSG_TYPE_PEER_DOWN_NOTIFICATION = 0x02
+    BMP_MSG_TYPE_PEER_UP_NOTIFICATION = 0x03
     BMP_MSG_TYPE_INITIATION = 0x04
     BMP_MSG_TYPE_TERMINATION = 0x05
     BMP_MSG_TYPE_ROUTE_MIRRORING = 0x06
@@ -107,15 +110,15 @@ class BMPCodes:
 
     # peer down reason code
     BMP_PEER_DOWN_LOCAL_NOTIFY = 0x01
-    BMP_PEER_DOWN_LOCAL_NO_NOTIFY = 0X02
-    BMP_PEER_DOWN_REMOTE_NOTIFY = 0X03
-    BMP_PEER_DOWN_REMOTE_NO_NOTIFY = 0X04
+    BMP_PEER_DOWN_LOCAL_NO_NOTIFY = 0x02
+    BMP_PEER_DOWN_REMOTE_NOTIFY = 0x03
+    BMP_PEER_DOWN_REMOTE_NO_NOTIFY = 0x04
     BMP_PEER_DOWN_INFO_NO_LONGER = 0x05
-    BMP_PEER_DOWN_SYSTEM_CLOSED = 0X06
+    BMP_PEER_DOWN_SYSTEM_CLOSED = 0x06
 
     # termincation message types
     BMP_TERM_TYPE_STRING = 0x00
-    BMP_TERM_TYPE_REASON = 0X01
+    BMP_TERM_TYPE_REASON = 0x01
 
     # termination reason code
     BMP_TERM_REASON_ADMIN_CLOSE = 0x00
@@ -126,31 +129,32 @@ class BMPCodes:
 
     # policy route tlv
     BMP_ROUTE_POLICY_TLV_VRF = 0x00
-    BMP_ROUTE_POLICY_TLV_POLICY= 0x01
+    BMP_ROUTE_POLICY_TLV_POLICY = 0x01
     BMP_ROUTE_POLICY_TLV_PRE_POLICY = 0x02
     BMP_ROUTE_POLICY_TLV_POST_POLICY = 0x03
     BMP_ROUTE_POLICY_TLV_STRING = 0x04
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 class BMPMsg:
     """
     XXX: should we move register_msg_type and look_msg_type
     to generic Type class.
     """
+
     TYPES = {}
     UNKNOWN_TYPE = None
-    HDR_STR = '!BIB'
+    HDR_STR = "!BIB"
     MIN_LEN = struct.calcsize(HDR_STR)
     TYPES_STR = {
-        BMPCodes.BMP_MSG_TYPE_INITIATION: 'initiation',
-        BMPCodes.BMP_MSG_TYPE_PEER_DOWN_NOTIFICATION: 'peer down notification',
-        BMPCodes.BMP_MSG_TYPE_PEER_UP_NOTIFICATION: 'peer up notification',
-        BMPCodes.BMP_MSG_TYPE_ROUTE_MONITORING: 'route monitoring',
-        BMPCodes.BMP_MSG_TYPE_STATISTICS_REPORT: 'statistics report',
-        BMPCodes.BMP_MSG_TYPE_TERMINATION: 'termination',
-        BMPCodes.BMP_MSG_TYPE_ROUTE_MIRRORING: 'route mirroring',
-        BMPCodes.BMP_MSG_TYPE_ROUTE_POLICY: 'route policy',
+        BMPCodes.BMP_MSG_TYPE_INITIATION: "initiation",
+        BMPCodes.BMP_MSG_TYPE_PEER_DOWN_NOTIFICATION: "peer down notification",
+        BMPCodes.BMP_MSG_TYPE_PEER_UP_NOTIFICATION: "peer up notification",
+        BMPCodes.BMP_MSG_TYPE_ROUTE_MONITORING: "route monitoring",
+        BMPCodes.BMP_MSG_TYPE_STATISTICS_REPORT: "statistics report",
+        BMPCodes.BMP_MSG_TYPE_TERMINATION: "termination",
+        BMPCodes.BMP_MSG_TYPE_ROUTE_MIRRORING: "route mirroring",
+        BMPCodes.BMP_MSG_TYPE_ROUTE_POLICY: "route policy",
     }
 
     @classmethod
@@ -158,6 +162,7 @@ class BMPMsg:
         def _register_type(subcls):
             cls.TYPES[msgtype] = subcls
             return subcls
+
         return _register_type
 
     @classmethod
@@ -179,15 +184,15 @@ class BMPMsg:
         if len(data) < cls.MIN_LEN:
             pass
         else:
-            _version, _len, _type = struct.unpack(cls.HDR_STR, data[0:cls.MIN_LEN])
+            _version, _len, _type = struct.unpack(cls.HDR_STR, data[0 : cls.MIN_LEN])
             return _version, _len, _type
 
     @classmethod
-    def dissect(cls, data):
+    def dissect(cls, data, log_file=None):
         global SEQ
         version, msglen, msgtype = cls.dissect_header(data)
 
-        msg_data = data[cls.MIN_LEN:msglen]
+        msg_data = data[cls.MIN_LEN : msglen]
         data = data[msglen:]
 
         if version != BMPCodes.VERSION:
@@ -202,13 +207,13 @@ class BMPMsg:
         msg_cls.MSG_LEN = msglen - cls.MIN_LEN
         logs = msg_cls.dissect(msg_data)
         logs["seq"] = SEQ
-        log2file(logs)
+        log2file(logs, log_file if log_file else LOG_FILE)
         SEQ += 1
 
         return data
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 class BMPPerPeerMessage:
     """
     0 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8
@@ -229,30 +234,33 @@ class BMPPerPeerMessage:
     |                    Timestamp (microseconds)                   |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     """
-    PEER_UNPACK_STR = '!BB8s16sI4sII'
+
+    PEER_UNPACK_STR = "!BB8s16sI4sII"
     PEER_TYPE_STR = {
-        BMPCodes.BMP_PEER_GLOBAL_INSTANCE: 'global instance',
-        BMPCodes.BMP_PEER_RD_INSTANCE: 'route distinguisher instance',
-        BMPCodes.BMP_PEER_LOCAL_INSTANCE: 'local instance',
-        BMPCodes.BMP_PEER_LOC_RIB_INSTANCE: 'loc-rib instance',
+        BMPCodes.BMP_PEER_GLOBAL_INSTANCE: "global instance",
+        BMPCodes.BMP_PEER_RD_INSTANCE: "route distinguisher instance",
+        BMPCodes.BMP_PEER_LOCAL_INSTANCE: "local instance",
+        BMPCodes.BMP_PEER_LOC_RIB_INSTANCE: "loc-rib instance",
     }
 
     @classmethod
     def dissect(cls, data):
-        (peer_type,
-         peer_flags,
-         peer_distinguisher,
-         peer_address,
-         peer_asn,
-         peer_bgp_id,
-         timestamp_secs,
-         timestamp_microsecs) = struct.unpack_from(cls.PEER_UNPACK_STR, data)
-
-        msg = {'peer_type': cls.PEER_TYPE_STR[peer_type]}
+        (
+            peer_type,
+            peer_flags,
+            peer_distinguisher,
+            peer_address,
+            peer_asn,
+            peer_bgp_id,
+            timestamp_secs,
+            timestamp_microsecs,
+        ) = struct.unpack_from(cls.PEER_UNPACK_STR, data)
+
+        msg = {"peer_type": cls.PEER_TYPE_STR[peer_type]}
 
         if peer_type == 0x03:
-            msg['is_filtered'] = bool(peer_flags & IS_FILTERED)
-            msg['policy'] = 'loc-rib'
+            msg["is_filtered"] = bool(peer_flags & IS_FILTERED)
+            msg["policy"] = "loc-rib"
         else:
             # peer_flags = 0x0000 0000
             # ipv6, post-policy, as-path, adj-rib-out, reserverdx4
@@ -260,29 +268,29 @@ class BMPPerPeerMessage:
             is_as_path = bool(peer_flags & IS_AS_PATH)
             is_post_policy = bool(peer_flags & IS_POST_POLICY)
             is_ipv6 = bool(peer_flags & IS_IPV6)
-            msg['policy'] = 'post-policy' if is_post_policy else 'pre-policy'
-            msg['ipv6'] = is_ipv6
-            msg['peer_ip'] = bin2str_ipaddress(peer_address, is_ipv6)
-
+            msg["policy"] = "post-policy" if is_post_policy else "pre-policy"
+            msg["ipv6"] = is_ipv6
+            msg["peer_ip"] = bin2str_ipaddress(peer_address, is_ipv6)
 
         peer_bgp_id = bin2str_ipaddress(peer_bgp_id)
-        timestamp = float(timestamp_secs) + timestamp_microsecs * (10 ** -6)
-
-        data = data[struct.calcsize(cls.PEER_UNPACK_STR):]
-        msg.update({
-            'peer_distinguisher': str(RouteDistinguisher(peer_distinguisher)),
-            'peer_asn': peer_asn,
-            'peer_bgp_id': peer_bgp_id,
-            'timestamp': str(datetime.datetime.fromtimestamp(timestamp)),
-        })
+        timestamp = float(timestamp_secs) + timestamp_microsecs * (10**-6)
+
+        data = data[struct.calcsize(cls.PEER_UNPACK_STR) :]
+        msg.update(
+            {
+                "peer_distinguisher": str(RouteDistinguisher(peer_distinguisher)),
+                "peer_asn": peer_asn,
+                "peer_bgp_id": peer_bgp_id,
+                "timestamp": str(datetime.datetime.fromtimestamp(timestamp)),
+            }
+        )
 
         return data, msg
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 @BMPMsg.register_msg_type(BMPCodes.BMP_MSG_TYPE_ROUTE_MONITORING)
 class BMPRouteMonitoring(BMPPerPeerMessage):
-
     @classmethod
     def dissect(cls, data):
         data, peer_msg = super().dissect(data)
@@ -290,7 +298,7 @@ class BMPRouteMonitoring(BMPPerPeerMessage):
         return {**peer_msg, **update_msg}
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 class BMPStatisticsReport:
     """
     0 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8
@@ -303,10 +311,11 @@ class BMPStatisticsReport:
     ~                                                               ~
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     """
+
     pass
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 class BMPPeerDownNotification:
     """
     0 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8
@@ -316,10 +325,11 @@ class BMPPeerDownNotification:
     |            Data (present if Reason = 1, 2 or 3)               |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     """
+
     pass
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 @BMPMsg.register_msg_type(BMPCodes.BMP_MSG_TYPE_PEER_UP_NOTIFICATION)
 class BMPPeerUpNotification(BMPPerPeerMessage):
     """
@@ -336,7 +346,8 @@ class BMPPeerUpNotification(BMPPerPeerMessage):
     ~                                                               ~
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     """
-    UNPACK_STR = '!16sHH'
+
+    UNPACK_STR = "!16sHH"
     MIN_LEN = struct.calcsize(UNPACK_STR)
     MSG_LEN = None
 
@@ -344,16 +355,14 @@ class BMPPeerUpNotification(BMPPerPeerMessage):
     def dissect(cls, data):
         data, peer_msg = super().dissect(data)
 
-        (local_addr,
-         local_port,
-         remote_port) = struct.unpack_from(cls.UNPACK_STR, data)
+        (local_addr, local_port, remote_port) = struct.unpack_from(cls.UNPACK_STR, data)
 
         msg = {
             **peer_msg,
             **{
-                'local_ip': bin2str_ipaddress(local_addr, peer_msg.get('ipv6')),
-                'local_port': int(local_port),
-                'remote_port': int(remote_port),
+                "local_ip": bin2str_ipaddress(local_addr, peer_msg.get("ipv6")),
+                "local_port": int(local_port),
+                "remote_port": int(remote_port),
             },
         }
 
@@ -362,7 +371,7 @@ class BMPPeerUpNotification(BMPPerPeerMessage):
         return msg
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 @BMPMsg.register_msg_type(BMPCodes.BMP_MSG_TYPE_INITIATION)
 class BMPInitiation:
     """
@@ -374,30 +383,31 @@ class BMPInitiation:
     ~                                                               ~
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     """
-    TLV_STR = '!HH'
+
+    TLV_STR = "!HH"
     MIN_LEN = struct.calcsize(TLV_STR)
     FIELD_TO_STR = {
-        BMPCodes.BMP_INIT_INFO_STRING: 'information',
-        BMPCodes.BMP_INIT_ADMIN_LABEL: 'admin_label',
-        BMPCodes.BMP_INIT_SYSTEM_DESCRIPTION: 'system_description',
-        BMPCodes.BMP_INIT_SYSTEM_NAME: 'system_name',
-        BMPCodes.BMP_INIT_VRF_TABLE_NAME: 'vrf_table_name',
+        BMPCodes.BMP_INIT_INFO_STRING: "information",
+        BMPCodes.BMP_INIT_ADMIN_LABEL: "admin_label",
+        BMPCodes.BMP_INIT_SYSTEM_DESCRIPTION: "system_description",
+        BMPCodes.BMP_INIT_SYSTEM_NAME: "system_name",
+        BMPCodes.BMP_INIT_VRF_TABLE_NAME: "vrf_table_name",
     }
 
     @classmethod
     def dissect(cls, data):
         msg = {}
         while len(data) > cls.MIN_LEN:
-            _type, _len = struct.unpack_from(cls.TLV_STR, data[0:cls.MIN_LEN])
-            _value = data[cls.MIN_LEN: cls.MIN_LEN + _len].decode()
+            _type, _len = struct.unpack_from(cls.TLV_STR, data[0 : cls.MIN_LEN])
+            _value = data[cls.MIN_LEN : cls.MIN_LEN + _len].decode()
 
             msg[cls.FIELD_TO_STR[_type]] = _value
-            data = data[cls.MIN_LEN + _len:]
+            data = data[cls.MIN_LEN + _len :]
 
         return msg
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 class BMPTermination:
     """
     0 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8
@@ -408,14 +418,15 @@ class BMPTermination:
     ~                                                               ~
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     """
+
     pass
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 class BMPRouteMirroring:
     pass
 
 
-#------------------------------------------------------------------------------
+# ------------------------------------------------------------------------------
 class BMPRoutePolicy:
     pass
diff --git a/tests/topotests/lib/bmp_collector/bmpserver b/tests/topotests/lib/bmp_collector/bmpserver
index 25b4a52c5e..5257df7530 100755
--- a/tests/topotests/lib/bmp_collector/bmpserver
+++ b/tests/topotests/lib/bmp_collector/bmpserver
@@ -16,10 +16,12 @@ BGP_MAX_SIZE = 4096
 parser = argparse.ArgumentParser()
 parser.add_argument("-a", "--address", type=str, default="0.0.0.0")
 parser.add_argument("-p", "--port", type=int, default=1789)
+parser.add_argument("-l", "--logfile", type=str, default="/var/log/bmp.log")
 
 def main():
     args = parser.parse_args()
     ADDRESS, PORT = args.address, args.port
+    LOG_FILE = args.logfile
 
     with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
         s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
@@ -31,7 +33,7 @@ def main():
             while True:
                 data = connection.recv(BGP_MAX_SIZE)
                 while len(data) > BMPMsg.MIN_LEN:
-                    data = BMPMsg.dissect(data)
+                    data = BMPMsg.dissect(data, log_file=LOG_FILE)
         except Exception as e:
             # XXX: do something
             pass
diff --git a/tests/topotests/lib/topogen.py b/tests/topotests/lib/topogen.py
index 155d2d0986..78d1b1c42c 100644
--- a/tests/topotests/lib/topogen.py
+++ b/tests/topotests/lib/topogen.py
@@ -94,7 +94,9 @@ def get_exabgp_cmd(commander=None):
             return False
         version = m.group(1)
         if topotest.version_cmp(version, "4.2.11") < 0:
-            logging.debug("found exabgp version < 4.2.11 in %s will keep looking", exacmd)
+            logging.debug(
+                "found exabgp version < 4.2.11 in %s will keep looking", exacmd
+            )
             return False
         logger.info("Using ExaBGP version %s in %s", version, exacmd)
         return True
@@ -1253,9 +1255,12 @@ class TopoBMPCollector(TopoHost):
         gear += " TopoBMPCollector<>".format()
         return gear
 
-    def start(self):
+    def start(self, log_file=None):
+        log_arg = "-l {}".format(log_file) if log_file else ""
         self.run(
-            "{}/bmp_collector/bmpserver -a {} -p {}&".format(CWD, self.ip, self.port),
+            "{}/bmp_collector/bmpserver -a {} -p {} {}&".format(
+                CWD, self.ip, self.port, log_arg
+            ),
             stdout=None,
         )