Skip to content

Commit

Permalink
* All the component nh member of a composite NH are policy disabled
Browse files Browse the repository at this point in the history
  nexthop, since composite NH itself is policy enabled and there is
  no need for component member to policy disabled. When a transition
  happens from non-ecmp to ecmp, local interface nexthop could be
  added with policy enabled flag, and finding of the nexthop inside
  composite NH would fail since we always search for policy disabled
  nh inside composite NH. Since nexthop lookup fails flow ecmp index
  would not be set, resulting in drop
  Unit test added to verify the case where transition happens from
  non-ecmp to ecmp with local nexthop pre-existing
Closes-bug:#1381978

Change-Id: Ib097a77450afcc7ebba622a0732af3c1105c7db9
  • Loading branch information
naveen-n committed Oct 16, 2014
1 parent 7645408 commit a21d82a
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/vnsw/agent/oper/nexthop.cc
Expand Up @@ -1781,6 +1781,7 @@ void CompositeNHKey::Reorder(Agent *agent,
if (nh->GetType() != NextHop::COMPOSITE) {
DBEntryBase::KeyPtr key = nh->GetDBRequestKey();
NextHopKey *nh_key = static_cast<NextHopKey *>(key.release());
nh_key->SetPolicy(false);
std::auto_ptr<const NextHopKey> nh_key_ptr(nh_key);
//Insert exisiting nexthop at first slot
//This ensures that old flows are not disturbed
Expand Down
70 changes: 68 additions & 2 deletions src/vnsw/agent/test/test_nh.cc
Expand Up @@ -369,14 +369,15 @@ TEST_F(CfgTest, EcmpNH_2) {
{"vnet5", 5, "1.1.1.1", "00:00:00:02:02:05", 1, 5}
};

CreateVmportWithEcmp(input1, 1);
CreateVmportWithEcmp(input1, 1, 1);
client->WaitForIdle();
//First VM added, route points to composite NH
Ip4Address ip = Ip4Address::from_string("1.1.1.1");
Inet4UnicastRouteEntry *rt = RouteGet("vrf1", ip, 32);
EXPECT_TRUE(rt != NULL);
const NextHop *nh = rt->GetActiveNextHop();
EXPECT_TRUE(nh->GetType() == NextHop::INTERFACE);
EXPECT_TRUE(nh->PolicyEnabled() == true);

//Second VM added, route should point to composite NH
CreateVmportWithEcmp(input2, 1);
Expand All @@ -385,6 +386,9 @@ TEST_F(CfgTest, EcmpNH_2) {
EXPECT_TRUE(nh->GetType() == NextHop::COMPOSITE);
const CompositeNH *comp_nh = static_cast<const CompositeNH *>(nh);
EXPECT_TRUE(comp_nh->ComponentNHCount() == 2);
const InterfaceNH *intf_nh = static_cast<const InterfaceNH *>(comp_nh->Get(0)->nh());
EXPECT_TRUE(intf_nh->PolicyEnabled() == false);
EXPECT_TRUE(intf_nh->GetInterface()->name() == "vnet1");

CreateVmportWithEcmp(input3, 1);
client->WaitForIdle();
Expand All @@ -405,7 +409,7 @@ TEST_F(CfgTest, EcmpNH_2) {
//Verify all the component NH have right label and nexthop
ComponentNHList::const_iterator component_nh_it =
comp_nh->begin();
const InterfaceNH *intf_nh = static_cast<const InterfaceNH *>
intf_nh = static_cast<const InterfaceNH *>
((*component_nh_it)->nh());
EXPECT_TRUE(intf_nh->GetInterface()->name() == "vnet1");
MplsLabel *mpls = GetMplsLabel(MplsLabel::VPORT_NH,
Expand Down Expand Up @@ -1642,6 +1646,68 @@ TEST_F(CfgTest, EcmpNH_16) {
EXPECT_FALSE(VrfFind("vrf2"));
}

//Add a interface NH with policy
//Add a BGP peer route with one interface NH and one tunnel NH
//make sure interface NH gets added without policy
TEST_F(CfgTest, EcmpNH_17) {
//Add interface
struct PortInfo input[] = {
{"vnet1", 1, "1.1.1.1", "00:00:00:01:01:01", 1, 1},
};
CreateVmportEnv(input, 1, 1);
client->WaitForIdle();

const VmInterface *intf = static_cast<const VmInterface *>(VmPortGet(1));
Ip4Address ip = Ip4Address::from_string("1.1.1.1");
agent_->fabric_inet4_unicast_table()->
AddLocalVmRouteReq(bgp_peer, "vrf1", ip, 32,
MakeUuid(1), "vn1", intf->label(),
SecurityGroupList(), false, PathPreference(), Ip4Address(0));
client->WaitForIdle();

Inet4UnicastRouteEntry *rt = RouteGet("vrf1", ip, 32);
EXPECT_TRUE(rt != NULL);
const NextHop *nh = rt->GetActiveNextHop();

Ip4Address remote_server_ip1 = Ip4Address::from_string("10.10.10.100");
//Create component NH list
//Transition remote VM route to ECMP route
ComponentNHKeyPtr nh_data1(new ComponentNHKey(15, agent_->fabric_vrf_name(),
agent_->router_id(),
remote_server_ip1,
false,
TunnelType::DefaultType()));
DBEntryBase::KeyPtr key = nh->GetDBRequestKey();
NextHopKey *nh_key = static_cast<NextHopKey *>(key.release());
std::auto_ptr<const NextHopKey> nh_akey(nh_key);
nh_key->SetPolicy(false);
ComponentNHKeyPtr nh_data2(new ComponentNHKey(intf->label(), nh_akey));

ComponentNHKeyList comp_nh_list;
//Insert new NH first and then existing route NH
comp_nh_list.push_back(nh_data1);
comp_nh_list.push_back(nh_data2);

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

rt = RouteGet("vrf1", ip, 32);
EXPECT_TRUE(rt != NULL);
nh = rt->GetActiveNextHop();
EXPECT_TRUE(nh->GetType() == NextHop::COMPOSITE);
const CompositeNH *comp_nh = static_cast<const CompositeNH *>(nh);
const InterfaceNH *intf_nh = static_cast<const InterfaceNH *>(comp_nh->Get(0)->nh());
EXPECT_TRUE(intf_nh->PolicyEnabled() == false);
EXPECT_TRUE(intf_nh->GetInterface()->name() == "vnet1");

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

TEST_F(CfgTest, TunnelType_1) {
AddEncapList("MPLSoGRE", NULL, NULL);
client->WaitForIdle();
Expand Down

0 comments on commit a21d82a

Please sign in to comment.