Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Enable policy on ECMP Nexthops when any of component interfaces have …
…policy

enabled

Update UT.

Change-Id: I105ffc23c37f4d55bfa0bdaa39e3b034d9bca7d5
Closes-Bug: #1566650
  • Loading branch information
ashoksr committed Aug 5, 2016
1 parent 79de733 commit 40ace4b
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 29 deletions.
66 changes: 46 additions & 20 deletions src/vnsw/agent/oper/inet_unicast_route.cc
Expand Up @@ -449,15 +449,22 @@ AgentPath *InetUnicastRouteEntry::AllocateEcmpPath(Agent *agent,
// Allocate a new label for the ECMP path
uint32_t label = agent->mpls_table()->AllocLabel();

const NextHop* path1_nh = path1->ComputeNextHop(agent);
bool composite_nh_policy = path1_nh->NexthopToInterfacePolicy();

// Create Component NH to be added to ECMP path
DBEntryBase::KeyPtr key1 = path1->ComputeNextHop(agent)->GetDBRequestKey();
DBEntryBase::KeyPtr key1 = path1_nh->GetDBRequestKey();
NextHopKey *nh_key1 = static_cast<NextHopKey *>(key1.release());
std::auto_ptr<const NextHopKey> nh_akey1(nh_key1);
nh_key1->SetPolicy(false);
ComponentNHKeyPtr component_nh_data1(new ComponentNHKey(path1->label(),
nh_akey1));

DBEntryBase::KeyPtr key2 = path2->ComputeNextHop(agent)->GetDBRequestKey();
const NextHop* path2_nh = path2->ComputeNextHop(agent);
if (!composite_nh_policy) {
composite_nh_policy = path2_nh->NexthopToInterfacePolicy();
}
DBEntryBase::KeyPtr key2 = path2_nh->GetDBRequestKey();
NextHopKey *nh_key2 = static_cast<NextHopKey *>(key2.release());
std::auto_ptr<const NextHopKey> nh_akey2(nh_key2);
nh_key2->SetPolicy(false);
Expand All @@ -472,21 +479,22 @@ AgentPath *InetUnicastRouteEntry::AllocateEcmpPath(Agent *agent,
// It will also create CompositeNH if necessary
DBRequest nh_req(DBRequest::DB_ENTRY_ADD_CHANGE);
nh_req.key.reset(new CompositeNHKey(Composite::LOCAL_ECMP,
false, component_nh_list,
composite_nh_policy, component_nh_list,
vrf()->GetName()));
nh_req.data.reset(new CompositeNHData());

InetUnicastRouteEntry::ModifyEcmpPath(addr_, plen_, path2->dest_vn_list(),
label, true, vrf()->GetName(),
path2->sg_list(),
path2->communities(),
path2->path_preference(),
path2->tunnel_bmap(),
path2->ecmp_load_balance(),
nh_req, agent, path);
label, true, vrf()->GetName(),
path2->sg_list(),
path2->communities(),
path2->path_preference(),
path2->tunnel_bmap(),
path2->ecmp_load_balance(),
nh_req, agent, path);

//Make MPLS label point to Composite NH
MplsLabel::CreateEcmpLabel(agent, label, Composite::LOCAL_ECMP, component_nh_list,
MplsLabel::CreateEcmpLabel(agent, label, Composite::LOCAL_ECMP,
composite_nh_policy, component_nh_list,
vrf()->GetName());

RouteInfo rt_info;
Expand Down Expand Up @@ -763,7 +771,8 @@ void InetUnicastRouteEntry::AppendEcmpPath(Agent *agent,
AgentPath *ecmp_path = FindPath(agent->ecmp_peer());
assert(ecmp_path);

DBEntryBase::KeyPtr key = path->ComputeNextHop(agent)->GetDBRequestKey();
const NextHop* path_nh = path->ComputeNextHop(agent);
DBEntryBase::KeyPtr key = path_nh->GetDBRequestKey();
NextHopKey *nh_key = static_cast<NextHopKey *>(key.release());
std::auto_ptr<const NextHopKey> nh_akey(nh_key);
nh_key->SetPolicy(false);
Expand All @@ -773,11 +782,16 @@ void InetUnicastRouteEntry::AppendEcmpPath(Agent *agent,
const CompositeNH *comp_nh =
static_cast<const CompositeNH *>(ecmp_path->ComputeNextHop(agent));
component_nh_key_list = comp_nh->AddComponentNHKey(comp_nh_key_ptr);
bool composite_nh_policy = comp_nh->PolicyEnabled();

if (!composite_nh_policy) {
composite_nh_policy = path_nh->NexthopToInterfacePolicy();
}
// 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,
composite_nh_policy,
component_nh_key_list,
vrf()->GetName()));
nh_req.data.reset(new CompositeNHData());

Expand All @@ -791,7 +805,8 @@ void InetUnicastRouteEntry::AppendEcmpPath(Agent *agent,

//Make MPLS label point to composite NH
MplsLabel::CreateEcmpLabel(agent, ecmp_path->label(), Composite::LOCAL_ECMP,
component_nh_key_list, vrf()->GetName());
composite_nh_policy, component_nh_key_list,
vrf()->GetName());

RouteInfo rt_info;
FillTrace(rt_info, AgentRoute::CHANGE_PATH, path);
Expand All @@ -811,7 +826,8 @@ bool InetUnicastRouteEntry::UpdateComponentNH(Agent *agent,
return false;
}
//Build ComponentNHKey for new path
DBEntryBase::KeyPtr key = path->ComputeNextHop(agent)->GetDBRequestKey();
const NextHop* path_nh = path->ComputeNextHop(agent);
DBEntryBase::KeyPtr key = path_nh->GetDBRequestKey();
NextHopKey *nh_key = static_cast<NextHopKey *>(key.get());
nh_key->SetPolicy(false);

Expand All @@ -825,10 +841,16 @@ bool InetUnicastRouteEntry::UpdateComponentNH(Agent *agent,
return false;
}

bool composite_nh_policy = comp_nh->PolicyEnabled();

if (!composite_nh_policy) {
composite_nh_policy = path_nh->NexthopToInterfacePolicy();
}
// 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,
composite_nh_policy,
component_nh_key_list,
vrf()->GetName()));
nh_req.data.reset(new CompositeNHData());

