Skip to content

Commit

Permalink
* Upon local mpls ecmp label deallocation, control-node would withdraw
Browse files Browse the repository at this point in the history
  this mpls label in its path, if the label gets reused even before
  control-node retracts the label, due to quick activate and deactivate
  of the interface(oper state change), then mpls label in BGP path would
  point to internface NH, instead of composite NH as expected.
  Re-evaluation of BGP peer happens because it was one of the ecmp interface
  that got deactivated and added again.
  Add a check to verify that local ecmp mpls label is not deleted.
Unit test:
  Created ecmp with 2 instances, add the same route with aggregarte mpls
  label via bgp peer, deleted both interface so that route update is
  sent to BGP to withdraw this path, before BGP peer could withdraw
  same instance would be activated with aggregarate mpls label and
  route change would be triggered, verify that no crash is seen,
  and update of path happens fine
Closes-bug:#1381821

Change-Id: I27a0f8ca0f011cc4bab3de79787356d32563c4d6
  • Loading branch information
naveen-n committed Dec 20, 2014
1 parent 50d8422 commit cd58ed5
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 2 deletions.
3 changes: 2 additions & 1 deletion src/vnsw/agent/oper/agent_path.cc
Expand Up @@ -218,7 +218,8 @@ bool AgentPath::Sync(AgentRoute *sync_route) {

//Check if there was a change in local ecmp composite nexthop
if (nh_ && nh_->GetType() == NextHop::COMPOSITE &&
composite_nh_key_.get() != NULL) {
composite_nh_key_.get() != NULL &&
local_ecmp_mpls_label_.get() != NULL) {
boost::scoped_ptr<CompositeNHKey> composite_nh_key(composite_nh_key_->Clone());
if (ReorderCompositeNH(agent, composite_nh_key.get())) {
if (ChangeCompositeNH(agent, composite_nh_key.get())) {
Expand Down
4 changes: 3 additions & 1 deletion src/vnsw/agent/pkt/test/test_ecmp.cc
Expand Up @@ -49,6 +49,7 @@ class EcmpTest : public ::testing::Test {
boost::system::error_code ec;
bgp_peer = CreateBgpPeer(Ip4Address::from_string("0.0.0.1", ec),
"xmpp channel");
client->WaitForIdle();

//Add floating IP for vrf2 interface to talk to
//vrf3
Expand Down Expand Up @@ -1487,7 +1488,8 @@ TEST_F(EcmpTest, ServiceVlanTest_3) {
DeleteVmportEnv(input1, 1, true);
client->WaitForIdle();

EXPECT_TRUE(Agent::GetInstance()->pkt()->flow_table()->Size() == 0);
WAIT_FOR(1000, 1000,
(Agent::GetInstance()->pkt()->flow_table()->Size() == 0));
EXPECT_FALSE(VrfFind("vrf11"));
EXPECT_FALSE(VrfFind("vrf10"));
EXPECT_FALSE(VrfFind("service-vrf1"));
Expand Down
65 changes: 65 additions & 0 deletions src/vnsw/agent/test/test_nh.cc
Expand Up @@ -2241,6 +2241,71 @@ TEST_F(CfgTest, Nexthop_invalid_vrf) {
WAIT_FOR(1000, 1000, (VrfGet("vrf11") == NULL));
}

//Delete local ecmp mpls label by
//deleting all local interfaces, add them
//back with BGP path still retaining old local ecmp
//mpls and ensure there is no crash
TEST_F(CfgTest, EcmpNH_18) {
//Add interface
struct PortInfo input[] = {
{"vnet1", 1, "1.1.1.1", "00:00:00:01:01:01", 1, 1},
{"vnet2", 2, "1.1.1.1", "00:00:00:01:01:01", 1, 2},
};
CreateVmportWithEcmp(input, 2);
client->WaitForIdle();

Ip4Address ip = Ip4Address::from_string("1.1.1.1");
InetUnicastRouteEntry *rt = RouteGet("vrf1", ip, 32);
EXPECT_TRUE(rt != NULL);
const NextHop *nh = rt->GetActiveNextHop();

//Create component NH list
//Transition remote VM route to ECMP route
DBEntryBase::KeyPtr key = nh->GetDBRequestKey();
NextHopKey *nh_key = static_cast<NextHopKey *>(key.release());
std::auto_ptr<const NextHopKey> nh_akey(nh_key);
ComponentNHKeyPtr nh_data1(new ComponentNHKey(rt->GetActiveLabel(), nh_akey));

uint32_t label = rt->GetActiveLabel();
ComponentNHKeyList comp_nh_list;
//Insert new NH first and then existing route NH
comp_nh_list.push_back(nh_data1);

SecurityGroupList sg_list;
EcmpTunnelRouteAdd(bgp_peer, "vrf1", ip, 32,
comp_nh_list, false, "vn1", sg_list, PathPreference());
client->WaitForIdle();

DeleteVmportEnv(input, 2, false);
client->WaitForIdle();

struct PortInfo input1[] = {
{"vnet3", 3, "1.1.1.3", "00:00:00:01:01:01", 1, 3},
{"vnet4", 4, "1.1.1.4", "00:00:00:01:01:01", 1, 4},
};
CreateVmportWithEcmp(input1, 2, 1);
client->WaitForIdle();
//Add back the VM interface of ECMP
//This would result ing resync of route, due to policy change
CreateVmportWithEcmp(input, 1, 1);
client->WaitForIdle();
const VmInterface *vm_intf =
static_cast<const VmInterface *>(VmPortGet(1));

agent_->fabric_inet4_unicast_table()->
AddLocalVmRouteReq(bgp_peer, "vrf1", ip, 32,
MakeUuid(1), "vn1", vm_intf->label(),
SecurityGroupList(), false, PathPreference(), Ip4Address(0));
client->WaitForIdle();
EXPECT_TRUE(rt->GetActiveNextHop()->GetType() == NextHop::INTERFACE);

DeleteVmportEnv(input, 1, false);
DeleteVmportEnv(input1, 2, true);
WAIT_FOR(100, 1000, (VrfFind("vrf1") == false));
client->WaitForIdle();
EXPECT_FALSE(RouteFind("vrf1", ip, 32));
}

int main(int argc, char **argv) {
GETUSERARGS();
client = TestInit(init_file, ksync_init);
Expand Down

0 comments on commit cd58ed5

Please sign in to comment.