From 0a3332ee1b8e6b1bc10b10128fcf796e23e3a21f Mon Sep 17 00:00:00 2001 From: IWASE Yusuke Date: Thu, 26 Apr 2018 10:50:51 +0900 Subject: [PATCH] vrf: Clones VPN paths with different RD and matched RTs When VPN routes are shared between different VPNs (or VRFs) for example, the RDs assigned for each VRF can differ from others, but the same RT should be used. In this case, the duplicated IP unicast routes can be selected as the best path because VPN prefix ":" is different and GoBGP calculates the best path on the global table context. For example, $ gobgp vrf add 1 rd 1:1 rt both 1:1 $ gobgp global rib -a vpnv4 add 10.0.0.0/24 label 0 rd 1:1 rt 1:1 $ gobgp global rib -a vpnv4 add 10.0.0.0/24 label 0 rd 2:2 rt 1:1 $ gobgp global rib -a vpnv4 Network Labels Next Hop AS_PATH Age Attrs *> 1:1:10.0.0.0/24 [0] 0.0.0.0 00:00:00 [{Origin: ?} {Extcomms: [1:1]}] *> 2:2:10.0.0.0/24 [0] 0.0.0.0 00:00:00 [{Origin: ?} {Extcomms: [1:1]}] $ gobgp vrf 1 rib -a ipv4 Network Next Hop AS_PATH Age Attrs 10.0.0.0/24 0.0.0.0 00:00:10 [{Origin: ?}] <--- RD 1:1 on global 10.0.0.0/24 0.0.0.0 00:00:10 [{Origin: ?}] <--- RD 2:2 on global The both of the above VRF routes are selected as the best path unexpectedly. This patch clones the received VPN path before installing the global table and overwrites the RD of the cloned paths by the RD of the matched VRFs. Also filters out the duplicated path on each VRF. $ gobgp vrf add 1 rd 1:1 rt both 1:1 $ gobgp global rib -a vpnv4 add 10.0.0.0/24 label 0 rd 2:2 rt 1:1 $ gobgp global rib -a vpnv4 Network Labels Next Hop AS_PATH Age Attrs *> 1:1:10.0.0.0/24 [0] 0.0.0.0 00:00:00 [{Origin: ?} {Extcomms: [1:1]}] *> 2:2:10.0.0.0/24 [0] 0.0.0.0 00:00:00 [{Origin: ?} {Extcomms: [1:1]}] $ gobgp vrf 1 rib -a ipv4 Network Next Hop AS_PATH Age Attrs 10.0.0.0/24 0.0.0.0 00:00:10 [{Origin: ?}] $ gobgp global rib -a vpnv4 del 10.0.0.0/24 label 0 rd 2:2 $ gobgp vrf 1 rib -a ipv4 Network not in table $ gobgp global rib -a vpnv4 Network not in table Note: This feature only takes effect for only MPLS VPNv4/v6 routes and does not have any for other address families which have RD on its NLRI (e.g., EVPN, FlowSpec for VPN). Signed-off-by: IWASE Yusuke --- server/server.go | 12 +++++----- table/destination.go | 7 +++++- table/path.go | 28 ++++++++++++++++++++++- table/table_manager.go | 50 ++++++++++++++++++++++++++++++++++++++++++ table/vrf.go | 18 +++++++++++++++ 5 files changed, 108 insertions(+), 7 deletions(-) diff --git a/server/server.go b/server/server.go index abd32dfad..9e82fd3bc 100644 --- a/server/server.go +++ b/server/server.go @@ -484,7 +484,7 @@ func (server *BgpServer) notifyBestWatcher(best []*table.Path, multipath [][]*ta switch p.GetRouteFamily() { case bgp.RF_IPv4_VPN, bgp.RF_IPv6_VPN: for _, vrf := range server.globalRib.Vrfs { - if vrf.Id != 0 && table.CanImportToVrf(vrf, p) { + if vrf.Id != 0 && vrf.IsImported(p) { p.VrfIds = append(p.VrfIds, uint16(vrf.Id)) } } @@ -714,7 +714,8 @@ func (server *BgpServer) propagateUpdate(peer *Peer, pathList []*table.Path) { } dsts = rib.ProcessPaths(append(pathList, moded...)) } else { - for idx, path := range pathList { + newPathList := make([]*table.Path, 0) + for _, path := range pathList { var options *table.PolicyOptions if peer != nil { options = &table.PolicyOptions{ @@ -728,7 +729,8 @@ func (server *BgpServer) propagateUpdate(peer *Peer, pathList []*table.Path) { } else { path = path.Clone(true) } - pathList[idx] = path + newPathList = append(newPathList, path) + newPathList = append(newPathList, server.globalRib.ClonePathForVrf(path)...) // RFC4684 Constrained Route Distribution 6. Operation // // When a BGP speaker receives a BGP UPDATE that advertises or withdraws @@ -774,8 +776,8 @@ func (server *BgpServer) propagateUpdate(peer *Peer, pathList []*table.Path) { sendFsmOutgoingMsg(peer, paths, nil, false) } } - server.notifyPostPolicyUpdateWatcher(peer, pathList) - dsts = rib.ProcessPaths(pathList) + server.notifyPostPolicyUpdateWatcher(peer, newPathList) + dsts = rib.ProcessPaths(newPathList) gBestList, gOldList, mpathList = dstsToPaths(table.GLOBAL_RIB_NAME, dsts, false) server.notifyBestWatcher(gBestList, mpathList) diff --git a/table/destination.go b/table/destination.go index fd32371d2..e3fb4cc8f 100644 --- a/table/destination.go +++ b/table/destination.go @@ -128,6 +128,11 @@ func (lhs *PeerInfo) Equal(rhs *PeerInfo) bool { return false } + if lhs.Address == nil && rhs.Address == nil { + // The both peers are "local" + return true + } + if (lhs.AS == rhs.AS) && lhs.ID.Equal(rhs.ID) && lhs.LocalID.Equal(rhs.LocalID) && lhs.Address.Equal(rhs.Address) { return true } @@ -1047,7 +1052,7 @@ func (old *Destination) Select(option ...DestinationSelectOption) *Destination { if vrf != nil { ps := make([]*Path, 0, len(paths)) for _, p := range paths { - if CanImportToVrf(vrf, p) { + if vrf.IsImported(p) { ps = append(ps, p.ToLocal()) } } diff --git a/table/path.go b/table/path.go index 0c2014476..40bca5e1e 100644 --- a/table/path.go +++ b/table/path.go @@ -102,6 +102,21 @@ type originInfo struct { stale bool } +func (info *originInfo) Clone(nlri bgp.AddrPrefixInterface) *originInfo { + return &originInfo{ + nlri: nlri, + source: info.source, + timestamp: info.timestamp, + validation: info.validation, + key: nlri.String(), + uuid: info.uuid, + noImplicitWithdraw: info.noImplicitWithdraw, + isFromExternal: info.isFromExternal, + eor: info.eor, + stale: info.stale, + } +} + type RpkiValidationReasonType string const ( @@ -344,9 +359,20 @@ func (path *Path) Clone(isWithdraw bool) *Path { } } +func (path *Path) CloneWithNewNlri(isWithdraw bool, nlri bgp.AddrPrefixInterface) *Path { + cloned := path.Clone(isWithdraw) + cloned.info = path.OriginInfo().Clone(nlri) + attr := path.getPathAttr(bgp.BGP_ATTR_TYPE_MP_REACH_NLRI) + if attr != nil { + oldNlri := attr.(*bgp.PathAttributeMpReachNLRI) + cloned.setPathAttr(bgp.NewPathAttributeMpReachNLRI(oldNlri.Nexthop.String(), []bgp.AddrPrefixInterface{nlri})) + } + return cloned +} + func (path *Path) root() *Path { p := path - for p.parent != nil { + for p.info == nil && p.parent != nil { p = p.parent } return p diff --git a/table/table_manager.go b/table/table_manager.go index 7341df174..6ebbd5a1e 100644 --- a/table/table_manager.go +++ b/table/table_manager.go @@ -180,6 +180,56 @@ func (manager *TableManager) DeleteVrf(name string) ([]*Path, error) { return msgs, nil } +// ClonePathForVrf returns a cloned path list if the given path has RTs which +// match some import RTs on VRFs, but has the different RD with those VRFs. The +// returns paths have the same attributes other than the RD and whose RD is +// overwritten by the RD of the matched VRFs. +// Please note that the given path is not included in the returned list and +// currently this function clones only VPNv4 or VPNv6 paths for MPLS VPN. +func (manager *TableManager) ClonePathForVrf(path *Path) []*Path { + if path == nil { + return nil + } + var rdString string + var v4 *bgp.LabeledVPNIPAddrPrefix + var v6 *bgp.LabeledVPNIPv6AddrPrefix + switch nlri := path.GetNlri().(type) { + case *bgp.LabeledVPNIPAddrPrefix: + rdString = nlri.RD.String() + v4 = nlri + case *bgp.LabeledVPNIPv6AddrPrefix: + rdString = nlri.RD.String() + v6 = nlri + default: + return nil + } + if path.IsWithdraw { + // The withdrawn path can have no path attribute, then look up global + // table to get the path attributes from the existing path. + src := path.GetSource() + if dest := manager.GetDestination(path); dest != nil { + for _, p := range dest.knownPathList { + if src.Equal(p.GetSource()) { + path = p.Clone(true) + } + } + } + } + pathList := make([]*Path, 0, len(manager.Vrfs)) + for _, vrf := range manager.Vrfs { + if CanImportToVrf(vrf, path) && vrf.Rd.String() != rdString { + var nlri bgp.AddrPrefixInterface + if v4 != nil { + nlri = bgp.NewLabeledVPNIPAddrPrefix(v4.IPPrefixLen(), v4.Prefix.String(), v4.Labels, vrf.Rd) + } else { // if v6 != nil + nlri = bgp.NewLabeledVPNIPv6AddrPrefix(v6.IPPrefixLen(), v6.Prefix.String(), v6.Labels, vrf.Rd) + } + pathList = append(pathList, path.CloneWithNewNlri(path.IsWithdraw, nlri)) + } + } + return pathList +} + func (manager *TableManager) calculate(dsts []*Destination) []*Destination { emptyDsts := make([]*Destination, 0, len(dsts)) clonedDsts := make([]*Destination, 0, len(dsts)) diff --git a/table/vrf.go b/table/vrf.go index 480f963fd..3bd9a50f2 100644 --- a/table/vrf.go +++ b/table/vrf.go @@ -44,6 +44,24 @@ func (v *Vrf) Clone() *Vrf { } } +func (v *Vrf) IsImported(path *Path) bool { + if path == nil { + return false + } + // When VPNv4/VPNv6 address family, a path having the different RD with the + // matched VRF might have cloned paths whose RD is overwritten by the RD + // of the matched VRF. Then, here compares the RD of the given path and + // this VRF instead of comparing the RTs of the given path and the import + // RTs of this VRF. If with other address families, compares the RTs. + switch nlri := path.GetNlri().(type) { + case *bgp.LabeledVPNIPAddrPrefix: + return v.Rd.String() == nlri.RD.String() + case *bgp.LabeledVPNIPv6AddrPrefix: + return v.Rd.String() == nlri.RD.String() + } + return CanImportToVrf(v, path) +} + func isLastTargetUser(vrfs map[string]*Vrf, target bgp.ExtendedCommunityInterface) bool { for _, vrf := range vrfs { for _, rt := range vrf.ImportRt {