avoid installing routes with originator ID to global when softresetin

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

7 files changed, 23 insertions, 48 deletions

diff --git a/internal/pkg/config/util.go b/internal/pkg/config/util.go
index 228c9e3d..522129a3 100644
--- a/internal/pkg/config/util.go
+++ b/internal/pkg/config/util.go
@@ -225,6 +225,7 @@ func (n *Neighbor) NeedsResendOpenMessage(new *Neighbor) bool {
 	return !n.Config.Equal(&new.Config) ||
 		!n.Transport.Config.Equal(&new.Transport.Config) ||
 		!n.AddPaths.Config.Equal(&new.AddPaths.Config) ||
+		!n.AsPathOptions.Config.Equal(&new.AsPathOptions.Config) ||
 		!n.GracefulRestart.Config.Equal(&new.GracefulRestart.Config) ||
 		isAfiSafiChanged(n.AfiSafis, new.AfiSafis)
 }
diff --git a/internal/pkg/table/adj.go b/internal/pkg/table/adj.go
index 5dda2a29..aedc41d6 100644
--- a/internal/pkg/table/adj.go
+++ b/internal/pkg/table/adj.go
@@ -63,16 +63,16 @@ func (adj *AdjRib) Update(pathList []*Path) {
 				if len(d.knownPathList) == 0 {
 					t.deleteDest(d)
 				}
-				if !old.IsAsLooped() {
+				if !old.IsRejected() {
 					adj.accepted[rf]--
 				}
 			}
 			path.SetDropped(true)
 		} else {
 			if idx != -1 {
-				if old.IsAsLooped() && !path.IsAsLooped() {
+				if old.IsRejected() && !path.IsRejected() {
 					adj.accepted[rf]++
-				} else if !old.IsAsLooped() && path.IsAsLooped() {
+				} else if !old.IsRejected() && path.IsRejected() {
 					adj.accepted[rf]--
 				}
 				if old.Equal(path) {
@@ -81,7 +81,7 @@ func (adj *AdjRib) Update(pathList []*Path) {
 				d.knownPathList[idx] = path
 			} else {
 				d.knownPathList = append(d.knownPathList, path)
-				if !path.IsAsLooped() {
+				if !path.IsRejected() {
 					adj.accepted[rf]++
 				}
 			}
@@ -121,7 +121,7 @@ func (adj *AdjRib) PathList(rfList []bgp.RouteFamily, accepted bool) []*Path {
 	pathList := make([]*Path, 0, adj.Count(rfList))
 	adj.walk(rfList, func(d *Destination) bool {
 		for _, p := range d.knownPathList {
-			if accepted && p.IsAsLooped() {
+			if accepted && p.IsRejected() {
 				continue
 			}
 			pathList = append(pathList, p)
@@ -189,9 +189,9 @@ 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())
+			n.SetRejected(p.IsRejected())
 			d.knownPathList[i] = n
-			if !n.IsAsLooped() {
+			if !n.IsRejected() {
 				pathList = append(pathList, n)
 			}
 		}
@@ -210,9 +210,9 @@ func (adj *AdjRib) MarkLLGRStaleOrDrop(rfList []bgp.RouteFamily) []*Path {
 				pathList = append(pathList, n)
 			} else {
 				n := p.Clone(false)
-				n.SetAsLooped(p.IsAsLooped())
+				n.SetRejected(p.IsRejected())
 				n.SetCommunities([]uint32{uint32(bgp.COMMUNITY_LLGR_STALE)}, false)
-				if p.IsAsLooped() {
+				if p.IsRejected() {
 					d.knownPathList[i] = n
 				} else {
 					pathList = append(pathList, n)
diff --git a/internal/pkg/table/adj_test.go b/internal/pkg/table/adj_test.go
index bb1f07b9..3ae59465 100644
--- a/internal/pkg/table/adj_test.go
+++ b/internal/pkg/table/adj_test.go
@@ -98,7 +98,7 @@ 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)
+	p2.SetRejected(true)
 
 	family := p1.GetRouteFamily()
 	families := []bgp.RouteFamily{family}
@@ -135,11 +135,11 @@ func TestLLGRStale(t *testing.T) {
 
 	nlri2 := bgp.NewIPAddrPrefix(24, "20.20.20.0")
 	p2 := NewPath(pi, nlri2, false, attrs, time.Now(), false)
-	p2.SetAsLooped(true) // Not accepted
+	p2.SetRejected(true) // Not accepted
 
 	nlri3 := bgp.NewIPAddrPrefix(24, "20.20.30.0")
 	p3 := NewPath(pi, nlri3, false, attrs, time.Now(), false)
-	p3.SetAsLooped(true)
+	p3.SetRejected(true)
 	// Not accepted and then dropped on MarkLLGRStaleOrDrop
 	p3.SetCommunities([]uint32{uint32(bgp.COMMUNITY_NO_LLGR)}, false)
 
diff --git a/internal/pkg/table/path.go b/internal/pkg/table/path.go
index 0a6debca..805fa63c 100644
--- a/internal/pkg/table/path.go
+++ b/internal/pkg/table/path.go
@@ -137,7 +137,7 @@ type Path struct {
 	pathAttrs []bgp.PathAttributeInterface
 	dels      []bgp.BGPAttrType
 	attrsHash uint32
-	aslooped  bool
+	rejected  bool
 	// doesn't exist in the adj
 	dropped bool
 
@@ -382,12 +382,12 @@ func (path *Path) IsStale() bool {
 	return path.OriginInfo().stale
 }
 
-func (path *Path) IsAsLooped() bool {
-	return path.aslooped
+func (path *Path) IsRejected() bool {
+	return path.rejected
 }
 
-func (path *Path) SetAsLooped(y bool) {
-	path.aslooped = y
+func (path *Path) SetRejected(y bool) {
+	path.rejected = y
 }
 
 func (path *Path) IsDropped() bool {
diff --git a/pkg/server/peer.go b/pkg/server/peer.go
index c213a9ea..a3f2be7b 100644
--- a/pkg/server/peer.go
+++ b/pkg/server/peer.go
@@ -481,7 +481,7 @@ func (peer *peer) handleUpdate(e *fsmMsg) ([]*table.Path, []bgp.RouteFamily, *bg
 				allowOwnAS := int(peer.fsm.pConf.AsPathOptions.Config.AllowOwnAs)
 				peer.fsm.lock.RUnlock()
 				if hasOwnASLoop(localAS, allowOwnAS, aspath) {
-					path.SetAsLooped(true)
+					path.SetRejected(true)
 					continue
 				}
 			}
@@ -500,6 +500,7 @@ func (peer *peer) handleUpdate(e *fsmMsg) ([]*table.Path, []bgp.RouteFamily, *bg
 						"OriginatorID": id,
 						"Data":         path,
 					}).Debug("Originator ID is mine, ignore")
+					path.SetRejected(true)
 					continue
 				}
 			}
diff --git a/pkg/server/server.go b/pkg/server/server.go
index 2312b39d..f95a988c 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -2255,34 +2255,7 @@ func (s *BgpServer) softResetIn(addr string, family bgp.RouteFamily) error {
 		return err
 	}
 	for _, peer := range peers {
-		families := familiesForSoftreset(peer, family)
-
-		pathList := make([]*table.Path, 0, peer.adjRibIn.Count(families))
-		for _, path := range peer.adjRibIn.PathList(families, false) {
-			// RFC4271 9.1.2 Phase 2: Route Selection
-			//
-			// If the AS_PATH attribute of a BGP route contains an AS loop, the BGP
-			// route should be excluded from the Phase 2 decision function.
-			isLooped := false
-			if aspath := path.GetAsPath(); aspath != nil {
-				peer.fsm.lock.RLock()
-				localAS := peer.fsm.peerInfo.LocalAS
-				allowOwnAS := int(peer.fsm.pConf.AsPathOptions.Config.AllowOwnAs)
-				peer.fsm.lock.RUnlock()
-				isLooped = hasOwnASLoop(localAS, allowOwnAS, aspath)
-			}
-			if path.IsAsLooped() != isLooped {
-				// can't modify the existing one. needs to create one
-				path = path.Clone(false)
-				path.SetAsLooped(isLooped)
-				// update accepted counter
-				peer.adjRibIn.Update([]*table.Path{path})
-			}
-			if !path.IsAsLooped() {
-				pathList = append(pathList, path)
-			}
-		}
-		s.propagateUpdate(peer, pathList)
+		s.propagateUpdate(peer, peer.adjRibIn.PathList(familiesForSoftreset(peer, family), true))
 	}
 	return err
 }
@@ -3007,7 +2980,6 @@ func (s *BgpServer) updateNeighbor(c *config.Neighbor) (needsSoftResetIn bool, e
 			"Topic": "Peer",
 			"Key":   peer.ID(),
 		}).Info("Update aspath options")
-		peer.fsm.pConf.AsPathOptions = c.AsPathOptions
 		needsSoftResetIn = true
 	}
 
diff --git a/test/scenario_test/aspath_test.py b/test/scenario_test/aspath_test.py
index c93cc7e5..e8227adf 100644
--- a/test/scenario_test/aspath_test.py
+++ b/test/scenario_test/aspath_test.py
@@ -91,7 +91,7 @@ class GoBGPTestBase(unittest.TestCase):
     def test_03_update_peer(self):
         self.g2.update_peer(self.q1, allow_as_in=10)
 
-        self.q1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=self.g2)
+        self.g2.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=self.q1)
 
     def test_04_check_accept_as_loop(self):
         def f():
@@ -102,6 +102,7 @@ class GoBGPTestBase(unittest.TestCase):
             for afisafi in r['afi_safis']:
                 self.assertTrue('state' in afisafi)
                 s = afisafi.get('state')
+                self.assertTrue('received' in s)
                 received += s.get('received')
                 accepted += s.get('accepted')
             self.assertEqual(received, 1)