Skip to content

Commit 6b50b03

Browse files
committed
Control node changes for supporting a knob to retain AS_PATH in a service chain.
The knob is in the service policy and translated by schema transformer into a boolean in the service RIs for the control node. This is a per service-chain knob. Depends-On: Ia69b39bd5c615ba896d2919482467c6cd5e8f425 Closes-Jira-Bug: CEM-14977 Change-Id: I726f5c908b7c6f3bf8da7e89aaa297f5a6bbd381
1 parent 769b367 commit 6b50b03

File tree

5 files changed

+90
-14
lines changed

5 files changed

+90
-14
lines changed

src/bgp/bgp_config.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@ struct ServiceChainConfig {
331331
std::string source_routing_instance;
332332
std::string service_chain_id;
333333
bool sc_head;
334+
bool retain_as_path;
334335
};
335336

336337
struct StaticRouteConfig {

src/bgp/bgp_config_ifmap.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,6 +1015,7 @@ static void SetServiceChainConfig(BgpInstanceConfig *rti,
10151015
inet_chain.source_routing_instance,
10161016
inet_chain.service_chain_id,
10171017
inet_chain.sc_head,
1018+
inet_chain.retain_as_path,
10181019
};
10191020
list.push_back(item);
10201021
}
@@ -1032,6 +1033,7 @@ static void SetServiceChainConfig(BgpInstanceConfig *rti,
10321033
inet6_chain.source_routing_instance,
10331034
inet6_chain.service_chain_id,
10341035
inet6_chain.sc_head,
1036+
inet6_chain.retain_as_path,
10351037
};
10361038
list.push_back(item);
10371039
}
@@ -1049,6 +1051,7 @@ static void SetServiceChainConfig(BgpInstanceConfig *rti,
10491051
evpn_chain.source_routing_instance,
10501052
evpn_chain.service_chain_id,
10511053
evpn_chain.sc_head,
1054+
evpn_chain.retain_as_path,
10521055
};
10531056
list.push_back(item);
10541057
}
@@ -1066,6 +1069,7 @@ static void SetServiceChainConfig(BgpInstanceConfig *rti,
10661069
evpnv6_chain.source_routing_instance,
10671070
evpnv6_chain.service_chain_id,
10681071
evpnv6_chain.sc_head,
1072+
evpnv6_chain.retain_as_path,
10691073
};
10701074
list.push_back(item);
10711075
}

