From 5cb60229f6d3e1080834022ae0cdffce5f8f1a87 Mon Sep 17 00:00:00 2001
From: Daniel Czerwonk <daniel@dan-nrw.de>
Date: Tue, 24 Jul 2018 00:30:13 +0200
Subject: [PATCH] testing withdraw func to increase test coverage

---
 protocols/bgp/server/update_sender.go      |  26 ++--
 protocols/bgp/server/update_sender_test.go | 167 ++++++++++++---------
 2 files changed, 113 insertions(+), 80 deletions(-)

diff --git a/protocols/bgp/server/update_sender.go b/protocols/bgp/server/update_sender.go
index 5d5804ac..9e40303f 100644
--- a/protocols/bgp/server/update_sender.go
+++ b/protocols/bgp/server/update_sender.go
@@ -243,7 +243,7 @@ func (u *UpdateSender) copyAttributesWithoutNextHop(pa *packet.PathAttribute) (a
 
 // RemovePath withdraws prefix `pfx` from a peer
 func (u *UpdateSender) RemovePath(pfx bnet.Prefix, p *route.Path) bool {
-	err := u.withdrawPrefix(pfx, p)
+	err := u.withdrawPrefix(u.fsm.con, pfx, p)
 	if err != nil {
 		log.Errorf("Unable to withdraw prefix: %v", err)
 		return false
@@ -252,27 +252,27 @@ func (u *UpdateSender) RemovePath(pfx bnet.Prefix, p *route.Path) bool {
 	return true
 }
 
-func (u *UpdateSender) withdrawPrefix(pfx bnet.Prefix, p *route.Path) error {
+func (u *UpdateSender) withdrawPrefix(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")
+	}
+
 	if u.addressFamily.afi == packet.IPv4AFI && !u.addressFamily.multiProtocol {
-		return u.withdrawPrefixIPv4(u.fsm.con, pfx, p)
+		return u.withdrawPrefixIPv4(out, pfx, p)
 	}
 
 	if !u.addressFamily.multiProtocol {
 		return fmt.Errorf(packet.AFIName(u.addressFamily.afi) + " was not negotiated")
 	}
 
-	return u.withdrawPrefixesMultiProtocol(u.fsm.con, pfx, p)
+	return u.withdrawPrefixMultiProtocol(out, pfx, p)
 }
 
 func (u *UpdateSender) withdrawPrefixIPv4(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,
@@ -284,7 +284,7 @@ func (u *UpdateSender) withdrawPrefixIPv4(out io.Writer, pfx bnet.Prefix, p *rou
 	return serializeAndSendUpdate(out, update, u.options)
 }
 
-func (u *UpdateSender) withdrawPrefixesMultiProtocol(out io.Writer, pfx bnet.Prefix, p *route.Path) error {
+func (u *UpdateSender) withdrawPrefixMultiProtocol(out io.Writer, pfx bnet.Prefix, p *route.Path) error {
 	pathID := uint32(0)
 	if p.BGPPath != nil {
 		pathID = p.BGPPath.PathIdentifier
diff --git a/protocols/bgp/server/update_sender_test.go b/protocols/bgp/server/update_sender_test.go
index 359e2da3..4b6ae307 100644
--- a/protocols/bgp/server/update_sender_test.go
+++ b/protocols/bgp/server/update_sender_test.go
@@ -942,69 +942,48 @@ func TestSender(t *testing.T) {
 		}
 	}
 }
-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
-			},
-		},
-	}
-
-	t.Parallel()
-
-	for _, test := range tests {
-		t.Run(test.Name, func(t *testing.T) {
-			buf := bytes.NewBuffer([]byte{})
 
-			u := &UpdateSender{
-				fsm: &FSM{},
-				addressFamily: &fsmAddressFamily{
-					afi:  packet.IPv6AFI,
-					safi: packet.UnicastSAFI,
-				},
-				options: &packet.EncodeOptions{},
-			}
-
-			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 TestWithdrawPrefixIPv4(t *testing.T) {
+func TestWithdrawPrefix(t *testing.T) {
 	testcases := []struct {
 		name          string
 		addPathTX     bool
+		afi           uint16
+		multiProtocol bool
 		prefix        bnet.Prefix
 		path          *route.Path
 		expected      []byte
 		expectedError error
 	}{
 		{
-			name:      "Normal withdraw with ADD-PATH",
-			addPathTX: true,
-			prefix:    bnet.NewPfx(bnet.IPv4(1413010532), 24),
+			name:          "Non bgp withdraw with ADD-PATH",
+			afi:           packet.IPv4AFI,
+			multiProtocol: false,
+			addPathTX:     true,
+			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 with ADD-PATH",
+			afi:           packet.IPv4AFI,
+			multiProtocol: false,
+			addPathTX:     true,
+			prefix:        bnet.NewPfx(bnet.IPv4(1413010532), 24),
+			path: &route.Path{
+				Type: route.BGPPathType,
+			},
+			expected:      []byte{},
+			expectedError: errors.New("got nil BGPPath"),
+		},
+		{
+			name:          "Normal withdraw with ADD-PATH",
+			afi:           packet.IPv4AFI,
+			multiProtocol: false,
+			addPathTX:     true,
+			prefix:        bnet.NewPfx(bnet.IPv4(1413010532), 24),
 			path: &route.Path{
 				Type: route.BGPPathType,
 				BGPPath: &route.BGPPath{
@@ -1024,9 +1003,11 @@ func TestWithdrawPrefixIPv4(t *testing.T) {
 			expectedError: nil,
 		},
 		{
-			name:      "Normal withdraw without ADD-PATH",
-			addPathTX: false,
-			prefix:    bnet.NewPfx(bnet.IPv4(1413010532), 24),
+			name:          "Normal withdraw without ADD-PATH",
+			afi:           packet.IPv4AFI,
+			multiProtocol: false,
+			addPathTX:     false,
+			prefix:        bnet.NewPfx(bnet.IPv4(1413010532), 24),
 			path: &route.Path{
 				Type: route.BGPPathType,
 				BGPPath: &route.BGPPath{
@@ -1045,24 +1026,70 @@ func TestWithdrawPrefixIPv4(t *testing.T) {
 			expectedError: nil,
 		},
 		{
-			name:      "Non bgp withdraw",
-			addPathTX: true,
-			prefix:    bnet.NewPfx(bnet.IPv4(1413010532), 24),
+			name:          "IPv6 MP_UNREACH_NLRI",
+			afi:           packet.IPv6AFI,
+			multiProtocol: true,
+			addPathTX:     false,
+			prefix:        bnet.NewPfx(bnet.IPv6FromBlocks(0x2804, 0x148c, 0, 0, 0, 0, 0, 0), 32),
 			path: &route.Path{
-				Type: route.StaticPathType,
+				Type:    route.BGPPathType,
+				BGPPath: &route.BGPPath{},
+			},
+			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
 			},
-			expected:      []byte{},
-			expectedError: errors.New("wrong path type, expected BGPPathType"),
 		},
 		{
-			name:      "Nil BGPPathType",
-			addPathTX: true,
-			prefix:    bnet.NewPfx(bnet.IPv4(1413010532), 24),
+			name:          "IPv6 MP_UNREACH_NLRI with ADD-PATH",
+			afi:           packet.IPv6AFI,
+			multiProtocol: true,
+			addPathTX:     true,
+			prefix:        bnet.NewPfx(bnet.IPv6FromBlocks(0x2804, 0x148c, 0, 0, 0, 0, 0, 0), 32),
 			path: &route.Path{
 				Type: route.BGPPathType,
+				BGPPath: &route.BGPPath{
+					PathIdentifier: 100,
+				},
+			},
+			expected: []byte{
+				0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // BGP Marker
+				0x00, 0x26, // BGP Message Length
+				0x02,       // BGP Message Type == Update
+				0x00, 0x00, // WithDraw Octet length
+				0x00, 0x0f, // Length
+				0x80,       // Flags
+				0x0f,       // Attribute Code
+				0x0c,       // Attribute length
+				0x00, 0x02, // AFI
+				0x01,                  // SAFI
+				0x00, 0x00, 0x00, 100, // Path Identifier
+				0x20, 0x28, 0x04, 0x14, 0x8c, // Prefix
+			},
+		},
+		{
+			name:          "IPv6 MP_UNREACH_NLRI without multi protocol beeing negotiated",
+			afi:           packet.IPv6AFI,
+			multiProtocol: false,
+			addPathTX:     false,
+			prefix:        bnet.NewPfx(bnet.IPv6FromBlocks(0x2804, 0x148c, 0, 0, 0, 0, 0, 0), 32),
+			path: &route.Path{
+				Type: route.BGPPathType,
+				BGPPath: &route.BGPPath{
+					PathIdentifier: 1,
+				},
 			},
 			expected:      []byte{},
-			expectedError: errors.New("got nil BGPPath"),
+			expectedError: errors.New("IPv6 was not negotiated"),
 		},
 	}
 
@@ -1074,12 +1101,18 @@ func TestWithdrawPrefixIPv4(t *testing.T) {
 
 			u := &UpdateSender{
 				fsm: &FSM{},
+				addressFamily: &fsmAddressFamily{
+					addPathTX:     tc.addPathTX,
+					multiProtocol: tc.multiProtocol,
+					afi:           tc.afi,
+					safi:          packet.UnicastSAFI,
+				},
 				options: &packet.EncodeOptions{
 					UseAddPath: tc.addPathTX,
 				},
 			}
 
-			err := u.withdrawPrefixIPv4(buf, tc.prefix, tc.path)
+			err := u.withdrawPrefix(buf, tc.prefix, tc.path)
 			assert.Equal(t, tc.expectedError, err, "error mismatch")
 			assert.Equal(t, tc.expected, buf.Bytes(), "expected different bytes")
 		})
-- 
GitLab