Cleanup BMP server locking and make VRF id human readable (#276)

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

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")
 

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

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)
+}

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)
+	}
+}