track bgp session state in dataplane by Fredi-raspall · Pull Request #1645 · githedgehog/dataplane

Fredi-raspall · 2026-07-18T15:54:21Z

~~This goes on top of #1643~~

Log (info) events for BGP session changes

2026-07-18T15:40:24.800367Z  INFO                 routerIO dataplane_routing::router::ctl: 284: BGP session with 7.0.0.2 (ASN 65002) changed Idle -> Established

Generate events so that all changes get recorded independently of logging and that these include:

reason for reset / down
downtime

dataplane(✔)# show router events
 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ROUTER_EVENTS ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 generated: 13 stored: 13 capacity: 2000

 0    2026-07-19T 12:08:40: Started!
 1    2026-07-19T 12:08:40: Connected to frr-agent
 2    2026-07-19T 12:08:46: Router config request received for generation 1
 3    2026-07-19T 12:08:46: Router config request for generation 1 SUCCEEDED
 4    2026-07-19T 12:08:46: Requested FRR configuration for generation 1
 5    2026-07-19T 12:08:46: CPI status changed to Connected
 6    2026-07-19T 12:08:47: FRR configuration for generation 1 SUCCEEDED
 7    2026-07-19T 12:08:57: Status of BGP peer 10.0.0.13 (id:7.0.0.2 ASN:65002) changed: Unset -> Established
 8    2026-07-19T 12:08:57: Status of BGP peer 10.0.1.13 (id:7.0.0.6 ASN:65006) changed: Unset -> Established
 9    2026-07-19T 12:09:08: Status of BGP peer 10.0.0.13 (id:7.0.0.2 ASN:65002) changed: Established -> Idle reason: Remote system closed
 10   2026-07-19T 12:09:10: Status of BGP peer 10.0.0.13 (id:7.0.0.2 ASN:65002) changed: Idle -> Established (downtime: 00:00:02)
 11   2026-07-19T 12:09:50: Status of BGP peer 10.0.1.13 (id:7.0.0.6 ASN:65006) changed: Established -> Idle reason: Local system closed
 12   2026-07-19T 12:10:01: Status of BGP peer 10.0.1.13 (id:7.0.0.6 ASN:65006) changed: Idle -> Established (downtime: 00:00:11)

coderabbitai · 2026-07-18T15:54:33Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a95d0979-d8d9-41ba-9ff8-4816a608dde0

📥 Commits

Reviewing files that changed from the base of the PR and between 0ca0ef1 and c7fc043.

📒 Files selected for processing (13)

config/src/converters/k8s/status/bgp.rs
config/src/internal/status/mod.rs
dataplane/src/runtime.rs
mgmt/src/processor/launch.rs
mgmt/src/processor/proc.rs
routing/src/bmp/bmp_render.rs
routing/src/bmp/handler.rs
routing/src/bmp/mod.rs
routing/src/cli/display.rs
routing/src/cli/mod.rs
routing/src/frr/test.rs
routing/src/router/ctl.rs
routing/src/router/revent.rs

🚧 Files skipped from review as they are similar to previous changes (11)

routing/src/frr/test.rs
mgmt/src/processor/launch.rs
routing/src/cli/mod.rs
routing/src/bmp/mod.rs
config/src/converters/k8s/status/bgp.rs
config/src/internal/status/mod.rs
routing/src/cli/display.rs
mgmt/src/processor/proc.rs
dataplane/src/runtime.rs
routing/src/bmp/handler.rs
routing/src/bmp/bmp_render.rs

📝 Walkthrough

Walkthrough

Changes

BGP neighbor status now preserves optional reset metadata, records session transitions from BMP notifications, sends those transitions through the router control channel, and renders them as router events. BMP startup receives the router control sender after router initialization, with related callers using immutable access.

BGP neighbor event propagation

