diff options
author | FUJITA Tomonori <fujita.tomonori@gmail.com> | 2019-08-27 15:46:32 +0900 |
---|---|---|
committer | FUJITA Tomonori <fujita.tomonori@gmail.com> | 2019-08-27 15:46:32 +0900 |
commit | 2682343deac42e6384f915dd5a1cde0154508019 (patch) | |
tree | 9fa12cbda7b714109ebb3f43ea55aa222ae40c2e | |
parent | 8e348d6f184db9cd802cb81bf7b4a9dbb338fb74 (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.go | 9 | ||||
-rw-r--r-- | internal/pkg/table/destination.go | 2 | ||||
-rw-r--r-- | internal/pkg/table/path.go | 10 | ||||
-rw-r--r-- | internal/pkg/table/table.go | 8 |
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 { |