table: remove Destination's WithdrawnList and UpdatedPathList used for validation

Withdrawn pathes are kept to be referenced thus the memory for them
are not freed. Nobody uses this so let's remove it.

Paths in UpdatedPathList are also in KnownPathList so doesn't lead to
memory leak. But remove it for simplicity.

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

4 files changed, 66 insertions, 64 deletions

diff --git a/server/server.go b/server/server.go
index 9588ecd5..ef10f1f6 100644
--- a/server/server.go
+++ b/server/server.go
@@ -589,8 +589,8 @@ func (server *BgpServer) dropPeerAllRoutes(peer *Peer, families []bgp.RouteFamil
 
 	options := &table.PolicyOptions{}
 	for _, rf := range families {
-		dsts := server.globalRib.DeletePathsByPeer(peer.fsm.peerInfo, rf)
-		server.validatePaths(dsts, true)
+		dsts, withdrawn := server.globalRib.DeletePathsByPeer(peer.fsm.peerInfo, rf)
+		server.validatePaths(nil, withdrawn, true)
 		if peer.isRouteServerClient() {
 			for _, targetPeer := range server.neighborMap {
 				if !targetPeer.isRouteServerClient() || targetPeer == peer || targetPeer.fsm.state != bgp.BGP_FSM_ESTABLISHED {
@@ -806,55 +806,54 @@ func (server *BgpServer) isRpkiMonitored() bool {
 	return false
 }
 
-func (server *BgpServer) validatePaths(dsts []*table.Destination, peerDown bool) {
+func (server *BgpServer) validatePaths(newly, withdrawn []*table.Path, peerDown bool) {
 	isMonitor := server.isRpkiMonitored()
-	for _, dst := range dsts {
-		if isMonitor {
-			rrList := make([]*api.ROAResult, 0, len(dst.WithdrawnList))
-			for _, path := range dst.WithdrawnList {
-				if path.Validation() == config.RPKI_VALIDATION_RESULT_TYPE_INVALID {
-					reason := api.ROAResult_WITHDRAW
-					if peerDown {
-						reason = api.ROAResult_PEER_DOWN
-					}
-					rr := &api.ROAResult{
-						Reason:    reason,
-						Address:   path.GetSource().Address.String(),
-						Timestamp: path.GetTimestamp().Unix(),
-						OriginAs:  path.GetSourceAs(),
-						Prefix:    path.GetNlri().String(),
-						OldResult: api.ROAResult_ValidationResult(path.Validation().ToInt()),
-						NewResult: api.ROAResult_ValidationResult(path.Validation().ToInt()),
-					}
-					if b := path.GetAsPath(); b != nil {
-						rr.AspathAttr, _ = b.Serialize()
-					}
-					rrList = append(rrList, rr)
+	if isMonitor {
+		rrList := make([]*api.ROAResult, 0, len(withdrawn))
+		for _, path := range withdrawn {
+			if path.Validation() == config.RPKI_VALIDATION_RESULT_TYPE_INVALID {
+				reason := api.ROAResult_WITHDRAW
+				if peerDown {
+					reason = api.ROAResult_PEER_DOWN
 				}
+				rr := &api.ROAResult{
+					Reason:    reason,
+					Address:   path.GetSource().Address.String(),
+					Timestamp: path.GetTimestamp().Unix(),
+					OriginAs:  path.GetSourceAs(),
+					Prefix:    path.GetNlri().String(),
+					OldResult: api.ROAResult_ValidationResult(path.Validation().ToInt()),
+					NewResult: api.ROAResult_ValidationResult(path.Validation().ToInt()),
+				}
+				if b := path.GetAsPath(); b != nil {
+					rr.AspathAttr, _ = b.Serialize()
+				}
+				rrList = append(rrList, rr)
 			}
-			server.broadcastValidationResults(rrList)
 		}
-		if vResults := server.roaManager.validate(dst.UpdatedPathList, isMonitor); isMonitor {
-			for i, path := range dst.UpdatedPathList {
-				old := func() config.RpkiValidationResultType {
-					for _, withdrawn := range dst.WithdrawnList {
-						if path.GetSource().Equal(withdrawn.GetSource()) {
-							return withdrawn.Validation()
-						}
+		server.broadcastValidationResults(rrList)
+	}
+
+	if vResults := server.roaManager.validate(newly, isMonitor); isMonitor {
+		for i, path := range newly {
+			old := func() config.RpkiValidationResultType {
+				for _, withdrawn := range withdrawn {
+					if path.GetSource().Equal(withdrawn.GetSource()) {
+						return withdrawn.Validation()
 					}
-					return config.RPKI_VALIDATION_RESULT_TYPE_NONE
-				}()
-				vResults[i].OldResult = api.ROAResult_ValidationResult(old.ToInt())
-			}
-			rrList := make([]*api.ROAResult, 0, len(vResults))
-			for _, rr := range vResults {
-				invalid := api.ROAResult_ValidationResult(config.RPKI_VALIDATION_RESULT_TYPE_INVALID.ToInt())
-				if rr.NewResult == invalid || rr.OldResult == invalid {
-					rrList = append(rrList, rr)
 				}
+				return config.RPKI_VALIDATION_RESULT_TYPE_NONE
+			}()
+			vResults[i].OldResult = api.ROAResult_ValidationResult(old.ToInt())
+		}
+		rrList := make([]*api.ROAResult, 0, len(vResults))
+		for _, rr := range vResults {
+			invalid := api.ROAResult_ValidationResult(config.RPKI_VALIDATION_RESULT_TYPE_INVALID.ToInt())
+			if rr.NewResult == invalid || rr.OldResult == invalid {
+				rrList = append(rrList, rr)
 			}
-			server.broadcastValidationResults(rrList)
 		}
+		server.broadcastValidationResults(rrList)
 	}
 }
 
@@ -875,8 +874,8 @@ func (server *BgpServer) propagateUpdate(peer *Peer, pathList []*table.Path) ([]
 			}
 			moded = append(moded, server.RSimportPaths(targetPeer, pathList)...)
 		}
-		dsts := rib.ProcessPaths(append(pathList, moded...))
-		server.validatePaths(dsts, false)
+		dsts, newly, withdrawn := rib.ProcessPaths(append(pathList, moded...))
+		server.validatePaths(newly, withdrawn, false)
 		for _, targetPeer := range server.neighborMap {
 			if !targetPeer.isRouteServerClient() || targetPeer.fsm.state != bgp.BGP_FSM_ESTABLISHED || targetPeer.fsm.pConf.GracefulRestart.State.LocalRestarting {
 				continue
@@ -898,8 +897,8 @@ func (server *BgpServer) propagateUpdate(peer *Peer, pathList []*table.Path) ([]
 			pathList[idx] = server.policy.ApplyPolicy(table.GLOBAL_RIB_NAME, table.POLICY_DIRECTION_IMPORT, path, nil)
 		}
 		alteredPathList = pathList
-		dsts := rib.ProcessPaths(pathList)
-		server.validatePaths(dsts, false)
+		dsts, newly, withdrawn := rib.ProcessPaths(pathList)
+		server.validatePaths(newly, withdrawn, false)
 		sendPathList := make([]*table.Path, 0, len(dsts))
 		if server.bgpConfig.Global.Collector.Enabled {
 			sendPathList = pathList
diff --git a/table/destination.go b/table/destination.go
index f323a6d7..31072561 100644
--- a/table/destination.go
+++ b/table/destination.go
@@ -117,8 +117,6 @@ type Destination struct {
 	knownPathList    paths
 	withdrawList     paths
 	newPathList      paths
-	WithdrawnList    paths
-	UpdatedPathList  paths
 	RadixKey         string
 }
 
@@ -236,15 +234,13 @@ func (dd *Destination) validatePath(path *Path) {
 
 // Calculates best-path among known paths for this destination.
 //
-// Returns: - Best path
-//
 // Modifies destination's state related to stored paths. Removes withdrawn
 // paths from known paths. Also, adds new paths to known paths.
-func (dest *Destination) Calculate() {
+func (dest *Destination) Calculate() ([]*Path, []*Path) {
 	dest.oldKnownPathList = dest.knownPathList
-	dest.UpdatedPathList = dest.newPathList
+	updated := dest.newPathList
 	// First remove the withdrawn paths.
-	dest.WithdrawnList = dest.explicitWithdraw()
+	withdrawnList := dest.explicitWithdraw()
 	// Do implicit withdrawal
 	dest.implicitWithdraw()
 	// Collect all new paths into known paths.
@@ -253,6 +249,7 @@ func (dest *Destination) Calculate() {
 	dest.newPathList = make([]*Path, 0)
 	// Compute new best path
 	dest.computeKnownBestPath()
+	return updated, withdrawnList
 }
 
 func (dest *Destination) NewFeed(id string) *Path {
diff --git a/table/table_manager.go b/table/table_manager.go
index afb4deac..f908bd69 100644
--- a/table/table_manager.go
+++ b/table/table_manager.go
@@ -216,26 +216,32 @@ func (manager *TableManager) DeleteVrf(name string) ([]*Path, error) {
 	return msgs, nil
 }
 
-func (manager *TableManager) calculate(destinations []*Destination) {
+func (manager *TableManager) calculate(destinations []*Destination) ([]*Path, []*Path) {
+	newly := make([]*Path, 0, len(destinations))
+	withdrawn := make([]*Path, 0, len(destinations))
 	for _, destination := range destinations {
 		log.WithFields(log.Fields{
 			"Topic": "table",
 			"Key":   destination.GetNlri().String(),
 		}).Debug("Processing destination")
-		destination.Calculate()
+		n, w := destination.Calculate()
+		newly = append(newly, n...)
+		withdrawn = append(withdrawn, w...)
 	}
+	return newly, withdrawn
 }
 
-func (manager *TableManager) DeletePathsByPeer(info *PeerInfo, rf bgp.RouteFamily) []*Destination {
+func (manager *TableManager) DeletePathsByPeer(info *PeerInfo, rf bgp.RouteFamily) ([]*Destination, []*Path) {
 	if t, ok := manager.Tables[rf]; ok {
 		dsts := t.DeleteDestByPeer(info)
-		manager.calculate(dsts)
-		return dsts
+		// no newly added paths
+		_, withdrawn := manager.calculate(dsts)
+		return dsts, withdrawn
 	}
-	return nil
+	return nil, nil
 }
 
-func (manager *TableManager) ProcessPaths(pathList []*Path) []*Destination {
+func (manager *TableManager) ProcessPaths(pathList []*Path) ([]*Destination, []*Path, []*Path) {
 	m := make(map[string]bool, len(pathList))
 	dsts := make([]*Destination, 0, len(pathList))
 	for _, path := range pathList {
@@ -261,8 +267,8 @@ func (manager *TableManager) ProcessPaths(pathList []*Path) []*Destination {
 			}
 		}
 	}
-	manager.calculate(dsts)
-	return dsts
+	newly, withdrawn := manager.calculate(dsts)
+	return dsts, newly, withdrawn
 }
 
 // EVPN MAC MOBILITY HANDLING
diff --git a/table/table_manager_test.go b/table/table_manager_test.go
index 02d96299..becf69a1 100644
--- a/table/table_manager_test.go
+++ b/table/table_manager_test.go
@@ -30,7 +30,7 @@ import (
 // this function processes only BGPUpdate
 func (manager *TableManager) ProcessUpdate(fromPeer *PeerInfo, message *bgp.BGPMessage) ([]*Path, error) {
 	paths := ProcessMessage(message, fromPeer, time.Now())
-	dsts := manager.ProcessPaths(paths)
+	dsts, _, _ := manager.ProcessPaths(paths)
 	paths2 := make([]*Path, 0, len(paths))
 	for _, dst := range dsts {
 		p := dst.NewFeed(GLOBAL_RIB_NAME)