diff options
| -rw-r--r-- | internal/pkg/table/adj.go | 37 | ||||
| -rw-r--r-- | internal/pkg/table/adj_test.go | 29 | ||||
| -rw-r--r-- | pkg/server/server.go | 6 | ||||
| -rw-r--r-- | test/lib/gobgp.py | 13 | ||||
| -rw-r--r-- | test/scenario_test/addpath_test.py | 34 |
5 files changed, 103 insertions, 16 deletions
diff --git a/internal/pkg/table/adj.go b/internal/pkg/table/adj.go index d63537f8..9ae1e156 100644 --- a/internal/pkg/table/adj.go +++ b/internal/pkg/table/adj.go @@ -89,6 +89,43 @@ func (adj *AdjRib) Update(pathList []*Path) { } } +/* The provided pathList is expected to be the real candidate routes after policy evaluation. + For routes that are filtered by policy, there could be a mismatch between display + and actual rib sent to the peer (if softreset out was not run). + Only used to display adj-out because we do not maintain a separate adj-out table +*/ +func (adj *AdjRib) UpdateAdjRibOut(pathList []*Path) { + for _, path := range pathList { + if path == nil || path.IsEOR() { + continue + } + t := adj.table[path.GetRouteFamily()] + d := t.getOrCreateDest(path.GetNlri(), 0) + + var old *Path + idx := -1 + for i, p := range d.knownPathList { + if p.GetNlri().PathLocalIdentifier() == path.GetNlri().PathLocalIdentifier() { + idx = i + break + } + } + if idx != -1 { + old = d.knownPathList[idx] + } + + // No withdraw use case for adj-out + if idx != -1 { + if old.Equal(path) { + path.setTimestamp(old.GetTimestamp()) + } + d.knownPathList[idx] = path + } else { + d.knownPathList = append(d.knownPathList, path) + } + } +} + func (adj *AdjRib) walk(families []bgp.RouteFamily, fn func(*Destination) bool) { for _, f := range families { if t, ok := adj.table[f]; ok { diff --git a/internal/pkg/table/adj_test.go b/internal/pkg/table/adj_test.go index 9e48538f..99aba0ff 100644 --- a/internal/pkg/table/adj_test.go +++ b/internal/pkg/table/adj_test.go @@ -61,6 +61,35 @@ func TestAddPath(t *testing.T) { assert.Equal(t, 0, len(adj.table[family].destinations)) } +func TestAddPathAdjOut(t *testing.T) { + pi := &PeerInfo{} + attrs := []bgp.PathAttributeInterface{bgp.NewPathAttributeOrigin(0)} + + nlri1 := bgp.NewIPAddrPrefix(24, "20.20.20.0") + nlri1.SetPathIdentifier(1) + nlri1.SetPathLocalIdentifier(1) + p1 := NewPath(pi, nlri1, false, attrs, time.Now(), false) + nlri2 := bgp.NewIPAddrPrefix(24, "20.20.20.0") + nlri2.SetPathIdentifier(1) + nlri2.SetPathLocalIdentifier(2) + p2 := NewPath(pi, nlri2, false, attrs, time.Now(), false) + nlri3 := bgp.NewIPAddrPrefix(24, "20.20.20.0") + nlri3.SetPathIdentifier(2) + nlri3.SetPathLocalIdentifier(2) + p3 := NewPath(pi, nlri3, false, attrs, time.Now(), false) + nlri4 := bgp.NewIPAddrPrefix(24, "20.20.20.0") + nlri4.SetPathIdentifier(3) + nlri4.SetPathLocalIdentifier(2) + p4 := NewPath(pi, nlri4, false, attrs, time.Now(), false) + family := p1.GetRouteFamily() + families := []bgp.RouteFamily{family} + + adj := NewAdjRib(families) + adj.UpdateAdjRibOut([]*Path{p1, p2, p3, p4}) + assert.Equal(t, len(adj.table[family].destinations), 1) + assert.Equal(t, adj.Count([]bgp.RouteFamily{family}), 2) +} + func TestStale(t *testing.T) { pi := &PeerInfo{} attrs := []bgp.PathAttributeInterface{bgp.NewPathAttributeOrigin(0)} diff --git a/pkg/server/server.go b/pkg/server/server.go index fa0f5225..966f48b7 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -2485,11 +2485,11 @@ func (s *BgpServer) getAdjRib(addr string, family bgp.RouteFamily, in bool, enab if p == nil { filtered[path.GetNlri().String()] = path } - adjRib.Update([]*table.Path{path}) + adjRib.UpdateAdjRibOut([]*table.Path{path}) } } else { accepted, _ := s.getBestFromLocal(peer, peer.configuredRFlist()) - adjRib.Update(accepted) + adjRib.UpdateAdjRibOut(accepted) } } rib, err = adjRib.Select(family, false, table.TableSelectOption{ID: id, AS: as, LookupPrefixes: prefixes}) @@ -2616,7 +2616,7 @@ func (s *BgpServer) getAdjRibInfo(addr string, family bgp.RouteFamily, in bool) } else { adjRib = table.NewAdjRib(peer.configuredRFlist()) accepted, _ := s.getBestFromLocal(peer, peer.configuredRFlist()) - adjRib.Update(accepted) + adjRib.UpdateAdjRibOut(accepted) } info, err = adjRib.TableInfo(family) return err diff --git a/test/lib/gobgp.py b/test/lib/gobgp.py index 1abdb782..6ffa6024 100644 --- a/test/lib/gobgp.py +++ b/test/lib/gobgp.py @@ -277,12 +277,15 @@ class GoBGPContainer(BGPContainer): t.daemon = True t.start() - def _get_adj_rib(self, adj_type, peer, prefix='', rf='ipv4'): + def _get_adj_rib(self, adj_type, peer, prefix='', rf='ipv4', add_path_enabled=False): peer_addr = self.peer_name(peer) cmd = 'gobgp neighbor {0} adj-{1} {2} -a {3} -j'.format(peer_addr, adj_type, prefix, rf) output = self.local(cmd, capture=True) + if add_path_enabled: + return self._get_rib(json.loads(output)) + ret = [p[0] for p in json.loads(output).values()] for p in ret: p["nexthop"] = self._get_nexthop(p) @@ -292,11 +295,11 @@ class GoBGPContainer(BGPContainer): p["med"] = self._get_med(p) return ret - def get_adj_rib_in(self, peer, prefix='', rf='ipv4'): - return self._get_adj_rib('in', peer, prefix, rf) + def get_adj_rib_in(self, peer, prefix='', rf='ipv4', add_path_enabled=False): + return self._get_adj_rib('in', peer, prefix, rf, add_path_enabled) - def get_adj_rib_out(self, peer, prefix='', rf='ipv4'): - return self._get_adj_rib('out', peer, prefix, rf) + def get_adj_rib_out(self, peer, prefix='', rf='ipv4', add_path_enabled=False): + return self._get_adj_rib('out', peer, prefix, rf, add_path_enabled) def get_neighbor(self, peer): cmd = 'gobgp -j neighbor {0}'.format(self.peer_name(peer)) diff --git a/test/scenario_test/addpath_test.py b/test/scenario_test/addpath_test.py index c8a8491b..23d3b5cb 100644 --- a/test/scenario_test/addpath_test.py +++ b/test/scenario_test/addpath_test.py @@ -164,8 +164,17 @@ class GoBGPTestBase(unittest.TestCase): assert_several_times(f) + def test_11_check_g1_adj_out(self): + adj_out = self.g1.get_adj_rib_out(self.g2, add_path_enabled=True) + self.assertEqual(len(adj_out), 1) + self.assertEqual(len(adj_out[0]['paths']), 1) + + adj_out = self.g1.get_adj_rib_out(self.g3, add_path_enabled=True) + self.assertEqual(len(adj_out), 1) + self.assertEqual(len(adj_out[0]['paths']), 3) + # test the best path is replaced due to the CLI route from g1 rib - def test_11_check_g2_global_rib(self): + def test_12_check_g2_global_rib(self): def f(): rib = self.g2.get_global_rib() self.assertEqual(len(rib), 1) @@ -175,7 +184,7 @@ class GoBGPTestBase(unittest.TestCase): assert_several_times(f) # test the route from CLI is advertised from g1 - def test_12_check_g3_global_rib(self): + def test_13_check_g3_global_rib(self): def f(): rib = self.g3.get_global_rib() self.assertEqual(len(rib), 1) @@ -190,13 +199,13 @@ class GoBGPTestBase(unittest.TestCase): assert_several_times(f) # remove non-existing route with path_id via GoBGP CLI (no error check) - def test_13_remove_non_existing_add_paths_route_via_cli(self): + def test_14_remove_non_existing_add_paths_route_via_cli(self): # specify locally non-existing identifier which has the same value # with the identifier of the route from e1 self.g1.del_route(route='192.168.100.0/24', identifier=20) # test none of route is removed by non-existing path_id via CLI - def test_14_check_g1_global_rib(self): + def test_15_check_g1_global_rib(self): def f(): rib = self.g1.get_global_rib() self.assertEqual(len(rib), 1) @@ -211,11 +220,20 @@ class GoBGPTestBase(unittest.TestCase): assert_several_times(f) # remove route with path_id via GoBGP CLI (no error check) - def test_15_remove_add_paths_route_via_cli(self): + def test_16_remove_add_paths_route_via_cli(self): self.g1.del_route(route='192.168.100.0/24', identifier=10) + def test_17_check_g1_adj_out(self): + adj_out = self.g1.get_adj_rib_out(self.g2, add_path_enabled=True) + self.assertEqual(len(adj_out), 1) + self.assertEqual(len(adj_out[0]['paths']), 1) + + adj_out = self.g1.get_adj_rib_out(self.g3, add_path_enabled=True) + self.assertEqual(len(adj_out), 1) + self.assertEqual(len(adj_out[0]['paths']), 2) + # test the route is removed from the rib via CLI - def test_16_check_g1_global_rib(self): + def test_18_check_g1_global_rib(self): def f(): rib = self.g1.get_global_rib() self.assertEqual(len(rib), 1) @@ -227,7 +245,7 @@ class GoBGPTestBase(unittest.TestCase): assert_several_times(f) # test the best path is replaced the removal from g1 rib - def test_17_check_g2_global_rib(self): + def test_19_check_g2_global_rib(self): def f(): rib = self.g2.get_global_rib() self.assertEqual(len(rib), 1) @@ -237,7 +255,7 @@ class GoBGPTestBase(unittest.TestCase): assert_several_times(f) # test the removed route from CLI is withdrawn by g1 - def test_18_check_g3_global_rib(self): + def test_20_check_g3_global_rib(self): def f(): rib = self.g3.get_global_rib() self.assertEqual(len(rib), 1) |