fixing an issue when gobgp sits between a MP-capable peer and a non-MP-capable peer

whereby MP-capable peer uses MP_REACH_NLRI to send updates to gobgp
gobgp accepts it but without setting NEXT_HOP pathattr
then gobgp propagates the paths to the non-MP-capable peer with update.NLRI set but without NEXT_HOP
the non-MP-capable peer will treat the update as withdraw message cause routes cannot propagate properly

2 files changed, 13 insertions, 0 deletions

diff --git a/internal/pkg/table/message.go b/internal/pkg/table/message.go
index 014e8978..9bd843e2 100644
--- a/internal/pkg/table/message.go
+++ b/internal/pkg/table/message.go
@@ -442,6 +442,12 @@ func (p *packerV4) pack(options ...*bgp.MarshallingOption) []*bgp.BGPMessage {
 			paths := c.paths
 
 			attrs := paths[0].GetPathAttrs()
+			// we can apply a fix here when gobgp receives from MP peer
+			// and propagtes to non-MP peer
+			// we should make sure that we next-hop exists in pathattrs
+			// while we build the update message
+			// we do not want to modify the `path` though
+			attrs = append(attrs, bgp.NewPathAttributeNextHop(paths[0].GetNexthop().String()))
 			attrsLen := 0
 			for _, a := range attrs {
 				attrsLen += a.Len()
diff --git a/internal/pkg/table/table_manager.go b/internal/pkg/table/table_manager.go
index b16f135b..0bf60bc1 100644
--- a/internal/pkg/table/table_manager.go
+++ b/internal/pkg/table/table_manager.go
@@ -60,6 +60,8 @@ func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time)
 			l = append(l, a.Value...)
 			dels = append(dels, l...)
 		default:
+			// update msg may not contain next_hop (type:3) in attr
+			// due to it uses MpReachNLRI and it also has empty update.NLRI
 			attrs = append(attrs, attr)
 		}
 	}
@@ -92,6 +94,11 @@ func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time)
 		reachAttrs[len(reachAttrs)-1] = reach
 
 		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}
 			p := NewPath(peerInfo, nlri, false, reachAttrs, timestamp, false)
 			p.SetHash(hash)
 			pathList = append(pathList, p)