From 44e77ccbb09368e922035009750ac2eaaacab49e Mon Sep 17 00:00:00 2001
From: Tuetuopay <tuetuopay@me.com>
Date: Thu, 2 May 2024 16:32:44 +0200
Subject: [PATCH] evpn: fix evpn losing type-2 routes

When fixing the EVPN MAC mobility complexity, the way destinations are
indexed in the routing table changed from RD+ETAG+MAC+IP to only RD+MAC.
This is incorrect per the BGP EVPN RFC. It works in most cases, as when
an IP is present, virtually all EVPN implementations will announce two
paths: with and without the IP. This way routes announces are balanced
and pose no issues.

Issues arise when GoBGP is connected to multiple peers announcing the
same things (read: route reflectors), at a high rate, with lots of
routes (hundreds of thousands), and if multiple paths exist for the same
mac (e.g. with and without an overlay IP address). The issue does not
appear time if any of the four above conditions is false.

There, processing ends up racy and over time, some routes end up missing
due to the concurrent updates. Such missing routes have been observed
with a production setup with:

- hundreds of thousands of routes
- tens of updates per second
- four route reflectors

With this setup, we ended up with a handful of routes missing (usually
10 to 20) after a few days of runtime.

This commit reverts back the custom `tableKey` implementation done
previously, to use the plain `String` view of the prefix. It is to be
noted this is suboptimal performance wise, but is correct.

Fixes: c393f43 ("evpn: fix quadratic evpn mac-mobility handling")
---
 internal/pkg/table/table.go | 46 +++++++++++--------------------------
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/internal/pkg/table/table.go b/internal/pkg/table/table.go
index 48f410ec3..7bf5aa73c 100644
--- a/internal/pkg/table/table.go
+++ b/internal/pkg/table/table.go
@@ -57,9 +57,9 @@ type Table struct {
 	routeFamily  bgp.RouteFamily
 	destinations map[string]*Destination
 	logger       log.Logger
-	// index of route distinguishers with paths to a specific MAC
-	// this is a map[MAC address]map[RD]struct{}
-	// this holds a map for a set of RD.
+	// index of evpn prefixes with paths to a specific MAC
+	// this is a map[MAC address]map[prefix]struct{}
+	// this holds a map for a set of prefixes.
 	macIndex map[string]map[string]struct{}
 }
 
@@ -147,11 +147,10 @@ func (t *Table) deleteDest(dest *Destination) {
 	if nlri, ok := dest.nlri.(*bgp.EVPNNLRI); ok {
 		if macadv, ok := nlri.RouteTypeData.(*bgp.EVPNMacIPAdvertisementRoute); ok {
 			mac := *(*string)(unsafe.Pointer(&macadv.MacAddress))
-			serializedRD, _ := macadv.RD.Serialize()
-			rd := *(*string)(unsafe.Pointer(&serializedRD))
-			if rds, ok := t.macIndex[mac]; ok {
-				delete(rds, rd)
-				if len(rds) == 0 {
+			key := t.tableKey(nlri)
+			if keys, ok := t.macIndex[mac]; ok {
+				delete(keys, key)
+				if len(keys) == 0 {
 					delete(t.macIndex, mac)
 				}
 			}
@@ -398,12 +397,11 @@ func (t *Table) setDestination(dst *Destination) {
 	if nlri, ok := dst.nlri.(*bgp.EVPNNLRI); ok {
 		if macadv, ok := nlri.RouteTypeData.(*bgp.EVPNMacIPAdvertisementRoute); ok {
 			mac := *(*string)(unsafe.Pointer(&macadv.MacAddress))
-			serializedRD, _ := macadv.RD.Serialize()
-			rd := *(*string)(unsafe.Pointer(&serializedRD))
-			if rds, ok := t.macIndex[mac]; ok {
-				rds[rd] = struct{}{}
+			key := t.tableKey(nlri)
+			if keys, ok := t.macIndex[mac]; ok {
+				keys[key] = struct{}{}
 			} else {
-				t.macIndex[mac] = map[string]struct{}{rd: {}}
+				t.macIndex[mac] = map[string]struct{}{key: {}}
 			}
 		}
 	}
@@ -435,17 +433,6 @@ func (t *Table) tableKey(nlri bgp.AddrPrefixInterface) string {
 		copy(b[8:24], T.Prefix.To16())
 		b[24] = T.Length
 		return *(*string)(unsafe.Pointer(&b))
-	// we need fast lookup to routes for a specific mac address for evpn mac mobility
-	case *bgp.EVPNNLRI:
-		switch U := T.RouteTypeData.(type) {
-		case *bgp.EVPNMacIPAdvertisementRoute:
-			b := make([]byte, 15)
-			serializedRD, _ := U.RD.Serialize()
-			copy(b, serializedRD)
-			b[8] = bgp.EVPN_ROUTE_TYPE_MAC_IP_ADVERTISEMENT
-			copy(b[9:15], U.MacAddress)
-			return *(*string)(unsafe.Pointer(&b))
-		}
 	}
 	return nlri.String()
 }
@@ -482,14 +469,9 @@ func (t *Table) GetKnownPathList(id string, as uint32) []*Path {
 
 func (t *Table) GetKnownPathListWithMac(id string, as uint32, mac net.HardwareAddr, onlyBest bool) []*Path {
 	var paths []*Path
-	if rds, ok := t.macIndex[*(*string)(unsafe.Pointer(&mac))]; ok {
-		for rd := range rds {
-			b := make([]byte, 15)
-			copy(b, rd)
-			b[8] = bgp.EVPN_ROUTE_TYPE_MAC_IP_ADVERTISEMENT
-			copy(b[9:15], mac)
-			key := *(*string)(unsafe.Pointer(&b))
-			if dst, ok := t.destinations[key]; ok {
+	if prefixes, ok := t.macIndex[*(*string)(unsafe.Pointer(&mac))]; ok {
+		for prefix := range prefixes {
+			if dst, ok := t.destinations[prefix]; ok {
 				if onlyBest {
 					paths = append(paths, dst.GetBestPath(id, as))
 				} else {