From f1370e28e6e1cb8e9bc6b4bdac7b4c4c1442aefc Mon Sep 17 00:00:00 2001 From: Naveen N Date: Thu, 20 Nov 2014 22:18:14 -0800 Subject: [PATCH] * All the component nh member of a composite NH are policy disabled 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: Iba53ae69c9bbdf8e796ee56d77b0a961b2f4ebf4 --- src/vnsw/agent/oper/nexthop.cc | 1 + src/vnsw/agent/test/test_nh.cc | 70 +++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/vnsw/agent/oper/nexthop.cc b/src/vnsw/agent/oper/nexthop.cc index 16330e8c646..31f52a72a8d 100644 --- a/src/vnsw/agent/oper/nexthop.cc +++ b/src/vnsw/agent/oper/nexthop.cc @@ -1773,6 +1773,7 @@ void CompositeNHKey::Reorder(Agent *agent, if (nh->GetType() != NextHop::COMPOSITE) { DBEntryBase::KeyPtr key = nh->GetDBRequestKey(); NextHopKey *nh_key = static_cast(key.release()); + nh_key->SetPolicy(false); std::auto_ptr nh_key_ptr(nh_key); //Insert exisiting nexthop at first slot //This ensures that old flows are not disturbed diff --git a/src/vnsw/agent/test/test_nh.cc b/src/vnsw/agent/test/test_nh.cc index f59031ecd20..d4039c80d5a 100644 --- a/src/vnsw/agent/test/test_nh.cc +++ b/src/vnsw/agent/test/test_nh.cc @@ -370,7 +370,7 @@ 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"); @@ -378,6 +378,7 @@ TEST_F(CfgTest, EcmpNH_2) { 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); @@ -386,6 +387,9 @@ TEST_F(CfgTest, EcmpNH_2) { EXPECT_TRUE(nh->GetType() == NextHop::COMPOSITE); const CompositeNH *comp_nh = static_cast(nh); EXPECT_TRUE(comp_nh->ComponentNHCount() == 2); + const InterfaceNH *intf_nh = static_cast(comp_nh->Get(0)->nh()); + EXPECT_TRUE(intf_nh->PolicyEnabled() == false); + EXPECT_TRUE(intf_nh->GetInterface()->name() == "vnet1"); CreateVmportWithEcmp(input3, 1); client->WaitForIdle(); @@ -406,7 +410,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 + intf_nh = static_cast ((*component_nh_it)->nh()); EXPECT_TRUE(intf_nh->GetInterface()->name() == "vnet1"); MplsLabel *mpls = GetActiveLabel(MplsLabel::VPORT_NH, @@ -1643,6 +1647,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(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(); + + InetUnicastRouteEntry *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(key.release()); + std::auto_ptr 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(nh); + const InterfaceNH *intf_nh = static_cast(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();