diff --git a/config/bgp_configs.go b/config/bgp_configs.go index 062ef8f75..9204bcddf 100644 --- a/config/bgp_configs.go +++ b/config/bgp_configs.go @@ -3334,12 +3334,22 @@ func (lhs *LongLivedGracefulRestart) Equal(rhs *LongLivedGracefulRestart) bool { type RouteTargetMembershipState struct { // original -> gobgp:deferral-time DeferralTime uint16 `mapstructure:"deferral-time" json:"deferral-time,omitempty"` + // original -> gobgp:advertise-default + // gobgp:advertise-default's original type is boolean. + // Configure whether advertise the default route target or not. This can + // be used only when the given neighbor is a route reflector client. + AdvertiseDefault bool `mapstructure:"advertise-default" json:"advertise-default,omitempty"` } // struct for container gobgp:config. type RouteTargetMembershipConfig struct { // original -> gobgp:deferral-time DeferralTime uint16 `mapstructure:"deferral-time" json:"deferral-time,omitempty"` + // original -> gobgp:advertise-default + // gobgp:advertise-default's original type is boolean. + // Configure whether advertise the default route target or not. This can + // be used only when the given neighbor is a route reflector client. + AdvertiseDefault bool `mapstructure:"advertise-default" json:"advertise-default,omitempty"` } func (lhs *RouteTargetMembershipConfig) Equal(rhs *RouteTargetMembershipConfig) bool { @@ -3349,6 +3359,9 @@ func (lhs *RouteTargetMembershipConfig) Equal(rhs *RouteTargetMembershipConfig) if lhs.DeferralTime != rhs.DeferralTime { return false } + if lhs.AdvertiseDefault != rhs.AdvertiseDefault { + return false + } return true } diff --git a/config/default.go b/config/default.go index f5bf809e6..6c21cc576 100644 --- a/config/default.go +++ b/config/default.go @@ -203,6 +203,9 @@ func setDefaultNeighborConfigValuesWithViper(v *viper.Viper, n *Neighbor, g *Glo n.AfiSafis[i].AddPaths.Config.SendMax = n.AddPaths.Config.SendMax } n.AfiSafis[i].AddPaths.State.SendMax = n.AfiSafis[i].AddPaths.Config.SendMax + if !n.RouteReflector.Config.RouteReflectorClient && n.AfiSafis[i].RouteTargetMembership.Config.AdvertiseDefault { + return fmt.Errorf("advertise-default can be enabled for only route reflector client") + } } } diff --git a/server/peer.go b/server/peer.go index e39810246..516105170 100644 --- a/server/peer.go +++ b/server/peer.go @@ -168,6 +168,15 @@ func (peer *Peer) isDynamicNeighbor() bool { return peer.fsm.pConf.Config.NeighborAddress == "" && peer.fsm.pConf.Config.NeighborInterface == "" } +func (peer *Peer) isAdvertiseDefaultRTEnabled() bool { + for _, afiSafi := range peer.fsm.pConf.AfiSafis { + if afiSafi.State.Family == bgp.RF_RTC_UC && afiSafi.RouteTargetMembership.Config.AdvertiseDefault { + return true + } + } + return false +} + func (peer *Peer) recvedAllEOR() bool { for _, a := range peer.fsm.pConf.AfiSafis { if s := a.MpGracefulRestart.State; s.Enabled && !s.EndOfRibReceived { @@ -321,6 +330,73 @@ func (peer *Peer) getAccepted(rfList []bgp.RouteFamily) []*table.Path { return peer.adjRibIn.PathList(rfList, true) } +func (peer *Peer) filterRTCPath(path, old *table.Path) *table.Path { + if path == nil || path.GetRouteFamily() != bgp.RF_RTC_UC { + return path + } + + rt := path.GetNlri().(*bgp.RouteTargetMembershipNLRI).RouteTarget + isAdvDefaultRT := peer.isAdvertiseDefaultRTEnabled() + if rt != nil && isAdvDefaultRT { + // If "advertise-default" is enabled, the default route tartget is + // already advertised to this peer, then there is no other path to be + // advertised. + return nil + } else if rt == nil && !isAdvDefaultRT { + // Avoid the default route target to be advertised to the peers which + // "advertise-default" is disabled. + return nil + } + + if path.IsWithdraw { + return path + } + + if path.IsLocal() && path == old { + // We assumes "path" was already sent before. This assumption avoids + // the infinite UPDATE loop between a Route Reflector and its clients. + log.WithFields(log.Fields{ + "Topic": "Peer", + "Key": peer.fsm.pConf.State.NeighborAddress, + "Path": path, + }).Debug("given rtm nlri is already sent, skipping to advertise") + return nil + } + + if old != nil && old.IsLocal() { + // We assumes VRF with the specific RT is deleted. + return old.Clone(true) + } + + if peer.isRouteReflectorClient() { + // We need to send the path even if the peer is the originator of the + // given path in order to signal that the client should distribute + // route with the given RT. + return path + } + + // We send a path even if it is not the best path. See comments in + // (*Destination) GetChanges(). + dst := peer.localRib.GetDestination(path) + path = nil + for _, p := range dst.GetKnownPathList(peer.TableID(), peer.AS()) { + srcPeer := p.GetSource() + if peer.ID() != srcPeer.Address.String() { + if srcPeer.RouteReflectorClient { + // The path from a RR client is preferred than others for the + // case that RR and non RR client peering (e.g., peering of + // different RR clusters). + path = p + break + } else if path == nil { + path = p + } + } + } + + return path +} + func (peer *Peer) filterPathFromSourcePeer(path, old *table.Path) *table.Path { if peer.ID() != path.GetSource().Address.String() { return path diff --git a/server/server.go b/server/server.go index 5a1dd8fc0..34389e819 100644 --- a/server/server.go +++ b/server/server.go @@ -465,46 +465,8 @@ func filterpath(peer *Peer, path, old *table.Path) *table.Path { func (s *BgpServer) filterpath(peer *Peer, path, old *table.Path) *table.Path { // Special handling for RTM NLRI. - if path != nil && path.GetRouteFamily() == bgp.RF_RTC_UC && !path.IsWithdraw { - // If the given "path" is locally generated and the same with "old", we - // assumes "path" was already sent before. This assumption avoids the - // infinite UPDATE loop between Route Reflector and its clients. - if path.IsLocal() && path == old { - log.WithFields(log.Fields{ - "Topic": "Peer", - "Key": peer.fsm.pConf.State.NeighborAddress, - "Path": path, - }).Debug("given rtm nlri is already sent, skipping to advertise") - return nil - } - - if old != nil && old.IsLocal() { - // We assumes VRF with the specific RT is deleted. - path = old.Clone(true) - } else if peer.isRouteReflectorClient() { - // We need to send the path even if the peer is originator of the - // path in order to signal that the client should distribute route - // with the given RT. - } else { - // We send a path even if it is not the best path. See comments in - // (*Destination) GetChanges(). - dst := peer.localRib.GetDestination(path) - path = nil - for _, p := range dst.GetKnownPathList(peer.TableID(), peer.AS()) { - srcPeer := p.GetSource() - if peer.ID() != srcPeer.Address.String() { - if srcPeer.RouteReflectorClient { - // The path from a RR client is preferred than others - // for the case that RR and non RR client peering - // (e.g., peering of different RR clusters). - path = p - break - } else if path == nil { - path = p - } - } - } - } + if path = peer.filterRTCPath(path, old); path == nil { + return nil } // only allow vpnv4 and vpnv6 paths to be advertised to VRFed neighbors. @@ -988,7 +950,9 @@ func (server *BgpServer) propagateUpdateToNeighbors(source *Peer, newPath *table } family := newPath.GetRouteFamily() for _, targetPeer := range server.neighborMap { - if (source == nil && targetPeer.isRouteServerClient()) || (source != nil && source.isRouteServerClient() != targetPeer.isRouteServerClient()) { + if (source == nil && targetPeer.isRouteServerClient()) || + (source != nil && source.isRouteServerClient() != targetPeer.isRouteServerClient()) || + (family == bgp.RF_RTC_UC && targetPeer.isAdvertiseDefaultRTEnabled()) { continue } f := func() bgp.RouteFamily { @@ -1162,6 +1126,16 @@ func (server *BgpServer) handleFSMMessage(peer *Peer, e *FsmMsg) { }, false) } } + if peer.isAdvertiseDefaultRTEnabled() { + // Insert a RTC path with the default route target. + pi := &table.PeerInfo{ + AS: server.bgpConfig.Global.Config.As, + LocalID: net.ParseIP(server.bgpConfig.Global.Config.RouterId).To4(), + } + path := table.NewDefaultRouteTargetPath(pi) + server.globalRib.Update(path) + sendFsmOutgoingMsg(peer, []*table.Path{path}, nil, false) + } if !peer.fsm.pConf.GracefulRestart.State.LocalRestarting { // When graceful-restart cap (which means intention // of sending EOR) and route-target address family are negotiated, diff --git a/table/path.go b/table/path.go index 62f277d4c..6c08e1a7c 100644 --- a/table/path.go +++ b/table/path.go @@ -179,6 +179,16 @@ func NewEOR(family bgp.RouteFamily) *Path { } } +func NewDefaultRouteTargetPath(source *PeerInfo) *Path { + nlri := bgp.NewRouteTargetMembershipNLRI(0, nil) + attrs := []bgp.PathAttributeInterface{ + bgp.NewPathAttributeOrigin(bgp.BGP_ORIGIN_ATTR_TYPE_INCOMPLETE), + bgp.NewPathAttributeAsPath(nil), + bgp.NewPathAttributeMpReachNLRI("0.0.0.0", []bgp.AddrPrefixInterface{nlri}), + } + return NewPath(source, nlri, false, attrs, time.Now(), false) +} + func (path *Path) IsEOR() bool { if path.info != nil && path.info.eor { return true diff --git a/table/table.go b/table/table.go index b26095f85..1f93e668f 100644 --- a/table/table.go +++ b/table/table.go @@ -23,8 +23,9 @@ import ( "unsafe" "github.com/armon/go-radix" - "github.com/osrg/gobgp/packet/bgp" log "github.com/sirupsen/logrus" + + "github.com/osrg/gobgp/packet/bgp" ) type LookupOption uint8 @@ -103,9 +104,12 @@ func (t *Table) deleteRTCPathsByVrf(vrf *Vrf, vrfs map[string]*Vrf) []*Path { for _, target := range vrf.ImportRt { lhs := target.String() for _, dest := range t.destinations { - nlri := dest.GetNlri().(*bgp.RouteTargetMembershipNLRI) - rhs := nlri.RouteTarget.String() - if lhs == rhs && isLastTargetUser(vrfs, target) { + rhs := dest.GetNlri().(*bgp.RouteTargetMembershipNLRI).RouteTarget + if rhs == nil { + // Ignores the default route target. + continue + } + if lhs == rhs.String() && isLastTargetUser(vrfs, target) { for _, p := range dest.knownPathList { if p.IsLocal() { pathList = append(pathList, p.Clone(true)) diff --git a/test/lib/base.py b/test/lib/base.py index 99a9e97ca..8d0716d8d 100644 --- a/test/lib/base.py +++ b/test/lib/base.py @@ -384,7 +384,8 @@ def add_peer(self, peer, passwd=None, vpn=False, is_rs_client=False, graceful_restart=None, local_as=None, prefix_limit=None, v6=False, llgr=None, vrf='', interface='', allow_as_in=0, remove_private_as=None, replace_peer_as=False, addpath=False, - treat_as_withdraw=False, remote_as=None): + treat_as_withdraw=False, remote_as=None, + advertise_default_rt=False): neigh_addr = '' local_addr = '' it = itertools.product(self.ip_addrs, peer.ip_addrs) @@ -431,7 +432,8 @@ def add_peer(self, peer, passwd=None, vpn=False, is_rs_client=False, 'replace_peer_as': replace_peer_as, 'addpath': addpath, 'treat_as_withdraw': treat_as_withdraw, - 'remote_as': remote_as or peer.asn} + 'remote_as': remote_as or peer.asn, + 'advertise_default_rt': advertise_default_rt} if self.is_running and reload_config: self.create_config() self.reload_config() diff --git a/test/lib/gobgp.py b/test/lib/gobgp.py index bc1f96e95..d54b879ea 100644 --- a/test/lib/gobgp.py +++ b/test/lib/gobgp.py @@ -388,7 +388,15 @@ def _create_config_bgp(self): afi_safi_list.append({'config': {'afi-safi-name': 'l3vpn-ipv4-unicast'}}) afi_safi_list.append({'config': {'afi-safi-name': 'l3vpn-ipv6-unicast'}}) afi_safi_list.append({'config': {'afi-safi-name': 'l2vpn-evpn'}}) - afi_safi_list.append({'config': {'afi-safi-name': 'rtc'}, 'route-target-membership': {'config': {'deferral-time': 10}}}) + if info['is_rr_client'] and info['advertise_default_rt']: + afi_safi_list.append( + {'config': {'afi-safi-name': 'rtc'}, + 'route-target-membership': {'config': {'deferral-time': 10, + 'advertise-default': True}}}) + else: + afi_safi_list.append( + {'config': {'afi-safi-name': 'rtc'}, + 'route-target-membership': {'config': {'deferral-time': 10}}}) if info['flowspec']: afi_safi_list.append({'config': {'afi-safi-name': 'ipv4-flowspec'}}) diff --git a/test/scenario_test/rtc_test.py b/test/scenario_test/rtc_test.py index 13bc8b25e..c431cf572 100644 --- a/test/scenario_test/rtc_test.py +++ b/test/scenario_test/rtc_test.py @@ -982,6 +982,134 @@ def test_56_rr_addpath_cleanup(self): self.g4.local("gobgp vrf del vrf2") self.g5.local("gobgp vrf del vrf1") + def test_60_adv_default_rt_setup(self): + # +------+ + # | g3 | + # +------| (RR) |------+ + # | +------+ | + # (iBGP) (iBGP) + # | | + # +-------------+ +-------------+ + # | g4 | | g5 | + # | (RR Client) | | (RR Client) | + # +-------------+ +-------------+ + g3, g4, g5 = self.g3, self.g4, self.g5 + + g3.update_peer(g4, vpn=True, is_rr_client=True, advertise_default_rt=True) + g4.update_peer(g3, vpn=True) + + g3.update_peer(g5, vpn=True, is_rr_client=True, advertise_default_rt=True) + g5.update_peer(g3, vpn=True) + + g3.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g4) + g3.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g5) + + def test_61_adv_default_rt_check_adj_rib_from_rr(self): + # VRF<#> g3 g4 g5 + # 1 ( ) ( ) + # 2 + # 3 + self.g4.local("gobgp vrf add vrf1 rd 100:100 rt both 100:100") + self.g5.local("gobgp vrf add vrf1 rd 100:100 rt both 100:100") + time.sleep(1) + + def check_rtc(client): + rib = self.g3.get_adj_rib_out(client, rf='rtc') + self.assertEqual(1, len(rib)) + path = rib[0] + self.assertEqual('0:default', path['prefix']) # default RT + self.assertEqual(self.g3.peers[client]['local_addr'].split('/')[0], path['nexthop']) + ids = [attr['value'] for attr in path['attrs'] if attr['type'] == base.BGP_ATTR_TYPE_ORIGINATOR_ID] + self.assertEqual(1, len(ids)) + self.assertEqual(self.g3.router_id, ids[0]) + + check_rtc(self.g4) + check_rtc(self.g5) + + # VRF<#> g3 g4 g5 + # 1 (*) (*) + # 2 + # 3 + self.g4.local("gobgp vrf vrf1 rib add 40.0.0.0/24") + self.g5.local("gobgp vrf vrf1 rib add 50.0.0.0/24") + time.sleep(1) + + def check_ipv4_l3vpn(client): + rib = self.g3.get_adj_rib_out(client, rf='ipv4-l3vpn') + self.assertEqual(1, len(rib)) + path = rib[0] + self.assertNotEqual(self.g3.peers[client]['local_addr'].split('/')[0], path['nexthop']) + ids = [attr['value'] for attr in path['attrs'] if attr['type'] == base.BGP_ATTR_TYPE_ORIGINATOR_ID] + self.assertEqual(1, len(ids)) + self.assertNotEqual(client.router_id, ids[0]) + + check_ipv4_l3vpn(self.g4) + check_ipv4_l3vpn(self.g5) + + def test_62_adv_default_rt_add_vrf(self): + # VRF<#> g3 g4 g5 + # 1 (*) (*) + # 2 ( ) + # 3 + self.g4.local("gobgp vrf add vrf2 rd 200:200 rt both 200:200") + time.sleep(1) + + self.assert_adv_count(self.g4, self.g3, 'rtc', 2) + self.assert_adv_count(self.g4, self.g3, 'ipv4-l3vpn', 1) + + self.assert_adv_count(self.g3, self.g4, 'rtc', 1) # default RT + self.assert_adv_count(self.g3, self.g4, 'ipv4-l3vpn', 1) + + self.assert_adv_count(self.g5, self.g3, 'rtc', 1) + self.assert_adv_count(self.g5, self.g3, 'ipv4-l3vpn', 1) + + self.assert_adv_count(self.g3, self.g5, 'rtc', 1) # default RT + self.assert_adv_count(self.g3, self.g5, 'ipv4-l3vpn', 1) + + def test_63_adv_default_rt_add_route_on_vrf(self): + # VRF<#> g3 g4 g5 + # 1 (*) (*) + # 2 (*) + # 3 + self.g4.local("gobgp vrf vrf2 rib add 40.0.0.0/24") + time.sleep(1) + + self.assert_adv_count(self.g4, self.g3, 'rtc', 2) + self.assert_adv_count(self.g4, self.g3, 'ipv4-l3vpn', 2) + + self.assert_adv_count(self.g3, self.g4, 'rtc', 1) # default RT + self.assert_adv_count(self.g3, self.g4, 'ipv4-l3vpn', 1) + + self.assert_adv_count(self.g5, self.g3, 'rtc', 1) + self.assert_adv_count(self.g5, self.g3, 'ipv4-l3vpn', 1) + + self.assert_adv_count(self.g3, self.g5, 'rtc', 1) # default RT + self.assert_adv_count(self.g3, self.g5, 'ipv4-l3vpn', 1) + + def test_64_adv_default_rt_del_vrf_with_route(self): + # VRF<#> g3 g4 g5 + # 1 (*) + # 2 (*) + # 3 + self.g4.local("gobgp vrf del vrf1") + time.sleep(1) + + self.assert_adv_count(self.g4, self.g3, 'rtc', 1) + self.assert_adv_count(self.g4, self.g3, 'ipv4-l3vpn', 1) + + self.assert_adv_count(self.g3, self.g4, 'rtc', 1) # default RT + self.assert_adv_count(self.g3, self.g4, 'ipv4-l3vpn', 0) + + self.assert_adv_count(self.g5, self.g3, 'rtc', 1) + self.assert_adv_count(self.g5, self.g3, 'ipv4-l3vpn', 1) + + self.assert_adv_count(self.g3, self.g5, 'rtc', 1) # default RT + self.assert_adv_count(self.g3, self.g5, 'ipv4-l3vpn', 0) + + def test_65_adv_default_rt_cleanup(self): + self.g4.local("gobgp vrf del vrf2") + self.g5.local("gobgp vrf del vrf1") + if __name__ == '__main__': output = local("which docker 2>&1 > /dev/null ; echo $?", capture=True) diff --git a/tools/pyang_plugins/gobgp.yang b/tools/pyang_plugins/gobgp.yang index 053ef0a0e..463605545 100644 --- a/tools/pyang_plugins/gobgp.yang +++ b/tools/pyang_plugins/gobgp.yang @@ -1327,9 +1327,15 @@ module gobgp { } grouping route-target-membership-config { - leaf deferral-time { - type uint16; - } + leaf deferral-time { + type uint16; + } + leaf advertise-default { + description + "Configure whether advertise the default route target or not. This can + be used only when the given neighbor is a route reflector client."; + type boolean; + } } grouping afi-safi-state {