Update adjrib for LLGR and preserve aslooped attr

Fixes LLGR community cleared on softreset.
Fixes AS Path looped routes added back to rib on Graceful Restart.

5 files changed, 128 insertions, 27 deletions

diff --git a/internal/pkg/table/adj.go b/internal/pkg/table/adj.go
index 4b9d7c5f..d63537f8 100644
--- a/internal/pkg/table/adj.go
+++ b/internal/pkg/table/adj.go
@@ -173,11 +173,39 @@ func (adj *AdjRib) StaleAll(rfList []bgp.RouteFamily) []*Path {
 		for i, p := range d.knownPathList {
 			n := p.Clone(false)
 			n.MarkStale(true)
+			n.SetAsLooped(p.IsAsLooped())
 			d.knownPathList[i] = n
-			pathList = append(pathList, n)
+			if !n.IsAsLooped() {
+				pathList = append(pathList, n)
+			}
+		}
+		return false
+	})
+	return pathList
+}
+
+func (adj *AdjRib) MarkLLGRStaleOrDrop(rfList []bgp.RouteFamily) []*Path {
+	pathList := make([]*Path, 0, adj.Count(rfList))
+	adj.walk(rfList, func(d *Destination) bool {
+		for i, p := range d.knownPathList {
+			if p.HasNoLLGR() {
+				n := p.Clone(true)
+				n.SetDropped(true)
+				pathList = append(pathList, n)
+			} else {
+				n := p.Clone(false)
+				n.SetAsLooped(p.IsAsLooped())
+				n.SetCommunities([]uint32{uint32(bgp.COMMUNITY_LLGR_STALE)}, false)
+				if p.IsAsLooped() {
+					d.knownPathList[i] = n
+				} else {
+					pathList = append(pathList, n)
+				}
+			}
 		}
 		return false
 	})
+	adj.Update(pathList)
 	return pathList
 }
 
