table: fix adjrib regression

fix adjrib regression due to the d2801681 commit.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

3 files changed, 33 insertions, 62 deletions

diff --git a/server/peer.go b/server/peer.go
index 967a0e5d..0ac082d3 100644
--- a/server/peer.go
+++ b/server/peer.go
@@ -59,8 +59,8 @@ func NewPeer(g config.Global, conf config.Neighbor, loc *table.TableManager, pol
 	conf.State.SessionState = config.IntToSessionStateMap[int(bgp.BGP_FSM_IDLE)]
 	conf.Timers.State.Downtime = time.Now().Unix()
 	rfs, _ := config.AfiSafis(conf.AfiSafis).ToRfList()
-	peer.adjRibIn = table.NewAdjRib(peer.ID(), rfs)
-	peer.adjRibOut = table.NewAdjRib(peer.ID(), rfs)
+	peer.adjRibIn = table.NewAdjRib(peer.ID(), rfs, g.Collector.Enabled)
+	peer.adjRibOut = table.NewAdjRib(peer.ID(), rfs, g.Collector.Enabled)
 	peer.fsm = NewFSM(&g, &conf, policy)
 	return peer
 }
diff --git a/table/adj.go b/table/adj.go
index ee9cf549..9e88e155 100644
--- a/table/adj.go
+++ b/table/adj.go
@@ -20,25 +20,23 @@ import (
 	"reflect"
 )
 
-type Dest struct {
-	pathList []*Path
-}
-
 type AdjRib struct {
-	id       string
-	accepted map[bgp.RouteFamily]int
-	table    map[bgp.RouteFamily]map[string]*Dest
+	id          string
+	accepted    map[bgp.RouteFamily]int
+	table       map[bgp.RouteFamily]map[string]*Path
+	isCollector bool
 }
 
