From 0235f7c67539e199eb59e7ffb09409e07a8d13e0 Mon Sep 17 00:00:00 2001 From: mageshgv Date: Tue, 8 Oct 2019 14:35:33 -0700 Subject: Support vrfs in zapi multipath --- .travis.yml | 3 + pkg/server/server.go | 33 +++- pkg/server/server_test.go | 22 ++- pkg/server/zclient.go | 24 +-- test/lib/gobgp.py | 6 +- test/scenario_test/zapi_v3_multipath_test.py | 262 +++++++++++++++++++++++++++ 6 files changed, 319 insertions(+), 31 deletions(-) create mode 100644 test/scenario_test/zapi_v3_multipath_test.py diff --git a/.travis.yml b/.travis.yml index 2692518e..f894115d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -170,6 +170,9 @@ matrix: - <<: *_docker env: - TEST=zapi_v3_test.py FROM_IMAGE=osrg/quagga:v1.0 + - <<: *_docker + env: + - TEST=zapi_v3_multipath_test.py FROM_IMAGE=osrg/quagga:v1.0 - <<: *_docker env: - TEST=long_lived_graceful_restart_test.py diff --git a/pkg/server/server.go b/pkg/server/server.go index c5dd18f3..4fbb76ff 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -698,26 +698,39 @@ func clonePathList(pathList []*table.Path) []*table.Path { return l } +func (s *BgpServer) setPathVrfIdMap(paths []*table.Path, m map[uint32]bool) { + for _, p := range paths { + switch p.GetRouteFamily() { + case bgp.RF_IPv4_VPN, bgp.RF_IPv6_VPN: + for _, vrf := range s.globalRib.Vrfs { + if vrf.Id != 0 && table.CanImportToVrf(vrf, p) { + m[uint32(vrf.Id)] = true + } + } + default: + m[zebra.VRF_DEFAULT] = true + } + } +} + +// Note: the destination would be the same for all the paths passed here +// The wather (only zapi) needs a unique list of vrf IDs func (s *BgpServer) notifyBestWatcher(best []*table.Path, multipath [][]*table.Path) { if table.SelectionOptions.DisableBestPathSelection { // Note: If best path selection disabled, no best path to notify. return } + m := make(map[uint32]bool) clonedM := make([][]*table.Path, len(multipath)) for i, pathList := range multipath { clonedM[i] = clonePathList(pathList) + if table.UseMultiplePaths.Enabled { + s.setPathVrfIdMap(clonedM[i], m) + } } clonedB := clonePathList(best) - m := make(map[uint32]bool) - for _, p := range clonedB { - switch p.GetRouteFamily() { - case bgp.RF_IPv4_VPN, bgp.RF_IPv6_VPN: - for _, vrf := range s.globalRib.Vrfs { - if vrf.Id != 0 && table.CanImportToVrf(vrf, p) { - m[uint32(vrf.Id)] = true - } - } - } + if !table.UseMultiplePaths.Enabled { + s.setPathVrfIdMap(clonedB, m) } w := &watchEventBestPath{PathList: clonedB, MultiPathList: clonedM} if len(m) > 0 { diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index a390ce58..3cbf53a9 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -533,7 +533,8 @@ func TestMonitor(test *testing.T) { assert.Equal(1, len(b.PathList)) assert.Equal("10.0.0.0/24", b.PathList[0].GetNlri().String()) assert.False(b.PathList[0].IsWithdraw) - assert.Equal(0, len(b.Vrf)) + assert.Equal(1, len(b.Vrf)) + assert.True(b.Vrf[0]) // Withdraws the previous route. // NOTE: Withdraw should not require any path attribute. @@ -545,7 +546,8 @@ func TestMonitor(test *testing.T) { assert.Equal(1, len(b.PathList)) assert.Equal("10.0.0.0/24", b.PathList[0].GetNlri().String()) assert.True(b.PathList[0].IsWithdraw) - assert.Equal(0, len(b.Vrf)) + assert.Equal(1, len(b.Vrf)) + assert.True(b.Vrf[0]) // Stops the watcher still having an item. w.Stop() @@ -590,7 +592,7 @@ func TestMonitor(test *testing.T) { assert.False(u.PathList[0].IsWithdraw) // Withdraws the previous route. - // NOTE: Withdow should not require any path attribute. + // NOTE: Withdraw should not require any path attribute. if err := t.addPathList("", []*table.Path{table.NewPath(nil, bgp.NewIPAddrPrefix(24, "10.2.0.0"), true, nil, time.Now(), false)}); err != nil { log.Fatal(err) } @@ -620,7 +622,19 @@ func TestMonitor(test *testing.T) { assert.True(b.Vrf[1]) assert.True(b.Vrf[2]) - // Stops the watcher still having an item. + // Withdraw the route + if err := s.addPathList("vrf1", []*table.Path{table.NewPath(nil, bgp.NewIPAddrPrefix(24, "10.0.0.0"), true, attrs, time.Now(), false)}); err != nil { + log.Fatal(err) + } + ev = <-w.Event() + b = ev.(*watchEventBestPath) + assert.Equal(1, len(b.PathList)) + assert.Equal("111:111:10.0.0.0/24", b.PathList[0].GetNlri().String()) + assert.True(b.PathList[0].IsWithdraw) + assert.Equal(2, len(b.Vrf)) + assert.True(b.Vrf[1]) + assert.True(b.Vrf[2]) + w.Stop() } diff --git a/pkg/server/zclient.go b/pkg/server/zclient.go index 390628f0..c002851a 100644 --- a/pkg/server/zclient.go +++ b/pkg/server/zclient.go @@ -421,27 +421,19 @@ func (z *zebraClient) loop() { if table.UseMultiplePaths.Enabled { for _, paths := range msg.MultiPathList { z.updatePathByNexthopCache(paths) - if body, isWithdraw := newIPRouteBody(paths, 0, z); body != nil { - z.client.SendIPRoute(0, body, isWithdraw) - } - if body := newNexthopRegisterBody(paths, z.nexthopCache); body != nil { - z.client.SendNexthopRegister(0, body, false) + for i := range msg.Vrf { + if body, isWithdraw := newIPRouteBody(paths, i, z); body != nil { + z.client.SendIPRoute(i, body, isWithdraw) + } + if body := newNexthopRegisterBody(paths, z.nexthopCache); body != nil { + z.client.SendNexthopRegister(i, body, false) + } } } } else { z.updatePathByNexthopCache(msg.PathList) for _, path := range msg.PathList { - vrfs := []uint32{0} - if msg.Vrf != nil { - for vrfId := range msg.Vrf { - vrfs = append(vrfs, vrfId) - } - } - for _, i := range vrfs { - routeFamily := path.GetRouteFamily() - if i == zebra.VRF_DEFAULT && (routeFamily == bgp.RF_IPv4_VPN || routeFamily == bgp.RF_IPv6_VPN) { - continue - } + for i := range msg.Vrf { if body, isWithdraw := newIPRouteBody([]*table.Path{path}, i, z); body != nil { err := z.client.SendIPRoute(i, body, isWithdraw) if err != nil { diff --git a/test/lib/gobgp.py b/test/lib/gobgp.py index 9384fd2a..1abdb782 100644 --- a/test/lib/gobgp.py +++ b/test/lib/gobgp.py @@ -62,7 +62,8 @@ class GoBGPContainer(BGPContainer): def __init__(self, name, asn, router_id, ctn_image_name='osrg/gobgp', log_level='debug', zebra=False, config_format='toml', - zapi_version=2, bgp_config=None, ospfd_config=None): + zapi_version=2, bgp_config=None, ospfd_config=None, + zebra_multipath_enabled=False): super(GoBGPContainer, self).__init__(name, asn, router_id, ctn_image_name) self.shared_volumes.append((self.config_dir, self.SHARED_VOLUME)) @@ -77,6 +78,7 @@ class GoBGPContainer(BGPContainer): self.default_policy = None self.zebra = zebra self.zapi_version = zapi_version + self.zebra_multipath_enabled = zebra_multipath_enabled self.config_format = config_format # bgp_config is equivalent to config.BgpConfigSet structure @@ -378,6 +380,8 @@ class GoBGPContainer(BGPContainer): if self.zebra and self.zapi_version == 2: config['global']['use-multiple-paths'] = {'config': {'enabled': True}} + else: + config['global']['use-multiple-paths'] = {'config': {'enabled': self.zebra_multipath_enabled}} for peer, info in self.peers.items(): afi_safi_list = [] diff --git a/test/scenario_test/zapi_v3_multipath_test.py b/test/scenario_test/zapi_v3_multipath_test.py new file mode 100644 index 00000000..53ed9d2a --- /dev/null +++ b/test/scenario_test/zapi_v3_multipath_test.py @@ -0,0 +1,262 @@ +# 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 sys +import time +import unittest + +import nose + +from lib.noseplugin import OptionParser, parser_option + +from lib import base +from lib.base import BGP_FSM_ESTABLISHED, local +from lib.gobgp import GoBGPContainer + + +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, + zebra=True, zapi_version=3, zebra_multipath_enabled=True) + + g2 = GoBGPContainer(name='g2', asn=65001, router_id='192.168.0.2', + ctn_image_name=gobgp_ctn_image_name, + log_level=parser_option.gobgp_log_level, + zebra=True, zapi_version=3, zebra_multipath_enabled=True) + + g3 = GoBGPContainer(name='g3', asn=65001, router_id='192.168.0.3', + ctn_image_name=gobgp_ctn_image_name, + log_level=parser_option.gobgp_log_level, + zebra=True, zapi_version=3, zebra_multipath_enabled=True) + + initial_wait_time = max(ctn.run() for ctn in [g1, g2, g3]) + + time.sleep(initial_wait_time) + + g1.add_peer(g2, vpn=True, addpath=True) + g2.add_peer(g1, vpn=True, addpath=True) + g1.add_peer(g3, vpn=True, addpath=True) + g3.add_peer(g1, vpn=True, addpath=True) + + cls.g1 = g1 + cls.g2 = g2 + cls.g3 = g3 + + """ + # Multipath route + 10.0.0.0/24 proto zebra metric 20 + nexthop via 127.0.0.1 dev lo weight 1 + nexthop via 127.0.0.2 dev lo weight 1 + # Single nexthop route + 10.0.0.0/24 via 127.0.0.2 dev lo proto zebra metric 20 + """ + def parse_ip_route(self, ip_route_output): + routes = {} + current_mpath_dest = "" + for line in ip_route_output: + tokens = line.split() + if len(tokens) == 0: + continue + if tokens[0] == "nexthop": + # multipath nexthops + routes[current_mpath_dest].add(tokens[2]) + elif tokens[1] == "via": + # single nexthop route + routes[tokens[0]] = set([tokens[2]]) + current_mpath_dest = "" + elif tokens[1] == "proto": + # multipath route line 1 + routes[tokens[0]] = set() + current_mpath_dest = tokens[0] + return routes + + def test_01_neighbors_established(self): + self.g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=self.g2) + self.g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=self.g3) + + def test_02_add_multipath_vrf_route(self): + self.g1.local('ip netns add ns01') + self.g1.local('ip netns add ns02') + self.g2.local('ip netns add ns01') + self.g2.local('ip netns add ns02') + self.g3.local('ip netns add ns01') + self.g3.local('ip netns add ns02') + + self.g1.local('ip netns exec ns01 ip li set up dev lo') + self.g1.local('ip netns exec ns01 ip addr add 127.0.0.2/8 dev lo') + self.g1.local('ip netns exec ns02 ip li set up dev lo') + self.g1.local('ip netns exec ns02 ip addr add 127.0.0.2/8 dev lo') + + self.g2.local('ip netns exec ns01 ip li set up dev lo') + self.g2.local('ip netns exec ns01 ip addr add 127.0.0.2/8 dev lo') + self.g2.local('ip netns exec ns02 ip li set up dev lo') + self.g2.local('ip netns exec ns02 ip addr add 127.0.0.2/8 dev lo') + + self.g3.local('ip netns exec ns01 ip li set up dev lo') + self.g3.local('ip netns exec ns01 ip addr add 127.0.0.2/8 dev lo') + self.g3.local('ip netns exec ns02 ip li set up dev lo') + self.g3.local('ip netns exec ns02 ip addr add 127.0.0.2/8 dev lo') + + self.g1.local("vtysh -c 'enable' -c 'conf t' -c 'vrf 1 netns ns01'") + self.g1.local("vtysh -c 'enable' -c 'conf t' -c 'vrf 2 netns ns02'") + self.g2.local("vtysh -c 'enable' -c 'conf t' -c 'vrf 1 netns ns01'") + self.g2.local("vtysh -c 'enable' -c 'conf t' -c 'vrf 2 netns ns02'") + self.g3.local("vtysh -c 'enable' -c 'conf t' -c 'vrf 1 netns ns01'") + self.g3.local("vtysh -c 'enable' -c 'conf t' -c 'vrf 2 netns ns02'") + + self.g1.local("gobgp vrf add vrf01 id 1 rd 1:1 rt both 1:1") + self.g1.local("gobgp vrf add vrf02 id 2 rd 2:2 rt both 2:2") + self.g2.local("gobgp vrf add vrf01 id 1 rd 1:1 rt both 1:1") + self.g2.local("gobgp vrf add vrf02 id 2 rd 2:2 rt both 2:2") + self.g3.local("gobgp vrf add vrf01 id 1 rd 1:1 rt both 1:1") + self.g3.local("gobgp vrf add vrf02 id 2 rd 2:2 rt both 2:2") + + self.g2.local("gobgp vrf vrf01 rib add 10.0.0.0/24 nexthop 127.0.0.1") + self.g2.local("gobgp vrf vrf02 rib add 20.0.0.0/24 nexthop 127.0.0.1") + + time.sleep(2) + + lines = self.g1.local("ip netns exec ns01 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['10.0.0.0/24'], set(['127.0.0.1'])) + + lines = self.g1.local("ip netns exec ns02 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1'])) + + self.g3.local("gobgp vrf vrf01 rib add 10.0.0.0/24 nexthop 127.0.0.2") + self.g3.local("gobgp vrf vrf02 rib add 20.0.0.0/24 nexthop 127.0.0.2") + + time.sleep(2) + + lines = self.g1.local("ip netns exec ns01 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['10.0.0.0/24'], set(['127.0.0.1', '127.0.0.2'])) + + lines = self.g1.local("ip netns exec ns02 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1', '127.0.0.2'])) + + def test_03_remove_vrf_route_from_multipath(self): + self.g3.local("gobgp vrf vrf01 rib del 10.0.0.0/24 nexthop 127.0.0.2") + self.g3.local("gobgp vrf vrf02 rib del 20.0.0.0/24 nexthop 127.0.0.2") + + time.sleep(2) + + lines = self.g1.local("ip netns exec ns01 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['10.0.0.0/24'], set(['127.0.0.1'])) + + lines = self.g1.local("ip netns exec ns02 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1'])) + + self.g2.local("gobgp vrf vrf01 rib del 10.0.0.0/24 nexthop 127.0.0.1") + self.g2.local("gobgp vrf vrf02 rib del 20.0.0.0/24 nexthop 127.0.0.1") + + lines = self.g1.local("ip netns exec ns01 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 0) + + lines = self.g1.local("ip netns exec ns02 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 0) + + def test_04_multipath_with_vrf_import(self): + self.g1.local("gobgp vrf del vrf01") + self.g1.local("gobgp vrf add vrf01 id 1 rd 1:1 rt import 1:1 2:2 3:3 export 1:1") + + self.g2.local("gobgp vrf vrf01 rib add 10.0.0.0/24 nexthop 127.0.0.1") + self.g2.local("gobgp vrf vrf02 rib add 20.0.0.0/24 nexthop 127.0.0.1") + + time.sleep(2) + + lines = self.g1.local("ip netns exec ns01 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 2) + self.assertEqual(kernel_routes['10.0.0.0/24'], set(['127.0.0.1'])) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1'])) + + lines = self.g1.local("ip netns exec ns02 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1'])) + + self.g3.local("gobgp vrf vrf01 rib add 10.0.0.0/24 nexthop 127.0.0.2") + self.g3.local("gobgp vrf vrf02 rib add 20.0.0.0/24 nexthop 127.0.0.2") + + time.sleep(2) + + lines = self.g1.local("ip netns exec ns01 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 2) + self.assertEqual(kernel_routes['10.0.0.0/24'], set(['127.0.0.1', '127.0.0.2'])) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1', '127.0.0.2'])) + + lines = self.g1.local("ip netns exec ns02 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1', '127.0.0.2'])) + + def test_05_cleanup_multipath_vrf_import(self): + self.g3.local("gobgp vrf vrf01 rib del 10.0.0.0/24 nexthop 127.0.0.2") + self.g3.local("gobgp vrf vrf02 rib del 20.0.0.0/24 nexthop 127.0.0.2") + + time.sleep(2) + + lines = self.g1.local("ip netns exec ns01 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 2) + self.assertEqual(kernel_routes['10.0.0.0/24'], set(['127.0.0.1'])) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1'])) + + lines = self.g1.local("ip netns exec ns02 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 1) + self.assertEqual(kernel_routes['20.0.0.0/24'], set(['127.0.0.1'])) + + self.g2.local("gobgp vrf vrf01 rib del 10.0.0.0/24 nexthop 127.0.0.1") + self.g2.local("gobgp vrf vrf02 rib del 20.0.0.0/24 nexthop 127.0.0.1") + + lines = self.g1.local("ip netns exec ns01 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 0) + + lines = self.g1.local("ip netns exec ns02 ip r", capture=True).split('\n') + kernel_routes = self.parse_ip_route(lines) + self.assertEqual(len(kernel_routes), 0) + + +if __name__ == '__main__': + 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]) -- cgit v1.2.3