diff --git a/internal/pkg/table/adj_test.go b/internal/pkg/table/adj_test.go
index 8513bc97..9e48538f 100644
--- a/internal/pkg/table/adj_test.go
+++ b/internal/pkg/table/adj_test.go
@@ -69,14 +69,19 @@ func TestStale(t *testing.T) {
 	p1 := NewPath(pi, nlri1, false, attrs, time.Now(), false)
 	nlri2 := bgp.NewIPAddrPrefix(24, "20.20.20.0")
 	p2 := NewPath(pi, nlri2, false, attrs, time.Now(), false)
+	p2.SetAsLooped(true)
+
 	family := p1.GetRouteFamily()
 	families := []bgp.RouteFamily{family}
 
 	adj := NewAdjRib(families)
 	adj.Update([]*Path{p1, p2})
 	assert.Equal(t, adj.Count([]bgp.RouteFamily{family}), 2)
+	assert.Equal(t, adj.Accepted([]bgp.RouteFamily{family}), 1)
 
-	adj.StaleAll(families)
+	stalePathList := adj.StaleAll(families)
+	// As looped path should not be returned
+	assert.Equal(t, 1, len(stalePathList))
 
 	for _, p := range adj.PathList([]bgp.RouteFamily{family}, false) {
 		assert.True(t, p.IsStale())
@@ -86,7 +91,45 @@ func TestStale(t *testing.T) {
 	p3 := NewPath(pi, nlri3, false, attrs, time.Now(), false)
 	adj.Update([]*Path{p1, p3})
 
-	adj.DropStale(families)
+	droppedPathList := adj.DropStale(families)
+	assert.Equal(t, 2, len(droppedPathList))
 	assert.Equal(t, adj.Count([]bgp.RouteFamily{family}), 1)
 	assert.Equal(t, 1, len(adj.table[family].destinations))
 }
+
+func TestLLGRStale(t *testing.T) {
+	pi := &PeerInfo{}
+	attrs := []bgp.PathAttributeInterface{bgp.NewPathAttributeOrigin(0)}
+
+	nlri1 := bgp.NewIPAddrPrefix(24, "20.20.10.0")
+	p1 := NewPath(pi, nlri1, false, attrs, time.Now(), false)
+
+	nlri2 := bgp.NewIPAddrPrefix(24, "20.20.20.0")
+	p2 := NewPath(pi, nlri2, false, attrs, time.Now(), false)
+	p2.SetAsLooped(true) // Not accepted
+
+	nlri3 := bgp.NewIPAddrPrefix(24, "20.20.30.0")
+	p3 := NewPath(pi, nlri3, false, attrs, time.Now(), false)
+	p3.SetAsLooped(true)
+	// Not accepted and then dropped on MarkLLGRStaleOrDrop
+	p3.SetCommunities([]uint32{uint32(bgp.COMMUNITY_NO_LLGR)}, false)
+
+	nlri4 := bgp.NewIPAddrPrefix(24, "20.20.40.0")
+	p4 := NewPath(pi, nlri4, false, attrs, time.Now(), false)
+	// dropped on MarkLLGRStaleOrDrop
+	p4.SetCommunities([]uint32{uint32(bgp.COMMUNITY_NO_LLGR)}, false)
+
+	family := p1.GetRouteFamily()
+	families := []bgp.RouteFamily{family}
+
+	adj := NewAdjRib(families)
+	adj.Update([]*Path{p1, p2, p3, p4})
+	assert.Equal(t, adj.Count([]bgp.RouteFamily{family}), 4)
+	assert.Equal(t, adj.Accepted([]bgp.RouteFamily{family}), 2)
+
+	pathList := adj.MarkLLGRStaleOrDrop(families)
+	assert.Equal(t, 3, len(pathList)) // Does not return aslooped path that is retained in adjrib
+	assert.Equal(t, adj.Count([]bgp.RouteFamily{family}), 2)
+	assert.Equal(t, adj.Accepted([]bgp.RouteFamily{family}), 1)
+	assert.Equal(t, 2, len(adj.table[family].destinations))
+}
diff --git a/internal/pkg/table/path.go b/internal/pkg/table/path.go
index 32616d2e..9f68fde3 100644
--- a/internal/pkg/table/path.go
+++ b/internal/pkg/table/path.go
@@ -398,6 +398,15 @@ func (path *Path) SetDropped(y bool) {
 	path.dropped = y
 }
 
+func (path *Path) HasNoLLGR() bool {
+	for _, c := range path.GetCommunities() {
+		if c == uint32(bgp.COMMUNITY_NO_LLGR) {
+			return true
+		}
+	}
+	return false
+}
+
 func (path *Path) IsLLGRStale() bool {
 	for _, c := range path.GetCommunities() {
 		if c == uint32(bgp.COMMUNITY_LLGR_STALE) {
diff --git a/pkg/server/peer.go b/pkg/server/peer.go
index 7d85ec52..a7843f49 100644
--- a/pkg/server/peer.go
+++ b/pkg/server/peer.go
@@ -333,23 +333,7 @@ func (peer *peer) llgrRestartTimerExpired(family bgp.RouteFamily) bool {
 }
 
 func (peer *peer) markLLGRStale(fs []bgp.RouteFamily) []*table.Path {
-	paths := peer.adjRibIn.PathList(fs, true)
-	for i, p := range paths {
-		doStale := true
-		for _, c := range p.GetCommunities() {
-			if c == uint32(bgp.COMMUNITY_NO_LLGR) {
-				doStale = false
-				p = p.Clone(true)
-				break
-			}
-		}
-		if doStale {
-			p = p.Clone(false)
-			p.SetCommunities([]uint32{uint32(bgp.COMMUNITY_LLGR_STALE)}, false)
-		}
-		paths[i] = p
-	}
-	return paths
+	return peer.adjRibIn.MarkLLGRStaleOrDrop(fs)
 }
 
 func (peer *peer) stopPeerRestarting() {
diff --git a/test/scenario_test/long_lived_graceful_restart_test.py b/test/scenario_test/long_lived_graceful_restart_test.py
index 0ea38ff1..1fb5edac 100644
--- a/test/scenario_test/long_lived_graceful_restart_test.py
+++ b/test/scenario_test/long_lived_graceful_restart_test.py
@@ -95,6 +95,11 @@ class GoBGPTestBase(unittest.TestCase):
         time.sleep(1)
 
         self.assertEqual(len(g1.get_global_rib('10.0.0.0/24')), 1)
+        # check llgr-stale community is added to 10.0.0.0/24
+        r = g1.get_global_rib('10.0.0.0/24')[0]['paths'][0]
+        comms = list(chain.from_iterable([attr['communities'] for attr in r['attrs'] if attr['type'] == 8]))
+        self.assertTrue(0xffff0006 in comms)
+
         # 10.10.0.0/24 is announced with no-llgr community
         # must not exist in the rib
         self.assertEqual(len(g1.get_global_rib('10.10.0.0/24')), 0)
@@ -102,24 +107,56 @@ class GoBGPTestBase(unittest.TestCase):
             for p in d['paths']:
                 self.assertTrue(p['stale'])
 
+        # check llgr-stale community is present in received route 10.0.0.0/24
         self.assertEqual(len(g3.get_global_rib('10.0.0.0/24')), 1)
-        # check llgr-stale community is added to 10.0.0.0/24
         r = g3.get_global_rib('10.0.0.0/24')[0]['paths'][0]
         comms = list(chain.from_iterable([attr['communities'] for attr in r['attrs'] if attr['type'] == 8]))
         self.assertTrue(0xffff0006 in comms)
+
         # g4 is not llgr capable, llgr-stale route must be
         # withdrawn
         self.assertEqual(len(g4.get_global_rib('10.0.0.0/24')), 0)
 
-        g2.start_gobgp(graceful_restart=True)
-        g2.local('gobgp global rib add 10.0.0.0/24')
-        g2.local('gobgp global rib add 10.10.0.0/24')
+    def test_03_softreset_preserves_llgr_community(self):
+        g1 = self.bgpds['g1']
+        g2 = self.bgpds['g2']
+        g3 = self.bgpds['g3']
+        g4 = self.bgpds['g4']
 
-    def test_03_neighbor_established(self):
+        g1.softreset(g2)
+        time.sleep(1)
+
+        # 10.10.0.0/24 received with no-llgr community is not reinstalled to rib
+        self.assertEqual(len(g1.get_global_rib('10.10.0.0/24')), 0)
+        # Stale flags are not cleared
+        for d in g1.get_global_rib():
+            for p in d['paths']:
+                self.assertTrue(p['stale'])
+
+        # check llgr-stale community is not cleared from 10.0.0.0/24
+        r = g1.get_global_rib('10.0.0.0/24')[0]['paths'][0]
+        comms = list(chain.from_iterable([attr['communities'] for attr in r['attrs'] if attr['type'] == 8]))
+        self.assertTrue(0xffff0006 in comms)
+
+        # check llgr-stale community is not cleared in route 10.0.0.0/24 on g3
+        self.assertEqual(len(g3.get_global_rib('10.0.0.0/24')), 1)
+        r = g3.get_global_rib('10.0.0.0/24')[0]['paths'][0]
+        comms = list(chain.from_iterable([attr['communities'] for attr in r['attrs'] if attr['type'] == 8]))
+        self.assertTrue(0xffff0006 in comms)
+
+        # g4 is not llgr capable, llgr-stale route must not be advertized on softreset g1
+        self.assertEqual(len(g4.get_global_rib('10.0.0.0/24')), 0)
+
+    def test_04_neighbor_established(self):
         g1 = self.bgpds['g1']
         g2 = self.bgpds['g2']
         # g3 = self.bgpds['g3']
         # g4 = self.bgpds['g4']
+
+        g2.start_gobgp(graceful_restart=True)
+        g2.local('gobgp global rib add 10.0.0.0/24')
+        g2.local('gobgp global rib add 10.10.0.0/24')
+
         g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g2)
         time.sleep(1)
         self.assertEqual(len(g1.get_global_rib('10.0.0.0/24')), 1)
@@ -128,7 +165,7 @@ class GoBGPTestBase(unittest.TestCase):
             for p in d['paths']:
                 self.assertFalse(p.get('stale', False))
 
-    def test_04_llgr_stale_route_depreferenced(self):
+    def test_05_llgr_stale_route_depreferenced(self):
         g1 = self.bgpds['g1']
         g2 = self.bgpds['g2']
         g3 = self.bgpds['g3']
@@ -156,7 +193,7 @@ class GoBGPTestBase(unittest.TestCase):
         self.assertEqual(len(rib), 1)
         self.assertTrue(g2.asn in rib[0]['paths'][0]['aspath'])
 
-    def test_05_llgr_restart_timer_expire(self):
+    def test_06_llgr_restart_timer_expire(self):
         time.sleep(LONG_LIVED_GRACEFUL_RESTART_TIME + 5)
         g3 = self.bgpds['g3']
         rib = g3.get_global_rib('10.10.0.0/24')