table: avoid unnecessary attributes in Path object

- withdraw path doesn't need any attributes.
- mp reach path doesn't need mp unreach attribute (some bgp speackers
  send an update message including both mp reach and mp unreach).

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

2 files changed, 59 insertions, 98 deletions

diff --git a/table/message.go b/table/message.go
index 5f5e02f6..9d82ce31 100644
--- a/table/message.go
+++ b/table/message.go
@@ -17,10 +17,12 @@ package table
 
 import (
 	"bytes"
-	"github.com/osrg/gobgp/packet/bgp"
-	log "github.com/sirupsen/logrus"
 	"hash/fnv"
 	"reflect"
+	"sort"
+
+	"github.com/osrg/gobgp/packet/bgp"
+	log "github.com/sirupsen/logrus"
 )
 
 func UpdatePathAttrs2ByteAs(msg *bgp.BGPUpdate) error {
@@ -278,16 +280,7 @@ func createUpdateMsgFromPath(path *Path, msg *bgp.BGPMessage) *bgp.BGPMessage {
 				u := msg.Body.(*bgp.BGPUpdate)
 				u.NLRI = append(u.NLRI, nlri)
 			} else {
-				attrs := make([]bgp.PathAttributeInterface, 0, 8)
-				for _, p := range path.GetPathAttrs() {
-					switch p.GetType() {
-					case bgp.BGP_ATTR_TYPE_MP_REACH_NLRI:
-					case bgp.BGP_ATTR_TYPE_MP_UNREACH_NLRI:
-					default:
-						attrs = append(attrs, p)
-					}
-				}
-				return bgp.NewBGPUpdateMessage(nil, attrs, []*bgp.IPAddrPrefix{nlri})
+				return bgp.NewBGPUpdateMessage(nil, path.GetPathAttrs(), []*bgp.IPAddrPrefix{nlri})
 			}
 		}
 	} else {
@@ -301,15 +294,7 @@ func createUpdateMsgFromPath(path *Path, msg *bgp.BGPMessage) *bgp.BGPMessage {
 					}
 				}
 			} else {
-				var nlris []bgp.AddrPrefixInterface
-				attr := path.getPathAttr(bgp.BGP_ATTR_TYPE_MP_REACH_NLRI)
-				if attr == nil {
-					// for bmp post-policy
-					attr = path.getPathAttr(bgp.BGP_ATTR_TYPE_MP_UNREACH_NLRI)
-					nlris = attr.(*bgp.PathAttributeMpUnreachNLRI).Value
-				} else {
-					nlris = []bgp.AddrPrefixInterface{path.GetNlri()}
-				}
+				nlris := []bgp.AddrPrefixInterface{path.GetNlri()}
 				return bgp.NewBGPUpdateMessage(nil, []bgp.PathAttributeInterface{bgp.NewPathAttributeMpUnreachNLRI(nlris)}, nil)
 			}
 		} else {
@@ -323,19 +308,16 @@ func createUpdateMsgFromPath(path *Path, msg *bgp.BGPMessage) *bgp.BGPMessage {
 				}
 			} else {
 				attrs := make([]bgp.PathAttributeInterface, 0, 8)
-				for _, p := range path.GetPathAttrs() {
-					switch p.GetType() {
-					case bgp.BGP_ATTR_TYPE_MP_REACH_NLRI:
+				for _, a := range path.GetPathAttrs() {
+					if a.GetType() == bgp.BGP_ATTR_TYPE_MP_REACH_NLRI {
 						attrs = append(attrs, bgp.NewPathAttributeMpReachNLRI(path.GetNexthop().String(), []bgp.AddrPrefixInterface{path.GetNlri()}))
-					case bgp.BGP_ATTR_TYPE_MP_UNREACH_NLRI:
-					default:
-						attrs = append(attrs, p)
+					} else {
+						attrs = append(attrs, a)
 					}
 				}
-
-				// we don't need to clone here but we
-				// might merge path to this message in
-				// the future so let's clone anyway.
+				sort.Slice(attrs, func(i, j int) bool {
+					return attrs[i].GetType() < attrs[j].GetType()
+				})
 				return bgp.NewBGPUpdateMessage(nil, attrs, nil)
 			}
 		}
diff --git a/table/table_manager.go b/table/table_manager.go
index 023672fc..0331cc70 100644
--- a/table/table_manager.go
+++ b/table/table_manager.go
@@ -29,85 +29,64 @@ const (
 	GLOBAL_RIB_NAME = "global"
 )
 
-func nlri2Path(m *bgp.BGPMessage, p *PeerInfo, now time.Time) []*Path {
-	updateMsg := m.Body.(*bgp.BGPUpdate)
-	pathAttributes := updateMsg.PathAttributes
-	pathList := make([]*Path, 0)
-	for _, nlri := range updateMsg.NLRI {
-		path := NewPath(p, nlri, false, pathAttributes, now, false)
-		pathList = append(pathList, path)
-	}
-	return pathList
-}
+func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time) []*Path {
+	update := m.Body.(*bgp.BGPUpdate)
 
-func withdraw2Path(m *bgp.BGPMessage, p *PeerInfo, now time.Time) []*Path {
-	updateMsg := m.Body.(*bgp.BGPUpdate)
-	pathAttributes := updateMsg.PathAttributes
-	pathList := make([]*Path, 0)
-	for _, nlri := range updateMsg.WithdrawnRoutes {
-		path := NewPath(p, nlri, true, pathAttributes, now, false)
-		pathList = append(pathList, path)
+	if y, f := update.IsEndOfRib(); y {
+		// this message has no normal updates or withdrawals.
+		return []*Path{NewEOR(f)}
 	}
-	return pathList
-}
-
-func mpreachNlri2Path(m *bgp.BGPMessage, p *PeerInfo, now time.Time) []*Path {
-	updateMsg := m.Body.(*bgp.BGPUpdate)
-	pathAttributes := updateMsg.PathAttributes
-	attrList := []*bgp.PathAttributeMpReachNLRI{}
 
-	for _, attr := range pathAttributes {
-		a, ok := attr.(*bgp.PathAttributeMpReachNLRI)
-		if ok {
-			attrList = append(attrList, a)
-			break
-		}
+	adds := make([]bgp.AddrPrefixInterface, 0, len(update.NLRI))
+	for _, nlri := range update.NLRI {
+		adds = append(adds, nlri)
 	}
-	pathList := make([]*Path, 0)
 
-	for _, mp := range attrList {
-		nlri_info := mp.Value
-		for _, nlri := range nlri_info {
-			path := NewPath(p, nlri, false, pathAttributes, now, false)
-			pathList = append(pathList, path)
-		}
+	dels := make([]bgp.AddrPrefixInterface, 0, len(update.WithdrawnRoutes))
+	for _, nlri := range update.WithdrawnRoutes {
+		dels = append(dels, nlri)
 	}
-	return pathList
-}
-
-func mpunreachNlri2Path(m *bgp.BGPMessage, p *PeerInfo, now time.Time) []*Path {
-	updateMsg := m.Body.(*bgp.BGPUpdate)
-	pathAttributes := updateMsg.PathAttributes
-	attrList := []*bgp.PathAttributeMpUnreachNLRI{}
 
-	for _, attr := range pathAttributes {
-		a, ok := attr.(*bgp.PathAttributeMpUnreachNLRI)
-		if ok {
-			attrList = append(attrList, a)
-			break
+	attrs := make([]bgp.PathAttributeInterface, 0, len(update.PathAttributes))
+	var reach *bgp.PathAttributeMpReachNLRI
+	for _, attr := range update.PathAttributes {
+		switch a := attr.(type) {
+		case *bgp.PathAttributeMpReachNLRI:
+			reach = a
+		case *bgp.PathAttributeMpUnreachNLRI:
+			l := make([]bgp.AddrPrefixInterface, 0, len(a.Value))
+			for _, nlri := range a.Value {
+				l = append(l, nlri)
+			}
+			dels = append(dels, l...)
+		default:
+			attrs = append(attrs, attr)
 		}
 	}
-	pathList := make([]*Path, 0)
 
-	for _, mp := range attrList {
-		nlri_info := mp.Value
-
-		for _, nlri := range nlri_info {
-			path := NewPath(p, nlri, true, pathAttributes, now, false)
-			pathList = append(pathList, path)
+	listLen := len(adds) + len(dels)
+	if reach != nil {
+		listLen += len(reach.Value)
+	}
+	pathList := make([]*Path, 0, listLen)
+	for _, nlri := range adds {
+		p := NewPath(peerInfo, nlri, false, attrs, timestamp, false)
+		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
+
+		for _, nlri := range reach.Value {
+			p := NewPath(peerInfo, nlri, false, reachAttrs, timestamp, false)
+			pathList = append(pathList, p)
 		}
 	}
-	return pathList
-}
-
-func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time) []*Path {
-	pathList := make([]*Path, 0)
-	pathList = append(pathList, nlri2Path(m, peerInfo, timestamp)...)
-	pathList = append(pathList, withdraw2Path(m, peerInfo, timestamp)...)
-	pathList = append(pathList, mpreachNlri2Path(m, peerInfo, timestamp)...)
-	pathList = append(pathList, mpunreachNlri2Path(m, peerInfo, timestamp)...)
-	if y, f := m.Body.(*bgp.BGPUpdate).IsEndOfRib(); y {
-		pathList = append(pathList, NewEOR(f))
+	for _, nlri := range dels {
+		p := NewPath(peerInfo, nlri, true, []bgp.PathAttributeInterface{}, timestamp, false)
+		pathList = append(pathList, p)
 	}
 	return pathList
 }