Expand All @@ -843,7 +865,8 @@ bool InetUnicastRouteEntry::UpdateComponentNH(Agent *agent,

//Make MPLS label point to updated composite NH
MplsLabel::CreateEcmpLabel(agent, ecmp_path->label(), Composite::LOCAL_ECMP,
component_nh_key_list, vrf()->GetName());
composite_nh_policy, component_nh_key_list,
vrf()->GetName());

RouteInfo rt_info;
FillTrace(rt_info, AgentRoute::CHANGE_PATH, path);
Expand All @@ -865,14 +888,16 @@ void InetUnicastRouteEntry::DeleteComponentNH(Agent *agent, AgentPath *path) {
ComponentNHKeyPtr comp_nh_key_ptr(new ComponentNHKey(path->label(), nh_akey));

ComponentNHKeyList component_nh_key_list;
bool comp_nh_policy = false;
const CompositeNH *comp_nh =
static_cast<const CompositeNH *>(ecmp_path->ComputeNextHop(agent));
component_nh_key_list = comp_nh->DeleteComponentNHKey(comp_nh_key_ptr);
component_nh_key_list = comp_nh->DeleteComponentNHKey(comp_nh_key_ptr,
comp_nh_policy);

// 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,
comp_nh_policy, component_nh_key_list,
vrf()->GetName()));
nh_req.data.reset(new CompositeNHData());

Expand All @@ -889,7 +914,8 @@ void InetUnicastRouteEntry::DeleteComponentNH(Agent *agent, AgentPath *path) {

//Make MPLS label point to composite NH
MplsLabel::CreateEcmpLabel(agent, ecmp_path->label(), Composite::LOCAL_ECMP,
component_nh_key_list, vrf()->GetName());
comp_nh_policy, component_nh_key_list,
vrf()->GetName());

RouteInfo rt_info;
FillTrace(rt_info, AgentRoute::CHANGE_PATH, path);
Expand Down
4 changes: 2 additions & 2 deletions src/vnsw/agent/oper/mpls.cc
Expand Up @@ -202,7 +202,7 @@ void MplsLabel::CreateVPortLabel(const Agent *agent,
}