Layer / File(s)	Summary
Status and event contracts `config/src/internal/status/mod.rs`, `config/src/converters/k8s/status/bgp.rs`, `routing/src/bmp/bmp_render.rs`, `routing/src/cli/*`, `routing/src/router/revent.rs`	Neighbor reset fields are optional, session states implement display formatting, BGP events carry transition metadata, and router events render reasons or downtime.
BMP event generation and routing `routing/src/bmp/*`, `routing/src/router/ctl.rs`	BMP peer-up and peer-down handling produces events for session changes and forwards them through router control messages into router events.
Runtime wiring and shared control access `dataplane/src/runtime.rs`, `mgmt/src/processor/*`, `routing/src/frr/test.rs`	BMP startup is moved after router initialization and receives its control sender; related control-sender callers use immutable borrows.

Possibly related PRs

githedgehog/dataplane#1607: Refactors the router control-message sending path used by the new BGP neighbor status message.
githedgehog/dataplane#1643: Updates BMP server startup and spawn wiring that this change extends with rtr_ctl.
githedgehog/dataplane#1646: Modifies the same BGP neighbor status model and BMP neighbor-event pipeline.

Suggested reviewers: sergeymatov, copilot

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: tracking BGP session state in the dataplane.
Description check	✅ Passed	The description matches the changeset by describing BGP session change logging and event generation with reason and downtime.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Pull request overview

Adds BGP neighbor session state change tracking to the dataplane by propagating BMP peer up/down notifications into the router control loop, emitting both info logs and persistent router events.

Changes:

