summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorFUJITA Tomonori <fujita.tomonori@gmail.com>2019-08-27 15:46:32 +0900
committerFUJITA Tomonori <fujita.tomonori@gmail.com>2019-08-27 15:46:32 +0900
commit2682343deac42e6384f915dd5a1cde0154508019 (patch)
tree9fa12cbda7b714109ebb3f43ea55aa222ae40c2e
parent8e348d6f184db9cd802cb81bf7b4a9dbb338fb74 (diff)
fix duplicated local path id bug
Fix a bug that the same path id is assigned to two paths. The bug happens in the following way: 1. a new path is assigned to a local path id. 2. an import policy dropping the path from the master rib is added and execute softreset in. 3. the path still exists in the adj but doesn't in the master. the path id was freed (marked as unused). 5. a new path with the same prefix comes from another peer. The same id is assigned to the new path. 6 deleted the policy and execute softreset in. 7 there are two paths in the master with the same path id. This path guarantees that only when a path is removed in the adj, the id for path is freed. Note that this doesn't fatten Path strcuture, which should be avoided for any reason. Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
-rw-r--r--internal/pkg/table/adj.go9
-rw-r--r--internal/pkg/table/destination.go2
-rw-r--r--internal/pkg/table/path.go10
-rw-r--r--internal/pkg/table/table.go8
4 files changed, 26 insertions, 3 deletions
diff --git a/internal/pkg/table/adj.go b/internal/pkg/table/adj.go
index acb2c07b..d434c938 100644
--- a/internal/pkg/table/adj.go
+++ b/internal/pkg/table/adj.go
@@ -53,6 +53,7 @@ func (adj *AdjRib) Update(pathList []*Path) {
adj.accepted[rf]--
}
}
+ path.SetDropped(true)
} else {
if found {
if old.IsAsLooped() && !path.IsAsLooped() {
@@ -111,7 +112,9 @@ func (adj *AdjRib) Drop(rfList []bgp.RouteFamily) []*Path {
for _, rf := range rfList {
if _, ok := adj.table[rf]; ok {
for _, p := range adj.table[rf] {
- l = append(l, p.Clone(true))
+ w := p.Clone(true)
+ w.SetDropped(true)
+ l = append(l, w)
}
adj.table[rf] = make(map[string]*Path)
adj.accepted[rf] = 0
@@ -130,7 +133,9 @@ func (adj *AdjRib) DropStale(rfList []bgp.RouteFamily) []*Path {
if !p.IsAsLooped() {
adj.accepted[rf]--
}
- pathList = append(pathList, p.Clone(true))
+ w := p.Clone(true)
+ w.SetDropped(true)
+ pathList = append(pathList, w)
}
}
}
diff --git a/internal/pkg/table/destination.go b/internal/pkg/table/destination.go
index 2cb902e1..4adc6290 100644
--- a/internal/pkg/table/destination.go
+++ b/internal/pkg/table/destination.go
@@ -265,7 +265,7 @@ func (dest *Destination) Calculate(newPath *Path) *Update {
if newPath.IsWithdraw {
p := dest.explicitWithdraw(newPath)
- if p != nil {
+ if p != nil && newPath.IsDropped() {
if id := p.GetNlri().PathLocalIdentifier(); id != 0 {
dest.localIdMap.Unflag(uint(id))
}
diff --git a/internal/pkg/table/path.go b/internal/pkg/table/path.go
index 743f1c1f..32616d2e 100644
--- a/internal/pkg/table/path.go
+++ b/internal/pkg/table/path.go
@@ -138,6 +138,8 @@ type Path struct {
dels []bgp.BGPAttrType
attrsHash uint32
aslooped bool
+ // doesn't exist in the adj
+ dropped bool
// For BGP Nexthop Tracking, this field shows if nexthop is invalidated by IGP.
IsNexthopInvalid bool
@@ -388,6 +390,14 @@ func (path *Path) SetAsLooped(y bool) {
path.aslooped = y
}
+func (path *Path) IsDropped() bool {
+ return path.dropped
+}
+
+func (path *Path) SetDropped(y bool) {
+ path.dropped = y
+}
+
func (path *Path) IsLLGRStale() bool {
for _, c := range path.GetCommunities() {
if c == uint32(bgp.COMMUNITY_LLGR_STALE) {
diff --git a/internal/pkg/table/table.go b/internal/pkg/table/table.go
index c7e505cc..9a9bdd8b 100644
--- a/internal/pkg/table/table.go
+++ b/internal/pkg/table/table.go
@@ -17,6 +17,7 @@ package table
import (
"fmt"
+ "math/bits"
"net"
"strings"
"unsafe"
@@ -118,6 +119,13 @@ func (t *Table) deleteRTCPathsByVrf(vrf *Vrf, vrfs map[string]*Vrf) []*Path {
}
func (t *Table) deleteDest(dest *Destination) {
+ count := 0
+ for _, v := range dest.localIdMap.bitmap {
+ count += bits.OnesCount64(v)
+ }
+ if len(dest.localIdMap.bitmap) != 0 && count != 1 {
+ return
+ }
destinations := t.GetDestinations()
delete(destinations, t.tableKey(dest.GetNlri()))
if len(destinations) == 0 {