src/bgp/routing-instance/service_chaining.cc

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ template <typename T>
265265
ServiceChain<T>::ServiceChain(ServiceChainMgrT *manager,
266266
ServiceChainGroup *group, RoutingInstance *src, RoutingInstance *dest,
267267
RoutingInstance *connected, const vector<string> &subnets, AddressT addr,
268-
bool head)
268+
bool head, bool retain_as_path)
269269
: manager_(manager),
270270
group_(group),
271271
src_(src),
@@ -278,6 +278,7 @@ ServiceChain<T>::ServiceChain(ServiceChainMgrT *manager,
278278
dest_table_unregistered_(false),
279279
aggregate_(false),
280280
sc_head_(head),
281+
retain_as_path_(retain_as_path),
281282
src_table_delete_ref_(this, src_table()->deleter()),
282283
dest_table_delete_ref_(this, dest_table()->deleter()),
283284
connected_table_delete_ref_(this, connected_table()->deleter()) {
@@ -816,6 +817,7 @@ void ServiceChain<T>::UpdateServiceChainRouteInternal(const RouteT *orig_route,
816817
bool load_balance_present = false;
817818
const Community *orig_community = NULL;
818819
const OriginVnPath *orig_ovnpath = NULL;
820+
const AsPath *orig_aspath = NULL;
819821
RouteDistinguisher orig_rd;
820822
if (orig_route) {
821823
const BgpPath *orig_path = orig_route->BestPath();
@@ -828,6 +830,7 @@ void ServiceChain<T>::UpdateServiceChainRouteInternal(const RouteT *orig_route,
828830
ext_community = orig_attr->ext_community();
829831
orig_ovnpath = orig_attr->origin_vn_path();
830832
orig_rd = orig_attr->source_rd();
833+
orig_aspath = orig_attr->as_path();
831834
}
832835
if (ext_community) {
833836
BOOST_FOREACH(const ExtCommunity::ExtCommunityValue &comm,
@@ -990,9 +993,16 @@ void ServiceChain<T>::UpdateServiceChainRouteInternal(const RouteT *orig_route,
990993
new_attr = attr_db->ReplaceOriginVnPathAndLocate(new_attr.get(),
991994
new_ovnpath);
992995

993-
// Strip aspath. This is required when the connected route is
994-
// learnt via BGP.
995-
new_attr = attr_db->ReplaceAsPathAndLocate(new_attr.get(), AsPathPtr());
996+
// Strip as_path if needed. This is required when the connected route is
997+
// learnt via BGP. If retain_as_path knob is configured replace the
998+
// AsPath with the value from the original route.
999+
if (retain_as_path() && orig_aspath) {
1000+
new_attr = attr_db->ReplaceAsPathAndLocate(new_attr.get(),
1001+
orig_aspath);
1002+
} else {
1003+
new_attr = attr_db->ReplaceAsPathAndLocate(new_attr.get(),
1004+
AsPathPtr());
1005+
}
9961006

9971007
// If the connected path is learnt via XMPP, construct RD based on
9981008
// the id registered with source table instead of connected table.
@@ -1786,7 +1796,7 @@ bool ServiceChainMgr<T>::LocateServiceChain(RoutingInstance *rtinstance,
17861796
// Allocate the new service chain.
17871797
ServiceChainPtr chain = ServiceChainPtr(new ServiceChainT(this, group,
17881798
rtinstance, dest, connected_ri, config.prefix, chain_addr,
1789-
config.sc_head));
1799+
config.sc_head, config.retain_as_path));
17901800

17911801
if (aggregate_host_route()) {
17921802
ServiceChainT *obj = static_cast<ServiceChainT *>(chain.get());

src/bgp/routing-instance/service_chaining.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ class ServiceChain : public ConditionMatch {
143143

144144
ServiceChain(ServiceChainMgrT *manager, ServiceChainGroup *group,
145145
RoutingInstance *src, RoutingInstance *dest, RoutingInstance *connected,
146-
const std::vector<std::string> &subnets, AddressT addr, bool head);
146+
const std::vector<std::string> &subnets, AddressT addr, bool head,
147+
bool retain_as_path);
147148
Address::Family GetFamily() const { return manager_->GetFamily(); }
148149
Address::Family GetConnectedFamily() const {
149150
return manager_->GetConnectedFamily();
@@ -219,6 +220,7 @@ class ServiceChain : public ConditionMatch {
219220

220221
bool aggregate_enable() const { return aggregate_; }
221222
bool is_sc_head() const { return sc_head_; }
223+
bool retain_as_path() const { return retain_as_path_; }
222224
void set_aggregate_enable() { aggregate_ = true; }
223225
bool group_oper_state_up() const { return group_oper_state_up_; }
224226
void set_group_oper_state_up(bool up) { group_oper_state_up_ = up; }
@@ -239,6 +241,7 @@ class ServiceChain : public ConditionMatch {
239241
bool dest_table_unregistered_;
240242
bool aggregate_; // Whether the host route needs to be aggregated
241243
bool sc_head_; // Whether this SI is at the head of the chain
244+
bool retain_as_path_;
242245
LifetimeRef<ServiceChain> src_table_delete_ref_;
243246
LifetimeRef<ServiceChain> dest_table_delete_ref_;
244247
LifetimeRef<ServiceChain> connected_table_delete_ref_;

src/bgp/test/service_chain_test.cc

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,8 @@ class ServiceChainTest : public ::testing::Test {
640640
extcomm_spec.communities.push_back(lb.GetExtCommunityValue());
641641

642642
attr_spec.push_back(&extcomm_spec);
643+
644+
643645
BgpAttrPtr attr = bgp_server_->attr_db()->Locate(attr_spec);
644646

645647
request.data.reset(new BgpTable::RequestData(attr, flags, label));
@@ -916,16 +918,23 @@ class ServiceChainTest : public ::testing::Test {
916918
const vector<uint32_t> sg_ids, const set<string> tunnel_encaps,
917919
const SiteOfOrigin &soo, const vector<uint32_t> &commlist,
918920
const vector<string> &origin_vn_path, const LoadBalance &lb,
919-
const vector<int> tag_list) {
921+
const vector<int> tag_list, bool retain_as_path=false) {
920922
BgpAttrPtr attr = path->GetAttr();
921923
if (attr->nexthop().to_v4().to_string() != path_id)
922924
return false;
923925
if (GetOriginVnFromRoute(path) != origin_vn)
924926
return false;
925927
if (label && path->GetLabel() != label)
926928
return false;
927-
if (attr->as_path_count())
928-
return false;
929+
if (retain_as_path) {
930+
if (!attr->as_path_count()) {
931+
return false;
932+
}
933+
} else {
934+
if (attr->as_path_count()) {
935+
return false;
936+
}
937+
}
929938
if (sg_ids.size()) {
930939
vector<uint32_t> path_sg_ids = GetSGIDListFromRoute(path);
931940
if (path_sg_ids.size() != sg_ids.size())
@@ -1019,7 +1028,7 @@ class ServiceChainTest : public ::testing::Test {
10191028
const SiteOfOrigin &soo, const vector<uint32_t> &commlist,
10201029
const vector<string> &origin_vn_path,
10211030
const LoadBalance &lb, const vector<int> tag_list,
1022-
bool replication=false) {
1031+
bool replication=false, bool retain_as_path=false) {
10231032
task_util::TaskSchedulerLock lock;
10241033
BgpRoute *route = RouteLookup(instance, prefix, replication);
10251034
if (!route)
@@ -1036,7 +1045,7 @@ class ServiceChainTest : public ::testing::Test {
10361045
found = true;
10371046
if (MatchPathAttributes(path, path_id, origin_vn, label,
10381047
sg_ids, tunnel_encap, soo, commlist, origin_vn_path,
1039-
lb, tag_list)) {
1048+
lb, tag_list, retain_as_path)) {
10401049
break;
10411050
}
10421051
return false;
@@ -1063,14 +1072,14 @@ class ServiceChainTest : public ::testing::Test {
10631072

10641073
void VerifyRouteAttributes(const string &instance,
10651074
const string &prefix, const string &path_id, const string &origin_vn,
1066-
int label = 0, bool replication=false) {
1075+
int label = 0, bool replication=false, bool retain_as_path=false) {
10671076
task_util::WaitForIdle();
10681077
vector<string> path_ids = list_of(path_id);
10691078
vector<uint32_t> commlist = list_of(CommunityType::AcceptOwnNexthop);
10701079
TASK_UTIL_EXPECT_TRUE(CheckRouteAttributes(
10711080
instance, prefix, path_ids, origin_vn, label, vector<uint32_t>(),
10721081
set<string>(), SiteOfOrigin(), commlist, vector<string>(),
1073-
LoadBalance(), vector<int>(), replication));
1082+
LoadBalance(), vector<int>(), replication, retain_as_path));
10741083
}
10751084

10761085
void VerifyRouteAttributes(const string &instance, const string &prefix,
@@ -1263,9 +1272,10 @@ class ServiceChainTest : public ::testing::Test {
12631272
}
12641273

12651274
void SetServiceChainInformation(const string &instance,
1266-
const string &filename) {
1275+
const string &filename, bool retain_as_path=false) {
12671276
auto_ptr<autogen::ServiceChainInfo> params = GetChainConfig(filename);
12681277
params->sc_head = true;
1278+
params->retain_as_path = retain_as_path;
12691279
ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance",
12701280
instance, sc_family_ == SCAddress::INET ?
12711281
"service-chain-information" : (sc_family_ == SCAddress::INET6 ?
@@ -3876,6 +3886,54 @@ TYPED_TEST(ServiceChainTest, ExtConnectRouteOriginVnUnresolved1) {
38763886
this->DeleteConnectedRoute(NULL, this->BuildConnPrefix("1.1.2.3", 32));
38773887
}
38783888

3889+
//
3890+
// Service chain route should be added for routes with unresolved origin
3891+
// vn if there is at least one route target matching an export target of
3892+
// the destination instance. Also AsPath should be retained.
3893+
//
3894+
// 1. Create Service Chain with 192.168.1.0/24 as vn subnet
3895+
// 2. Add connected route
3896+
// 3. Add MX leaked route 10.1.1.0/24 with unresolved OriginVn
3897+
// 4. Verify that ext connect route 10.1.1.0/24 is added
3898+
//
3899+
TYPED_TEST(ServiceChainTest, ExtConnectRouteRetainAsPath1) {
3900+
if (this->GetFamily() == Address::EVPN) return;
3901+
vector<string> instance_names =
3902+
list_of("blue")("blue-i1")("red-i2")("red")("green");
3903+
multimap<string, string> connections =
3904+
map_list_of("blue", "blue-i1") ("red-i2", "red");
3905+
this->NetworkConfig(instance_names, connections);
3906+
this->VerifyNetworkConfig(instance_names);
3907+
3908+
this->SetServiceChainInformation("blue-i1",
3909+
"controller/src/bgp/testdata/service_chain_1.xml", true);
3910+
3911+
// Add Connected
3912+
this->AddConnectedRoute(NULL, this->BuildConnPrefix("1.1.2.3", 32), 100,
3913+
this->BuildNextHopAddress("2.3.4.5"));
3914+
3915+
// Add Ext connect route with targets of both red and green.
3916+
this->AddVpnRoute(NULL, "red", this->BuildPrefix("10.1.1.0", 24), 100);
3917+
3918+
// Verify that MX leaked route is present in red
3919+
this->VerifyRouteExists("red", this->BuildPrefix("10.1.1.0", 24));
3920+
3921+
// Verify that ExtConnect route is present in blue
3922+
this->VerifyRouteExists("blue", this->BuildPrefix("10.1.1.0", 24));
3923+
this->VerifyRouteAttributes("blue", this->BuildPrefix("10.1.1.0", 24),
3924+
this->BuildNextHopAddress("2.3.4.5"), "red",
3925+
0, false, true);
3926+
this->VerifyRouteExists("blue",
3927+
this->BuildReplicationPrefix("10.1.1.0", 24), true);
3928+
this->VerifyRouteAttributes("blue",
3929+
this->BuildReplicationPrefix("10.1.1.0", 24),
3930+
this->BuildNextHopAddress("2.3.4.5"), "red", 0, true, true);
3931+
3932+
// Delete ExtRoute and connected route
3933+
this->DeleteVpnRoute(NULL, "red", this->BuildPrefix("10.1.1.0", 24));
3934+
this->DeleteConnectedRoute(NULL, this->BuildConnPrefix("1.1.2.3", 32));
3935+
}
3936+
38793937
//
38803938
// Service chain route must not be added for routes with unresolved origin
38813939
// vn if there is no route target matching an export target of destination

0 commit comments

Comments
 (0)