From e0cc90eac4e8fbb5c99b401419e9addf9c941df2 Mon Sep 17 00:00:00 2001 From: Noel Georgi Date: Mon, 3 Aug 2026 12:42:52 +0530 Subject: [PATCH] bfd: use transport local address for BFD Bind outgoing BFD sessions to the neighbor transport local address so numbered peers use the same explicit source as BGP while preserving interface binding and kernel-selected defaults. Signed-off-by: Noel Georgi --- docs/sources/bfd.md | 4 ++ pkg/server/bfd_peer.go | 21 ++++++++- pkg/server/bfd_peer_test.go | 81 +++++++++++++++++++++++++++++++++ pkg/server/bfd_server.go | 13 ++++-- pkg/server/bfd_server_test.go | 86 +++++++++++++++++++++++++++++++++++ pkg/server/server.go | 15 ++++-- 6 files changed, 210 insertions(+), 10 deletions(-) diff --git a/docs/sources/bfd.md b/docs/sources/bfd.md index ea4dfcf8b..c5abe6933 100644 --- a/docs/sources/bfd.md +++ b/docs/sources/bfd.md @@ -25,6 +25,8 @@ Supported behavior: - peer-group BFD configuration inherited by neighbors; - default destination UDP port `3784`; - source UDP port selected from the RFC 5881 dynamic range `49152..65535`; +- source IP selected from the neighbor transport `local-address` when configured, + or by the kernel when unset; - outgoing BFD packets sent with TTL/Hop Limit `255`; - BFD states `DOWN`, `INIT`, `UP`, and `ADMIN_DOWN`; - Poll/Final handling in control packets; @@ -242,6 +244,8 @@ If the BFD session does not come up: neighbor address; - verify that UDP `3784` is reachable in both directions; - verify that the remote system accepts source ports from `49152..65535`; +- verify that the neighbor transport `local-address`, when configured, exists on + the host and is compatible with `bind-interface`; - avoid setting a non-default `port` unless the remote peer is known to listen there; - check JSON peer output for `state.bfd_state.session_state`; diff --git a/pkg/server/bfd_peer.go b/pkg/server/bfd_peer.go index 4696c67d2..dda3df968 100644 --- a/pkg/server/bfd_peer.go +++ b/pkg/server/bfd_peer.go @@ -43,6 +43,7 @@ type bfdPeer struct { logger *slog.Logger peerAddress netip.Addr peerPort int + localAddress netip.Addr bindInterface string udpClient *net.UDPConn @@ -69,6 +70,10 @@ type bfdPeer struct { } func NewBfdPeer(ps peerState, logger *slog.Logger, peerAddress netip.Addr, config oc.BfdConfig, bindInterface string) *bfdPeer { + return newBfdPeer(ps, logger, peerAddress, config, netip.Addr{}, bindInterface) +} + +func newBfdPeer(ps peerState, logger *slog.Logger, peerAddress netip.Addr, config oc.BfdConfig, localAddress netip.Addr, bindInterface string) *bfdPeer { peerPort := int(config.Port) if peerPort == 0 { peerPort = BfdServerPort @@ -79,6 +84,7 @@ func NewBfdPeer(ps peerState, logger *slog.Logger, peerAddress netip.Addr, confi logger: logger, peerAddress: peerAddress, peerPort: peerPort, + localAddress: localAddress, bindInterface: bindInterface, myDiscriminator: randomBFDMyDiscriminator(), @@ -202,11 +208,22 @@ func (p *bfdPeer) remoteUDPAddr() *net.UDPAddr { } } -func (p *bfdPeer) startClient() { - localAddress := &net.UDPAddr{ +func (p *bfdPeer) localUDPAddr() *net.UDPAddr { + addr := &net.UDPAddr{ Port: randRange(bfdSourcePortMin, bfdSourcePortMax), } + if p.localAddress.IsValid() && !p.localAddress.IsUnspecified() { + addr.IP = p.localAddress.AsSlice() + addr.Zone = p.localAddress.Zone() + } + + return addr +} + +func (p *bfdPeer) startClient() { + localAddress := p.localUDPAddr() + remoteAddress := p.remoteUDPAddr() var err error diff --git a/pkg/server/bfd_peer_test.go b/pkg/server/bfd_peer_test.go index acd674b54..523fc1412 100644 --- a/pkg/server/bfd_peer_test.go +++ b/pkg/server/bfd_peer_test.go @@ -3,6 +3,7 @@ package server import ( "fmt" "log/slog" + "net" "net/netip" "sync/atomic" "testing" @@ -70,6 +71,86 @@ func Test_BfdPeerRemoteUDPAddrZone(t *testing.T) { assert.Empty(g.remoteUDPAddr().Zone) } +func Test_BfdPeerLocalUDPAddr(t *testing.T) { + tests := []struct { + name string + localAddress netip.Addr + expectedIP string + expectedZone string + }{ + {name: "unset"}, + {name: "unspecified IPv4", localAddress: netip.IPv4Unspecified()}, + {name: "unspecified IPv6", localAddress: netip.IPv6Unspecified()}, + {name: "IPv4", localAddress: netip.MustParseAddr("192.0.2.1"), expectedIP: "192.0.2.1"}, + {name: "IPv6", localAddress: netip.MustParseAddr("2001:db8::1"), expectedIP: "2001:db8::1"}, + {name: "link-local IPv6", localAddress: netip.MustParseAddr("fe80::1%eth0"), expectedIP: "fe80::1", expectedZone: "eth0"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addr := (&bfdPeer{localAddress: tt.localAddress}).localUDPAddr() + + if tt.expectedIP == "" { + assert.Nil(t, addr.IP) + } else { + assert.Equal(t, tt.expectedIP, addr.IP.String()) + } + + assert.Equal(t, tt.expectedZone, addr.Zone) + assert.GreaterOrEqual(t, addr.Port, bfdSourcePortMin) + assert.LessOrEqual(t, addr.Port, bfdSourcePortMax) + }) + } +} + +func Test_BfdPeerStartClientUsesLocalAddress(t *testing.T) { + for _, localAddress := range []netip.Addr{ + netip.MustParseAddr("127.0.0.1"), + netip.MustParseAddr("::1"), + } { + t.Run(localAddress.String(), func(t *testing.T) { + listener, err := net.ListenUDP("udp", &net.UDPAddr{IP: localAddress.AsSlice()}) + if !assert.NoError(t, err) { + return + } + defer listener.Close() + + peer := &bfdPeer{ + logger: slog.Default(), + peerAddress: localAddress, + peerPort: listener.LocalAddr().(*net.UDPAddr).Port, + localAddress: localAddress, + } + peer.startClient() + if !assert.NotNil(t, peer.udpClient) { + return + } + defer peer.udpClient.Close() + + actual := peer.udpClient.LocalAddr().(*net.UDPAddr) + assert.Equal(t, localAddress.String(), actual.IP.String()) + assert.GreaterOrEqual(t, actual.Port, bfdSourcePortMin) + assert.LessOrEqual(t, actual.Port, bfdSourcePortMax) + }) + } +} + +func Test_NewBfdPeerLocalAddressAndBindInterface(t *testing.T) { + localAddress := netip.MustParseAddr("2001:db8::1") + p := newBfdPeer( + &mockPeerState{}, + slog.Default(), + netip.MustParseAddr("2001:db8::2"), + oc.BfdConfig{Enabled: true}, + localAddress, + "eth0", + ) + defer p.Stop() + + assert.Equal(t, localAddress, p.localAddress) + assert.Equal(t, "eth0", p.bindInterface) +} + func Test_BfdPeerStopIdempotent(t *testing.T) { assert := assert.New(t) diff --git a/pkg/server/bfd_server.go b/pkg/server/bfd_server.go index e7cc3f0c5..b456d8123 100644 --- a/pkg/server/bfd_server.go +++ b/pkg/server/bfd_server.go @@ -32,6 +32,7 @@ type bfdEventPeerUpdate struct { isAdd bool peerAddress netip.Addr config oc.BfdConfig + localAddress netip.Addr bindInterface string } @@ -117,6 +118,10 @@ func (s *bfdServer) Stop() { } func (s *bfdServer) AddPeer(ctx context.Context, peerAddress netip.Addr, config oc.BfdConfig, bindInterface string) error { + return s.addPeer(ctx, peerAddress, config, netip.Addr{}, bindInterface) +} + +func (s *bfdServer) addPeer(ctx context.Context, peerAddress netip.Addr, config oc.BfdConfig, localAddress netip.Addr, bindInterface string) error { if s.stopped.Load() { return errors.New("bfd server stopped") } @@ -126,7 +131,7 @@ func (s *bfdServer) AddPeer(ctx context.Context, peerAddress netip.Addr, config } select { - case s.eventPeerUpdate <- &bfdEventPeerUpdate{isAdd: true, peerAddress: peerAddress, config: config, bindInterface: bindInterface}: + case s.eventPeerUpdate <- &bfdEventPeerUpdate{isAdd: true, peerAddress: peerAddress, config: config, localAddress: localAddress, bindInterface: bindInterface}: if s.stopped.Load() { return errors.New("bfd server stopped") } @@ -205,7 +210,7 @@ func (s *bfdServer) loop() { s.config = ev case ev := <-s.eventPeerUpdate: if ev.isAdd { - s.addBfdPeer(ev.peerAddress, ev.config, ev.bindInterface) + s.addBfdPeer(ev.peerAddress, ev.config, ev.localAddress, ev.bindInterface) } else { s.deleteBfdPeer(ev.peerAddress) } @@ -292,7 +297,7 @@ func (s *bfdServer) stop() { ) } -func (s *bfdServer) addBfdPeer(peerAddress netip.Addr, config oc.BfdConfig, bindInterface string) { +func (s *bfdServer) addBfdPeer(peerAddress netip.Addr, config oc.BfdConfig, localAddress netip.Addr, bindInterface string) { s.peersMutex.RLock() _, ok := s.peers[peerAddress] s.peersMutex.RUnlock() @@ -306,7 +311,7 @@ func (s *bfdServer) addBfdPeer(peerAddress netip.Addr, config oc.BfdConfig, bind return } - bfdPeer := NewBfdPeer(s.peerState, s.logger, peerAddress, config, bindInterface) + bfdPeer := newBfdPeer(s.peerState, s.logger, peerAddress, config, localAddress, bindInterface) if bfdPeer != nil { s.logger.Info("Insert BFD peer", slog.String("Topic", "bfd"), diff --git a/pkg/server/bfd_server_test.go b/pkg/server/bfd_server_test.go index edf77727b..904d2d761 100644 --- a/pkg/server/bfd_server_test.go +++ b/pkg/server/bfd_server_test.go @@ -146,6 +146,82 @@ func addPeer(s *bfdServer, port uint16) error { }, "") } +func Test_AddPeerLocalAddressAndBindInterface(t *testing.T) { + s := newServer(0) + defer s.Stop() + + peerAddress := netip.MustParseAddr("127.0.0.1") + localAddress := netip.MustParseAddr("127.0.0.2") + err := s.addPeer(context.Background(), peerAddress, oc.BfdConfig{Enabled: true}, localAddress, "lo") + assert.NoError(t, err) + + err = eventually(time.Second, func() error { + s.peersMutex.RLock() + defer s.peersMutex.RUnlock() + + peer := s.peers[peerAddress] + if peer == nil { + return fmt.Errorf("BFD peer not created") + } + + if peer.localAddress != localAddress || peer.bindInterface != "lo" { + return fmt.Errorf("unexpected BFD source configuration: address=%s interface=%q", peer.localAddress, peer.bindInterface) + } + + return nil + }) + assert.NoError(t, err) +} + +func Test_UpdateBfdPeerLocalAddress(t *testing.T) { + bfd := newServer(0) + defer bfd.Stop() + + s := &BgpServer{bfdServer: bfd} + peerAddress := netip.MustParseAddr("127.0.0.1") + oldLocalAddress := netip.MustParseAddr("127.0.0.2") + newLocalAddress := netip.MustParseAddr("127.0.0.3") + config := oc.BfdConfig{Enabled: true} + + assert.NoError(t, bfd.addPeer(context.Background(), peerAddress, config, oldLocalAddress, "lo")) + + var oldPeer *bfdPeer + assert.NoError(t, eventually(time.Second, func() error { + bfd.peersMutex.RLock() + defer bfd.peersMutex.RUnlock() + + oldPeer = bfd.peers[peerAddress] + if oldPeer == nil { + return fmt.Errorf("BFD peer not created") + } + + return nil + })) + + assert.NoError(t, s.updateBfdPeer(peerAddress.String(), config, config, oldLocalAddress, newLocalAddress, "lo", "lo")) + assert.NoError(t, eventually(time.Second, func() error { + bfd.peersMutex.RLock() + defer bfd.peersMutex.RUnlock() + + peer := bfd.peers[peerAddress] + if peer == nil || peer == oldPeer || peer.localAddress != newLocalAddress { + return fmt.Errorf("BFD peer was not recreated with local address %s", newLocalAddress) + } + + return nil + })) + + bfd.peersMutex.RLock() + unchangedPeer := bfd.peers[peerAddress] + bfd.peersMutex.RUnlock() + + assert.NoError(t, s.updateBfdPeer(peerAddress.String(), config, config, newLocalAddress, newLocalAddress, "lo", "lo")) + + bfd.peersMutex.RLock() + assert.Same(t, unchangedPeer, bfd.peers[peerAddress]) + bfd.peersMutex.RUnlock() +} + func Test_AddDeletePeer(t *testing.T) { assert := assert.New(t) @@ -265,6 +341,7 @@ func Test_BgpAddDeletePeer(t *testing.T) { assert.NoError(err) defer s.Stop() + localAddress := netip.MustParseAddr("127.0.0.10") nConf1 := &oc.Neighbor{ Config: oc.NeighborConfig{ NeighborAddress: netip.MustParseAddr("127.0.0.1"), @@ -281,6 +358,11 @@ func Test_BgpAddDeletePeer(t *testing.T) { Config: oc.PeerGroupConfig{ PeerGroupName: "group_on", }, + Transport: oc.Transport{ + Config: oc.TransportConfig{ + LocalAddress: localAddress, + }, + }, Bfd: oc.Bfd{ Config: oc.BfdConfig{ Enabled: true, @@ -318,6 +400,10 @@ func Test_BgpAddDeletePeer(t *testing.T) { }) assert.Equal(count, 1) + s.bfdServer.peersMutex.RLock() + assert.Equal(localAddress, s.bfdServer.peers[nConf1.Config.NeighborAddress].localAddress) + s.bfdServer.peersMutex.RUnlock() + // Delete 1 peer err = s.DeletePeer(context.Background(), &api.DeletePeerRequest{ Address: "127.0.0.1", diff --git a/pkg/server/server.go b/pkg/server/server.go index b7b1beaba..8c6b8d900 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -372,7 +372,7 @@ func (s *BgpServer) passConnToPeer(conn net.Conn) { // register BFD for the dynamic neighbor too (explicit neighbors do this in addNeighbor): the // BFD config is inherited from the peer group. Without this, BFD never runs for dynamic peers. if s.bfdServer != nil && conf.Bfd.Config.Enabled { - if err := s.bfdServer.AddPeer(context.Background(), addr, conf.Bfd.Config, s.bgpConfig.Global.Config.BindToDevice); err != nil { + if err := s.bfdServer.addPeer(context.Background(), addr, conf.Bfd.Config, conf.Transport.Config.LocalAddress, s.bgpConfig.Global.Config.BindToDevice); err != nil { s.logger.Warn("failed to add BFD peer for dynamic neighbor", slog.String("Topic", "Peer"), slog.String("Key", addr.String()), @@ -3536,7 +3536,7 @@ func (s *BgpServer) addNeighbor(c *oc.Neighbor) error { if err != nil { return fmt.Errorf("failed to parse IP address: %v", err) } - if err := s.bfdServer.AddPeer(context.Background(), ipAddr, c.Bfd.Config, c.Transport.Config.BindInterface); err != nil { + if err := s.bfdServer.addPeer(context.Background(), ipAddr, c.Bfd.Config, c.Transport.Config.LocalAddress, c.Transport.Config.BindInterface); err != nil { s.logger.Warn("failed to add BFD peer", slog.String("Topic", "Peer"), slog.String("Key", addr), @@ -3565,9 +3565,10 @@ func apiBfdSessionStateToOC(state api.BfdSessionState) oc.BfdSessionState { func (s *BgpServer) updateBfdPeer( addr string, oldConfig, newConfig oc.BfdConfig, + oldLocalAddress, newLocalAddress netip.Addr, oldBindInterface, newBindInterface string, ) error { - if s.bfdServer == nil || oldConfig.Equal(&newConfig) && oldBindInterface == newBindInterface { + if s.bfdServer == nil || oldConfig.Equal(&newConfig) && oldLocalAddress == newLocalAddress && oldBindInterface == newBindInterface { return nil } @@ -3583,7 +3584,7 @@ func (s *BgpServer) updateBfdPeer( } if newConfig.Enabled { - if err := s.bfdServer.AddPeer(context.Background(), ipAddr, newConfig, newBindInterface); err != nil { + if err := s.bfdServer.addPeer(context.Background(), ipAddr, newConfig, newLocalAddress, newBindInterface); err != nil { return err } } @@ -3860,6 +3861,11 @@ func (s *BgpServer) updateNeighbor(c *oc.Neighbor) (needsSoftResetIn bool, err e conf.Bfd.Config = c.Bfd.Config } + if original.Transport.Config.LocalAddress != c.Transport.Config.LocalAddress { + peer.fsm.logger.Info("Update BFD local address") + bfdConfigChanged = true + } + if original.Transport.Config.BindInterface != c.Transport.Config.BindInterface { peer.fsm.logger.Info("Update BFD interface binding") bfdConfigChanged = true @@ -3912,6 +3918,7 @@ func (s *BgpServer) updateNeighbor(c *oc.Neighbor) (needsSoftResetIn bool, err e err = s.updateBfdPeer( addr, original.Bfd.Config, c.Bfd.Config, + original.Transport.Config.LocalAddress, c.Transport.Config.LocalAddress, original.Transport.Config.BindInterface, c.Transport.Config.BindInterface, ) }