Merge pull request #18 from hikhvar/feature/bgp-withdraw

Feature/bgp withdraw

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