From 68e33f6acea55a067aaa42a9bc350088d2555ed7 Mon Sep 17 00:00:00 2001
From: Maximilian Wilhelm <max@sdn.clinic>
Date: Sun, 24 Jun 2018 19:23:29 +0200
Subject: [PATCH] Use RWLock with menaingful name, overflow check, white-sapce
fixes.
Signed-off-by: Maximilian Wilhelm <max@sdn.clinic>
---
protocols/bgp/server/server.go | 3 +-
routingtable/BUILD.bazel | 2 +-
routingtable/contributing_asn_list.go | 41 ++++++++++++++++++---------
3 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/protocols/bgp/server/server.go b/protocols/bgp/server/server.go
index 4015f44b..db6ae24f 100644
--- a/protocols/bgp/server/server.go
+++ b/protocols/bgp/server/server.go
@@ -7,10 +7,9 @@ import (
"strings"
"sync"
- "github.com/bio-routing/bio-rd/routingtable/locRIB"
-
"github.com/bio-routing/bio-rd/config"
"github.com/bio-routing/bio-rd/protocols/bgp/packet"
+ "github.com/bio-routing/bio-rd/routingtable/locRIB"
log "github.com/sirupsen/logrus"
)
diff --git a/routingtable/BUILD.bazel b/routingtable/BUILD.bazel
index 22b4ef8c..fdea6278 100644
--- a/routingtable/BUILD.bazel
+++ b/routingtable/BUILD.bazel
@@ -26,7 +26,7 @@ go_test(
name = "go_default_test",
srcs = [
"client_manager_test.go",
- "contributing_asn_list_test.go",
+ "contributing_asn_list_test.go",
"table_test.go",
"trie_test.go",
"update_helper_test.go",
diff --git a/routingtable/contributing_asn_list.go b/routingtable/contributing_asn_list.go
index f08058e2..dd6b9885 100644
--- a/routingtable/contributing_asn_list.go
+++ b/routingtable/contributing_asn_list.go
@@ -1,6 +1,8 @@
package routingtable
import (
+ "fmt"
+ "math"
"sync"
)
@@ -11,8 +13,8 @@ type contributingASN struct {
// ContributingASNs contains a list of contributing ASN to a LocRIB to check ASPaths for possible routing loops.
type ContributingASNs struct {
- contributingASNs []*contributingASN
- mu sync.RWMutex
+ contributingASNs []*contributingASN
+ contributingASNsLock sync.RWMutex
}
// NewContributingASNs creates a list of contributing ASNs to a LocRIB for routing loop prevention.
@@ -26,12 +28,17 @@ func NewContributingASNs() *ContributingASNs {
// Add a new ASN to the list of contributing ASNs or add the ref count of an existing one.
func (c *ContributingASNs) Add(asn uint32) {
- c.mu.RLock()
- defer c.mu.RUnlock()
+ c.contributingASNsLock.Lock()
+ defer c.contributingASNsLock.Unlock()
for _, cASN := range c.contributingASNs {
if cASN.asn == asn {
cASN.count++
+
+ if cASN.count == math.MaxUint32 {
+ panic(fmt.Sprintf("Contributing ASNs counter overflow triggered for AS %d. Dyning of shame.", asn))
+ }
+
return
}
}
@@ -44,27 +51,33 @@ func (c *ContributingASNs) Add(asn uint32) {
// Remove a ASN to the list of contributing ASNs or decrement the ref count of an existing one.
func (c *ContributingASNs) Remove(asn uint32) {
- c.mu.RLock()
- defer c.mu.RUnlock()
+ c.contributingASNsLock.Lock()
+ defer c.contributingASNsLock.Unlock()
asnList := c.contributingASNs
for i, cASN := range asnList {
- if cASN.asn == asn {
- cASN.count--
+ if cASN.asn != asn {
+ continue
+ }
- if cASN.count == 0 {
- copy(asnList[i:], asnList[i+1:])
- asnList = asnList[:len(asnList)]
- c.contributingASNs = asnList[:len(asnList)-1]
- }
- return
+ cASN.count--
+
+ if cASN.count == 0 {
+ copy(asnList[i:], asnList[i+1:])
+ asnList = asnList[:len(asnList)]
+ c.contributingASNs = asnList[:len(asnList)-1]
}
+
+ return
}
}
// IsContributingASN checks if a given ASN is part of the contributing ASNs
func (c *ContributingASNs) IsContributingASN(asn uint32) bool {
+ c.contributingASNsLock.Lock()
+ defer c.contributingASNsLock.Unlock()
+
for _, cASN := range c.contributingASNs {
if asn == cASN.asn {
return true
--
GitLab