Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Ensure flow-stickiness in case of ECMP with bridged forward flow and …
…routed reverse flow

Removed support for muptile ECMP members on one compute node.
Multiple ECMP member in a compute node is not a requirement and
supporting it adds complexity that is avoidable. So, rolling back
support for multiple ECMP members in single compute node.

Change-Id: Idf69ce4a5ad9e99d6b578f72226a8c8857e65f77
Closes-Bug: #1648696
  • Loading branch information
praveenkv committed Jan 10, 2017
1 parent ec7d52e commit a5827fc
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 67 deletions.
74 changes: 11 additions & 63 deletions src/vnsw/agent/pkt/pkt_flow_info.cc
Expand Up @@ -1715,6 +1715,9 @@ static bool ShouldSwapFlows(const PktFlowInfo *info, const PktInfo *pkt,
// same session. The checks to find layer-2 flow depends on type of flow.
// Note, the layer-2 flow would always be created with interface-nh as key
//
// Caveat : We only support single instance of ECMP member running on a given
// compute node
//
// Notations:
// ----------
// Layer-2 flows are denoted as L2-Fwd-Flow and L2-Rev-Flow
Expand All @@ -1736,73 +1739,18 @@ static bool ShouldSwapFlows(const PktFlowInfo *info, const PktInfo *pkt,
//
// Ingress Flow:
// ------------
// If L3-Fwd-Flow is created with interface-nh
// Key for L3-Fwd-Flow matches key for L2-Rev-Flow
// Stitch L3-Fwd-Flow and L2-Fwd-Flow
// FlowTable::Add ensures layer-2 flows is re-used since keys match
// Layer-2 flow would be creaed with interface-nh as key
// Reverse flow *will* be created with interface-nh as key
//
// If L3-Fwd-Flow is created with Ecmp-nh
// Nexthop in L2-Fwd-Flow would be a member in Ecmp-NH
// Iterate thru all local interface-nh in Ecmp-NH
// Find L2-Fwd-Flow with interface-nh as 5-tuple in L3-Rev-Flow as key
// If flow is found
// Stitch L3-Fwd-Flow and L2-Fwd-Flow
// FlowTable::Add ensures layer-2 flows is re-used since keys match
//
// StitchL2Flow is dummy since we dont support multiple instances on a single
// compute node.
// Commit 08dfca551faf420f2c15738ebfb4f26a6c875a51 has code to support
// multiple instances in single compute node
static bool StitchL2Flow(const Agent *agent, const PktFlowInfo *info,
const PktInfo *pkt, FlowEntryPtr &flow,
FlowEntryPtr &rflow) {
if (info->short_flow) {
return false;
}

// If this is message processing, then retain forward and reverse flows
if (pkt->type == PktType::MESSAGE) {
return false;
}

FlowTable *flow_table = info->get_flow_table();
NextHopTable *nh_table = agent->nexthop_table();
const NextHop *nh = nh_table->FindNextHop(rflow->key().nh);
// If reverse flow has interface-nh as key, find L2-Fwd-Flow with
// interface-nh and 5-tuple as key
if (dynamic_cast<const InterfaceNH *>(nh)) {
return false;
}

// When reverse flow uses interface-nh, the keys for layer-2 and layer-3
// flow will match and FlowTable::AddInternal will stitch them.
//
// If NH is composite-NH, look for flow interface-nh in composite and find
// layer-2 flows using the interface-nh.
if (const CompositeNH *comp_nh = dynamic_cast<const CompositeNH *>(nh)) {
FlowKey key = rflow->key();
// Iterate thru all local members
ComponentNHList::const_iterator it = comp_nh->begin();
while (it != comp_nh->end()) {
const InterfaceNH *intf_nh =
dynamic_cast<const InterfaceNH *>(it->get()->nh());
it++;
if (intf_nh == NULL)
continue;

const VmInterface *vmi =
dynamic_cast<const VmInterface *>(intf_nh->GetInterface());
if (vmi == NULL)
continue;

intf_nh = dynamic_cast<const InterfaceNH *>(vmi->flow_key_nh());
if (intf_nh == NULL)
continue;

key.nh = intf_nh->id();
FlowEntry *l2_fwd_flow = flow_table->Find(key);
if (l2_fwd_flow == NULL) {
continue;
}
rflow->SetKey(l2_fwd_flow->key());
return true;
}
}

return false;
}

Expand Down
14 changes: 10 additions & 4 deletions src/vnsw/agent/pkt/test/test_ecmp_bridge_route.cc
Expand Up @@ -495,8 +495,10 @@ TEST_F(EcmpTest, Egress_Non_Ecmp_To_Non_Ecmp_1) {
// Egress Flow
// Source : Non-ECMP
// Destination : ECMP
//
// DISABLED since we dont support multiple instances on a compute node
/////////////////////////////////////////////////////////////////////////////
TEST_F(EcmpTest, Egress_Non_Ecmp_To_Ecmp_1) {
TEST_F(EcmpTest, DISABLED_Egress_Non_Ecmp_To_Ecmp_1) {
// Inner iteration for all 4 members of destination
for (int i = 0; i < 4; i++) {
char sip[64];
Expand Down Expand Up @@ -561,8 +563,10 @@ TEST_F(EcmpTest, Egress_Ecmp_To_Non_Ecmp_1) {
// Egress Flow
// Source : ECMP
// Destination : ECMP
//
// DISABLED since we dont support multiple instances on a compute node
/////////////////////////////////////////////////////////////////////////////
TEST_F(EcmpTest, Egress_Ecmp_To_Ecmp_1) {
TEST_F(EcmpTest, DISABLED_Egress_Ecmp_To_Ecmp_1) {
// Outer iteration for all 4 members of source
for (int i = 0; i < 4; i++) {
// Inner iteration for all 4 members of destination
Expand Down Expand Up @@ -661,8 +665,9 @@ TEST_F(EcmpTest, Ingress_Non_Ecmp_To_Ecmp_1) {
// Ingress Flow
// Source : ECMP
// Destination : Non-ECMP
// DISABLED since we dont support multiple instances on a compute node
/////////////////////////////////////////////////////////////////////////////
TEST_F(EcmpTest, Ingress_Ecmp_To_Non_Ecmp_1) {
TEST_F(EcmpTest, DISABLED_Ingress_Ecmp_To_Non_Ecmp_1) {
for (uint32_t i = 0; i < 4; i++) {
char sip[64];
VmInterface *vmi = static_cast<VmInterface *>(VmPortGet(i + 1));
Expand Down Expand Up @@ -693,8 +698,9 @@ TEST_F(EcmpTest, Ingress_Ecmp_To_Non_Ecmp_1) {
// Ingress Flow
// Source : ECMP
// Destination : ECMP
// DISABLED since we dont support multiple instances on a compute node
/////////////////////////////////////////////////////////////////////////////
TEST_F(EcmpTest, Ingress_Ecmp_To_Ecmp_1) {
TEST_F(EcmpTest, DISABLED_Ingress_Ecmp_To_Ecmp_1) {
for (uint32_t i = 0; i < 4; i++) {
for (uint32_t j = 0; j < 4; j++) {
char sip[64];
Expand Down

0 comments on commit a5827fc

Please sign in to comment.