diff --git a/protocols/bgp/server/update_sender.go b/protocols/bgp/server/update_sender.go index 0f2096aa9d5c5d89b14dc3da9fb33b9480c49113..a5d05887f026cf336e3792e405fa7c456eae6e74 100644 --- a/protocols/bgp/server/update_sender.go +++ b/protocols/bgp/server/update_sender.go @@ -65,8 +65,8 @@ func (u *UpdateSender) AddPath(pfx net.Prefix, p *route.Path) error { // RemovePath withdraws prefix `pfx` from a peer func (u *UpdateSender) RemovePath(pfx net.Prefix, p *route.Path) bool { - log.Warningf("BGP Update Sender: RemovePath not implemented") - return false + err := withDrawPrefixes(u.fsm.con, pfx) + return err == nil } // UpdateNewClient does nothing diff --git a/protocols/bgp/server/update_sender_add_path.go b/protocols/bgp/server/update_sender_add_path.go index f63f485e738f0540666bea9e624163ca422e72f8..e7b46577c594c5b5f84e115981ac560777899d11 100644 --- a/protocols/bgp/server/update_sender_add_path.go +++ b/protocols/bgp/server/update_sender_add_path.go @@ -66,8 +66,8 @@ func (u *UpdateSenderAddPath) AddPath(pfx net.Prefix, p *route.Path) error { // RemovePath withdraws prefix `pfx` from a peer func (u *UpdateSenderAddPath) RemovePath(pfx net.Prefix, p *route.Path) bool { - log.Warningf("BGP Update Sender: RemovePath not implemented") - return false + err := withDrawPrefixesAddPath(u.fsm.con, pfx, p) + return err == nil } // UpdateNewClient does nothing diff --git a/protocols/bgp/server/withdraw.go b/protocols/bgp/server/withdraw.go new file mode 100644 index 0000000000000000000000000000000000000000..3769c63fa1dd5d9a59f200aadff4b945fae61d20 --- /dev/null +++ b/protocols/bgp/server/withdraw.go @@ -0,0 +1,68 @@ +package server + +import ( + "errors" + "io" + + "github.com/bio-routing/bio-rd/net" + "github.com/bio-routing/bio-rd/protocols/bgp/packet" + "github.com/bio-routing/bio-rd/route" +) + +// withDrawPrefixes generates a BGPUpdate message and write it to the given +// io.Writer. +func withDrawPrefixes(out io.Writer, prefixes ...net.Prefix) error { + if len(prefixes) < 1 { + return nil + } + var rootNLRI *packet.NLRI + var currentNLRI *packet.NLRI + for _, pfx := range prefixes { + if rootNLRI == nil { + rootNLRI = &packet.NLRI{ + IP: pfx.Addr(), + Pfxlen: pfx.Pfxlen(), + } + currentNLRI = rootNLRI + } else { + currentNLRI.Next = &packet.NLRI{ + IP: pfx.Addr(), + Pfxlen: pfx.Pfxlen(), + } + currentNLRI = currentNLRI.Next + } + } + update := &packet.BGPUpdate{ + WithdrawnRoutes: rootNLRI, + } + data, err := update.SerializeUpdate() + if err != nil { + return err + } + _, err = out.Write(data) + return err +} + +// withDrawPrefixesAddPath generates a BGPUpdateAddPath message and write it to the given +// io.Writer. +func withDrawPrefixesAddPath(out io.Writer, pfx net.Prefix, p *route.Path) error { + if p.Type != route.BGPPathType { + return errors.New("wrong path type, expected BGPPathType") + } + if p.BGPPath == nil { + return errors.New("got nil BGPPath") + } + update := &packet.BGPUpdateAddPath{ + WithdrawnRoutes: &packet.NLRIAddPath{ + PathIdentifier: p.BGPPath.PathIdentifier, + IP: pfx.Addr(), + Pfxlen: pfx.Pfxlen(), + }, + } + data, err := update.SerializeUpdate() + if err != nil { + return err + } + _, err = out.Write(data) + return err +} diff --git a/protocols/bgp/server/withdraw_test.go b/protocols/bgp/server/withdraw_test.go new file mode 100644 index 0000000000000000000000000000000000000000..0df94f75259aa446af0d78d1a65aa9ea7a4c67b2 --- /dev/null +++ b/protocols/bgp/server/withdraw_test.go @@ -0,0 +1,115 @@ +package server + +import ( + "testing" + + "errors" + + "bytes" + + "github.com/bio-routing/bio-rd/net" + "github.com/bio-routing/bio-rd/route" + "github.com/stretchr/testify/assert" +) + +func TestWithDrawPrefixes(t *testing.T) { + testcases := []struct { + Name string + Prefix []net.Prefix + Expected []byte + ExpectedError error + }{ + { + Name: "One withdraw", + Prefix: []net.Prefix{net.NewPfx(1413010532, 24)}, + Expected: []byte{ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // BGP Marker + 0x00, 0x1b, // BGP Message Length + 0x02, // BGP Message Type == Update + 0x00, 0x04, // WithDraw Octet length + 0x18, // Prefix Length + 0x54, 0x38, 0xd4, // Prefix, + 0x00, 0x00, // Total Path Attribute Length + }, + ExpectedError: nil, + }, + { + Name: "two withdraws", + Prefix: []net.Prefix{net.NewPfx(1413010532, 24), net.NewPfx(1413010534, 25)}, + Expected: []byte{ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // BGP Marker + 0x00, 0x20, // BGP Message Length + 0x02, // BGP Message Type == Update + 0x00, 0x09, // WithDraw Octet length + 0x18, // Prefix Length first + 0x54, 0x38, 0xd4, // Prefix, + 0x19, // Prefix Length second + 0x54, 0x38, 0xd4, 0x66, // Prefix, + 0x00, 0x00, // Total Path Attribute Length + }, + ExpectedError: nil, + }, + } + for _, tc := range testcases { + buf := bytes.NewBuffer([]byte{}) + err := withDrawPrefixes(buf, tc.Prefix...) + assert.Equal(t, tc.ExpectedError, err, "error mismatch in testcase %v", tc.Name) + assert.Equal(t, tc.Expected, buf.Bytes(), "expected different bytes in testcase %v", tc.Name) + } +} + +func TestWithDrawPrefixesAddPath(t *testing.T) { + testcases := []struct { + Name string + Prefix net.Prefix + Path *route.Path + Expected []byte + ExpectedError error + }{ + { + Name: "Normal withdraw", + Prefix: net.NewPfx(1413010532, 24), + Path: &route.Path{ + Type: route.BGPPathType, + BGPPath: &route.BGPPath{ + PathIdentifier: 1, + }, + }, + Expected: []byte{ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // BGP Marker + 0x00, 0x1f, // BGP Message Length + 0x02, // BGP Message Type == Update + 0x00, 0x08, // WithDraw Octet length + 0x00, 0x00, 0x00, 0x01, // NLRI Path Identifier + 0x18, // Prefix Length + 0x54, 0x38, 0xd4, // Prefix, + 0x00, 0x00, // Total Path Attribute Length + }, + ExpectedError: nil, + }, + { + Name: "Non bgp withdraw", + Prefix: net.NewPfx(1413010532, 24), + Path: &route.Path{ + Type: route.StaticPathType, + }, + Expected: []byte{}, + ExpectedError: errors.New("wrong path type, expected BGPPathType"), + }, + { + Name: "Nil BGPPathType", + Prefix: net.NewPfx(1413010532, 24), + Path: &route.Path{ + Type: route.BGPPathType, + }, + Expected: []byte{}, + ExpectedError: errors.New("got nil BGPPath"), + }, + } + for _, tc := range testcases { + buf := bytes.NewBuffer([]byte{}) + err := withDrawPrefixesAddPath(buf, tc.Prefix, tc.Path) + assert.Equal(t, tc.ExpectedError, err, "error mismatch in testcase %v", tc.Name) + assert.Equal(t, tc.Expected, buf.Bytes(), "expected different bytes in testcase %v", tc.Name) + } +} diff --git a/route/path.go b/route/path.go index 6cfaf993303e727ddad5484c23683dc893f1d2ff..7c0edb7e11443fcad6f2bb0024794c32036fb629 100644 --- a/route/path.go +++ b/route/path.go @@ -53,19 +53,7 @@ func (p *Path) Equal(q *Path) bool { if p == nil || q == nil { return false } - - if p.Type != q.Type { - return false - } - - switch p.Type { - case BGPPathType: - if *p.BGPPath != *q.BGPPath { - return false - } - } - - return true + return p.Compare(q) == 0 } // PathsDiff gets the list of elements contained by a but not b diff --git a/routingtable/locRIB/loc_rib.go b/routingtable/locRIB/loc_rib.go index b5c0b522ac4866cedba8de80213afa3564758905..584954a954e394bf7f859693b87ec827529a4b05 100644 --- a/routingtable/locRIB/loc_rib.go +++ b/routingtable/locRIB/loc_rib.go @@ -120,7 +120,7 @@ func (a *LocRIB) removePathsFromClients(oldRoute *route.Route, newRoute *route.R withdraw := route.PathsDiff(oldRoute.Paths()[0:oldPathsLimit], newRoute.Paths()[0:newPathsLimit]) for _, p := range withdraw { - client.RemovePath(newRoute.Prefix(), p) + client.RemovePath(oldRoute.Prefix(), p) } } } @@ -130,18 +130,18 @@ func (a *LocRIB) removePathsFromClients(oldRoute *route.Route, newRoute *route.R func (a *LocRIB) ContainsPfxPath(pfx net.Prefix, p *route.Path) bool { a.mu.RLock() defer a.mu.RUnlock() - + r := a.rt.Get(pfx) if r == nil { return false } - + for _, path := range r.Paths() { if path.Compare(p) == 0 { return true } } - + return false }