Do not preserve MPReachNLRI attribute the way we receive it from wire

Right now table.ProcessMessage() rebuilds attribute list, but preserves
MPReachNLRI attribute as is. This causes trouble if GoBGP has a neighbor
that merges multiple NLRIs having same nexthop in a single BGP update
message and if GoBGP has a watcher. Thus watcher receives up to n^2
nlris (n paths each containing n NLRIs) causing enormous memory
consumption (since serialization/deserialization removes deduplication).

Since having all NLRIs in each path is useless in most cases, the
behavior is changed so only relevant NLRI is present in BGP path.

In our tests memory consumption of such watcher had reduced from 19 Gb
to 2Gb (with around 300K routes in RIB).

Author: Sergey Klyaus

internal/pkg/table/table_manager.go

@@ -88,17 +88,19 @@ func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time)
 		pathList = append(pathList, p)
 	}
 	if reach != nil {
-		reachAttrs := make([]bgp.PathAttributeInterface, len(attrs)+1)
-		copy(reachAttrs, attrs)
-		// we sort attributes when creating a bgp message from paths
-		reachAttrs[len(reachAttrs)-1] = reach
+		nexthop := reach.Nexthop.String()
 
 		for _, nlri := range reach.Value {
 			// when build path from reach
 			// reachAttrs might not contain next_hop if `attrs` does not have one
 			// this happens when a MP peer send update to gobgp
 			// However nlri is always populated because how we build the path
 			// path.info{nlri: nlri}
+			// Compute a new attribute array for each path with one NLRI to make serialization
+			// of path attrs faster
+			nlriAttr := bgp.NewPathAttributeMpReachNLRI(nexthop, []bgp.AddrPrefixInterface{nlri})
+			reachAttrs := makeAttributeList(attrs, nlriAttr)
+
 			p := NewPath(peerInfo, nlri, false, reachAttrs, timestamp, false)
 			p.SetHash(hash)
 			pathList = append(pathList, p)
@@ -111,6 +113,17 @@ func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time)
 	return pathList
 }
 
