Skip to content

Commit

Permalink
Merge "Update Path of static-route with correct label on VMI’s label …
Browse files Browse the repository at this point in the history 
…change." into R3.0.3.x
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Oct 24, 2016
2 parents fb3aae0 + 29c1622 commit 5bcd008
Show file tree
Hide file tree
Showing 8 changed files with 250 additions and 108 deletions.
16 changes: 0 additions & 16 deletions src/vnsw/agent/oper/inet_unicast_route.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,22 +161,6 @@ static void InetUnicastTableProcess(Agent *agent, const string &vrf_name,
}
}

void InetUnicastAgentRouteTable::ReEvaluatePaths(const Agent* agent,
const string &vrf_name,
const IpAddress &addr,
uint8_t plen) {
DBRequest rt_req(DBRequest::DB_ENTRY_ADD_CHANGE);
InetUnicastRouteKey *rt_key = new InetUnicastRouteKey(agent->local_peer(),
vrf_name,
addr,
plen);

rt_key->sub_op_ = AgentKey::RESYNC;
rt_req.key.reset(rt_key);
rt_req.data.reset(NULL);
InetUnicastTableEnqueue(Agent::GetInstance(), vrf_name, &rt_req);
}

/*
* Traverse all smaller subnets w.r.t. route sent and mark the arp flood flag
* accordingly.
Expand Down
4 changes: 0 additions & 4 deletions src/vnsw/agent/oper/inet_unicast_route.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,6 @@ class InetUnicastAgentRouteTable : public AgentRouteTable {
}

static DBTableBase *CreateTable(DB *db, const std::string &name);
static void ReEvaluatePaths(const Agent *agent,
const string &vrf_name,
const IpAddress &ip,
uint8_t plen);
static void DeleteReq(const Peer *peer, const string &vrf_name,
const IpAddress &addr, uint8_t plen,
AgentRouteData *data);
Expand Down
48 changes: 1 addition & 47 deletions src/vnsw/agent/oper/test/test_aap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <cmn/agent_cmn.h>

#include "test_cmn_util.h"
#include "cfg/cfg_init.h"
#include "cfg/cfg_interface.h"
#include "oper/operdb_init.h"
Expand All @@ -29,7 +30,6 @@
#include "oper/vn.h"
#include "oper/path_preference.h"
#include "filter/acl.h"
#include "test_cmn_util.h"
#include "vr_types.h"
#include <controller/controller_export.h>
#include <ksync/ksync_sock_user.h>
Expand Down Expand Up @@ -64,51 +64,6 @@ class TestAap : public ::testing::Test {
~TestAap() {
DeleteBgpPeer(peer_);
}
void AddAap(std::string intf_name, int intf_id,
std::vector<Ip4Address> aap_list) {
std::ostringstream buf;
buf << "<virtual-machine-interface-allowed-address-pairs>";
std::vector<Ip4Address>::iterator it = aap_list.begin();
while (it != aap_list.end()) {
buf << "<allowed-address-pair>";
buf << "<ip>";
buf << "<ip-prefix>" << it->to_string()<<"</ip-prefix>";
buf << "<ip-prefix-len>"<< 32 << "</ip-prefix-len>";
buf << "</ip>";
buf << "<mac><mac-address>" << "00:00:00:00:00:00"
<< "</mac-address></mac>";
buf << "<flag>" << "act-stby" << "</flag>";
buf << "</allowed-address-pair>";
it++;
}
buf << "</virtual-machine-interface-allowed-address-pairs>";
char cbuf[10000];
strcpy(cbuf, buf.str().c_str());
AddNode("virtual-machine-interface", intf_name.c_str(),
intf_id, cbuf);
client->WaitForIdle();
}

void AddAap(std::string intf_name, int intf_id, Ip4Address ip,
const std::string &mac) {
std::ostringstream buf;
buf << "<virtual-machine-interface-allowed-address-pairs>";
buf << "<allowed-address-pair>";
buf << "<ip>";
buf << "<ip-prefix>" << ip.to_string() <<"</ip-prefix>";
buf << "<ip-prefix-len>"<< 32 << "</ip-prefix-len>";
buf << "</ip>";
buf << "<mac>" << mac << "</mac>";
buf << "<flag>" << "act-stby" << "</flag>";
buf << "</allowed-address-pair>";
buf << "</virtual-machine-interface-allowed-address-pairs>";
char cbuf[10000];
strcpy(cbuf, buf.str().c_str());
AddNode("virtual-machine-interface", intf_name.c_str(),
intf_id, cbuf);
client->WaitForIdle();
}

void AddEcmpAap(std::string intf_name, int intf_id, Ip4Address ip,
const std::string &mac) {
std::ostringstream buf;
Expand All @@ -130,7 +85,6 @@ class TestAap : public ::testing::Test {
client->WaitForIdle();
}


void AddVlan(std::string intf_name, int intf_id, uint32_t vlan) {
std::ostringstream buf;
buf << "<virtual-machine-interface-properties>";
Expand Down
120 changes: 120 additions & 0 deletions src/vnsw/agent/oper/test/test_intf_policy.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,126 @@ TEST_F(PolicyTest, IntfPolicyDisable_Flow) {
EXPECT_EQ(0U, flow_proto_->FlowCount());
}

//When policy is enabled/disabled on VMI, label of VMI changes. When a VMI
//has static_route associated with it and its policy-status changes verify that
//the label of the static-route's path is updated accordingly.
TEST_F(PolicyTest, IntfStaticRoute) {
struct PortInfo input[] = {
{"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1}
};

client->Reset();
CreateVmportEnv(input, 1, 1);
client->WaitForIdle();
EXPECT_TRUE(VmPortActive(input, 0));
const VmInterface *intf1 = VmInterfaceGet(input[0].intf_id);
EXPECT_TRUE(intf1 != NULL);

//Add a static route
struct TestIp4Prefix static_route[] = {
{ Ip4Address::from_string("2.1.1.10"), 32}
};

AddInterfaceRouteTable("static_route", 1, static_route, 1);
AddLink("virtual-machine-interface", "vnet1",
"interface-route-table", "static_route");
client->WaitForIdle();

InetUnicastRouteEntry *rt = RouteGet("vrf1", static_route[0].addr_,
static_route[0].plen_);
EXPECT_TRUE(rt != NULL);
const AgentPath *path = rt->GetActivePath();
EXPECT_TRUE(path != NULL);
EXPECT_TRUE(path->label() == intf1->label());
uint32_t vmi_label = intf1->label();

WAIT_FOR(100, 1000, ((VmPortPolicyEnabled(input, 0)) == true));

//Disable policy on vnet1 VMI
SetPolicyDisabledStatus(&input[0], true);
client->WaitForIdle();

//Verify that policy status of interface
WAIT_FOR(100, 1000, ((VmPortPolicyEnabled(input, 0)) == false));

//Verify that vnet1's label has changed after disabling policy on it.
EXPECT_TRUE(vmi_label != intf1->label());

//Verify that label of static_route's path is updated with new label of VMI
EXPECT_TRUE(path->label() == intf1->label());

//Delete the link between interface and route table
DelLink("virtual-machine-interface", "vnet1",
"interface-route-table", "static_route");
client->WaitForIdle();
EXPECT_FALSE(RouteFind("vrf1", static_route[0].addr_,
static_route[0].plen_));

DelNode("interface-route-table", "static_route");
client->WaitForIdle();
DeleteVmportEnv(input, 1, true, 1);
client->WaitForIdle();
WAIT_FOR(100, 1000, (VrfFind("vrf1") == false));
}

//Add and delete allowed address pair route
TEST_F(PolicyTest, AapRoute) {
struct PortInfo input[] = {
{"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1}
};

client->Reset();
CreateVmportEnv(input, 1, 1);
client->WaitForIdle();
EXPECT_TRUE(VmPortActive(input, 0));
const VmInterface *intf1 = VmInterfaceGet(input[0].intf_id);
EXPECT_TRUE(intf1 != NULL);

//Add an AAP route
Ip4Address ip = Ip4Address::from_string("10.10.10.10");
std::vector<Ip4Address> v;
v.push_back(ip);

AddAap("vnet1", 1, v);
EXPECT_TRUE(RouteFind("vrf1", ip, 32));

InetUnicastRouteEntry *rt = RouteGet("vrf1", ip, 32);
EXPECT_TRUE(rt != NULL);
const AgentPath *path = rt->GetActivePath();
EXPECT_TRUE(path != NULL);
EXPECT_TRUE(path->label() == intf1->label());
uint32_t vmi_label = intf1->label();

WAIT_FOR(100, 1000, ((VmPortPolicyEnabled(input, 0)) == true));

//Disable policy on vnet1 VMI
AddAapWithDisablePolicy("vnet1", 1, v, true);
client->WaitForIdle();

//Verify that policy status of interface
WAIT_FOR(100, 1000, ((VmPortPolicyEnabled(input, 0)) == false));

//Verify that vnet1's label has changed after disabling policy on it.
EXPECT_TRUE(vmi_label != intf1->label());

//Verify that label of aap_route's path is updated with new label of VMI
rt = RouteGet("vrf1", ip, 32);
EXPECT_TRUE(rt != NULL);
path = rt->GetActivePath();
EXPECT_TRUE(path != NULL);
EXPECT_TRUE(path->label() == intf1->label());

//Remove AAP route
v.clear();
AddAap("vnet1", 1, v);
EXPECT_FALSE(RouteFind("vrf1", ip, 32));

//cleanup
DeleteVmportEnv(input, 1, true, 1);
client->WaitForIdle();
WAIT_FOR(100, 1000, (VrfFind("vrf1") == false));
}

int main(int argc, char **argv) {
GETUSERARGS();

Expand Down
54 changes: 20 additions & 34 deletions src/vnsw/agent/oper/vm_interface.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1376,8 +1376,8 @@ void VmInterface::UpdateL3(bool old_ipv4_active, VrfEntry *old_vrf,
const Ip4Address &old_dhcp_addr) {
if (ipv4_active_) {
if (do_dhcp_relay_) {
UpdateIpv4InterfaceRoute(old_ipv4_active, force_update,
policy_change,
UpdateIpv4InterfaceRoute(old_ipv4_active,
force_update||policy_change,
old_vrf, old_dhcp_addr);
}
UpdateIpv4InstanceIp(force_update, policy_change, false,
Expand All @@ -1393,7 +1393,7 @@ void VmInterface::UpdateL3(bool old_ipv4_active, VrfEntry *old_vrf,
old_ipv6_active);
UpdateAllowedAddressPair(force_update, policy_change, false, false, false);
UpdateVrfAssignRule();
UpdateStaticRoute(force_update, policy_change);
UpdateStaticRoute(force_update||policy_change);
}

void VmInterface::DeleteL3(bool old_ipv4_active, VrfEntry *old_vrf,
Expand Down Expand Up @@ -2969,16 +2969,16 @@ IpAddress VmInterface::GetServiceIp(const IpAddress &vm_ip) const {
}

// Add/Update route. Delete old route if VRF or address changed
void VmInterface::UpdateIpv4InterfaceRoute(bool old_ipv4_active, bool force_update,
bool policy_change,
VrfEntry * old_vrf,
const Ip4Address &old_addr) {
void VmInterface::UpdateIpv4InterfaceRoute(bool old_ipv4_active,
bool force_update,
VrfEntry * old_vrf,
const Ip4Address &old_addr) {
Ip4Address ip = GetServiceIp(primary_ip_addr_).to_v4();

// If interface was already active earlier and there is no force_update or
// policy_change, return
if (old_ipv4_active == true && force_update == false
&& policy_change == false && old_addr == primary_ip_addr_ &&
&& old_addr == primary_ip_addr_ &&
vm_ip_service_addr_ == ip) {
return;
}
Expand All @@ -2990,14 +2990,8 @@ void VmInterface::UpdateIpv4InterfaceRoute(bool old_ipv4_active, bool force_upda
old_addr != primary_ip_addr_ || vm_ip_service_addr_ != ip) {
vm_ip_service_addr_ = ip;
AddRoute(vrf_->GetName(), primary_ip_addr_, 32, vn_->GetName(),
false, ecmp_, vm_ip_service_addr_, Ip4Address(0),
CommunityList(), label_);
} else if (policy_change) {
// If old-l3-active and there is change in policy, invoke RESYNC of
// route to account for change in NH policy
InetUnicastAgentRouteTable::ReEvaluatePaths(agent(),
vrf_->GetName(),
primary_ip_addr_, 32);
false, ecmp_, vm_ip_service_addr_,
Ip4Address(0), CommunityList(), label_);
}
}

Expand Down Expand Up @@ -3147,7 +3141,7 @@ void VmInterface::DeleteServiceVlan() {
}
}

void VmInterface::UpdateStaticRoute(bool force_update, bool policy_change) {
void VmInterface::UpdateStaticRoute(bool force_update) {
StaticRouteSet::iterator it = static_route_list_.list_.begin();
while (it != static_route_list_.list_.end()) {
StaticRouteSet::iterator prev = it++;
Expand All @@ -3162,7 +3156,7 @@ void VmInterface::UpdateStaticRoute(bool force_update, bool policy_change) {
prev->DeActivate(this);
static_route_list_.list_.erase(prev);
} else {
prev->Activate(this, force_update, policy_change);
prev->Activate(this, force_update);
}
}
}
Expand Down Expand Up @@ -3205,7 +3199,7 @@ void VmInterface::UpdateAllowedAddressPair(bool force_update, bool policy_change
it->L2Activate(this, force_update, policy_change,
old_layer2_forwarding, old_layer3_forwarding);
} else {
it->Activate(this, force_update, policy_change);
it->Activate(this, (force_update || policy_change));
}
}
}
Expand Down Expand Up @@ -4107,19 +4101,15 @@ bool VmInterface::StaticRoute::IsLess(const StaticRoute *rhs) const {
}

void VmInterface::StaticRoute::Activate(VmInterface *interface,
bool force_update,
bool policy_change) const {
if (installed_ && force_update == false && policy_change == false)
bool force_update) const {
if (installed_ && force_update == false)
return;

if (vrf_ != interface->vrf()->GetName()) {
vrf_ = interface->vrf()->GetName();
}

if (installed_ == true && policy_change) {
InetUnicastAgentRouteTable::ReEvaluatePaths(interface->agent(),
vrf_, addr_, plen_);
} else if (installed_ == false || force_update) {
if (installed_ == false || force_update) {
Ip4Address gw_ip(0);
if (gw_.is_v4() && addr_.is_v4() && gw_.to_v4() != gw_ip) {
SecurityGroupList sg_id_list;
Expand Down Expand Up @@ -4348,11 +4338,10 @@ void VmInterface::AllowedAddressPair::CreateLabelAndNH(Agent *agent,
}

void VmInterface::AllowedAddressPair::Activate(VmInterface *interface,
bool force_update,
bool policy_change) const {
bool force_update) const {
IpAddress ip = interface->GetServiceIp(addr_);

if (installed_ && force_update == false && policy_change == false &&
if (installed_ && force_update == false &&
service_ip_ == ip && l3_ecmp_config_changed_ == false) {
return;
}
Expand All @@ -4362,11 +4351,8 @@ void VmInterface::AllowedAddressPair::Activate(VmInterface *interface,
vrf_ = interface->vrf()->GetName();
}

if (installed_ == true && policy_change) {
InetUnicastAgentRouteTable::ReEvaluatePaths(agent,
vrf_, addr_, plen_);
} else if (installed_ == false || force_update || service_ip_ != ip ||
l3_ecmp_config_changed_) {
if (installed_ == false || force_update || service_ip_ != ip ||
l3_ecmp_config_changed_) {
service_ip_ = ip;
if (mac_ == MacAddress::kZeroMac ||
mac_ == interface->vm_mac_) {
Expand Down

0 comments on commit 5bcd008

Please sign in to comment.