policy: fix bug of export neighbor condition matching

export neighbor condition must match to the remote address of
destination peer.

Signed-off-by: ISHIDA Wataru <ishida.wataru@lab.ntt.co.jp>

5 files changed, 36 insertions, 25 deletions

diff --git a/policy/policy.go b/policy/policy.go
index 2a1524af..839cb926 100644
--- a/policy/policy.go
+++ b/policy/policy.go
@@ -296,10 +296,12 @@ func (c *NeighborCondition) evaluate(path *table.Path) bool {
 		return true
 	}
 
-	sAddr := path.GetSource().Address
+	if path.Owner == nil {
+		return false
+	}
 	result := false
 	for _, neighbor := range c.NeighborList {
-		if sAddr.Equal(neighbor) {
+		if path.Owner.Equal(neighbor) {
 			result = true
 			break
 		}
@@ -312,7 +314,7 @@ func (c *NeighborCondition) evaluate(path *table.Path) bool {
 	log.WithFields(log.Fields{
 		"Topic":           "Policy",
 		"Condition":       "neighbor",
-		"NeighborAddress": sAddr.String(),
+		"NeighborAddress": path.Owner,
 		"Matched":         result,
 	}).Debug("evaluate neighbor")
 
@@ -1406,7 +1408,7 @@ func (p *Policy) Apply(path *table.Path) (RouteType, *table.Path) {
 				return ROUTE_TYPE_ACCEPT, path
 			}
 			// apply all modification actions
-			cloned := path.Clone(p.IsWithdraw)
+			cloned := path.Clone(p.Owner, p.IsWithdraw)
 			for _, action := range statement.modificationActions {
 				cloned = action.apply(cloned)
 			}
diff --git a/server/server.go b/server/server.go
index 08775593..2900f55b 100644
--- a/server/server.go
+++ b/server/server.go
@@ -470,6 +470,8 @@ func filterpath(peer *Peer, pathList []*table.Path) []*table.Path {
 			continue
 		}
 
+		remoteAddr := peer.conf.NeighborConfig.NeighborAddress
+
 		//iBGP handling
 		if !path.IsLocal() && peer.isIBGPPeer() {
 			ignore := true
@@ -485,7 +487,7 @@ func filterpath(peer *Peer, pathList []*table.Path) []*table.Path {
 			if id := path.GetOriginatorID(); peer.gConf.GlobalConfig.RouterId.Equal(id) {
 				log.WithFields(log.Fields{
 					"Topic":        "Peer",
-					"Key":          peer.conf.NeighborConfig.NeighborAddress,
+					"Key":          remoteAddr,
 					"OriginatorID": id,
 					"Data":         path,
 				}).Debug("Originator ID is mine, ignore")
@@ -502,7 +504,7 @@ func filterpath(peer *Peer, pathList []*table.Path) []*table.Path {
 					if clusterId.Equal(peer.peerInfo.RouteReflectorClusterID) {
 						log.WithFields(log.Fields{
 							"Topic":     "Peer",
-							"Key":       peer.conf.NeighborConfig.NeighborAddress,
+							"Key":       remoteAddr,
 							"ClusterID": clusterId,
 							"Data":      path,
 						}).Debug("cluster list path attribute has local cluster id, ignore")
@@ -515,17 +517,17 @@ func filterpath(peer *Peer, pathList []*table.Path) []*table.Path {
 			if ignore {
 				log.WithFields(log.Fields{
 					"Topic": "Peer",
-					"Key":   peer.conf.NeighborConfig.NeighborAddress,
+					"Key":   remoteAddr,
 					"Data":  path,
 				}).Debug("From same AS, ignore.")
 				continue
 			}
 		}
 
-		if peer.conf.NeighborConfig.NeighborAddress.Equal(path.GetSource().Address) {
+		if remoteAddr.Equal(path.GetSource().Address) {
 			log.WithFields(log.Fields{
 				"Topic": "Peer",
-				"Key":   peer.conf.NeighborConfig.NeighborAddress,
+				"Key":   remoteAddr,
 				"Data":  path,
 			}).Debug("From me, ignore.")
 			continue
@@ -542,12 +544,12 @@ func filterpath(peer *Peer, pathList []*table.Path) []*table.Path {
 		if !send {
 			log.WithFields(log.Fields{
 				"Topic": "Peer",
-				"Key":   peer.conf.NeighborConfig.NeighborAddress,
+				"Key":   remoteAddr,
 				"Data":  path,
 			}).Debug("AS PATH loop, ignore.")
 			continue
 		}
-		filtered = append(filtered, path.Clone(path.IsWithdraw))
+		filtered = append(filtered, path.Clone(remoteAddr, path.IsWithdraw))
 	}
 	return filtered
 }
@@ -671,11 +673,11 @@ func (server *BgpServer) propagateUpdate(peer *Peer, pathList []*table.Path) []*
 			if rib.OwnerName() == GLOBAL_RIB_NAME || rib.OwnerName() == neighborAddress {
 				continue
 			}
-			sendPathList, _ := rib.ProcessPaths(targetPeer.ApplyPolicy(POLICY_DIRECTION_IMPORT, filterpath(targetPeer, pathList)))
+			sendPathList, _ := rib.ProcessPaths(targetPeer.ApplyPolicy(POLICY_DIRECTION_IMPORT, pathList))
 			if targetPeer.fsm.state != bgp.BGP_FSM_ESTABLISHED || len(sendPathList) == 0 {
 				continue
 			}
-			sendPathList = targetPeer.ApplyPolicy(POLICY_DIRECTION_EXPORT, sendPathList)
+			sendPathList = targetPeer.ApplyPolicy(POLICY_DIRECTION_EXPORT, filterpath(targetPeer, sendPathList))
 			if len(sendPathList) == 0 {
 				continue
 			}
@@ -757,15 +759,14 @@ func (server *BgpServer) handleFSMMessage(peer *Peer, e *fsmMsg, incoming chan *
 			pathList := make([]*table.Path, 0)
 			if peer.isRouteServerClient() {
 				rib := server.localRibMap[peer.conf.NeighborConfig.NeighborAddress.String()]
-				pathList = peer.ApplyPolicy(POLICY_DIRECTION_EXPORT, peer.getBests(rib))
+				pathList = peer.ApplyPolicy(POLICY_DIRECTION_EXPORT, filterpath(peer, peer.getBests(rib)))
 			} else {
 				rib := server.localRibMap[GLOBAL_RIB_NAME]
 				l, _ := peer.fsm.LocalHostPort()
 				peer.conf.Transport.TransportConfig.LocalAddress = net.ParseIP(l)
 				for _, path := range filterpath(peer, peer.getBests(rib)) {
-					p := path.Clone(path.IsWithdraw)
-					p.UpdatePathAttrs(&server.bgpConfig.Global, &peer.conf)
-					pathList = append(pathList, p)
+					path.UpdatePathAttrs(&server.bgpConfig.Global, &peer.conf)
+					pathList = append(pathList, path)
 				}
 			}
 			if len(pathList) > 0 {
diff --git a/table/path.go b/table/path.go
index 02b24416..812046bc 100644
--- a/table/path.go
+++ b/table/path.go
@@ -39,6 +39,7 @@ type Path struct {
 	Validation             config.RpkiValidationResultType
 	IsFromZebra            bool
 	Filtered               bool
+	Owner                  net.IP
 }
 
 func NewPath(source *PeerInfo, nlri bgp.AddrPrefixInterface, isWithdraw bool, pattrs []bgp.PathAttributeInterface, medSetByTargetNeighbor bool, timestamp time.Time, noImplicitWithdraw bool) *Path {
@@ -51,6 +52,11 @@ func NewPath(source *PeerInfo, nlri bgp.AddrPrefixInterface, isWithdraw bool, pa
 		return nil
 	}
 
+	var owner net.IP
+	if source != nil {
+		owner = source.Address
+	}
+
 	return &Path{
 		source:                 source,
 		IsWithdraw:             isWithdraw,
@@ -59,6 +65,7 @@ func NewPath(source *PeerInfo, nlri bgp.AddrPrefixInterface, isWithdraw bool, pa
 		medSetByTargetNeighbor: medSetByTargetNeighbor,
 		timestamp:              timestamp,
 		NoImplicitWithdraw:     noImplicitWithdraw,
+		Owner:                  owner,
 	}
 }
 
@@ -213,7 +220,7 @@ func (path *Path) MarshalJSON() ([]byte, error) {
 }
 
 // create new PathAttributes
-func (path *Path) Clone(isWithdraw bool) *Path {
+func (path *Path) Clone(owner net.IP, isWithdraw bool) *Path {
 	newPathAttrs := make([]bgp.PathAttributeInterface, len(path.pathAttrs))
 	for i, v := range path.pathAttrs {
 		newPathAttrs[i] = v
@@ -221,6 +228,7 @@ func (path *Path) Clone(isWithdraw bool) *Path {
 
 	p := NewPath(path.source, path.nlri, isWithdraw, newPathAttrs, false, path.timestamp, path.NoImplicitWithdraw)
 	p.Validation = path.Validation
+	p.Owner = owner
 	return p
 }
 
diff --git a/table/table_manager.go b/table/table_manager.go
index 25807566..6ec65617 100644
--- a/table/table_manager.go
+++ b/table/table_manager.go
@@ -265,7 +265,7 @@ func (manager *TableManager) calculate(destinationList []*Destination) ([]*Path,
 					}).Debug("best path is lost")
 
 					p := destination.GetBestPath()
-					newPaths = append(newPaths, p.Clone(true))
+					newPaths = append(newPaths, p.Clone(p.Owner, true))
 				}
 				destination.setBestPath(nil)
 			} else {
diff --git a/test/scenario_test/route_server_policy_test.py b/test/scenario_test/route_server_policy_test.py
index 6827199d..7acaecc6 100644
--- a/test/scenario_test/route_server_policy_test.py
+++ b/test/scenario_test/route_server_policy_test.py
@@ -162,7 +162,7 @@ def setup(env):
            'PrefixList': [p0]}
     g1.set_prefix_set(ps0)
 
-    n0 = {'Address': g1.peers[e1]['neigh_addr'].split('/')[0]}
+    n0 = {'Address': g1.peers[q2]['neigh_addr'].split('/')[0]}
 
     ns0 = {'NeighborSetName': 'ns0',
            'NeighborInfoList': [n0]}
@@ -362,7 +362,7 @@ def setup(env):
            'PrefixList': [p0, p1]}
     g1.set_prefix_set(ps0)
 
-    n0 = {'Address': g1.peers[e1]['neigh_addr'].split('/')[0]}
+    n0 = {'Address': g1.peers[q2]['neigh_addr'].split('/')[0]}
 
     ns0 = {'NeighborSetName': 'ns0',
            'NeighborInfoList': [n0]}
@@ -411,7 +411,7 @@ def setup2(env):
            'PrefixList': [p0]}
     g1.set_prefix_set(ps0)
 
-    n0 = {'Address': g1.peers[e1]['neigh_addr'].split('/')[0]}
+    n0 = {'Address': g1.peers[q2]['neigh_addr'].split('/')[0]}
 
     ns0 = {'NeighborSetName': 'ns0',
            'NeighborInfoList': [n0]}
@@ -560,7 +560,7 @@ def setup(env):
            'PrefixList': [p0]}
     g1.set_prefix_set(ps0)
 
-    n0 = {'Address': g1.peers[e1]['neigh_addr'].split('/')[0]}
+    n0 = {'Address': g1.peers[q2]['neigh_addr'].split('/')[0]}
 
     ns0 = {'NeighborSetName': 'ns0',
            'NeighborInfoList': [n0]}
@@ -748,7 +748,7 @@ def setup(env):
            'PrefixList': [p0, p1]}
     g1.set_prefix_set(ps0)
 
-    n0 = {'Address': g1.peers[e1]['neigh_addr'].split('/')[0]}
+    n0 = {'Address': g1.peers[q2]['neigh_addr'].split('/')[0]}
 
     ns0 = {'NeighborSetName': 'ns0',
            'NeighborInfoList': [n0]}
@@ -793,7 +793,7 @@ def setup2(env):
            'PrefixList': [p0]}
     g1.set_prefix_set(ps0)
 
-    n0 = {'Address': g1.peers[e1]['neigh_addr'].split('/')[0]}
+    n0 = {'Address': g1.peers[q2]['neigh_addr'].split('/')[0]}
 
     ns0 = {'NeighborSetName': 'ns0',
            'NeighborInfoList': [n0]}