void MplsLabel::CreateEcmpLabel(const Agent *agent,
uint32_t label, COMPOSITETYPE type,
uint32_t label, COMPOSITETYPE type, bool policy,
ComponentNHKeyList &component_nh_key_list,
const std::string vrf_name) {
DBRequest req;
Expand All @@ -211,7 +211,7 @@ void MplsLabel::CreateEcmpLabel(const Agent *agent,
MplsLabelKey *key = new MplsLabelKey(MplsLabel::VPORT_NH, label);
req.key.reset(key);

MplsLabelData *data = new MplsLabelData(type, false, component_nh_key_list,
MplsLabelData *data = new MplsLabelData(type, policy, component_nh_key_list,
vrf_name);
req.data.reset(data);

Expand Down
3 changes: 2 additions & 1 deletion src/vnsw/agent/oper/mpls.h
Expand Up @@ -55,7 +55,8 @@ class MplsLabel : AgentRefCount<MplsLabel>, public AgentDBEntry {
bool policy,
InterfaceNHFlags::Type type,
const MacAddress &mac);
static void CreateEcmpLabel(const Agent *agent, uint32_t label, COMPOSITETYPE type,
static void CreateEcmpLabel(const Agent *agent, uint32_t label,
COMPOSITETYPE type, bool policy,
ComponentNHKeyList &component_nh_key_list,
const std::string vrf_name);
// Delete MPLS Label entry
Expand Down
35 changes: 34 additions & 1 deletion src/vnsw/agent/oper/nexthop.cc
Expand Up @@ -216,6 +216,19 @@ void NextHop::FillObjectLogMac(const unsigned char *m,
info.set_mac(mac);
}

bool NextHop::NexthopToInterfacePolicy() const {
if (GetType() == NextHop::INTERFACE) {
const InterfaceNH *intf_nh =
static_cast<const InterfaceNH *>(this);
const VmInterface *intf = dynamic_cast<const VmInterface *>
(intf_nh->GetInterface());
if (intf && intf->policy_enabled()) {
return true;
}
}
return false;
}

std::auto_ptr<DBEntry> NextHopTable::AllocEntry(const DBRequestKey *k) const {
return std::auto_ptr<DBEntry>(static_cast<DBEntry *>(AllocWithKey(k)));
}
Expand Down Expand Up @@ -1901,7 +1914,8 @@ ComponentNHKeyList CompositeNH::AddComponentNHKey(ComponentNHKeyPtr cnh) const {
}

ComponentNHKeyList
CompositeNH::DeleteComponentNHKey(ComponentNHKeyPtr cnh) const {
CompositeNH::DeleteComponentNHKey(ComponentNHKeyPtr cnh,
bool &comp_nh_new_policy) const {
Agent *agent = static_cast<NextHopTable *>(get_table())->agent();
const NextHop *nh = static_cast<const NextHop *>(agent->nexthop_table()->
FindActiveEntry(cnh->nh_key()));
Expand All @@ -1910,12 +1924,31 @@ CompositeNH::DeleteComponentNHKey(ComponentNHKeyPtr cnh) const {
ComponentNHKeyList component_nh_key_list = component_nh_key_list_;
ComponentNHKeyPtr component_nh_key;
ComponentNHList::const_iterator it = begin();
comp_nh_new_policy = false;
bool removed = false;
int index = 0;
for (;it != end(); it++, index++) {
ComponentNHKeyPtr dummy_ptr;
dummy_ptr.reset();
if ((*it) && ((*it)->label() == cnh->label() && (*it)->nh() == nh)) {
component_nh_key_list[index] = dummy_ptr;
removed = true;
} else {
/* Go through all the component Interface Nexthops of this
* CompositeNH to figure out the new policy status of this
* CompositeNH. Ignore the component NH being deleted while
* iterating. */
if ((*it) && (*it)->nh() && !comp_nh_new_policy) {
/* If any one of component NH's interface has policy enabled,
* the policy-status of compositeNH is true. So we need to
* look only until we find the first Interface which has
* policy enabled */
comp_nh_new_policy = (*it)->nh()->NexthopToInterfacePolicy();
}
}
if (removed && comp_nh_new_policy) {
/* No need to iterate further if we done with both deleting key and
* figuring out policy-status */
break;
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/vnsw/agent/oper/nexthop.h
Expand Up @@ -380,6 +380,7 @@ class NextHop : AgentRefCount<NextHop>, public AgentDBEntry {
NextHopObjectLogInfo &info);
static void FillObjectLogMac(const unsigned char *m,
NextHopObjectLogInfo &info);
bool NexthopToInterfacePolicy() const;
protected:
void FillObjectLog(AgentLogEvent::type event,
NextHopObjectLogInfo &info) const;
Expand Down Expand Up @@ -1358,7 +1359,8 @@ class CompositeNH : public NextHop {
ComponentNHKeyList AddComponentNHKey(ComponentNHKeyPtr
component_nh_key) const;
ComponentNHKeyList DeleteComponentNHKey(ComponentNHKeyPtr
component_nh_key) const;
component_nh_key,
bool &comp_nh_new_policy) const;
bool UpdateComponentNHKey(uint32_t label, NextHopKey *nh_key,
ComponentNHKeyList &component_nh_key_list) const;
ComponentNHList& component_nh_list() {
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/test/test_intf_policy.cc
Expand Up @@ -723,7 +723,7 @@ TEST_F(PolicyTest, EcmpNH_1) {
EXPECT_TRUE(nh->GetType() == NextHop::COMPOSITE);
comp_nh = static_cast<const CompositeNH *>(nh);
EXPECT_TRUE(comp_nh->ComponentNHCount() == 2);
EXPECT_TRUE(comp_nh->PolicyEnabled() == false);
EXPECT_TRUE(comp_nh->PolicyEnabled() == true);

uint32_t label1 = comp_nh->Get(0)->label();
uint32_t label2 = comp_nh->Get(1)->label();
Expand Down
9 changes: 6 additions & 3 deletions src/vnsw/agent/test/test_ecmp_nh.cc
Expand Up @@ -217,14 +217,17 @@ TEST_F(EcmpNhTest, EcmpNH_2) {
const NextHop *nh = rt->GetActiveNextHop();
EXPECT_TRUE(nh->GetType() == NextHop::INTERFACE);
EXPECT_TRUE(nh->PolicyEnabled() == true);
VmInterface *intf = VmInterfaceGet(input1[0].intf_id);
EXPECT_TRUE(intf != NULL);
EXPECT_TRUE(intf->policy_enabled());

//Second VM added, route should point to composite NH
CreateVmportWithEcmp(input2, 1);
client->WaitForIdle();
nh = rt->GetActiveNextHop();
EXPECT_TRUE(nh->GetType() == NextHop::COMPOSITE);
const CompositeNH *comp_nh = static_cast<const CompositeNH *>(nh);
EXPECT_TRUE(comp_nh->PolicyEnabled() == false);
EXPECT_TRUE(comp_nh->PolicyEnabled() == true);
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);
Expand All @@ -245,7 +248,7 @@ TEST_F(EcmpNhTest, EcmpNH_2) {
comp_nh = static_cast<const CompositeNH *>(rt->GetActiveNextHop());
EXPECT_TRUE(comp_nh->GetType() == NextHop::COMPOSITE);
EXPECT_TRUE(comp_nh->ComponentNHCount() == 5);
EXPECT_TRUE(comp_nh->PolicyEnabled() == false);
EXPECT_TRUE(comp_nh->PolicyEnabled() == true);

//Verify all the component NH have right label and nexthop
ComponentNHList::const_iterator component_nh_it =
Expand Down Expand Up @@ -305,7 +308,7 @@ TEST_F(EcmpNhTest, EcmpNH_2) {

//Verify all the component NH have right label and nexthop
comp_nh = static_cast<const CompositeNH *>(rt->GetActiveNextHop());
EXPECT_TRUE(comp_nh->PolicyEnabled() == false);
EXPECT_TRUE(comp_nh->PolicyEnabled() == true);
component_nh_it = comp_nh->begin();
intf_nh = static_cast<const InterfaceNH *>((*component_nh_it)->nh());
EXPECT_TRUE(intf_nh->GetInterface()->name() == "vnet1");
Expand Down

0 comments on commit 40ace4b

Please sign in to comment.