Skip to content

Commit

Permalink
Merge "Revert "Enable policy on ECMP Nexthops when any of component i…
Browse files Browse the repository at this point in the history 
…nterfaces have policy enabled"" into R3.1
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Aug 5, 2016
2 parents 4b590b0 + 165de0e commit d62ca32
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 94 deletions.
66 changes: 20 additions & 46 deletions src/vnsw/agent/oper/inet_unicast_route.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,22 +449,15 @@ 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_nh->GetDBRequestKey();
DBEntryBase::KeyPtr key1 = path1->ComputeNextHop(agent)->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));

const NextHop* path2_nh = path2->ComputeNextHop(agent);
if (!composite_nh_policy) {
composite_nh_policy = path2_nh->NexthopToInterfacePolicy();
}
DBEntryBase::KeyPtr key2 = path2_nh->GetDBRequestKey();
DBEntryBase::KeyPtr key2 = path2->ComputeNextHop(agent)->GetDBRequestKey();
NextHopKey *nh_key2 = static_cast<NextHopKey *>(key2.release());
std::auto_ptr<const NextHopKey> nh_akey2(nh_key2);
nh_key2->SetPolicy(false);
Expand All @@ -479,22 +472,21 @@ 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,
composite_nh_policy, component_nh_list,
false, 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,
composite_nh_policy, component_nh_list,
MplsLabel::CreateEcmpLabel(agent, label, Composite::LOCAL_ECMP, component_nh_list,
vrf()->GetName());

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

const NextHop* path_nh = path->ComputeNextHop(agent);
DBEntryBase::KeyPtr key = path_nh->GetDBRequestKey();
DBEntryBase::KeyPtr key = path->ComputeNextHop(agent)->GetDBRequestKey();
NextHopKey *nh_key = static_cast<NextHopKey *>(key.release());
std::auto_ptr<const NextHopKey> nh_akey(nh_key);
nh_key->SetPolicy(false);
Expand All @@ -782,16 +773,11 @@ 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,
composite_nh_policy,
component_nh_key_list,
false, component_nh_key_list,
vrf()->GetName()));
nh_req.data.reset(new CompositeNHData());

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

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

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

Expand All @@ -841,16 +825,10 @@ 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,
composite_nh_policy,
component_nh_key_list,
false, component_nh_key_list,
vrf()->GetName()));
nh_req.data.reset(new CompositeNHData());

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

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

RouteInfo rt_info;
FillTrace(rt_info, AgentRoute::CHANGE_PATH, path);
Expand All @@ -888,16 +865,14 @@ 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,
comp_nh_policy);
component_nh_key_list = comp_nh->DeleteComponentNHKey(comp_nh_key_ptr);

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

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

//Make MPLS label point to composite NH
MplsLabel::CreateEcmpLabel(agent, ecmp_path->label(), Composite::LOCAL_ECMP,
comp_nh_policy, component_nh_key_list,
vrf()->GetName());
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ void MplsLabel::CreateVPortLabel(const Agent *agent,
}

void MplsLabel::CreateEcmpLabel(const Agent *agent,
uint32_t label, COMPOSITETYPE type, bool policy,
uint32_t label, COMPOSITETYPE type,
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, policy, component_nh_key_list,
MplsLabelData *data = new MplsLabelData(type, false, component_nh_key_list,
vrf_name);
req.data.reset(data);

Expand Down
3 changes: 1 addition & 2 deletions src/vnsw/agent/oper/mpls.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ 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, bool policy,
static void CreateEcmpLabel(const Agent *agent, uint32_t label, COMPOSITETYPE type,
ComponentNHKeyList &component_nh_key_list,
const std::string vrf_name);
// Delete MPLS Label entry
Expand Down
35 changes: 1 addition & 34 deletions src/vnsw/agent/oper/nexthop.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,19 +216,6 @@ 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 @@ -1933,8 +1920,7 @@ ComponentNHKeyList CompositeNH::AddComponentNHKey(ComponentNHKeyPtr cnh) const {
}

ComponentNHKeyList
CompositeNH::DeleteComponentNHKey(ComponentNHKeyPtr cnh,
bool &comp_nh_new_policy) const {
CompositeNH::DeleteComponentNHKey(ComponentNHKeyPtr cnh) 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 @@ -1943,31 +1929,12 @@ CompositeNH::DeleteComponentNHKey(ComponentNHKeyPtr cnh,
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: 1 addition & 3 deletions src/vnsw/agent/oper/nexthop.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,6 @@ 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 @@ -1365,8 +1364,7 @@ class CompositeNH : public NextHop {
ComponentNHKeyList AddComponentNHKey(ComponentNHKeyPtr
component_nh_key) const;
ComponentNHKeyList DeleteComponentNHKey(ComponentNHKeyPtr
component_nh_key,
bool &comp_nh_new_policy) const;
component_nh_key) 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
View file
Original file line number Diff line number Diff line change
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() == true);
EXPECT_TRUE(comp_nh->PolicyEnabled() == false);

uint32_t label1 = comp_nh->Get(0)->label();
uint32_t label2 = comp_nh->Get(1)->label();
Expand Down
9 changes: 3 additions & 6 deletions src/vnsw/agent/test/test_ecmp_nh.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,17 +217,14 @@ 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() == true);
EXPECT_TRUE(comp_nh->PolicyEnabled() == false);
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 @@ -248,7 +245,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() == true);
EXPECT_TRUE(comp_nh->PolicyEnabled() == false);

//Verify all the component NH have right label and nexthop
ComponentNHList::const_iterator component_nh_it =
Expand Down Expand Up @@ -308,7 +305,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() == true);
EXPECT_TRUE(comp_nh->PolicyEnabled() == false);
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 d62ca32

Please sign in to comment.