server: support route reflector behavior

scenario_test is also added

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

8 files changed, 268 insertions, 37 deletions

diff --git a/server/peer.go b/server/peer.go
index 70d464ec..80d4d0dc 100644
--- a/server/peer.go
+++ b/server/peer.go
@@ -44,7 +44,6 @@ type Peer struct {
 	inPolicies            []*policy.Policy
 	defaultInPolicy       config.DefaultPolicyType
 	isConfederationMember bool
-	isEBGP                bool
 }
 
 func NewPeer(g config.Global, conf config.Neighbor) *Peer {
@@ -61,17 +60,19 @@ func NewPeer(g config.Global, conf config.Neighbor) *Peer {
 		k, _ := bgp.GetRouteFamily(rf.AfiSafiName)
 		peer.rfMap[k] = true
 	}
+	id := net.ParseIP(string(conf.RouteReflector.RouteReflectorConfig.RouteReflectorClusterId)).To4()
 	peer.peerInfo = &table.PeerInfo{
-		AS:      conf.NeighborConfig.PeerAs,
-		LocalAS: g.GlobalConfig.As,
-		LocalID: g.GlobalConfig.RouterId,
-		Address: conf.NeighborConfig.NeighborAddress,
+		AS:                      conf.NeighborConfig.PeerAs,
+		LocalAS:                 g.GlobalConfig.As,
+		LocalID:                 g.GlobalConfig.RouterId,
+		Address:                 conf.NeighborConfig.NeighborAddress,
+		RouteReflectorClient:    peer.isRouteReflectorClient(),
+		RouteReflectorClusterID: id,
 	}
 	peer.adjRib = table.NewAdjRib(peer.configuredRFlist())
 	peer.fsm = NewFSM(&g, &conf)
 
 	if conf.NeighborConfig.PeerAs != g.GlobalConfig.As {
-		peer.isEBGP = true
 		for _, member := range g.Confederation.ConfederationConfig.MemberAs {
 			if member == conf.NeighborConfig.PeerAs {
 				peer.isConfederationMember = true
@@ -83,10 +84,22 @@ func NewPeer(g config.Global, conf config.Neighbor) *Peer {
 	return peer
 }
 
+func (peer *Peer) isEBGPPeer() bool {
+	return peer.conf.NeighborConfig.PeerAs != peer.gConf.GlobalConfig.As
+}
+
+func (peer *Peer) isIBGPPeer() bool {
+	return peer.conf.NeighborConfig.PeerAs == peer.gConf.GlobalConfig.As
+}
+
 func (peer *Peer) isRouteServerClient() bool {
 	return peer.conf.RouteServer.RouteServerConfig.RouteServerClient
 }
 
+func (peer *Peer) isRouteReflectorClient() bool {
+	return peer.conf.RouteReflector.RouteReflectorConfig.RouteReflectorClient
+}
+
 func (peer *Peer) configuredRFlist() []bgp.RouteFamily {
 	rfList := []bgp.RouteFamily{}
 	for _, rf := range peer.conf.AfiSafis.AfiSafiList {
@@ -177,7 +190,7 @@ func (peer *Peer) handleBGPmessage(m *bgp.BGPMessage) ([]*table.Path, bool, []*b
 		update = true
 		peer.conf.Timers.TimersState.UpdateRecvTime = time.Now().Unix()
 		body := m.Body.(*bgp.BGPUpdate)
-		confedCheckRequired := !peer.isConfederationMember && peer.isEBGP
+		confedCheckRequired := !peer.isConfederationMember && peer.isEBGPPeer()
 		_, err := bgp.ValidateUpdateMsg(body, peer.rfMap, confedCheckRequired)
 		if err != nil {
 			log.WithFields(log.Fields{
@@ -213,8 +226,7 @@ func (peer *Peer) startFSMHandler(incoming chan *fsmMsg) {
 }
 
 func (peer *Peer) PassConn(conn *net.TCPConn) {
-	isEBGP := peer.gConf.GlobalConfig.As != peer.conf.NeighborConfig.PeerAs
-	if isEBGP {
+	if peer.isEBGPPeer() {
 		ttl := 1
 		SetTcpTTLSockopts(conn, ttl)
 	}
diff --git a/server/server.go b/server/server.go
index 51b84c73..92e3d178 100644
--- a/server/server.go
+++ b/server/server.go
@@ -378,16 +378,56 @@ func filterpath(peer *Peer, pathList []*table.Path) []*table.Path {
 			continue
 		}
 
-		selfGenerated := path.GetSource().ID == nil
-		fromAS := path.GetSource().AS
-		myAS := peer.gConf.GlobalConfig.As
-		if !selfGenerated && !peer.isEBGP && myAS == fromAS {
-			log.WithFields(log.Fields{
-				"Topic": "Peer",
-				"Key":   peer.conf.NeighborConfig.NeighborAddress,
-				"Data":  path,
-			}).Debug("From same AS, ignore.")
-			continue
+		//iBGP handling
+		if !path.IsLocal() && peer.isIBGPPeer() {
+			ignore := true
+			info := path.GetSource()
+
+			//if the path comes from eBGP peer
+			if info.AS != peer.conf.NeighborConfig.PeerAs {
+				ignore = false
+			}
+			// RFC4456 8. Avoiding Routing Information Loops
+			// A router that recognizes the ORIGINATOR_ID attribute SHOULD
+			// ignore a route received with its BGP Identifier as the ORIGINATOR_ID.
+			if id := path.GetOriginatorID(); peer.gConf.GlobalConfig.RouterId.Equal(id) {
+				log.WithFields(log.Fields{
+					"Topic":        "Peer",
+					"Key":          peer.conf.NeighborConfig.NeighborAddress,
+					"OriginatorID": id,
+					"Data":         path,
+				}).Debug("Originator ID is mine, ignore")
+				continue
+			}
+			if info.RouteReflectorClient {
+				ignore = false
+			}
+			if peer.isRouteReflectorClient() {
+				// RFC4456 8. Avoiding Routing Information Loops
+				// If the local CLUSTER_ID is found in the CLUSTER_LIST,
+				// the advertisement received SHOULD be ignored.
+				for _, clusterId := range path.GetClusterList() {
+					if clusterId.Equal(peer.peerInfo.RouteReflectorClusterID) {
+						log.WithFields(log.Fields{
+							"Topic":     "Peer",
+							"Key":       peer.conf.NeighborConfig.NeighborAddress,
+							"ClusterID": clusterId,
+							"Data":      path,
+						}).Debug("cluster list path attribute has local cluster id, ignore")
+						continue
+					}
+				}
+				ignore = false
+			}
+
+			if ignore {
+				log.WithFields(log.Fields{
+					"Topic": "Peer",
+					"Key":   peer.conf.NeighborConfig.NeighborAddress,
+					"Data":  path,
+				}).Debug("From same AS, ignore.")
+				continue
+			}
 		}
 
 		if peer.conf.NeighborConfig.NeighborAddress.Equal(path.GetSource().Address) {
@@ -568,14 +608,14 @@ func (server *BgpServer) broadcastPeerState(peer *Peer) {
 	server.broadcastReqs = remainReqs
 }
 
-func (server *BgpServer) propagateUpdate(neighborAddress string, RouteServerClient bool, pathList []*table.Path) []*SenderMsg {
+func (server *BgpServer) propagateUpdate(peer *Peer, pathList []*table.Path) []*SenderMsg {
 	msgs := make([]*SenderMsg, 0)
 
-	if RouteServerClient {
-		p := server.neighborMap[neighborAddress]
-		newPathList := applyPolicies(p, nil, POLICY_DIRECTION_IN, pathList)
+	if peer != nil && peer.isRouteServerClient() {
+		newPathList := applyPolicies(peer, nil, POLICY_DIRECTION_IN, pathList)
 		for _, loc := range server.localRibMap {
 			targetPeer := server.neighborMap[loc.OwnerName()]
+			neighborAddress := peer.conf.NeighborConfig.NeighborAddress.String()
 			if loc.isGlobal() || loc.OwnerName() == neighborAddress {
 				continue
 			}
@@ -695,8 +735,7 @@ func (server *BgpServer) handleFSMMessage(peer *Peer, e *fsmMsg, incoming chan *
 					server.roaClient.validate(pathList)
 				}
 			}
-			msgs = append(msgs, server.propagateUpdate(peer.conf.NeighborConfig.NeighborAddress.String(),
-				peer.isRouteServerClient(), pathList)...)
+			msgs = append(msgs, server.propagateUpdate(peer, pathList)...)
 		default:
 			log.WithFields(log.Fields{
 				"Topic": "Peer",
@@ -1156,7 +1195,7 @@ func (server *BgpServer) handleGrpc(grpcReq *GrpcRequest) []*SenderMsg {
 	case REQ_MOD_PATH:
 		pathList := server.handleModPathRequest(grpcReq)
 		if len(pathList) > 0 {
-			msgs = server.propagateUpdate("", false, pathList)
+			msgs = server.propagateUpdate(nil, pathList)
 			grpcReq.ResponseCh <- &GrpcResponse{}
 			close(grpcReq.ResponseCh)
 		}
@@ -1282,7 +1321,7 @@ func (server *BgpServer) handleGrpc(grpcReq *GrpcRequest) []*SenderMsg {
 			break
 		}
 		pathList := peer.adjRib.GetInPathList(grpcReq.RouteFamily)
-		msgs = server.propagateUpdate(peer.conf.NeighborConfig.NeighborAddress.String(), peer.isRouteServerClient(), pathList)
+		msgs = server.propagateUpdate(peer, pathList)
 
 		if grpcReq.RequestType == REQ_NEIGHBOR_SOFT_RESET_IN {
 			grpcReq.ResponseCh <- &GrpcResponse{}
@@ -1488,7 +1527,7 @@ func (server *BgpServer) handleGrpc(grpcReq *GrpcRequest) []*SenderMsg {
 	case REQ_VRF, REQ_VRFS, REQ_VRF_MOD:
 		pathList := server.handleVrfRequest(grpcReq)
 		if len(pathList) > 0 {
-			msgs = server.propagateUpdate("", false, pathList)
+			msgs = server.propagateUpdate(nil, pathList)
 		}
 	default:
 		errmsg := fmt.Errorf("Unknown request type: %v", grpcReq.RequestType)
diff --git a/table/destination.go b/table/destination.go
index 290830a4..5889fb5b 100644
--- a/table/destination.go
+++ b/table/destination.go
@@ -52,11 +52,13 @@ func CidrToRadixkey(cidr string) string {
 }
 
 type PeerInfo struct {
-	AS      uint32
-	ID      net.IP
-	LocalAS uint32
-	LocalID net.IP
-	Address net.IP
+	AS                      uint32
+	ID                      net.IP
+	LocalAS                 uint32
+	LocalID                 net.IP
+	Address                 net.IP
+	RouteReflectorClient    bool
+	RouteReflectorClusterID net.IP
 }
 
 func (lhs *PeerInfo) Equal(rhs *PeerInfo) bool {
diff --git a/table/path.go b/table/path.go
index 62f0e437..6dbb1f24 100644
--- a/table/path.go
+++ b/table/path.go
@@ -118,6 +118,32 @@ func (path *Path) UpdatePathAttrs(global *config.Global, peer *config.Neighbor)
 		} else {
 			path.pathAttrs[idx] = p
 		}
+
+		// RFC4456: BGP Route Reflection
+		// 8. Avoiding Routing Information Loops
+		info := path.source
+		if peer.RouteReflector.RouteReflectorConfig.RouteReflectorClient {
+			// This attribute will carry the BGP Identifier of the originator of the route in the local AS.
+			// A BGP speaker SHOULD NOT create an ORIGINATOR_ID attribute if one already exists.
+			idx, _ = path.getPathAttr(bgp.BGP_ATTR_TYPE_ORIGINATOR_ID)
+			if idx < 0 {
+				p := bgp.NewPathAttributeOriginatorId(info.ID.String())
+				path.pathAttrs = append(path.pathAttrs, p)
+			}
+			// When an RR reflects a route, it MUST prepend the local CLUSTER_ID to the CLUSTER_LIST.
+			// If the CLUSTER_LIST is empty, it MUST create a new one.
+			idx, _ = path.getPathAttr(bgp.BGP_ATTR_TYPE_CLUSTER_LIST)
+			id := string(peer.RouteReflector.RouteReflectorConfig.RouteReflectorClusterId)
+			if idx < 0 {
+				p := bgp.NewPathAttributeClusterList([]string{id})
+				path.pathAttrs = append(path.pathAttrs, p)
+			} else {
+				p := path.pathAttrs[idx].(*bgp.PathAttributeClusterList)
+				p.Value = append([]net.IP{net.ParseIP(id).To4()}, p.Value...)
+				path.pathAttrs[idx] = p
+			}
+		}
+
 	} else {
 		log.WithFields(log.Fields{
 			"Topic": "Peer",
@@ -582,6 +608,20 @@ func (path *Path) SetMed(med int64, doReplace bool) error {
 	return nil
 }
 
+func (path *Path) GetOriginatorID() net.IP {
+	if _, attr := path.getPathAttr(bgp.BGP_ATTR_TYPE_ORIGINATOR_ID); attr != nil {
+		return attr.(*bgp.PathAttributeOriginatorId).Value
+	}
+	return nil
+}
+
+func (path *Path) GetClusterList() []net.IP {
+	if _, attr := path.getPathAttr(bgp.BGP_ATTR_TYPE_CLUSTER_LIST); attr != nil {
+		return attr.(*bgp.PathAttributeClusterList).Value
+	}
+	return nil
+}
+
 func (lhs *Path) Equal(rhs *Path) bool {
 	if rhs == nil {
 		return false
diff --git a/test/scenario_test/lib/base.py b/test/scenario_test/lib/base.py
index 1bba6f5b..7f73516b 100644
--- a/test/scenario_test/lib/base.py
+++ b/test/scenario_test/lib/base.py
@@ -224,6 +224,9 @@ class BGPContainer(Container):
         self.policies = {}
         super(BGPContainer, self).__init__(name, ctn_image_name)
 
+    def __repr__(self):
+        return str({'name':self.name, 'asn':self.asn, 'router_id':self.router_id})
+
     def run(self):
         self.create_config()
         super(BGPContainer, self).run()
@@ -231,7 +234,7 @@ class BGPContainer(Container):
 
     def add_peer(self, peer, passwd=None, evpn=False, is_rs_client=False,
                  policies=None, passive=False,
-                 is_rr_client=False, cluster_id='',
+                 is_rr_client=False, cluster_id=None,
                  flowspec=False):
         neigh_addr = ''
         local_addr = ''
diff --git a/test/scenario_test/lib/gobgp.py b/test/scenario_test/lib/gobgp.py
index aed7f16d..04b89ffe 100644
--- a/test/scenario_test/lib/gobgp.py
+++ b/test/scenario_test/lib/gobgp.py
@@ -186,9 +186,11 @@ class GoBGPContainer(BGPContainer):
                 n['RouteServer'] = {'RouteServerConfig': {'RouteServerClient': True}}
 
             if info['is_rr_client']:
-                clusterId = info['cluster_id']
-                n['RouteReflector'] = {'RouteReflectorClient': True,
-                                       'RouteReflectorClusterId': clusterId}
+                clusterId = self.router_id
+                if 'cluster_id' in info and info['cluster_id'] is not None:
+                    clusterId = info['cluster_id']
+                n['RouteReflector'] = {'RouteReflectorConfig' : {'RouteReflectorClient': True,
+                                                                 'RouteReflectorClusterId': clusterId}}
 
             f = lambda typ: [p for p in info['policies'].itervalues() if p['type'] == typ]
             import_policies = f('import')
diff --git a/test/scenario_test/route_reflector_test.py b/test/scenario_test/route_reflector_test.py
new file mode 100644
index 00000000..7650a4ac
--- /dev/null
+++ b/test/scenario_test/route_reflector_test.py
@@ -0,0 +1,129 @@
+# Copyright (C) 2015 Nippon Telegraph and Telephone Corporation.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import unittest
+from fabric.api import local
+from lib import base
+from lib.gobgp import *
+from lib.quagga import *
+import sys
+import os
+import time
+import nose
+from noseplugin import OptionParser, parser_option
+from itertools import combinations
+
+def wait_for(f, timeout=120):
+    interval = 1
+    count = 0
+    while True:
+        if f():
+            return
+
+        time.sleep(interval)
+        count += interval
+        if count >= timeout:
+            raise Exception('timeout')
+
+
+class GoBGPTestBase(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        gobgp_ctn_image_name = parser_option.gobgp_image
+        base.TEST_PREFIX = parser_option.test_prefix
+
+        g1 = GoBGPContainer(name='g1', asn=65000, router_id='192.168.0.1',
+                            ctn_image_name=gobgp_ctn_image_name,
+                            log_level=parser_option.gobgp_log_level)
+        q1 = QuaggaBGPContainer(name='q1', asn=65000, router_id='192.168.0.2')
+        q2 = QuaggaBGPContainer(name='q2', asn=65000, router_id='192.168.0.3')
+        q3 = QuaggaBGPContainer(name='q3', asn=65000, router_id='192.168.0.4')
+        q4 = QuaggaBGPContainer(name='q4', asn=65000, router_id='192.168.0.5')
+
+        qs = [q1, q2, q3, q4]
+        ctns = [g1, q1, q2, q3, q4]
+
+        # advertise a route from q1, q2
+        for idx, c in enumerate(qs):
+            route = '10.0.{0}.0/24'.format(idx+1)
+            c.add_route(route)
+
+        initial_wait_time = max(ctn.run() for ctn in ctns)
+
+        time.sleep(initial_wait_time)
+
+        br01 = Bridge(name='br01', subnet='192.168.10.0/24')
+        [br01.addif(ctn) for ctn in ctns]
+
+        # g1 as a route reflector
+        g1.add_peer(q1, is_rr_client=True)
+        q1.add_peer(g1)
+        g1.add_peer(q2, is_rr_client=True)
+        q2.add_peer(g1)
+        g1.add_peer(q3)
+        q3.add_peer(g1)
+        g1.add_peer(q4)
+        q4.add_peer(g1)
+
+        cls.gobgp = g1
+        cls.quaggas = {'q1': q1, 'q2': q2, 'q3': q3, 'q4': q4}
+        cls.bridges = {'br01': br01}
+
+    # test each neighbor state is turned establish
+    def test_01_neighbor_established(self):
+        for q in self.quaggas.itervalues():
+            self.gobgp.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=q)
+
+    def test_02_check_gobgp_global_rib(self):
+        for q in self.quaggas.itervalues():
+            # paths expected to exist in gobgp's global rib
+            def f():
+                routes = q.routes.keys()
+                global_rib = [p['prefix'] for p in self.gobgp.get_global_rib()]
+                for p in global_rib:
+                    if p in routes:
+                        routes.remove(p)
+
+                return len(routes) == 0
+            wait_for(f)
+ 
+    def test_03_check_gobgp_adj_rib_out(self):
+        for q in self.quaggas.itervalues():
+            paths = [p['nlri']['prefix'] for p in self.gobgp.get_adj_rib_out(q)]
+            for qq in self.quaggas.itervalues():
+                if q == qq:
+                    continue
+                if self.gobgp.peers[q]['is_rr_client']:
+                    for p in qq.routes.keys():
+                        self.assertTrue(p in paths)
+                else:
+                    for p in qq.routes.keys():
+                        if self.gobgp.peers[qq]['is_rr_client']:
+                            self.assertTrue(p in paths)
+                        else:
+                            self.assertFalse(p in paths)
+
+if __name__ == '__main__':
+    if os.geteuid() is not 0:
+        print "you are not root."
+        sys.exit(1)
+    output = local("which docker 2>&1 > /dev/null ; echo $?", capture=True)
+    if int(output) is not 0:
+        print "docker not found"
+        sys.exit(1)
+
+    nose.main(argv=sys.argv, addplugins=[OptionParser()],
+              defaultTest=sys.argv[0])
diff --git a/test/scenario_test/run_all_tests.sh b/test/scenario_test/run_all_tests.sh
index 87999c91..d6f72d54 100755
--- a/test/scenario_test/run_all_tests.sh
+++ b/test/scenario_test/run_all_tests.sh
@@ -55,6 +55,10 @@ PIDS=("${PIDS[@]}" $!)
 sudo -E python flow_spec_test.py --gobgp-image $GOBGP_IMAGE --test-prefix flow -s -x --with-xunit --xunit-file=${WS}/nosetest_flow.xml &
 PIDS=("${PIDS[@]}" $!)
 
+# flowspec test
+sudo -E python route_reflector_test.py --gobgp-image $GOBGP_IMAGE --test-prefix rr -s -x --with-xunit --xunit-file=${WS}/nosetest_rr.xml &
+PIDS=("${PIDS[@]}" $!)
+
 # route server malformed message test
 NUM=$(sudo -E python route_server_malformed_test.py -s 2> /dev/null | awk '/invalid/{print $NF}')
 PARALLEL_NUM=10