BMP Module initial implementation #1130

sergeymatov · 2025-12-11T13:00:46Z

this is WIP, desc will be added later

config/src/internal/routing/bgp.rs

routing/src/bmp/handler.rs

routing/src/frr/renderer/bgp.rs

routing/src/bmp/bmp_render.rs

dataplane/src/main.rs

args/src/lib.rs

mgmt/src/processor/proc.rs

routing/src/bmp/mod.rs

routing/src/frr/renderer/bgp.rs

Fredi-raspall

I like the approach and implementation!
I am going to request changes because I'd like to review this again once it is no longer a draft and some of the questions get clarified.
Thanks!

Signed-off-by: Sergey Matov <sergey.matov@githedgehog.com>

daniel-noland

just posting initial thoughts

daniel-noland · 2026-01-08T23:45:17Z

routing/src/bmp/server.rs

+                }
+            }
+            active += 1;
+            let cfg = self.cfg.clone();


i think this clone is wrong

With this strategy you won't change configs for established sessions no?

daniel-noland · 2026-01-08T23:45:51Z

config/src/internal/routing/bgp.rs

+    /// Name for `bmp targets <name>`
+    pub target_name: String,
+    /// Collector host/IP in `bmp connect`
+    pub connect_host: String,


Shouldn't this be IpAddr?

Also, shouldn't this be called collector?

daniel-noland · 2026-01-08T23:46:20Z

config/src/internal/routing/bgp.rs

+#[derive(Clone, Debug)]
+pub struct BmpOptions {
+    /// Name for `bmp targets <name>`
+    pub target_name: String,


Ideally we would enforce validity at the type level. What are the constraints on this string?

Also, I assume this would be called target

daniel-noland · 2026-01-08T23:46:46Z

config/src/internal/routing/bgp.rs

+    /// Collector host/IP in `bmp connect`
+    pub connect_host: String,
+    /// Collector TCP port
+    pub port: u16,


We have a type for TCP port in net

daniel-noland · 2026-01-08T23:47:20Z

config/src/internal/routing/bgp.rs

+#[derive(Clone, Debug)]
+pub enum BmpSource {
+    Address(IpAddr),
+    Interface(String),


Is this an interface name? If so, we should use the InterfaceName type

daniel-noland · 2026-01-08T23:56:27Z

routing/src/bmp/server.rs

+        Self {
+            bind_addr: "0.0.0.0:5000".parse().unwrap(),
+            tcp_nodelay: true,
+            tcp_recv_buf: Some(1 << 20), // 1 MiB


make this a constant on the type rather than hard code it in Default::default

daniel-noland · 2026-01-08T23:57:48Z

routing/src/bmp/server.rs

+    handler: &H,
+) -> Result<()> {
+    if cfg.tcp_nodelay {
+        let _ = sock.set_nodelay(true);


why eat the error?

daniel-noland · 2026-01-08T23:57:55Z

routing/src/bmp/server.rs

+        let _ = sock.set_nodelay(true);
+    }
+    if let Some(sz) = cfg.tcp_recv_buf {
+        let _ = sock.set_recv_buffer_size(sz);


why eat the error?

daniel-noland · 2026-01-08T23:58:21Z

routing/src/bmp/server.rs

+    tokio::pin!(reader);
+
+    // Use a scratch buffer for zero-copy clones if needed
+    let mut _scratch = BytesMut::new();


daniel-noland · 2026-01-09T00:00:11Z

routing/src/bmp/handler.rs

+    async fn on_message(&self, peer: std::net::SocketAddr, msg: BmpMessage);
+
+    /// Called when a connection terminates (EOF / error).
+    async fn on_disconnect(&self, peer: std::net::SocketAddr, reason: &str) {


I recommend making this a sync function so this can be called from a Drop trait

sergeymatov requested a review from a team as a code owner December 11, 2025 13:00

sergeymatov requested review from Fredi-raspall and removed request for a team December 11, 2025 13:00

sergeymatov marked this pull request as draft December 11, 2025 13:00

sergeymatov force-pushed the pr/smatov/bmp-frr-config branch from 43e8d27 to 6044efa Compare December 11, 2025 13:01