Introduce BgpNeighEvent generation from BMP PeerUp/PeerDown and forward it to the router via RouterCtlSender.
Add a new RouterEvent::BgpNeighStateChange variant and expand the router event log capacity (1000 → 2000).
Make RouterCtlSender APIs take &self (not &mut self) and thread the sender into the BMP server/handler.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
routing/src/router/revent.rs	Adds BGP neighbor state-change router events + increases event log capacity.
routing/src/router/ctl.rs	Adds ctl message for BGP neighbor status changes; makes sender methods take `&self`.
routing/src/frr/test.rs	Updates test to reflect `RouterCtlSender` no longer requiring `mut`.
routing/src/bmp/mod.rs	Threads `RouterCtlSender` into BMP server spawn path.
routing/src/bmp/handler.rs	Calls BMP render logic with router ctl sender to emit events.
routing/src/bmp/bmp_render.rs	Generates BGP neighbor state-change events from BMP messages.
mgmt/src/processor/proc.rs	Updates call sites to use `&RouterCtlSender` APIs.
mgmt/src/processor/launch.rs	Updates interface-event notifier to use non-mutable router ctl sender.
dataplane/src/runtime.rs	Starts BMP server after router so BMP handler can send router events.
config/src/internal/status/mod.rs	Adds `Display` for BGP session state; makes `last_reset_reason` optional.
config/src/converters/k8s/status/bgp.rs	Adjusts k8s status conversion/tests for optional `last_reset_reason`.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@routing/src/bmp/bmp_render.rs`:
- Around line 306-309: Update the PeerDown handling around
set_neighbor_session_state so last_reset_reason and last_reset_time are assigned
only when the previous neighbor state is not Idle. Preserve the existing
duplicate-message behavior and connection counter, while retaining the original
downtime start across repeated PeerDown notifications.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b77fc23b-f416-4325-8bc4-1d58b261a5c7

📥 Commits

Reviewing files that changed from the base of the PR and between c3cd8ed and 0ca0ef1.

📒 Files selected for processing (13)

config/src/converters/k8s/status/bgp.rs
config/src/internal/status/mod.rs
dataplane/src/runtime.rs
mgmt/src/processor/launch.rs
mgmt/src/processor/proc.rs
routing/src/bmp/bmp_render.rs
routing/src/bmp/handler.rs
routing/src/bmp/mod.rs
routing/src/cli/display.rs
routing/src/cli/mod.rs
routing/src/frr/test.rs
routing/src/router/ctl.rs
routing/src/router/revent.rs

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

BgpNeighEvent represents a status change of a BGP session. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Propagate a RouterCtlSender to the BMP server so that it can send events on BGP peer changes. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Mutability is not required by most methods of RouterCtlSender. Replace &mut by &self. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

We were not populating the last reset reason when parsing a peer down message. Fix this and make the field Option<String> instead of String. Also, enlarge the event buffer to 2000 events. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Reorganize the handling of peer up/down messages so that if a BGP neighbor event needs to be sent, the RwLock guard on the status has been released before attempting to send on the channel as, otherwise, the lock would unnecessarily be held if the channel was full and blocked us. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Instead of using the bgp router id, use the peer's key (address) in events. The router id is also kept. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Compute BGP session downtime and include it in the bgp events. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Add some sanity on bgp peer down to ensure that we only account for transitions from established. With FRR0s bmp implementation, we should only see peer downs when established state is left. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

+                    write!(f, " reason: {last_reset_reason}")?;
+                } else if let Some(downtime) = &bgp_ev.last_downtime {
+                    let downtime = PrettyDuration::new(*downtime);
+                    write!(f, " (downtime of {downtime})")?;


+use crate::bmp::bmp_render::BgpNeighEvent;
+use crate::cli::display::PrettyDuration;
 use crate::event::EventLog;


+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let duration = self.0.elapsed();
+        PrettyDuration(duration).fmt(f)
+    }


Fredi-raspall requested review from Copilot and sergeymatov July 18, 2026 15:54

Fredi-raspall requested a review from a team as a code owner July 18, 2026 15:54

Copilot started reviewing on behalf of Fredi-raspall July 18, 2026 15:54 View session

Fredi-raspall added the ci:+vlab Enable VLAB tests label Jul 18, 2026

Copilot AI reviewed Jul 18, 2026

View reviewed changes

Comment thread routing/src/bmp/handler.rs Outdated

Fredi-raspall mentioned this pull request Jul 18, 2026

bmp msg stats #1646

Open

Fredi-raspall force-pushed the pr/fredi/bgp_sess_change_record branch from f6c954b to 357d5a0 Compare July 19, 2026 10:54

Fredi-raspall self-assigned this Jul 19, 2026

Base automatically changed from pr/fredi/bmp_simplify to main July 20, 2026 12:50

Fredi-raspall force-pushed the pr/fredi/bgp_sess_change_record branch from 49819bc to 0ca0ef1 Compare July 20, 2026 14:55

coderabbitai Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread routing/src/bmp/bmp_render.rs Outdated

Fredi-raspall added 10 commits July 21, 2026 14:42

feat(config): impl Display for BgpNeighborSessionState

df21567

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(routing): define new event BgpNeighEvent

6169007

BgpNeighEvent represents a status change of a BGP session. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(bmp,routing): pass RouterCtlSender to bmp server

1d76978

Propagate a RouterCtlSender to the BMP server so that it can send events on BGP peer changes. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(routing): RouterCtlSender need not be mut

d43980c

Mutability is not required by most methods of RouterCtlSender. Replace &mut by &self. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(bmp): generate events on BGP peer status changes

0194bea

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(bmp): use the same neigh id as bmp for events

bd826ed

Instead of using the bgp router id, use the peer's key (address) in events. The router id is also kept. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(bmp): compute bgp session downtime

16844b2

Compute BGP session downtime and include it in the bgp events. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(bmp): sanities on peer down

c7fc043

Add some sanity on bgp peer down to ensure that we only account for transitions from established. With FRR0s bmp implementation, we should only see peer downs when established state is left. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Fredi-raspall added the ci:+release Enable VLAB release tests label Jul 21, 2026

Copilot AI review requested due to automatic review settings July 21, 2026 13:08

Fredi-raspall force-pushed the pr/fredi/bgp_sess_change_record branch from 0ca0ef1 to c7fc043 Compare July 21, 2026 13:08

Copilot started reviewing on behalf of Fredi-raspall July 21, 2026 13:09 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

track bgp session state in dataplane#1645

track bgp session state in dataplane#1645
Fredi-raspall wants to merge 10 commits into
mainfrom
pr/fredi/bgp_sess_change_record

Fredi-raspall commented Jul 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Fredi-raspall commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fredi-raspall commented Jul 18, 2026 •

edited

Loading

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading