From 96439443ef3c085d607f03578824472e57c1fc3f Mon Sep 17 00:00:00 2001
From: Daniel Czerwonk <daniel@dan-nrw.de>
Date: Tue, 17 Jul 2018 08:12:02 +0200
Subject: [PATCH] improved method signature

---
 protocols/bgp/server/update_sender.go      |  81 +++++++++-
 protocols/bgp/server/update_sender_test.go | 171 +++++++++++++++++++++
 protocols/bgp/server/withdraw.go           |  81 ----------
 protocols/bgp/server/withdraw_test.go      | 161 -------------------
 4 files changed, 250 insertions(+), 244 deletions(-)
 delete mode 100644 protocols/bgp/server/withdraw.go
 delete mode 100644 protocols/bgp/server/withdraw_test.go

diff --git a/protocols/bgp/server/update_sender.go b/protocols/bgp/server/update_sender.go
index ca650eed..6eda96c2 100644
--- a/protocols/bgp/server/update_sender.go
+++ b/protocols/bgp/server/update_sender.go
@@ -1,6 +1,8 @@
 package server
 
 import (
+	"errors"
+	"io"
 	"sync"
 	"time"
 
@@ -245,11 +247,86 @@ func (u *UpdateSender) RemovePath(pfx bnet.Prefix, p *route.Path) bool {
 
 func (u *UpdateSender) withdrawPrefix(pfx bnet.Prefix, p *route.Path) error {
 	if u.fsm.options.SupportsMultiProtocol {
-		return withDrawPrefixesMultiProtocol(u.fsm.con, u.fsm.options, pfx, p, u.afi, u.safi)
+		return u.withDrawPrefixesMultiProtocol(u.fsm.con, pfx, p)
+	}
+
+	return u.withDrawPrefixesAddPath(u.fsm.con, pfx, p)
+}
+
+// withDrawPrefixes generates a BGPUpdate message and write it to the given
+// io.Writer.
+func (u *UpdateSender) withDrawPrefixes(out io.Writer, prefixes ...bnet.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().ToUint32(),
+				Pfxlen: pfx.Pfxlen(),
+			}
+			currentNLRI = rootNLRI
+		} else {
+			currentNLRI.Next = &packet.NLRI{
+				IP:     pfx.Addr().ToUint32(),
+				Pfxlen: pfx.Pfxlen(),
+			}
+			currentNLRI = currentNLRI.Next
+		}
+	}
+
+	update := &packet.BGPUpdate{
+		WithdrawnRoutes: rootNLRI,
+	}
+
+	return serializeAndSendUpdate(out, update, u.fsm.options)
+
+}
+
+// withDrawPrefixesAddPath generates a BGPUpdateAddPath message and write it to the given
+// io.Writer.
+func (u *UpdateSender) withDrawPrefixesAddPath(out io.Writer, pfx bnet.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.BGPUpdate{
+		WithdrawnRoutes: &packet.NLRI{
+			PathIdentifier: p.BGPPath.PathIdentifier,
+			IP:             pfx.Addr().ToUint32(),
+			Pfxlen:         pfx.Pfxlen(),
+		},
+	}
+
+	return serializeAndSendUpdate(out, update, u.fsm.options)
+}
+
+func (u *UpdateSender) withDrawPrefixesMultiProtocol(out io.Writer, pfx bnet.Prefix, p *route.Path) error {
+	pathID := uint32(0)
+	if p.BGPPath != nil {
+		pathID = p.BGPPath.PathIdentifier
+	}
+
+	update := &packet.BGPUpdate{
+		PathAttributes: &packet.PathAttribute{
+			TypeCode: packet.MultiProtocolUnreachNLRICode,
+			Value: packet.MultiProtocolUnreachNLRI{
+				AFI:      u.afi,
+				SAFI:     u.safi,
+				Prefixes: []bnet.Prefix{pfx},
+				PathID:   pathID,
+			},
+		},
 	}
 
-	return withDrawPrefixesAddPath(u.fsm.con, u.fsm.options, pfx, p)
+	return serializeAndSendUpdate(out, update, u.fsm.options)
 }
 
 // UpdateNewClient does nothing
diff --git a/protocols/bgp/server/update_sender_test.go b/protocols/bgp/server/update_sender_test.go
index 40719d69..09a15e82 100644
--- a/protocols/bgp/server/update_sender_test.go
+++ b/protocols/bgp/server/update_sender_test.go
@@ -1,6 +1,8 @@
 package server
 
 import (
+	"bytes"
+	"errors"
 	"reflect"
 	"testing"
 	"time"
@@ -945,3 +947,172 @@ func TestSender(t *testing.T) {
 		}
 	}
 }
