Skip to content

Commit

Permalink
Merge "When policy changes, the label of VMI is updated. The updated …
Browse files Browse the repository at this point in the history 
…VMI label was not reflected in AAP routes. Added UT to verify this."
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Aug 5, 2016
2 parents 3fa2806 + 0ba5a3c commit 422ae27
Show file tree
Hide file tree
Showing 9 changed files with 188 additions and 124 deletions.
16 changes: 0 additions & 16 deletions src/vnsw/agent/oper/inet_unicast_route.cc
View file
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
Expand Up @@ -193,10 +193,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
67 changes: 1 addition & 66 deletions src/vnsw/agent/oper/test/test_aap.cc
View file
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 @@ -62,71 +62,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) {
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-address>" << "00:00:00:00:00:00"
<< "</mac-address></mac>";
buf << "<address-mode>" << "active-active" << "</address-mode>";
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 AddVlan(std::string intf_name, int intf_id, uint32_t vlan) {
std::ostringstream buf;
Expand Down
58 changes: 58 additions & 0 deletions src/vnsw/agent/oper/test/test_intf_policy.cc
View file
Expand Up @@ -996,6 +996,64 @@ TEST_F(PolicyTest, EcmpNH_3) {
EXPECT_FALSE(RouteFind("vrf1", ip, 32));
}

//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
49 changes: 19 additions & 30 deletions src/vnsw/agent/oper/vm_interface.cc
View file
Expand Up @@ -1531,8 +1531,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 @@ -1549,7 +1549,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 @@ -3148,16 +3148,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 @@ -3171,12 +3171,6 @@ void VmInterface::UpdateIpv4InterfaceRoute(bool old_ipv4_active, bool force_upda
AddRoute(vrf_->GetName(), primary_ip_addr_, 32, vn_->GetName(),
policy_enabled_, ecmp_, false, 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);
}
}

Expand Down Expand Up @@ -3364,7 +3358,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 @@ -3379,7 +3373,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 @@ -3422,7 +3416,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 @@ -4498,16 +4492,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_ == false || force_update || policy_change) {
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 @@ -4744,12 +4737,11 @@ 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 &&
service_ip_ == ip && ecmp_config_changed_ == false) {
if (installed_ && force_update == false && service_ip_ == ip &&
ecmp_config_changed_ == false) {
return;
}

Expand All @@ -4758,11 +4750,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 ||
ecmp_config_changed_) {
if (installed_ == false || force_update || service_ip_ != ip ||
ecmp_config_changed_) {
service_ip_ = ip;
IpAddress dependent_rt;
if (ecmp_ == true) {
Expand Down
12 changes: 5 additions & 7 deletions src/vnsw/agent/oper/vm_interface.h
View file
Expand Up @@ -243,8 +243,7 @@ class VmInterface : public Interface {

bool operator() (const StaticRoute &lhs, const StaticRoute &rhs) const;
bool IsLess(const StaticRoute *rhs) const;
void Activate(VmInterface *interface, bool force_update,
bool policy_change) const;
void Activate(VmInterface *interface, bool force_update) const;
void DeActivate(VmInterface *interface) const;

mutable std::string vrf_;
Expand Down Expand Up @@ -275,8 +274,7 @@ class VmInterface : public Interface {
bool operator() (const AllowedAddressPair &lhs,
const AllowedAddressPair &rhs) const;
bool IsLess(const AllowedAddressPair *rhs) const;
void Activate(VmInterface *interface, bool force_update,
bool policy_change) const;
void Activate(VmInterface *interface, bool force_update) const;
void DeActivate(VmInterface *interface) const;
void L2Activate(VmInterface *interface, bool force_update,
bool policy_change, bool old_layer2_forwarding,
Expand Down Expand Up @@ -805,8 +803,8 @@ class VmInterface : public Interface {
bool Ipv4Deactivated(bool old_ipv4_active);
bool Ipv6Deactivated(bool old_ipv6_active);
void UpdateIpv4InterfaceRoute(bool old_ipv4_active, bool force_update,
bool policy_change, VrfEntry * old_vrf,
const Ip4Address &old_addr);
VrfEntry * old_vrf,
const Ip4Address &old_addr);
void DeleteIpv4InterfaceRoute(VrfEntry *old_vrf,
const Ip4Address &old_addr);
void UpdateResolveRoute(bool old_ipv4_active, bool force_update,
Expand All @@ -830,7 +828,7 @@ class VmInterface : public Interface {
void UpdateServiceVlan(bool force_update, bool policy_change,
bool old_ipv4_active, bool old_ipv6_active);
void DeleteServiceVlan();
void UpdateStaticRoute(bool force_update, bool policy_change);
void UpdateStaticRoute(bool force_update);
void DeleteStaticRoute();
void UpdateAllowedAddressPair(bool force_update, bool policy_change,
bool l2, bool old_l2_forwarding,
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/port_ipc/config_stale_cleaner.cc
View file
Expand Up @@ -88,7 +88,7 @@ bool InterfaceConfigStaleCleaner::CfgIntfWalk(DBTablePartBase *partition,
if (!pih) {
return true;
}
string msg;
std::string msg;
pih->DeletePort(UuidToString(cfg_intf->GetUuid()), msg);
}
return true;
Expand Down
8 changes: 8 additions & 0 deletions src/vnsw/agent/test/test_cmn_util.h
View file
Expand Up @@ -439,6 +439,14 @@ void AddInterfaceRouteTable(const char *name, int id, TestIp4Prefix *addr,
void AddInterfaceRouteTableV6(const char *name, int id, TestIp6Prefix *addr,
int count);
void ShutdownAgentController(Agent *agent);
void AddAap(std::string intf_name, int intf_id,
std::vector<Ip4Address> aap_list);
void AddEcmpAap(std::string intf_name, int intf_id, Ip4Address ip);
void AddAap(std::string intf_name, int intf_id, Ip4Address ip,
const std::string &mac);
void AddAapWithDisablePolicy(std::string intf_name, int intf_id,
std::vector<Ip4Address> aap_list,
bool disable_policy);

class XmppChannelMock : public XmppChannel {
public:
Expand Down

0 comments on commit 422ae27

Please sign in to comment.