+func makeAttributeList(
+	attrs []bgp.PathAttributeInterface, reach *bgp.PathAttributeMpReachNLRI,
+) []bgp.PathAttributeInterface {
+	reachAttrs := make([]bgp.PathAttributeInterface, len(attrs)+1)
+	copy(reachAttrs, attrs)
+	// we sort attributes when creating a bgp message from paths
+	reachAttrs[len(reachAttrs)-1] = reach
+
+	return reachAttrs
+}
+
 type TableManager struct {
 	Tables map[bgp.RouteFamily]*Table
 	Vrfs   map[string]*Vrf

internal/pkg/table/table_manager_test.go

@@ -1979,35 +1979,37 @@ func TestProcessBGPUpdate_multiple_nlri_ipv6(t *testing.T) {
 
 	// check PathAttribute
 	checkPattr := func(expected *bgp.BGPMessage, actual *Path) {
-		pathAttributes := expected.Body.(*bgp.BGPUpdate).PathAttributes
-		pathNexthop := pathAttributes[4]
+		bgpPathAttributes := expected.Body.(*bgp.BGPUpdate).PathAttributes
+		bgpUpdateNexthop := bgpPathAttributes[4]
+		expectedNexthopAttr := bgp.NewPathAttributeMpReachNLRI(
+			bgpUpdateNexthop.(*bgp.PathAttributeMpReachNLRI).Nexthop.String(),
+			[]bgp.AddrPrefixInterface{actual.GetNlri()})
 		attr := actual.getPathAttr(bgp.BGP_ATTR_TYPE_MP_REACH_NLRI)
-		expectedNexthopAttr := attr.(*bgp.PathAttributeMpReachNLRI)
+		pathNexthop := attr.(*bgp.PathAttributeMpReachNLRI)
 		assert.Equal(t, expectedNexthopAttr, pathNexthop)
 
-		expectedOrigin := pathAttributes[0]
+		expectedOrigin := bgpPathAttributes[0]
 		attr = actual.getPathAttr(bgp.BGP_ATTR_TYPE_ORIGIN)
 		pathOrigin := attr.(*bgp.PathAttributeOrigin)
 		assert.Equal(t, expectedOrigin, pathOrigin)
 
-		expectedAsPath := pathAttributes[1]
+		expectedAsPath := bgpPathAttributes[1]
 		attr = actual.getPathAttr(bgp.BGP_ATTR_TYPE_AS_PATH)
 		pathAspath := attr.(*bgp.PathAttributeAsPath)
 		assert.Equal(t, expectedAsPath, pathAspath)
 
-		expectedMed := pathAttributes[2]
+		expectedMed := bgpPathAttributes[2]
 		attr = actual.getPathAttr(bgp.BGP_ATTR_TYPE_MULTI_EXIT_DISC)
 		pathMed := attr.(*bgp.PathAttributeMultiExitDisc)
 		assert.Equal(t, expectedMed, pathMed)
 
-		expectedLocalpref := pathAttributes[3]
+		expectedLocalpref := bgpPathAttributes[3]
 		attr = actual.getPathAttr(bgp.BGP_ATTR_TYPE_LOCAL_PREF)
 		localpref := attr.(*bgp.PathAttributeLocalPref)
 		assert.Equal(t, expectedLocalpref, localpref)
 
 		// check PathAttribute length
-		assert.Equal(t, len(pathAttributes), len(actual.GetPathAttrs()))
-
+		assert.Equal(t, len(bgpPathAttributes), len(actual.GetPathAttrs()))
 	}
 
 	checkBestPathResult := func(rf bgp.RouteFamily, prefix, nexthop string, p *Path, m *bgp.BGPMessage) {
@@ -2105,6 +2107,35 @@ func TestProcessBGPUpdate_multiple_nlri_ipv6(t *testing.T) {
 
 }
 
+func TestProcessBGPUpdate_multiple_nlri_ipv4_split(t *testing.T) {
+	tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC})
+
+	origin := bgp.NewPathAttributeOrigin(0)
+	aspath := createAsPathAttribute([]uint32{65000, 65100, 65200})
+	mpReach := createMpReach("10.50.60.70", []bgp.AddrPrefixInterface{
+		bgp.NewIPAddrPrefix(32, "10.0.0.1"),
+		bgp.NewIPAddrPrefix(32, "10.0.0.2"),
+		bgp.NewIPAddrPrefix(32, "10.0.0.3"),
+	})
+	med := bgp.NewPathAttributeMultiExitDisc(200)
+	localpref := bgp.NewPathAttributeLocalPref(200)
+	pathAttributes := []bgp.PathAttributeInterface{
+		mpReach, origin, aspath, med, localpref,
+	}
+	bgpMessage := bgp.NewBGPUpdateMessage(nil, pathAttributes, nil)
+
+	peer1 := peerR1()
+	pList, err := tm.ProcessUpdate(peer1, bgpMessage)
+	assert.Equal(t, len(mpReach.Value), len(pList))
+	for i, p := range pList {
+		attr := p.getPathAttr(bgp.BGP_ATTR_TYPE_MP_REACH_NLRI).(*bgp.PathAttributeMpReachNLRI)
+		assert.Equal(t, mpReach.Nexthop, attr.Nexthop)
+		assert.Equal(t, []bgp.AddrPrefixInterface{mpReach.Value[i]}, attr.Value)
+	}
+	assert.NoError(t, err)
+
+}
+
 func TestProcessBGPUpdate_Timestamp(t *testing.T) {
 	origin := bgp.NewPathAttributeOrigin(0)
 	aspathParam := []bgp.AsPathParamInterface{bgp.NewAs4PathParam(2, []uint32{65000})}