feat(bfd): add BFD server and peer state machine by Ivan-Pokhabov · Pull Request #3402 · osrg/gobgp

Ivan-Pokhabov · 2026-04-29T11:49:45Z

Implements RFC 5880/5881 BFD liveness detection for BGP peers:

UDP server listening on port 3784 with per-peer session management
State machine: Down → Init → Up with detection timeout and hard reset
Per-peer async counters and session state
Integration with BGP peer lifecycle (AddPeer/DeletePeer/ResetPeer)
Tests covering state transitions, peer reset, and BGP integration

Implements RFC 5880/5881 BFD liveness detection for BGP peers: - UDP server listening on port 3784 with per-peer session management - State machine: Down → Init → Up with detection timeout and hard reset - Per-peer async counters and session state - Integration with BGP peer lifecycle (AddPeer/DeletePeer/ResetPeer) - Tests covering state transitions, peer reset, and BGP integration Co-Authored-By: Ivan-Pokhabov <vanek3372@gmail.com>

fujita

You add some public API functions. They can be called by user applications that use GoBGP as a library. Are they safely called by multiple goroutines? Also functions that could be blocked need to take context.

fujita · 2026-04-29T21:46:30Z

 				Prefix:       p.Prefix,
 				RD:           p.Rd,
-				LookupOption: apiutil.LookupOptionFromAPI(p.Type),
+				LookupOption: apiutil.LookupOption(p.Type),


Please don't modify unrelated code.

fujita · 2026-04-29T21:48:51Z

+	s.eventConfig <- &config
+}
+
+func (s *bfdServer) Stop() {


Where is this called?

Fixed, we lose it

fujita · 2026-04-29T22:15:25Z

+
+	s.eventGetPeer <- list
+
+	select {


Looks like this code could be blocked. The caller of GetPeerState holds s.shared.mu so you can't.

fujita · 2026-04-29T22:16:24Z

+	return nil
+}
+
+func (s *bfdServer) GetPeerState(ctx context.Context, peerAddress string) (*bfdPeerState, error) {


Use netip.Addr

Replace all string with netip.Addr

fujita · 2026-04-29T22:16:36Z

+}
+
+func (s *bfdServer) DeletePeer(peerAddress string) error {
+	address, err := convertToIPAddress(peerAddress)


Use netip.Addr

fujita · 2026-04-29T22:16:48Z

+}
+
+func (s *bfdServer) AddPeer(peerAddress string, config oc.BfdConfig) error {
+	if !config.Enabled {


Use netip.Addr

Ivan-Pokhabov · 2026-04-30T10:39:40Z

You add some public API functions. They can be called by user applications that use GoBGP as a library. Are they safely called by multiple goroutines? Also functions that could be blocked need to take context.

Thank you for review, I fixed mistakes and made code more concurent-safety

fujita · 2026-05-24T12:01:39Z

+}
+
+func (p *bfdPeer) Stop() {
+	close(p.eventShutdown)


Needs to wait for the coroutine to exit here.

fujita · 2026-05-24T13:19:51Z

+
+	switch h.State {
+	case bfd.StateDown:
+		p.sendPacket(bfd.StateInit, false, false, h.MyDiscriminator)


If the current state is Up, needs to set the sate Down?

Fixed, made like more RFC

fujita · 2026-05-24T13:25:14Z

+		peerState:   ps,
+		logger:      logger,
+		peerAddress: peerAddress,
+		peerPort:    int(config.Port),


Needs to use the default port number if config.Port is zero?

Yeah, made this

- BgpServer.Stop() now calls bfdServer.Stop(), fixing goroutine leak where bfdServer.loop() ran indefinitely after server shutdown - Start/AddPeer/DeletePeer now accept context.Context and use select with ctx.Done() and eventShutdown, making them safe to call concurrently and non-blockable under cancellation or shutdown - GetPeerState/GetPeerStateList no longer route through the event loop: they read directly under peersMutex/atomic, eliminating the risk of deadlock when called while s.shared.mu is held (e.g. from toConfig) - Removed bfdEventGetPeerState and bfdEventGetPeerStateList types and their event loop cases — no longer needed - Fixed IPv4-mapped address mismatch: rxPacket now calls Addr().Unmap() so ReadFromUDP's ::ffff:1.2.3.4 form matches peers stored as pure IPv4 - AddPeer/DeletePeer API changed from string to netip.Addr, eliminating redundant string parsing in the hot path

- Wait for bfdPeer loop exit in Stop() and stop all peer timers during shutdown, so callers do not return while peer goroutine is still active - Treat remote BFD StateDown as a session failure when local state is Up, resetting the BGP peer immediately instead of waiting for timeout - Default per-peer BFD destination port to 3784 when BfdConfig.Port is zero - Aligned the peer state machine closer to RFC 5880: remote Down from Up now transitions local state to Down without immediately sending Init, local Init is represented explicitly, and remote Up no longer brings a Down session directly to Up - Add regression tests for peer shutdown, remote down handling, and default port selection

fujita · 2026-05-25T01:00:03Z

Pushed, thanks. Could we add documentation for configuring BFD?

Ivan-Pokhabov · 2026-05-25T09:04:52Z

Pushed, thanks. Could we add documentation for configuring BFD?

Thanks! I wiil do docs soon

Ivan-Pokhabov force-pushed the feat/bfd-server branch from bd258f0 to e5d4c70 Compare April 29, 2026 11:53

Ivan-Pokhabov force-pushed the feat/bfd-server branch from e5d4c70 to 60a9472 Compare April 29, 2026 15:17

fujita reviewed Apr 29, 2026

View reviewed changes

Ivan-Pokhabov force-pushed the feat/bfd-server branch from 52b3bad to c841d82 Compare April 30, 2026 10:01

fujita reviewed May 24, 2026

View reviewed changes

Ivan-Pokhabov force-pushed the feat/bfd-server branch from c841d82 to d249887 Compare May 24, 2026 15:57

Ivan-Pokhabov force-pushed the feat/bfd-server branch from d249887 to 130c415 Compare May 24, 2026 15:58

fujita closed this May 25, 2026

fujita mentioned this pull request May 25, 2026

Native BFD support #2563

Open

Conversation

Ivan-Pokhabov commented Apr 29, 2026

Uh oh!

fujita left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivan-Pokhabov commented Apr 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fujita commented May 25, 2026

Uh oh!

Ivan-Pokhabov commented May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants