diff --git a/net/helper.go b/net/helper.go
index 4f7319cddfd92ef975819b143e4c03488ec22fb8..4b82b2425fdc199023c675c904e92cd6bcce27b3 100644
--- a/net/helper.go
+++ b/net/helper.go
@@ -4,5 +4,6 @@ import "net"
// IPv4ToUint32 converts an `net.IP` to an uint32 interpretation
func IPv4ToUint32(ip net.IP) uint32 {
+ ip = ip.To4()
return uint32(ip[3]) + uint32(ip[2])<<8 + uint32(ip[1])<<16 + uint32(ip[0])<<24
}
diff --git a/net/helper_test.go b/net/helper_test.go
index 7a267c3c9e26715bd9b14e3579265e2057aac8db..d5a038cbb6bd2683b50ba50c658fae797e46d39c 100644
--- a/net/helper_test.go
+++ b/net/helper_test.go
@@ -3,6 +3,8 @@ package net
import (
"testing"
+ "net"
+
"github.com/stretchr/testify/assert"
)
@@ -23,6 +25,10 @@ func TestIPv4ToUint32(t *testing.T) {
input: []byte{172, 24, 5, 1},
expected: 2887255297,
},
+ {
+ input: net.ParseIP("172.24.5.1"),
+ expected: 2887255297,
+ },
}
for _, test := range tests {
diff --git a/protocols/bgp/server/fsm.go b/protocols/bgp/server/fsm.go
index 7681a127f17d6e4229d3ea79af22c55665a31831..d4c472066583d2c0190b64ef60b98f54a61b981e 100644
--- a/protocols/bgp/server/fsm.go
+++ b/protocols/bgp/server/fsm.go
@@ -724,7 +724,9 @@ func (fsm *FSM) established() int {
Address: tnet.IPv4ToUint32(fsm.remote),
}
- clientOptions := routingtable.ClientOptions{}
+ clientOptions := routingtable.ClientOptions{
+ BestOnly: true,
+ }
if fsm.capAddPathSend {
fsm.updateSender = newUpdateSenderAddPath(fsm)
fsm.adjRIBOut = adjRIBOutAddPath.New(n)
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/client_manager.go b/routingtable/client_manager.go
index ee0275c3f2358ae96b4e297b17b540e29139bf90..20d848b2c090a4c311cb4a81fc8c1c4697cf5891 100644
--- a/routingtable/client_manager.go
+++ b/routingtable/client_manager.go
@@ -63,6 +63,8 @@ func (c *ClientManager) RegisterWithOptions(client RouteTableClient, opt ClientO
// Unregister unregisters a client
func (c *ClientManager) Unregister(client RouteTableClient) {
+ c.mu.Lock()
+ defer c.mu.Unlock()
if _, ok := c.clients[client]; !ok {
return
}
@@ -71,6 +73,8 @@ func (c *ClientManager) Unregister(client RouteTableClient) {
// Clients returns a list of registered clients
func (c *ClientManager) Clients() []RouteTableClient {
+ c.mu.RLock()
+ defer c.mu.RUnlock()
ret := make([]RouteTableClient, 0)
for rtc := range c.clients {
ret = append(ret, rtc)
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
}