graceful restart: send initial paths list to all neighbors

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 speaker, leading to an inconsistent routing state.
osrg · Apr 30, 2024 · 3ae9121 · 3ae9121
1 parent 5aaabf0
commit 3ae9121
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 3 deletions.
diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -1650,9 +1650,10 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
 				if allEnd {
 					for _, p := range s.neighborMap {
 						p.fsm.lock.Lock()
+						peerLocalRestarting := p.fsm.pConf.GracefulRestart.State.LocalRestarting
 						p.fsm.pConf.GracefulRestart.State.LocalRestarting = false
 						p.fsm.lock.Unlock()
-						if !p.isGracefulRestartEnabled() {
+						if !p.isGracefulRestartEnabled() && !peerLocalRestarting {
 							continue
 						}
 						paths, _ := s.getBestFromLocal(p, p.configuredRFlist())
@@ -1791,9 +1792,10 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
 					if allEnd {
 						for _, p := range s.neighborMap {
 							p.fsm.lock.Lock()
+							peerLocalRestarting := p.fsm.pConf.GracefulRestart.State.LocalRestarting
 							p.fsm.pConf.GracefulRestart.State.LocalRestarting = false
 							p.fsm.lock.Unlock()
-							if !p.isGracefulRestartEnabled() {
+							if !p.isGracefulRestartEnabled() && !peerLocalRestarting {
 								continue
 							}
 							paths, _ := s.getBestFromLocal(p, p.negotiatedRFList())

diff --git a/test/scenario_test/graceful_restart_test.py b/test/scenario_test/graceful_restart_test.py
@@ -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']