Merge remote-tracking branch 'origin/master' into bgp_large_community_support

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
 }

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 {

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)

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

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

protocols/bgp/server/withdraw.go

+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
+}

protocols/bgp/server/withdraw_test.go

+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)
+	}
+}

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

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)

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
 }