From 6e7b27cf12c6df2684a64587ede9bf63c63b78ee Mon Sep 17 00:00:00 2001
From: Christoph Petrausch <christoph.petrausch@inovex.de>
Date: Sun, 10 Jun 2018 13:51:31 +0200
Subject: [PATCH] Removed duplicate code and fix bug in update_sender.
UpdateSenderAddPath send nonAddpath Update. UpdateSender send
BGPUpdateAddpath
---
protocols/bgp/server/update_helper.go | 20 +++++++
protocols/bgp/server/update_helper_test.go | 59 +++++++++++++++++++
protocols/bgp/server/update_sender.go | 21 ++-----
.../bgp/server/update_sender_add_path.go | 25 ++------
protocols/bgp/server/withdraw.go | 15 +----
5 files changed, 93 insertions(+), 47 deletions(-)
create mode 100644 protocols/bgp/server/update_helper_test.go
diff --git a/protocols/bgp/server/update_helper.go b/protocols/bgp/server/update_helper.go
index 6873ca8d..4a50f1d7 100644
--- a/protocols/bgp/server/update_helper.go
+++ b/protocols/bgp/server/update_helper.go
@@ -2,10 +2,12 @@ package server
import (
"fmt"
+ "io"
"strings"
"github.com/bio-routing/bio-rd/protocols/bgp/packet"
"github.com/bio-routing/bio-rd/route"
+ log "github.com/sirupsen/logrus"
)
func pathAttribues(p *route.Path, fsm *FSM) (*packet.PathAttribute, error) {
@@ -68,3 +70,21 @@ func addOptionalPathAttribues(p *route.Path, parent *packet.PathAttribute) error
return nil
}
+
+type serializeAbleUpdate interface {
+ SerializeUpdate() ([]byte, error)
+}
+
+func serializeAndSendUpdate(out io.Writer, update serializeAbleUpdate) error {
+ updateBytes, err := update.SerializeUpdate()
+ if err != nil {
+ log.Errorf("Unable to serialize BGP Update: %v", err)
+ return nil
+ }
+
+ _, err = out.Write(updateBytes)
+ if err != nil {
+ return fmt.Errorf("Failed sending Update: %v", err)
+ }
+ return nil
+}
diff --git a/protocols/bgp/server/update_helper_test.go b/protocols/bgp/server/update_helper_test.go
new file mode 100644
index 00000000..81792015
--- /dev/null
+++ b/protocols/bgp/server/update_helper_test.go
@@ -0,0 +1,59 @@
+package server
+
+import (
+ "testing"
+
+ "bytes"
+
+ "github.com/bio-routing/bio-rd/protocols/bgp/packet"
+
+ "github.com/bio-routing/bio-rd/net"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestSerializeAndSendUpdate(t *testing.T) {
+ tests := []struct {
+ name string
+ err error
+ testUpdate serializeAbleUpdate
+ expected []byte
+ }{
+ {
+ name: "normal bgp update",
+ err: nil,
+ testUpdate: &packet.BGPUpdate{
+ WithdrawnRoutesLen: 5,
+ WithdrawnRoutes: &packet.NLRI{
+ IP: strAddr("10.0.0.0"),
+ Pfxlen: 8,
+ Next: &packet.NLRI{
+ IP: strAddr("192.168.0.0"),
+ Pfxlen: 16,
+ },
+ },
+ },
+
+ expected: []byte{
+ 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, // Marker
+ 0, 28, // Length
+ 2, // Type = Update
+ 0, 5, 8, 10, 16, 192, 168, 0, 0, // 2 withdraws
+ },
+ },
+ }
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ buf := bytes.NewBuffer(nil)
+ err := serializeAndSendUpdate(buf, test.testUpdate)
+ assert.Equal(t, test.err, err)
+
+ assert.Equal(t, test.expected, buf.Bytes())
+ })
+
+ }
+}
+
+func strAddr(s string) uint32 {
+ ret, _ := net.StrToAddr(s)
+ return ret
+}
diff --git a/protocols/bgp/server/update_sender.go b/protocols/bgp/server/update_sender.go
index 54c1d1b0..6c6b7dfd 100644
--- a/protocols/bgp/server/update_sender.go
+++ b/protocols/bgp/server/update_sender.go
@@ -34,26 +34,15 @@ func (u *UpdateSender) AddPath(pfx net.Prefix, p *route.Path) error {
return nil
}
- update := &packet.BGPUpdateAddPath{
+ update := &packet.BGPUpdate{
PathAttributes: pathAttrs,
- NLRI: &packet.NLRIAddPath{
- PathIdentifier: p.BGPPath.PathIdentifier,
- IP: pfx.Addr(),
- Pfxlen: pfx.Pfxlen(),
+ NLRI: &packet.NLRI{
+ IP: pfx.Addr(),
+ Pfxlen: pfx.Pfxlen(),
},
}
- updateBytes, err := update.SerializeUpdate()
- if err != nil {
- log.Errorf("Unable to serialize BGP Update: %v", err)
- return nil
- }
-
- _, err = u.fsm.con.Write(updateBytes)
- if err != nil {
- return fmt.Errorf("Failed sending Update: %v", err)
- }
- return nil
+ return serializeAndSendUpdate(u.fsm.con, update)
}
// RemovePath withdraws prefix `pfx` from a peer
diff --git a/protocols/bgp/server/update_sender_add_path.go b/protocols/bgp/server/update_sender_add_path.go
index 84d67960..5c9f5457 100644
--- a/protocols/bgp/server/update_sender_add_path.go
+++ b/protocols/bgp/server/update_sender_add_path.go
@@ -1,8 +1,6 @@
package server
import (
- "fmt"
-
log "github.com/sirupsen/logrus"
"github.com/bio-routing/bio-rd/net"
@@ -32,26 +30,15 @@ func (u *UpdateSenderAddPath) AddPath(pfx net.Prefix, p *route.Path) error {
log.Errorf("Unable to create BGP Update: %v", err)
return nil
}
-
- update := &packet.BGPUpdate{
+ update := &packet.BGPUpdateAddPath{
PathAttributes: pathAttrs,
- NLRI: &packet.NLRI{
- IP: pfx.Addr(),
- Pfxlen: pfx.Pfxlen(),
+ NLRI: &packet.NLRIAddPath{
+ PathIdentifier: p.BGPPath.PathIdentifier,
+ IP: pfx.Addr(),
+ Pfxlen: pfx.Pfxlen(),
},
}
-
- updateBytes, err := update.SerializeUpdate()
- if err != nil {
- log.Errorf("Unable to serialize BGP Update: %v", err)
- return nil
- }
-
- _, err = u.fsm.con.Write(updateBytes)
- if err != nil {
- return fmt.Errorf("Failed sending Update: %v", err)
- }
- return nil
+ return serializeAndSendUpdate(u.fsm.con, update)
}
// RemovePath withdraws prefix `pfx` from a peer
diff --git a/protocols/bgp/server/withdraw.go b/protocols/bgp/server/withdraw.go
index 3769c63f..42e97a39 100644
--- a/protocols/bgp/server/withdraw.go
+++ b/protocols/bgp/server/withdraw.go
@@ -35,12 +35,8 @@ func withDrawPrefixes(out io.Writer, prefixes ...net.Prefix) error {
update := &packet.BGPUpdate{
WithdrawnRoutes: rootNLRI,
}
- data, err := update.SerializeUpdate()
- if err != nil {
- return err
- }
- _, err = out.Write(data)
- return err
+ return serializeAndSendUpdate(out, update)
+
}
// withDrawPrefixesAddPath generates a BGPUpdateAddPath message and write it to the given
@@ -59,10 +55,5 @@ func withDrawPrefixesAddPath(out io.Writer, pfx net.Prefix, p *route.Path) error
Pfxlen: pfx.Pfxlen(),
},
}
- data, err := update.SerializeUpdate()
- if err != nil {
- return err
- }
- _, err = out.Write(data)
- return err
+ return serializeAndSendUpdate(out, update)
}
--
GitLab