From 82d2877c7038336d14cbd19c687dd8026eadb149 Mon Sep 17 00:00:00 2001 From: Imcom Jin Date: Thu, 25 Feb 2021 14:05:58 +0800 Subject: 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 --- internal/pkg/table/message.go | 6 ++++++ internal/pkg/table/table_manager.go | 7 +++++++ 2 files changed, 13 insertions(+) (limited to 'internal') 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) -- cgit v1.2.3