zclient: Avoid flapping of nexthop reachability

Currently, GoBGP sends the ROUTE_DELETE messages to Zebra for the paths
whose nexthop was determined as unreachable by IGP, but when Zebra
received the ROUTE_DELETE messages for the unreachable nexthop, Zebra
will send the NEXTHOP_UPDATE message with the given nexthop is
reachable via the default interface. This message unexpectedly causes
updates of the unreachable path to be reachable (not true though). Then
GoBGP will send ROUTE_ADD messages to Zebra, but those paths soon will
be invalidated via the NEXTHOP_UPDATE message, GoBGP will send the
ROUTE_DELETE messages and ... (infinite loop).

This patch fixes to avoid sending the ROUTE_DELETE messages for the
unreachable paths because those paths are inactivated on Zebra and not
installed to FIB.

Signed-off-by: IWASE Yusuke <iwase.yusuke0@gmail.com>

2 files changed, 16 insertions, 6 deletions

diff --git a/server/zclient.go b/server/zclient.go
index eb3e0b81..5b1ba483 100644
--- a/server/zclient.go
+++ b/server/zclient.go
@@ -210,7 +210,11 @@ func (m *nexthopTrackingManager) filterPathToRegister(paths pathList) pathList {
 func filterOutExternalPath(paths pathList) pathList {
 	filteredPaths := make(pathList, 0, len(paths))
 	for _, path := range paths {
-		if path == nil || path.IsFromExternal() {
+		// Here filters out:
+		// - Nil path
+		// - External path (advertised from Zebra) in order avoid sending back
+		// - Unreachable path because invalidated by Zebra
+		if path == nil || path.IsFromExternal() || path.IsNexthopInvalid {
 			continue
 		}
 		filteredPaths = append(filteredPaths, path)
diff --git a/table/destination.go b/table/destination.go
index e063d89d..adf1c0cd 100644
--- a/table/destination.go
+++ b/table/destination.go
@@ -238,16 +238,17 @@ func getBestPath(id string, as uint32, pathList []*Path) *Path {
 		if rsFilter(id, as, p) {
 			continue
 		}
-
-		if !p.IsNexthopInvalid {
-			return p
-		}
+		return p
 	}
 	return nil
 }
 
 func (dd *Destination) GetBestPath(id string, as uint32) *Path {
-	return getBestPath(id, as, dd.knownPathList)
+	p := getBestPath(id, as, dd.knownPathList)
+	if p == nil || p.IsNexthopInvalid {
+		return nil
+	}
+	return p
 }
 
 func (dd *Destination) GetMultiBestPath(id string) []*Path {
@@ -573,6 +574,11 @@ func (u *Update) GetChanges(id string, as uint32, peerDown bool) (*Path, *Path,
 			// For BGP Nexthop Tracking, checks if the nexthop reachability
 			// was changed or not.
 			if best.IsNexthopInvalid != old.IsNexthopInvalid {
+				// If the nexthop of the best path became unreachable, we need
+				// to withdraw that path.
+				if best.IsNexthopInvalid {
+					return best.Clone(true), old
+				}
 				return best, old
 			}
 			return nil, old