-func NewAdjRib(id string, rfList []bgp.RouteFamily) *AdjRib {
-	table := make(map[bgp.RouteFamily]map[string]*Dest)
+func NewAdjRib(id string, rfList []bgp.RouteFamily, isCollector bool) *AdjRib {
+	table := make(map[bgp.RouteFamily]map[string]*Path)
 	for _, rf := range rfList {
-		table[rf] = make(map[string]*Dest)
+		table[rf] = make(map[string]*Path)
 	}
 	return &AdjRib{
-		id:       id,
-		table:    table,
-		accepted: make(map[bgp.RouteFamily]int),
+		id:          id,
+		table:       table,
+		accepted:    make(map[bgp.RouteFamily]int),
+		isCollector: isCollector,
 	}
 }
 
@@ -49,35 +47,21 @@ func (adj *AdjRib) Update(pathList []*Path) {
 		}
 		rf := path.GetRouteFamily()
 		key := path.getPrefix()
-		dst := adj.table[rf][key]
-		var old *Path
-		oldIdx := 0
-		if dst == nil {
-			dst = &Dest{}
-			dst.pathList = make([]*Path, 0)
-			adj.table[rf][key] = dst
-		} else {
-			for i, known := range dst.pathList {
-				if known.GetSource() == path.GetSource() {
-					old = known
-					oldIdx = i
-				}
-			}
+		if adj.isCollector {
+			key += path.GetSource().Address.String()
 		}
-		if path.IsWithdraw {
-			if old != nil {
-				dst.pathList = append(dst.pathList[:oldIdx], dst.pathList[oldIdx+1:]...)
-				if len(dst.pathList) == 0 {
-					delete(adj.table[rf], key)
-				}
 
+		old, found := adj.table[rf][key]
+		if path.IsWithdraw {
+			if found {
+				delete(adj.table[rf], key)
 				if old.Filtered(adj.id) == POLICY_DIRECTION_NONE {
 					adj.accepted[rf]--
 				}
 			}
 		} else {
 			n := path.Filtered(adj.id)
-			if old != nil {
+			if found {
 				o := old.Filtered(adj.id)
 				if o == POLICY_DIRECTION_IN && n == POLICY_DIRECTION_NONE {
 					adj.accepted[rf]++
@@ -89,17 +73,10 @@ func (adj *AdjRib) Update(pathList []*Path) {
 					adj.accepted[rf]++
 				}
 			}
-			if old != nil {
-				dst.pathList[oldIdx] = path
-				// avoid updating timestamp for the
-				// exact same message due to soft
-				// reset, etc
-				if reflect.DeepEqual(old.GetPathAttrs(), path.GetPathAttrs()) {
-					path.setTimestamp(old.GetTimestamp())
-				}
-			} else {
-				dst.pathList = append(dst.pathList, path)
+			if found && reflect.DeepEqual(old.GetPathAttrs(), path.GetPathAttrs()) {
+				path.setTimestamp(old.GetTimestamp())
 			}
+			adj.table[rf][key] = path
 		}
 	}
 }
@@ -107,11 +84,9 @@ func (adj *AdjRib) Update(pathList []*Path) {
 func (adj *AdjRib) RefreshAcceptedNumber(rfList []bgp.RouteFamily) {
 	for _, rf := range rfList {
 		adj.accepted[rf] = 0
-		for _, d := range adj.table[rf] {
-			for _, p := range d.pathList {
-				if p.Filtered(adj.id) != POLICY_DIRECTION_IN {
-					adj.accepted[rf]++
-				}
+		for _, p := range adj.table[rf] {
+			if p.Filtered(adj.id) != POLICY_DIRECTION_IN {
+				adj.accepted[rf]++
 			}
 		}
 	}
@@ -120,13 +95,11 @@ func (adj *AdjRib) RefreshAcceptedNumber(rfList []bgp.RouteFamily) {
 func (adj *AdjRib) PathList(rfList []bgp.RouteFamily, accepted bool) []*Path {
 	pathList := make([]*Path, 0, adj.Count(rfList))
 	for _, rf := range rfList {
-		for _, d := range adj.table[rf] {
-			for _, p := range d.pathList {
-				if accepted && p.Filtered(adj.id) > POLICY_DIRECTION_NONE {
-					continue
-				}
-				pathList = append(pathList, p)
+		for _, rr := range adj.table[rf] {
+			if accepted && rr.Filtered(adj.id) > POLICY_DIRECTION_NONE {
+				continue
 			}
+			pathList = append(pathList, rr)
 		}
 	}
 	return pathList
@@ -136,9 +109,7 @@ func (adj *AdjRib) Count(rfList []bgp.RouteFamily) int {
 	count := 0
 	for _, rf := range rfList {
 		if table, ok := adj.table[rf]; ok {
-			for _, d := range table {
-				count += len(d.pathList)
-			}
+			count += len(table)
 		}
 	}
 	return count
@@ -157,7 +128,7 @@ func (adj *AdjRib) Accepted(rfList []bgp.RouteFamily) int {
 func (adj *AdjRib) Drop(rfList []bgp.RouteFamily) {
 	for _, rf := range rfList {
 		if _, ok := adj.table[rf]; ok {
-			adj.table[rf] = make(map[string]*Dest)
+			adj.table[rf] = make(map[string]*Path)
 			adj.accepted[rf] = 0
 		}
 	}
diff --git a/table/table_manager_test.go b/table/table_manager_test.go
index 61019781..02d96299 100644
--- a/table/table_manager_test.go
+++ b/table/table_manager_test.go
@@ -2127,7 +2127,7 @@ func TestProcessBGPUpdate_Timestamp(t *testing.T) {
 
 	nlri := []*bgp.IPAddrPrefix{bgp.NewIPAddrPrefix(24, "10.10.10.0")}
 
-	adjRib := NewAdjRib("test", []bgp.RouteFamily{bgp.RF_IPv4_UC, bgp.RF_IPv6_UC})
+	adjRib := NewAdjRib("test", []bgp.RouteFamily{bgp.RF_IPv4_UC, bgp.RF_IPv6_UC}, false)
 	m1 := bgp.NewBGPUpdateMessage(nil, pathAttributes, nlri)
 	peer := peerR1()
 	pList1 := ProcessMessage(m1, peer, time.Now())