Skip to content

Commit

Permalink
graceful restart: send initial paths list to all neighbors
Browse files Browse the repository at this point in the history
Before this PR, when graceful restart was configured for a neighbor
and when the restart flag was set by the restarting speaker, if
the neighbor was not advertising the GR capability, the initial
paths list was never sent by the restarting speaker to its neighbor

This is a problem when the server is configured with graceful
restart for all its peers without knowing if the peer supports it.
If some of the peers don't support it, they may never receive the
routes from the restarting speeaker, leading to an inconsistent
routing state.
  • Loading branch information
hardeker committed Apr 11, 2024
1 parent 7fed268 commit c076205
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
6 changes: 0 additions & 6 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1608,9 +1608,6 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
p.fsm.lock.Lock()
p.fsm.pConf.GracefulRestart.State.LocalRestarting = false
p.fsm.lock.Unlock()
if !p.isGracefulRestartEnabled() {
continue
}
paths, _ := s.getBestFromLocal(p, p.configuredRFlist())
if len(paths) > 0 {
sendfsmOutgoingMsg(p, paths, nil, false)
Expand Down Expand Up @@ -1749,9 +1746,6 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
p.fsm.lock.Lock()
p.fsm.pConf.GracefulRestart.State.LocalRestarting = false
p.fsm.lock.Unlock()
if !p.isGracefulRestartEnabled() {
continue
}
paths, _ := s.getBestFromLocal(p, p.negotiatedRFList())
if len(paths) > 0 {
sendfsmOutgoingMsg(p, paths, nil, false)
Expand Down
16 changes: 15 additions & 1 deletion test/scenario_test/graceful_restart_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,25 @@ def test_04_add_non_graceful_restart_enabled_peer(self):
self.bgpds['g3'] = g3
time.sleep(g3.run())
g3.add_route('10.10.30.0/24')
g1.add_peer(g3)
g1.add_peer(g3, graceful_restart=True)
g3.add_peer(g1)
g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g3)
time.sleep(1)
self.assertEqual(len(g3.get_global_rib('10.10.20.0/24')), 1)
self.assertEqual(len(g1.get_global_rib('10.10.30.0/24')), 1)

# Restart g1 with graceful restart flag and check that g1 routes are still propagated to g3.
g1.stop_gobgp()
g3.stop_gobgp()
g1.routes = {}
g3.routes = {}
g1.start_gobgp(graceful_restart=True)
g1.add_route('10.10.20.0/24')
g3.start_gobgp()
g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g3)
time.sleep(1)
self.assertEqual(len(g3.get_global_rib('10.10.20.0/24')), 1)
g3.add_route('10.10.30.0/24')

def test_05_holdtime_expiry_graceful_restart(self):
g1 = self.bgpds['g1']
Expand Down

0 comments on commit c076205

Please sign in to comment.