From 3151aa21b03dbf28b8a01c0451268feb6d2fe367 Mon Sep 17 00:00:00 2001 From: ashoksingh Date: Sat, 16 Jul 2016 23:40:18 +0530 Subject: [PATCH] Update CompositeNH on VMI policy status change. When label of VMI changes and if that VMI (ie VMI's InterfaceNH) is part of ECMP, then update the CompositeNH for ECMP route to point to right label for that VMI. Label of VMI can change when policy-status of VMI changes Closes-Bug: #1602944 (cherry picked from commit bc1636641f3b1077620bbf76b7da1cdf6f26e35e) Change-Id: Ib8198586a210ce052af8908e2cff284a31ca12b4 --- src/vnsw/agent/oper/inet_unicast_route.cc | 57 +++++++++++ src/vnsw/agent/oper/inet_unicast_route.h | 1 + src/vnsw/agent/oper/nexthop.cc | 19 ++++ src/vnsw/agent/oper/nexthop.h | 2 + src/vnsw/agent/oper/test/test_intf_policy.cc | 101 ++++++++++++++++--- 5 files changed, 164 insertions(+), 16 deletions(-) diff --git a/src/vnsw/agent/oper/inet_unicast_route.cc b/src/vnsw/agent/oper/inet_unicast_route.cc index d41c19e51c9..2413d40fcaa 100644 --- a/src/vnsw/agent/oper/inet_unicast_route.cc +++ b/src/vnsw/agent/oper/inet_unicast_route.cc @@ -761,10 +761,14 @@ bool InetUnicastRouteEntry::EcmpAddPath(AgentPath *path) { ret = true; } else if (ecmp) { AgentPath *ecmp_path = FindPath(agent->ecmp_peer()); + bool updated = UpdateComponentNH(agent, ecmp_path, path); ret = SyncEcmpPath(ecmp_path, path->sg_list(), path->communities(), path->path_preference(), path->tunnel_bmap(), path->ecmp_load_balance()); + if (updated) { + ret = true; + } } return ret; @@ -813,6 +817,59 @@ void InetUnicastRouteEntry::AppendEcmpPath(Agent *agent, GETPEERNAME(agent->ecmp_peer())); } +/* When label of VMI changes and if that VMI (ie VMI's InterfaceNH) is part of + * ECMP, then update the CompositeNH for ECMP route to point to right label for + * that VMI. Label of VMI can change when policy-status of VMI changes */ +bool InetUnicastRouteEntry::UpdateComponentNH(Agent *agent, + AgentPath *ecmp_path, + AgentPath *path) { + if (!ecmp_path) { + return false; + } + //Build ComponentNHKey for new path + DBEntryBase::KeyPtr key = path->ComputeNextHop(agent)->GetDBRequestKey(); + NextHopKey *nh_key = static_cast(key.release()); + nh_key->SetPolicy(false); + + ComponentNHKeyList component_nh_key_list; + const CompositeNH *comp_nh = + static_cast(ecmp_path->ComputeNextHop(agent)); + bool updated = comp_nh->UpdateComponentNHKey(path->label(), nh_key, + component_nh_key_list); + + if (!updated) { + return false; + } + + // Form the request for Inet4UnicastEcmpRoute and invoke AddChangePath + DBRequest nh_req(DBRequest::DB_ENTRY_ADD_CHANGE); + nh_req.key.reset(new CompositeNHKey(Composite::LOCAL_ECMP, + false, component_nh_key_list, + vrf()->GetName())); + nh_req.data.reset(new CompositeNHData()); + + InetUnicastRouteEntry::ModifyEcmpPath(addr_, plen_, + ecmp_path->dest_vn_list(), + ecmp_path->label(), true, vrf()->GetName(), + ecmp_path->sg_list(), ecmp_path->communities(), + ecmp_path->path_preference(), + ecmp_path->tunnel_bmap(), + ecmp_path->ecmp_load_balance(), + nh_req, agent, ecmp_path); + + //Make MPLS label point to updated composite NH + MplsLabel::CreateEcmpLabel(agent, ecmp_path->label(), Composite::LOCAL_ECMP, + component_nh_key_list, vrf()->GetName()); + + RouteInfo rt_info; + FillTrace(rt_info, AgentRoute::CHANGE_PATH, path); + AgentRouteTable *table = static_cast(get_table()); + OPER_TRACE_ROUTE_ENTRY(Route, table, rt_info); + AGENT_ROUTE_LOG(table, "Path Update", ToString(), vrf()->GetName(), + GETPEERNAME(agent->ecmp_peer())); + return true; +} + void InetUnicastRouteEntry::DeleteComponentNH(Agent *agent, AgentPath *path) { AgentPath *ecmp_path = FindPath(agent->ecmp_peer()); diff --git a/src/vnsw/agent/oper/inet_unicast_route.h b/src/vnsw/agent/oper/inet_unicast_route.h index f169b0fab40..bd73d2ff486 100644 --- a/src/vnsw/agent/oper/inet_unicast_route.h +++ b/src/vnsw/agent/oper/inet_unicast_route.h @@ -73,6 +73,7 @@ class InetUnicastRouteEntry : public AgentRoute { virtual bool EcmpDeletePath(AgentPath *path); void AppendEcmpPath(Agent *agent, AgentPath *path); void DeleteComponentNH(Agent *agent, AgentPath *path); + bool UpdateComponentNH(Agent *agent, AgentPath *ecmp_path, AgentPath *path); AgentPath *AllocateEcmpPath(Agent *agent, const AgentPath *path1, const AgentPath *path2); diff --git a/src/vnsw/agent/oper/nexthop.cc b/src/vnsw/agent/oper/nexthop.cc index 8c42df99820..154ff5f4a67 100644 --- a/src/vnsw/agent/oper/nexthop.cc +++ b/src/vnsw/agent/oper/nexthop.cc @@ -1828,6 +1828,25 @@ void CompositeNHKey::erase(ComponentNHKeyPtr nh_key) { } } +bool CompositeNH::UpdateComponentNHKey(uint32_t label, NextHopKey *nh_key, + ComponentNHKeyList &component_nh_key_list) const { + bool ret = false; + ComponentNHKeyList::const_iterator it = component_nh_key_list_.begin(); + for (;it != component_nh_key_list_.end(); it++) { + const NextHopKey *lhs = (*it)->nh_key(); + uint32_t new_label = (*it)->label(); + if((*it) && (*it)->label() != label && lhs->IsEqual(*nh_key)) { + new_label = label; + ret = true; + } + std::auto_ptr nh_akey(lhs->Clone()); + ComponentNHKeyPtr comp_nh_key_ptr(new ComponentNHKey(new_label, + nh_akey)); + component_nh_key_list.push_back(comp_nh_key_ptr); + } + return ret; +} + ComponentNHKeyList CompositeNH::AddComponentNHKey(ComponentNHKeyPtr cnh) const { Agent *agent = static_cast(get_table())->agent(); const NextHop *nh = static_cast(agent->nexthop_table()-> diff --git a/src/vnsw/agent/oper/nexthop.h b/src/vnsw/agent/oper/nexthop.h index e0d1eca6389..9638c387f6a 100644 --- a/src/vnsw/agent/oper/nexthop.h +++ b/src/vnsw/agent/oper/nexthop.h @@ -1359,6 +1359,8 @@ class CompositeNH : public NextHop { component_nh_key) const; ComponentNHKeyList DeleteComponentNHKey(ComponentNHKeyPtr component_nh_key) const; + bool UpdateComponentNHKey(uint32_t label, NextHopKey *nh_key, + ComponentNHKeyList &component_nh_key_list) const; ComponentNHList& component_nh_list() { return component_nh_list_; } diff --git a/src/vnsw/agent/oper/test/test_intf_policy.cc b/src/vnsw/agent/oper/test/test_intf_policy.cc index f98c3f7556c..ad13593ef40 100644 --- a/src/vnsw/agent/oper/test/test_intf_policy.cc +++ b/src/vnsw/agent/oper/test/test_intf_policy.cc @@ -3,22 +3,6 @@ */ #include "base/os.h" -#if 0 -#include - -#include - -#ifdef __linux__ -#include -#include -#include -#endif - -#ifdef __FreeBSD__ -#include -#include -#endif -#endif #include "testing/gunit.h" #include @@ -54,6 +38,10 @@ #define vm1_ip "11.1.1.1" #define vm2_ip "11.1.1.2" +IpamInfo ipam_info[] = { + {"1.1.1.0", 24, "1.1.1.10"}, +}; + void RouterIdDepInit(Agent *agent) { } @@ -199,6 +187,15 @@ class PolicyTest : public ::testing::Test { flow_proto_ = agent_->pkt()->get_flow_proto(); } + void SetUp() { + AddIPAM("vn1", ipam_info, 1); + client->WaitForIdle(); + } + void TearDown() { + WAIT_FOR(1000, 1000, agent_->vrf_table()->Size() == 1); + DelIPAM("vn1"); + client->WaitForIdle(); + } void SetPolicyDisabledStatus(struct PortInfo *input, bool status) { ostringstream str; @@ -697,6 +694,78 @@ TEST_F(PolicyTest, IntfPolicyDisable_Flow) { EXPECT_EQ(0U, flow_proto_->FlowCount()); } +TEST_F(PolicyTest, EcmpNH_1) { + //Create mutliple VM interface with same IP + struct PortInfo input1[] = { + {"vnet1", 1, "1.1.1.1", "00:00:00:01:01:01", 1, 1}, + {"vnet2", 2, "1.1.1.1", "00:00:00:02:02:01", 1, 2}, + }; + + CreateVmportWithEcmp(input1, 2, 1); + client->WaitForIdle(); + + EXPECT_TRUE(VmPortActive(input1, 0)); + EXPECT_TRUE(VmPortActive(input1, 1)); + const VmInterface *intf1 = VmInterfaceGet(input1[0].intf_id); + EXPECT_TRUE(intf1 != NULL); + const VmInterface *intf2 = VmInterfaceGet(input1[1].intf_id); + EXPECT_TRUE(intf2 != NULL); + //Check that route points to composite NH, + //with 2 members + Ip4Address ip = Ip4Address::from_string("1.1.1.1"); + InetUnicastRouteEntry *rt = RouteGet("vrf1", ip, 32); + EXPECT_TRUE(rt != NULL); + const NextHop *nh = rt->GetActiveNextHop(); + EXPECT_TRUE(nh->GetType() == NextHop::COMPOSITE); + const CompositeNH *comp_nh = static_cast(nh); + EXPECT_TRUE(comp_nh->ComponentNHCount() == 2); + + //Get the MPLS label corresponding to this path and verify + //that mpls label also has 2 component NH + uint32_t mpls_label = rt->GetActiveLabel(); + EXPECT_TRUE(FindMplsLabel(MplsLabel::VPORT_NH, mpls_label)); + EXPECT_TRUE(nh->GetType() == NextHop::COMPOSITE); + comp_nh = static_cast(nh); + EXPECT_TRUE(comp_nh->ComponentNHCount() == 2); + EXPECT_TRUE(comp_nh->PolicyEnabled() == false); + + uint32_t label1 = comp_nh->Get(0)->label(); + uint32_t label2 = comp_nh->Get(1)->label(); + uint32_t vnet1_label = intf1->label(); + + EXPECT_TRUE(label1 == intf1->label()); + EXPECT_TRUE(label2 == intf2->label()); + + WAIT_FOR(100, 1000, ((VmPortPolicyEnabled(input1, 0)) == true)); + + //Disable policy on vnet1 VMI + SetPolicyDisabledStatus(&input1[0], true); + client->WaitForIdle(); + + //Verify that policy status of interfaces + WAIT_FOR(100, 1000, ((VmPortPolicyEnabled(input1, 0)) == false)); + + //Verify that vnet1's label has changed after disabling policy on it. + EXPECT_TRUE(vnet1_label != intf1->label()); + + nh = rt->GetActiveNextHop(); + EXPECT_TRUE(nh->GetType() == NextHop::COMPOSITE); + comp_nh = static_cast(nh); + //Verify that Component NH part of CompositeNH has its label updated after + //policy status change on vnet1 + EXPECT_TRUE(label1 != comp_nh->Get(0)->label()); + EXPECT_TRUE(comp_nh->Get(0)->label() == intf1->label()); + + + DeleteVmportEnv(input1, 2, true, 1); + client->WaitForIdle(); + WAIT_FOR(100, 1000, (VrfFind("vrf1") == false)); + EXPECT_FALSE(RouteFind("vrf1", ip, 32)); + + //Expect MPLS label to be not present + EXPECT_FALSE(FindMplsLabel(MplsLabel::VPORT_NH, mpls_label)); +} + int main(int argc, char **argv) { GETUSERARGS();