+
+func TestWithDrawPrefixes(t *testing.T) {
+	testcases := []struct {
+		Name          string
+		Prefix        []bnet.Prefix
+		Expected      []byte
+		ExpectedError error
+	}{
+		{
+			Name:   "One withdraw",
+			Prefix: []bnet.Prefix{bnet.NewPfx(bnet.IPv4(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: []bnet.Prefix{bnet.NewPfx(bnet.IPv4(1413010532), 24), bnet.NewPfx(bnet.IPv4(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{})
+
+		u := &UpdateSender{
+			fsm: &FSM{
+				options: &types.Options{},
+			},
+			afi:  packet.IPv6AFI,
+			safi: packet.UnicastSAFI,
+		}
+
+		err := u.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 TestWithDrawPrefixesMultiProtocol(t *testing.T) {
+	tests := []struct {
+		Name     string
+		Prefix   bnet.Prefix
+		Expected []byte
+	}{
+		{
+			Name:   "IPv6 MP_UNREACH_NLRI",
+			Prefix: bnet.NewPfx(bnet.IPv6FromBlocks(0x2804, 0x148c, 0, 0, 0, 0, 0, 0), 32),
+			Expected: []byte{
+				0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // BGP Marker
+				0x00, 0x22, // BGP Message Length
+				0x02,       // BGP Message Type == Update
+				0x00, 0x00, // WithDraw Octet length
+				0x00, 0x0b, // Length
+				0x80,       // Flags
+				0x0f,       // Attribute Code
+				0x08,       // Attribute length
+				0x00, 0x02, // AFI
+				0x01,                         // SAFI
+				0x20, 0x28, 0x04, 0x14, 0x8c, // Prefix
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			buf := bytes.NewBuffer([]byte{})
+
+			u := &UpdateSender{
+				fsm: &FSM{
+					options: &types.Options{
+						AddPathRX:             false,
+						SupportsMultiProtocol: true,
+					},
+				},
+				afi:  packet.IPv6AFI,
+				safi: packet.UnicastSAFI,
+			}
+
+			err := u.withDrawPrefixesMultiProtocol(buf, test.Prefix, &route.Path{})
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			assert.Equal(t, test.Expected, buf.Bytes())
+		})
+	}
+}
+
+func TestWithDrawPrefixesAddPath(t *testing.T) {
+	testcases := []struct {
+		Name          string
+		Prefix        bnet.Prefix
+		Path          *route.Path
+		Expected      []byte
+		ExpectedError error
+	}{
+		{
+			Name:   "Normal withdraw",
+			Prefix: bnet.NewPfx(bnet.IPv4(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: bnet.NewPfx(bnet.IPv4(1413010532), 24),
+			Path: &route.Path{
+				Type: route.StaticPathType,
+			},
+			Expected:      []byte{},
+			ExpectedError: errors.New("wrong path type, expected BGPPathType"),
+		},
+		{
+			Name:   "Nil BGPPathType",
+			Prefix: bnet.NewPfx(bnet.IPv4(1413010532), 24),
+			Path: &route.Path{
+				Type: route.BGPPathType,
+			},
+			Expected:      []byte{},
+			ExpectedError: errors.New("got nil BGPPath"),
+		},
+	}
+	for _, tc := range testcases {
+		buf := bytes.NewBuffer([]byte{})
+
+		u := &UpdateSender{
+			fsm: &FSM{
+				options: &types.Options{
+					AddPathRX: true,
+				},
+			},
+		}
+
+		err := u.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/protocols/bgp/server/withdraw.go b/protocols/bgp/server/withdraw.go
deleted file mode 100644
index 22cea17c..00000000
--- a/protocols/bgp/server/withdraw.go
+++ /dev/null
@@ -1,81 +0,0 @@
-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/protocols/bgp/types"
-	"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, opt *types.Options, 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().ToUint32(),
-				Pfxlen: pfx.Pfxlen(),
-			}
-			currentNLRI = rootNLRI
-		} else {
-			currentNLRI.Next = &packet.NLRI{
-				IP:     pfx.Addr().ToUint32(),
-				Pfxlen: pfx.Pfxlen(),
-			}
-			currentNLRI = currentNLRI.Next
-		}
-	}
-	update := &packet.BGPUpdate{
-		WithdrawnRoutes: rootNLRI,
-	}
-	return serializeAndSendUpdate(out, update, opt)
-
-}
-
-// withDrawPrefixesAddPath generates a BGPUpdateAddPath message and write it to the given
-// io.Writer.
-func withDrawPrefixesAddPath(out io.Writer, opt *types.Options, 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.BGPUpdate{
-		WithdrawnRoutes: &packet.NLRI{
-			PathIdentifier: p.BGPPath.PathIdentifier,
-			IP:             pfx.Addr().ToUint32(),
-			Pfxlen:         pfx.Pfxlen(),
-		},
-	}
-	return serializeAndSendUpdate(out, update, opt)
-}
-
-func withDrawPrefixesMultiProtocol(out io.Writer, opt *types.Options, pfx net.Prefix, p *route.Path, afi uint16, safi uint8) error {
-	pathID := uint32(0)
-	if p.BGPPath != nil {
-		pathID = p.BGPPath.PathIdentifier
-	}
-
-	update := &packet.BGPUpdate{
-		PathAttributes: &packet.PathAttribute{
-			TypeCode: packet.MultiProtocolUnreachNLRICode,
-			Value: packet.MultiProtocolUnreachNLRI{
-				AFI:      afi,
-				SAFI:     safi,
-				Prefixes: []net.Prefix{pfx},
-				PathID:   pathID,
-			},
-		},
-	}
-
-	return serializeAndSendUpdate(out, update, opt)
-}
diff --git a/protocols/bgp/server/withdraw_test.go b/protocols/bgp/server/withdraw_test.go
deleted file mode 100644
index 954b36e4..00000000
--- a/protocols/bgp/server/withdraw_test.go
+++ /dev/null
@@ -1,161 +0,0 @@
-package server
-
-import (
-	"bytes"
-	"errors"
-	"testing"
-
-	"github.com/bio-routing/bio-rd/net"
-	"github.com/bio-routing/bio-rd/protocols/bgp/packet"
-	"github.com/bio-routing/bio-rd/protocols/bgp/types"
-	"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(net.IPv4(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(net.IPv4(1413010532), 24), net.NewPfx(net.IPv4(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{})
-		opt := &types.Options{}
-		err := withDrawPrefixes(buf, opt, 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 TestWithDrawPrefixesMultiProtocol(t *testing.T) {
-	tests := []struct {
-		Name     string
-		Prefix   net.Prefix
-		Expected []byte
-	}{
-		{
-			Name:   "IPv6 MP_UNREACH_NLRI",
-			Prefix: net.NewPfx(net.IPv6FromBlocks(0x2804, 0x148c, 0, 0, 0, 0, 0, 0), 32),
-			Expected: []byte{
-				0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // BGP Marker
-				0x00, 0x22, // BGP Message Length
-				0x02,       // BGP Message Type == Update
-				0x00, 0x00, // WithDraw Octet length
-				0x00, 0x0b, // Length
-				0x80,       // Flags
-				0x0f,       // Attribute Code
-				0x08,       // Attribute length
-				0x00, 0x02, // AFI
-				0x01,                         // SAFI
-				0x20, 0x28, 0x04, 0x14, 0x8c, // Prefix
-			},
-		},
-	}
-	for _, test := range tests {
-		t.Run(test.Name, func(t *testing.T) {
-			buf := bytes.NewBuffer([]byte{})
-			opt := &types.Options{
-				AddPathRX: false,
-			}
-
-			err := withDrawPrefixesMultiProtocol(buf, opt, test.Prefix, &route.Path{}, packet.IPv6AFI, packet.UnicastSAFI)
-			if err != nil {
-				t.Fatalf("unexpected error: %v", err)
-			}
-
-			assert.Equal(t, test.Expected, buf.Bytes())
-		})
-	}
-}
-
-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(net.IPv4(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(net.IPv4(1413010532), 24),
-			Path: &route.Path{
-				Type: route.StaticPathType,
-			},
-			Expected:      []byte{},
-			ExpectedError: errors.New("wrong path type, expected BGPPathType"),
-		},
-		{
-			Name:   "Nil BGPPathType",
-			Prefix: net.NewPfx(net.IPv4(1413010532), 24),
-			Path: &route.Path{
-				Type: route.BGPPathType,
-			},
-			Expected:      []byte{},
-			ExpectedError: errors.New("got nil BGPPath"),
-		},
-	}
-	for _, tc := range testcases {
-		buf := bytes.NewBuffer([]byte{})
-		opt := &types.Options{
-			AddPathRX: true,
-		}
-		err := withDrawPrefixesAddPath(buf, opt, 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)
-	}
-}
-- 
GitLab