From a5827fc9515f64f24cd21743a0c50574719e2d45 Mon Sep 17 00:00:00 2001 From: Praveen K V Date: Tue, 10 Jan 2017 15:54:07 +0530 Subject: [PATCH] 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 --- src/vnsw/agent/pkt/pkt_flow_info.cc | 74 +++---------------- .../agent/pkt/test/test_ecmp_bridge_route.cc | 14 +++- 2 files changed, 21 insertions(+), 67 deletions(-) diff --git a/src/vnsw/agent/pkt/pkt_flow_info.cc b/src/vnsw/agent/pkt/pkt_flow_info.cc index de5cb598128..7992dd0e43b 100644 --- a/src/vnsw/agent/pkt/pkt_flow_info.cc +++ b/src/vnsw/agent/pkt/pkt_flow_info.cc @@ -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 @@ -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(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(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(it->get()->nh()); - it++; - if (intf_nh == NULL) - continue; - - const VmInterface *vmi = - dynamic_cast(intf_nh->GetInterface()); - if (vmi == NULL) - continue; - - intf_nh = dynamic_cast(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; } diff --git a/src/vnsw/agent/pkt/test/test_ecmp_bridge_route.cc b/src/vnsw/agent/pkt/test/test_ecmp_bridge_route.cc index d1a5b9a9933..9d386652cf5 100644 --- a/src/vnsw/agent/pkt/test/test_ecmp_bridge_route.cc +++ b/src/vnsw/agent/pkt/test/test_ecmp_bridge_route.cc @@ -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]; @@ -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 @@ -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(VmPortGet(i + 1)); @@ -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];