From 0da37b02108d79fc902d1d6731c9245ff733dc84 Mon Sep 17 00:00:00 2001 From: mageshgv Date: Fri, 4 Oct 2019 10:24:19 -0700 Subject: Maintain unique vrf IDs in watchEventBestPath vrf map Fixes missing route propagation to proper vrfs in zapi --- pkg/server/server.go | 6 ++--- pkg/server/server_test.go | 61 ++++++++++++++++++++++++++++++++++++++++------- pkg/server/zclient.go | 4 ++-- 3 files changed, 57 insertions(+), 14 deletions(-) (limited to 'pkg/server') diff --git a/pkg/server/server.go b/pkg/server/server.go index d6c54e10..c5dd18f3 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -708,13 +708,13 @@ func (s *BgpServer) notifyBestWatcher(best []*table.Path, multipath [][]*table.P clonedM[i] = clonePathList(pathList) } clonedB := clonePathList(best) - m := make(map[string]uint32) + 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[p.GetNlri().String()] = uint32(vrf.Id) + m[uint32(vrf.Id)] = true } } } @@ -3828,7 +3828,7 @@ type watchEventTable struct { type watchEventBestPath struct { PathList []*table.Path MultiPathList [][]*table.Path - Vrf map[string]uint32 + Vrf map[uint32]bool } type watchEventMessage struct { diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 0c005efb..a390ce58 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -468,6 +468,10 @@ func TestMonitor(test *testing.T) { assert.Nil(err) defer s.StopBgp(context.Background(), &api.StopBgpRequest{}) + // Vrf1 111:111 and vrf2 import 111:111 and 222:222 + addVrf(test, s, "vrf1", "111:111", []string{"111:111"}, []string{"111:111"}, 1) + addVrf(test, s, "vrf2", "222:222", []string{"111:111", "222:222"}, []string{"222:222"}, 2) + p1 := &api.Peer{ Conf: &api.PeerConf{ NeighborAddress: "127.0.0.1", @@ -529,9 +533,10 @@ 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)) // 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.0.0.0"), true, nil, time.Now(), false)}); err != nil { log.Fatal(err) } @@ -540,6 +545,7 @@ 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)) // Stops the watcher still having an item. w.Stop() @@ -594,6 +600,26 @@ func TestMonitor(test *testing.T) { assert.Equal("10.2.0.0/24", u.PathList[0].GetNlri().String()) assert.True(u.PathList[0].IsWithdraw) + // Test bestpath events with vrf and rt import + w.Stop() + w = s.watch(watchBestPath(false)) + attrs = []bgp.PathAttributeInterface{ + bgp.NewPathAttributeOrigin(0), + bgp.NewPathAttributeNextHop("10.0.0.1"), + } + + if err := s.addPathList("vrf1", []*table.Path{table.NewPath(nil, bgp.NewIPAddrPrefix(24, "10.0.0.0"), false, 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.False(b.PathList[0].IsWithdraw) + assert.Equal(2, len(b.Vrf)) + assert.True(b.Vrf[1]) + assert.True(b.Vrf[2]) + // Stops the watcher still having an item. w.Stop() } @@ -1161,17 +1187,34 @@ func parseRDRT(rdStr string) (bgp.RouteDistinguisherInterface, bgp.ExtendedCommu return rd, rt, nil } -func addVrf(t *testing.T, s *BgpServer, vrfName, rdStr string, id uint32) { - rd, rt, err := parseRDRT(rdStr) +func addVrf(t *testing.T, s *BgpServer, vrfName, rdStr string, importRtsStr []string, exportRtsStr []string, id uint32) { + rd, _, err := parseRDRT(rdStr) if err != nil { t.Fatal(err) } + importRts := make([]bgp.ExtendedCommunityInterface, 0, len(importRtsStr)) + for _, importRtStr := range importRtsStr { + _, rt, err := parseRDRT(importRtStr) + if err != nil { + t.Fatal(err) + } + importRts = append(importRts, rt) + } + + exportRts := make([]bgp.ExtendedCommunityInterface, 0, len(exportRtsStr)) + for _, exportRtStr := range exportRtsStr { + _, rt, err := parseRDRT(exportRtStr) + if err != nil { + t.Fatal(err) + } + exportRts = append(exportRts, rt) + } req := &api.AddVrfRequest{ Vrf: &api.Vrf{ Name: vrfName, - ImportRt: apiutil.MarshalRTs([]bgp.ExtendedCommunityInterface{rt}), - ExportRt: apiutil.MarshalRTs([]bgp.ExtendedCommunityInterface{rt}), + ImportRt: apiutil.MarshalRTs(importRts), + ExportRt: apiutil.MarshalRTs(exportRts), Rd: apiutil.MarshalRD(rd), Id: id, }, @@ -1188,8 +1231,8 @@ func TestDoNotReactToDuplicateRTCMemberships(t *testing.T) { s1 := runNewServer(1, "1.1.1.1", 10179) s2 := runNewServer(1, "2.2.2.2", 20179) - addVrf(t, s1, "vrf1", "111:111", 1) - addVrf(t, s2, "vrf1", "111:111", 1) + addVrf(t, s1, "vrf1", "111:111", []string{"111:111"}, []string{"111:111"}, 1) + addVrf(t, s2, "vrf1", "111:111", []string{"111:111"}, []string{"111:111"}, 1) if err := peerServers(t, ctx, []*BgpServer{s1, s2}, []config.AfiSafiType{config.AFI_SAFI_TYPE_L3VPN_IPV4_UNICAST, config.AFI_SAFI_TYPE_RTC}); err != nil { t.Fatal(err) @@ -1446,7 +1489,7 @@ func TestDeleteNonExistingVrf(t *testing.T) { log.SetLevel(log.DebugLevel) s := runNewServer(1, "1.1.1.1", 10179) - addVrf(t, s, "vrf1", "111:111", 1) + addVrf(t, s, "vrf1", "111:111", []string{"111:111"}, []string{"111:111"}, 1) req := &api.DeleteVrfRequest{Name: "Invalidvrf"} if err := s.DeleteVrf(context.Background(), req); err == nil { t.Fatal("Did not raise error for invalid vrf deletion.", err) @@ -1458,7 +1501,7 @@ func TestDeleteVrf(t *testing.T) { log.SetLevel(log.DebugLevel) s := runNewServer(1, "1.1.1.1", 10179) - addVrf(t, s, "vrf1", "111:111", 1) + addVrf(t, s, "vrf1", "111:111", []string{"111:111"}, []string{"111:111"}, 1) req := &api.DeleteVrfRequest{Name: "vrf1"} if err := s.DeleteVrf(context.Background(), req); err != nil { t.Fatal("Vrf delete failed", err) diff --git a/pkg/server/zclient.go b/pkg/server/zclient.go index 1c315a6c..390628f0 100644 --- a/pkg/server/zclient.go +++ b/pkg/server/zclient.go @@ -433,8 +433,8 @@ func (z *zebraClient) loop() { for _, path := range msg.PathList { vrfs := []uint32{0} if msg.Vrf != nil { - if v, ok := msg.Vrf[path.GetNlri().String()]; ok { - vrfs = append(vrfs, v) + for vrfId := range msg.Vrf { + vrfs = append(vrfs, vrfId) } } for _, i := range vrfs { -- cgit v1.2.3