server: Avoid infinite UPDATE loop of RTM NLRI

When GoBGP dropped adj-Rib-out per Peer, we fixed to send the same Route
Target Membership (RTM) NLRI even if it is already sent. This can cause
the infinite UPDATE loop when Route Reflector(RR) reflects RTM NLRI to
its clients.

For example, the following situation causes the infinite UPDATE loop.

Topology:

     +----- RR -----+
     |              |
  Client1        Client2

When Client1 has VRF with RT 65000:1 and sends a RTM NLRI to RR, then RR
reflects the NLRI to Client2. If a new VRF with the same RT 65000:1 on
Client2 is created, Client2 will notify it to RR, then RR calculates the
best, but RR will send the NLRI from Client2 to Client1 even if it is
not the best. Client1 receives the NLRI again, calculates the best and
re-sends the best. Then, RR reflects the received NLRI ... (infinite
loop).

This patch fixes to compare the candidate path to be sent with the old
path and assume the given candidate path was already sent before if the
candidate path and the old path is the same path. Then avoids the
infinite UPDATE loop.

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

1 files changed, 1 insertions, 25 deletions

diff --git a/server/server.go b/server/server.go
index e0f6f5cc..cd739594 100644
--- a/server/server.go
+++ b/server/server.go
@@ -443,31 +443,7 @@ func filterpath(peer *Peer, path, old *table.Path) *table.Path {
 		}
 	}
 
-	if peer.ID() == path.GetSource().Address.String() {
-		// Note: multiple paths having the same prefix could exist the
-		// withdrawals list in the case of Route Server setup with
-		// import policies modifying paths. In such case, gobgp sends
-		// duplicated update messages; withdraw messages for the same
-		// prefix.
-		if !peer.isRouteServerClient() {
-			// Say, peer A and B advertized same prefix P, and
-			// best path calculation chose a path from B as best.
-			// When B withdraws prefix P, best path calculation chooses
-			// the path from A as best.
-			// For peers other than A, this path should be advertised
-			// (as implicit withdrawal). However for A, we should advertise
-			// the withdrawal path.
-			// Thing is same when peer A and we advertized prefix P (as local
-			// route), then, we withdraws the prefix.
-			if !path.IsWithdraw && old != nil && old.GetSource().Address.String() != peer.ID() {
-				return old.Clone(true)
-			}
-		}
-		log.WithFields(log.Fields{
-			"Topic": "Peer",
-			"Key":   peer.ID(),
-			"Data":  path,
-		}).Debug("From me, ignore.")
+	if path = peer.filterPathFromSourcePeer(path, old); path == nil {
 		return nil
 	}