From f8b1767b64af60f67fb854512faaa8223731ad8b Mon Sep 17 00:00:00 2001
From: takt <oliver.herms@exaring.de>
Date: Tue, 30 Jun 2020 15:09:26 +0200
Subject: [PATCH] Cleanup BMP server locking and make VRF id human readable
(#276)
---
cmd/ris/risserver/server.go | 3 ++-
protocols/bgp/server/bmp_router.go | 4 ++--
protocols/bgp/server/bmp_server.go | 30 +++++++++++++++---------------
routingtable/vrf/vrf.go | 10 ++++++++++
routingtable/vrf/vrf_test.go | 24 ++++++++++++++++++++++++
5 files changed, 53 insertions(+), 18 deletions(-)
diff --git a/cmd/ris/risserver/server.go b/cmd/ris/risserver/server.go
index f7fae806..8c950c6c 100644
--- a/cmd/ris/risserver/server.go
+++ b/cmd/ris/risserver/server.go
@@ -10,6 +10,7 @@ import (
"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/bio-routing/bio-rd/routingtable/vrf"
"github.com/prometheus/client_golang/prometheus"
@@ -60,7 +61,7 @@ func (s Server) getRIB(rtr string, vrfID uint64, ipVersion netapi.IP_Version) (*
v := r.GetVRF(vrfID)
if v == nil {
- return nil, fmt.Errorf("Unable to get VRF %d", vrfID)
+ return nil, fmt.Errorf("Unable to get VRF %s", vrf.RouteDistinguisherHumanReadable(vrfID))
}
var rib *locRIB.LocRIB
diff --git a/protocols/bgp/server/bmp_router.go b/protocols/bgp/server/bmp_router.go
index 019a2b7a..31c8348a 100644
--- a/protocols/bgp/server/bmp_router.go
+++ b/protocols/bgp/server/bmp_router.go
@@ -241,7 +241,7 @@ func (r *Router) processPeerDownNotification(msg *bmppkt.PeerDownNotification) {
r.logger.WithFields(log.Fields{
"address": r.address.String(),
"router": r.name,
- "peer_distinguisher": msg.PerPeerHeader.PeerDistinguisher,
+ "peer_distinguisher": vrf.RouteDistinguisherHumanReadable(msg.PerPeerHeader.PeerDistinguisher),
"peer_address": addrToNetIP(msg.PerPeerHeader.PeerAddress).String(),
}).Infof("peer down notification received")
atomic.AddUint64(&r.counters.peerDownNotificationMessages, 1)
@@ -257,7 +257,7 @@ func (r *Router) processPeerUpNotification(msg *bmppkt.PeerUpNotification) error
r.logger.WithFields(log.Fields{
"address": r.address.String(),
"router": r.name,
- "peer_distinguisher": msg.PerPeerHeader.PeerDistinguisher,
+ "peer_distinguisher": vrf.RouteDistinguisherHumanReadable(msg.PerPeerHeader.PeerDistinguisher),
"peer_address": addrToNetIP(msg.PerPeerHeader.PeerAddress).String(),
}).Infof("peer up notification received")
diff --git a/protocols/bgp/server/bmp_server.go b/protocols/bgp/server/bmp_server.go
index 139e1e24..99dfe141 100644
--- a/protocols/bgp/server/bmp_server.go
+++ b/protocols/bgp/server/bmp_server.go
@@ -26,7 +26,6 @@ type BMPServer struct {
routers map[string]*Router
routersMu sync.RWMutex
ribClients map[string]map[afiClient]struct{}
- gloablMu sync.RWMutex
metrics *bmpMetricsService
}
@@ -52,9 +51,6 @@ func conString(host string, port uint16) string {
// AddRouter adds a router to which we connect with BMP
func (b *BMPServer) AddRouter(addr net.IP, port uint16) {
- b.gloablMu.Lock()
- defer b.gloablMu.Unlock()
-
r := newRouter(addr, port)
b.addRouter(r)
@@ -116,18 +112,26 @@ func (b *BMPServer) AddRouter(addr net.IP, port uint16) {
}
func (b *BMPServer) addRouter(r *Router) {
+ b.routersMu.Lock()
+ defer b.routersMu.Unlock()
+
b.routers[fmt.Sprintf("%s", r.address.String())] = r
}
-// RemoveRouter removes a BMP monitored router
-func (b *BMPServer) RemoveRouter(addr net.IP, port uint16) {
- b.gloablMu.Lock()
- defer b.gloablMu.Unlock()
+func (b *BMPServer) deleteRouter(addr net.IP) {
+ b.routersMu.Lock()
+ defer b.routersMu.Unlock()
+
+ delete(b.routers, addr.String())
+}
+// RemoveRouter removes a BMP monitored router
+func (b *BMPServer) RemoveRouter(addr net.IP) {
id := addr.String()
r := b.routers[id]
r.stop <- struct{}{}
- delete(b.routers, id)
+
+ b.deleteRouter(addr)
}
func (b *BMPServer) getRouters() []*Router {
@@ -183,12 +187,8 @@ func (b *BMPServer) GetRouter(name string) *Router {
b.routersMu.RLock()
defer b.routersMu.RUnlock()
- for x := range b.routers {
- if x != name {
- continue
- }
-
- return b.routers[x]
+ if _, ok := b.routers[name]; ok {
+ return b.routers[name]
}
return nil
diff --git a/routingtable/vrf/vrf.go b/routingtable/vrf/vrf.go
index 0e440ba5..1162595e 100644
--- a/routingtable/vrf/vrf.go
+++ b/routingtable/vrf/vrf.go
@@ -137,3 +137,13 @@ func (v *VRF) Dispose() {
delete(v.ribNames, ribName)
}
}
+
+// RouteDistinguisherHumanReadable converts 64bit route distinguisher to human readable string form
+func RouteDistinguisherHumanReadable(rdi uint64) string {
+ asn := rdi >> 32
+
+ netIDMask := uint64(0x00000000ffffffff)
+ netID := rdi & netIDMask
+
+ return fmt.Sprintf("%d:%d", asn, netID)
+}
diff --git a/routingtable/vrf/vrf_test.go b/routingtable/vrf/vrf_test.go
index 4bd93653..56466ca3 100644
--- a/routingtable/vrf/vrf_test.go
+++ b/routingtable/vrf/vrf_test.go
@@ -71,3 +71,27 @@ func TestUnregister(t *testing.T) {
assert.False(t, found, "vrf must not be in global registry")
}
+
+func TestRouteDistinguisherHumanReadable(t *testing.T) {
+ tests := []struct {
+ name string
+ rdi uint64
+ expected string
+ }{
+ {
+ name: "Test #1",
+ rdi: 0,
+ expected: "0:0",
+ },
+ {
+ name: "Test #2",
+ rdi: 220434901565105,
+ expected: "51324:65201",
+ },
+ }
+
+ for _, test := range tests {
+ res := RouteDistinguisherHumanReadable(test.rdi)
+ assert.Equal(t, test.expected, res, test.name)
+ }
+}
--
GitLab