From d55bc402ba103756b82afedf5d15ef46d8740fbf Mon Sep 17 00:00:00 2001
From: cedi <cedi@users.noreply.github.com>
Date: Mon, 22 Oct 2018 22:35:16 +0200
Subject: [PATCH] Cleanup code and address @BarbarossaTM pull request remarks
- Remove Makefile for adding it later in a seperate PR
- Remove main files which where added by accident
- Add constants for IPv4 and IPv6
- Improve log messages
- Simplify netlink_reader addPathsToClients and removePathsFromClients
- Refactor Route's Equal function
- Return false for ECMP in netlink_path since ecmp is currently not
implemented in netlink
---
Makefile | 26 ------
main.go | 63 --------------
main_ipv4.go | 49 -----------
main_ipv6.go | 72 ---------------
protocols/netlink/netlink.go | 4 +-
protocols/netlink/netlink_reader.go | 130 ++++++++++++++++------------
route/netlink_path.go | 2 +-
route/route.go | 34 ++++----
8 files changed, 94 insertions(+), 286 deletions(-)
delete mode 100644 Makefile
delete mode 100644 main.go
delete mode 100644 main_ipv4.go
delete mode 100644 main_ipv6.go
diff --git a/Makefile b/Makefile
deleted file mode 100644
index 22eb8c21..00000000
--- a/Makefile
+++ /dev/null
@@ -1,26 +0,0 @@
-NAME=bio-rd
-
-all: test
-
-$(NAME): gazelle
- bazel build //:bio-rd
-
-gazelle:
- bazel run //:gazelle -- update
-
-test: $(NAME)
- bazel test //...
-
-dep:
- bazel build //vendor/github.com/golang/dep/cmd/dep
-
-dep-clean:
- # hack: dep of dep gives us these, and it breaks gazelle
- rm -rf vendor/github.com/golang/dep/cmd/dep/testdata
- rm -rf vendor/github.com/golang/dep/internal/fs/testdata/symlinks/dir-symlink
-
-clean: dep-clean
- bazel clean
- rm $(NAME)
-
-.PHONY: $(NAME) gazelle clean dep dep-clean
diff --git a/main.go b/main.go
deleted file mode 100644
index 659857da..00000000
--- a/main.go
+++ /dev/null
@@ -1,63 +0,0 @@
-package main
-
-import (
- "net"
- "os"
- "time"
-
- "github.com/bio-routing/bio-rd/config"
- "github.com/bio-routing/bio-rd/protocols/bgp/server"
- "github.com/bio-routing/bio-rd/protocols/netlink"
- "github.com/bio-routing/bio-rd/routingtable/locRIB"
- log "github.com/sirupsen/logrus"
-
- bnet "github.com/bio-routing/bio-rd/net"
-)
-
-func strAddr(s string) uint32 {
- ret, _ := bnet.StrToAddr(s)
- return ret
-}
-
-func main() {
- log.SetLevel(log.DebugLevel)
-
- f, err := os.OpenFile("/var/log/bio-rd.log", os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
- if err != nil {
- log.Fatalf("error opening file: %v", err)
- }
- defer f.Close()
-
- log.SetOutput(f)
-
- log.Info("bio-routing started...\n")
-
- cfg := &config.Global{
- Listen: true,
- LocalAddressList: []net.IP{
- net.IPv4(169, 254, 0, 2),
- },
- }
-
- rib := locRIB.New()
- b := server.NewBgpServer()
- startBGPServer(b, rib, cfg)
-
- // Netlink communication
- n := protocolnetlink.NewNetlink(&config.Netlink{
- HoldTime: time.Second * 15,
- UpdateInterval: time.Second * 15,
- RoutingTable: config.RtMain,
- }, rib)
- n.Start()
-
- go func() {
- for {
- log.Debugf("LocRIB count: %d", rib.Count())
- log.Debugf(rib.Print())
- time.Sleep(time.Second * 10)
- }
- }()
-
- select {}
-}
diff --git a/main_ipv4.go b/main_ipv4.go
deleted file mode 100644
index 12bb67ae..00000000
--- a/main_ipv4.go
+++ /dev/null
@@ -1,49 +0,0 @@
-package main
-
-import (
- "time"
-
- "github.com/bio-routing/bio-rd/routingtable"
- "github.com/bio-routing/bio-rd/routingtable/locRIB"
-
- "github.com/bio-routing/bio-rd/config"
- "github.com/bio-routing/bio-rd/protocols/bgp/server"
- "github.com/bio-routing/bio-rd/routingtable/filter"
- log "github.com/sirupsen/logrus"
-
- bnet "github.com/bio-routing/bio-rd/net"
-)
-
-func startBGPServer(b server.BGPServer, rib *locRIB.LocRIB, cfg *config.Global) {
- err := b.Start(cfg)
- if err != nil {
- log.Fatalf("Unable to start BGP server: %v", err)
- }
-
- b.AddPeer(config.Peer{
- AdminEnabled: true,
- LocalAS: 65200,
- PeerAS: 65100,
- PeerAddress: bnet.IPv4FromOctets(169, 254, 0, 1),
- LocalAddress: bnet.IPv4FromOctets(169, 254, 0, 2),
- ReconnectInterval: time.Second * 20,
- HoldTime: time.Second * 20,
- KeepAlive: time.Second * 20,
- Passive: false,
- RouterID: b.RouterID(),
-
- //AddPathSend: routingtable.ClientOptions{
- // MaxPaths: 10,
- //},
- //RouteServerClient: true,
- IPv4: &config.AddressFamilyConfig{
- RIB: rib,
- ImportFilter: filter.NewAcceptAllFilter(),
- ExportFilter: filter.NewAcceptAllFilter(),
- AddPathSend: routingtable.ClientOptions{
- MaxPaths: 10,
- },
- AddPathRecv: true,
- },
- })
-}
diff --git a/main_ipv6.go b/main_ipv6.go
deleted file mode 100644
index cc26484b..00000000
--- a/main_ipv6.go
+++ /dev/null
@@ -1,72 +0,0 @@
-// +build ipv6
-
-package main
-
-import (
- "net"
- "time"
-
- "github.com/bio-routing/bio-rd/config"
- "github.com/bio-routing/bio-rd/protocols/bgp/server"
- "github.com/bio-routing/bio-rd/routingtable"
- "github.com/bio-routing/bio-rd/routingtable/filter"
- "github.com/bio-routing/bio-rd/routingtable/locRIB"
- "github.com/sirupsen/logrus"
-
- bnet "github.com/bio-routing/bio-rd/net"
-)
-
-func startServer(b server.BGPServer, rib *locRIB.LocRIB) {
-
- err := b.Start(&config.Global{
- Listen: true,
- LocalAddressList: []net.IP{
- net.IP{0x20, 0x01, 0x6, 0x78, 0x1, 0xe0, 0, 0, 0, 0, 0, 0, 0, 0, 0xca, 0xfe},
- },
- })
- if err != nil {
- logrus.Fatalf("Unable to start BGP server: %v", err)
- }
-
- b.AddPeer(config.Peer{
- AdminEnabled: true,
- LocalAS: 65200,
- PeerAS: 202739,
- PeerAddress: bnet.IPv6FromBlocks(0x2001, 0x678, 0x1e0, 0, 0, 0, 0, 1),
- LocalAddress: bnet.IPv6FromBlocks(0x2001, 0x678, 0x1e0, 0, 0, 0, 0, 0xcafe),
- ReconnectInterval: time.Second * 15,
- HoldTime: time.Second * 90,
- KeepAlive: time.Second * 30,
- Passive: true,
- RouterID: b.RouterID(),
- IPv6: &config.AddressFamilyConfig{
- RIB: rib,
- ImportFilter: filter.NewAcceptAllFilter(),
- ExportFilter: filter.NewDrainFilter(),
- AddPathSend: routingtable.ClientOptions{
- BestOnly: true,
- },
- },
- })
-
- b.AddPeer(config.Peer{
- AdminEnabled: true,
- LocalAS: 65200,
- PeerAS: 65400,
- PeerAddress: bnet.IPv6FromBlocks(0x2001, 0x678, 0x1e0, 0xcafe, 0, 0, 0, 5),
- LocalAddress: bnet.IPv6FromBlocks(0x2001, 0x678, 0x1e0, 0, 0, 0, 0, 0xcafe),
- ReconnectInterval: time.Second * 15,
- HoldTime: time.Second * 90,
- KeepAlive: time.Second * 30,
- Passive: true,
- RouterID: b.RouterID(),
- IPv6: &config.AddressFamilyConfig{
- RIB: rib,
- ImportFilter: filter.NewDrainFilter(),
- ExportFilter: filter.NewAcceptAllFilter(),
- AddPathSend: routingtable.ClientOptions{
- BestOnly: true,
- },
- },
- })
-}
diff --git a/protocols/netlink/netlink.go b/protocols/netlink/netlink.go
index f5fdedce..fa39d601 100644
--- a/protocols/netlink/netlink.go
+++ b/protocols/netlink/netlink.go
@@ -37,9 +37,9 @@ func (n *Netlink) Start() {
// 1. from locRib to Kernel
n.locRib.ClientManager.RegisterWithOptions(n.writer, options)
- // 2. vom Kernel to locRib
+ // 2. from Kernel to locRib
n.reader.ClientManager.RegisterWithOptions(n.locRib, options)
- // Listn for new routes from kernel
+ // Listen for new routes from kernel
go n.reader.Read()
}
diff --git a/protocols/netlink/netlink_reader.go b/protocols/netlink/netlink_reader.go
index 75a97637..b4d16ea5 100644
--- a/protocols/netlink/netlink_reader.go
+++ b/protocols/netlink/netlink_reader.go
@@ -14,6 +14,12 @@ import (
"github.com/vishvananda/netlink"
)
+// Constants for IP family
+const (
+ IPFamily4 = 4 // IPv4
+ IPFamily6 = 6 // IPv6
+)
+
// NetlinkReader read routes from the Linux Kernel and propagates it to the locRIB
type NetlinkReader struct {
options *config.Netlink
@@ -45,7 +51,7 @@ func (nr *NetlinkReader) Read() {
for {
// Family doesn't matter. I only filter by the rt_table here
- routes, err := netlink.RouteListFiltered(4, &netlink.Route{Table: nr.options.RoutingTable}, netlink.RT_FILTER_TABLE)
+ routes, err := netlink.RouteListFiltered(IPFamily4, &netlink.Route{Table: nr.options.RoutingTable}, netlink.RT_FILTER_TABLE)
if err != nil {
log.WithError(err).Panic("Failed to read routes from kernel")
}
@@ -84,39 +90,45 @@ func (nr *NetlinkReader) propagateChanges(routes []netlink.Route) {
// Add given paths to clients
func (nr *NetlinkReader) addPathsToClients(routes []netlink.Route) {
- for _, client := range nr.ClientManager.Clients() {
- // only advertise changed routes
+ // only advertise changed routes
+ nr.mu.RLock()
+ advertise := route.NetlinkRouteDiff(routes, nr.routes)
+ nr.mu.RUnlock()
+
+ for _, r := range advertise {
+ // Is it a BIO-Written route? if so, skip it, dont advertise it
+ if r.Protocol == route.ProtoBio {
+ log.WithFields(routeLogFields(r)).Debug("Skipping bio route")
+ continue
+ }
- nr.mu.RLock()
- advertise := route.NetlinkRouteDiff(routes, nr.routes)
- nr.mu.RUnlock()
+ // create pfx and path from route
+ pfx := bnet.NewPfxFromIPNet(r.Dst)
+ path, err := createPathFromRoute(&r)
+ if err != nil {
+ log.WithError(err).WithFields(log.Fields{
+ "prefix": pfx.String(),
+ "path": path.String(),
+ }).Error("Unable to create path")
+ continue
+ }
- for _, r := range advertise {
- // Is it a BIO-Written route? if so, skip it, dont advertise it
- if r.Protocol == route.ProtoBio {
- log.WithFields(routeLogFields(r)).Debug("Skipping bio route")
- continue
- }
+ // Apply filter (if existing)
+ if nr.filter != nil {
+ var reject bool
+ // TODO: Implement filter that cann handle netlinkRoute objects
+ path, reject = nr.filter.ProcessTerms(pfx, path)
+ if reject {
+ log.WithError(err).WithFields(log.Fields{
+ "prefix": pfx.String(),
+ "path": path.String(),
+ }).Debug("Skipping route due to filter")
- // create pfx and path from route
- pfx := bnet.NewPfxFromIPNet(r.Dst)
- path, err := createPathFromRoute(&r)
- if err != nil {
- log.WithError(err).Error("Unable to create path")
continue
}
+ }
- // Apply filter (if existing)
- if nr.filter != nil {
- var reject bool
- // TODO: Implement filter that cann handle netlinkRoute objects
- path, reject = nr.filter.ProcessTerms(pfx, path)
- if reject {
- log.Debug("Skipping route due to filter")
- continue
- }
- }
-
+ for _, client := range nr.ClientManager.Clients() {
log.WithFields(log.Fields{
"pfx": pfx,
"path": path,
@@ -128,42 +140,46 @@ func (nr *NetlinkReader) addPathsToClients(routes []netlink.Route) {
// Remove given paths from clients
func (nr *NetlinkReader) removePathsFromClients(routes []netlink.Route) {
- for _, client := range nr.ClientManager.Clients() {
- // If there where no routes yet, just skip this funktion. There's nothing to delete
- nr.mu.RLock()
- if len(nr.routes) == 0 {
- nr.mu.RUnlock()
- break
- }
+ nr.mu.RLock()
+
+ // get the number of routes
+ routeLength := len(nr.routes)
- // only withdraw changed routes
- withdraw := route.NetlinkRouteDiff(nr.routes, routes)
+ // If there where no routes yet, just skip this funktion. There's nothing to delete
+ if routeLength == 0 {
nr.mu.RUnlock()
+ return
+ }
- for _, r := range withdraw {
- // Is it a BIO-Written route? if so, skip it, dont advertise it
- if r.Protocol == route.ProtoBio {
- continue
- }
+ // only withdraw changed routes
+ withdraw := route.NetlinkRouteDiff(nr.routes, routes)
+ nr.mu.RUnlock()
- // create pfx and path from route
- pfx := bnet.NewPfxFromIPNet(r.Dst)
- path, err := createPathFromRoute(&r)
- if err != nil {
- log.WithError(err).Error("Unable to create path")
- continue
- }
+ for _, r := range withdraw {
+ // Is it a BIO-Written route? if so, skip it, dont advertise it
+ if r.Protocol == route.ProtoBio {
+ continue
+ }
+
+ // create pfx and path from route
+ pfx := bnet.NewPfxFromIPNet(r.Dst)
+ path, err := createPathFromRoute(&r)
+ if err != nil {
+ log.WithError(err).Error("Unable to create path")
+ continue
+ }
- // Apply filter (if existing)
- if nr.filter != nil {
- var reject bool
- // TODO: Implement filter that cann handle netlinkRoute objects
- path, reject = nr.filter.ProcessTerms(pfx, path)
- if reject {
- continue
- }
+ // Apply filter (if existing)
+ if nr.filter != nil {
+ var reject bool
+ // TODO: Implement filter that cann handle netlinkRoute objects
+ path, reject = nr.filter.ProcessTerms(pfx, path)
+ if reject {
+ continue
}
+ }
+ for _, client := range nr.ClientManager.Clients() {
log.WithFields(log.Fields{
"pfx": pfx,
"path": path,
diff --git a/route/netlink_path.go b/route/netlink_path.go
index a0f5e2f4..1f01495a 100644
--- a/route/netlink_path.go
+++ b/route/netlink_path.go
@@ -133,7 +133,7 @@ func (s *NetlinkPath) Select(t *NetlinkPath) int8 {
// ECMP determines if path s and t are equal in terms of ECMP
func (s *NetlinkPath) ECMP(t *NetlinkPath) bool {
- return true
+ return false
}
// Copy duplicates the current object
diff --git a/route/route.go b/route/route.go
index 59b25f71..e7f6730b 100644
--- a/route/route.go
+++ b/route/route.go
@@ -199,36 +199,38 @@ func (r *Route) PathSelection() {
r.updateEqualPathCount()
}
-// Euql compares Are two routes and return true if they are equal
+// Equal compares if two routes are the same
func (r *Route) Equal(other *Route) bool {
r.mu.Lock()
defer r.mu.Unlock()
- a := r.pfx.Equal(other.pfx)
- b := r.ecmpPaths == other.ecmpPaths
- c := true
+ pfxEqual := r.pfx.Equal(other.pfx)
+ ecmpPathsEqual := r.ecmpPaths == other.ecmpPaths
+ pathsEqual := comparePathSlice(r.paths, other.paths)
- if r.paths == nil && other.paths == nil {
- c = true
- return a && b && c
+ return pfxEqual && ecmpPathsEqual && pathsEqual
+}
+
+// Compare two path pointer slices if they are equal
+func comparePathSlice(left, right []*Path) bool {
+ if left == nil && right == nil {
+ return true
}
- if len(r.paths) != len(other.paths) {
- c = false
- return a && b && c
+ if len(left) != len(right) {
+ return false
}
- for _, myP := range r.paths {
- if !r.compareItemExists(myP, other.paths) {
- c = false
- return a && b && c
+ for _, leftPath := range left {
+ if !compareItemExists(leftPath, right) {
+ return false
}
}
- return a && b && c
+ return true
}
-func (r *Route) compareItemExists(needle *Path, haystack []*Path) bool {
+func compareItemExists(needle *Path, haystack []*Path) bool {
for _, compare := range haystack {
if needle.Equal(compare) {
return true
--
GitLab