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

Commit 1433319 supported stickiness
across bridge and routed flows. However, it doenst support multiple ECMP
members on same compute node.

This current supports multiple members of ECMP in same compute

When layer-3 flow is created with local-ecmp-nh as key, look for layer-2 flow
with one of the local-ecmp members as key. If layer-2 flow with interface-nh
is present, stitch layer-3 and layer-2 flows

Conflicts:
	src/vnsw/agent/pkt/flow_entry.cc
	src/vnsw/agent/pkt/flow_entry.h
	src/vnsw/agent/pkt/test/egress-flow.xml
	src/vnsw/agent/pkt/test/test_pkt_util.cc
	src/vnsw/agent/pkt/test/test_pkt_util.h

Change-Id: I3fd0185fb9856fbcbdc2b038a75fa65d54183802
closes-Bug: #1648696
  • Loading branch information
praveenkv authored and haripk committed Jan 6, 2017
1 parent 7e9c21b commit 08dfca5
Show file tree
Hide file tree
Showing 16 changed files with 1,088 additions and 168 deletions.
24 changes: 18 additions & 6 deletions src/vnsw/agent/pkt/flow_entry.cc
Expand Up @@ -83,6 +83,7 @@ const std::map<uint16_t, const char*>
((uint16_t)SHORT_FLOW_ON_TSN, "Short flow TSN flow")
((uint16_t)SHORT_NO_MIRROR_ENTRY, "Short flow No mirror entry ")
((uint16_t)SHORT_SAME_FLOW_RFLOW_KEY,"Short flow same flow and rflow")
((uint16_t)SHORT_INACTIVE_NH, "Shot flow - Inactive Nexthop")
((uint16_t)DROP_POLICY, "Flow drop Policy")
((uint16_t)DROP_OUT_POLICY, "Flow drop Out Policy")
((uint16_t)DROP_SG, "Flow drop SG")
Expand Down Expand Up @@ -415,6 +416,12 @@ void FlowEntry::Reset(const FlowKey &k) {
key_ = k;
}

// Change key for a flow-table. Caller must ensure that flow-entry is not
// in tree
void FlowEntry::SetKey(const FlowKey &k) {
key_ = k;
}

void FlowEntry::Init() {
alloc_count_ = 0;
}
Expand Down Expand Up @@ -500,7 +507,7 @@ void intrusive_ptr_release(FlowEntry *fe) {

bool FlowEntry::InitFlowCmn(const PktFlowInfo *info, const PktControlInfo *ctrl,
const PktControlInfo *rev_ctrl,
FlowEntry *rflow) {
FlowEntry *rflow, bool l3_flow) {
reverse_flow_entry_ = rflow;
reset_flags(FlowEntry::ReverseFlow);
peer_vrouter_ = info->peer_vrouter;
Expand Down Expand Up @@ -552,7 +559,7 @@ bool FlowEntry::InitFlowCmn(const PktFlowInfo *info, const PktControlInfo *ctrl,
data_.vn_entry = ctrl->vn_ ? ctrl->vn_ : rev_ctrl->vn_;
data_.in_vm_entry.SetVm(ctrl->vm_);
data_.out_vm_entry.SetVm(rev_ctrl->vm_);
l3_flow_ = info->l3_flow;
l3_flow_ = l3_flow;
data_.ecmp_rpf_nh_ = 0;
data_.acl_assigned_vrf_index_ = VrfEntry::kInvalidIndex;
return true;
Expand All @@ -561,10 +568,10 @@ bool FlowEntry::InitFlowCmn(const PktFlowInfo *info, const PktControlInfo *ctrl,
void FlowEntry::InitFwdFlow(const PktFlowInfo *info, const PktInfo *pkt,
const PktControlInfo *ctrl,
const PktControlInfo *rev_ctrl,
FlowEntry *rflow, Agent *agent) {
FlowEntry *rflow, Agent *agent, bool l3_flow) {
gen_id_ = pkt->GetAgentHdr().cmd_param_5;
flow_handle_ = pkt->GetAgentHdr().cmd_param;
if (InitFlowCmn(info, ctrl, rev_ctrl, rflow) == false) {
if (InitFlowCmn(info, ctrl, rev_ctrl, rflow, l3_flow) == false) {
return;
}
if (info->linklocal_bind_local_port) {
Expand Down Expand Up @@ -634,9 +641,9 @@ void FlowEntry::InitFwdFlow(const PktFlowInfo *info, const PktInfo *pkt,
void FlowEntry::InitRevFlow(const PktFlowInfo *info, const PktInfo *pkt,
const PktControlInfo *ctrl,
const PktControlInfo *rev_ctrl,
FlowEntry *rflow, Agent *agent) {
FlowEntry *rflow, Agent *agent, bool l3_flow) {
uint32_t intf_in;
if (InitFlowCmn(info, ctrl, rev_ctrl, rflow) == false) {
if (InitFlowCmn(info, ctrl, rev_ctrl, rflow, l3_flow) == false) {
return;
}
set_flags(FlowEntry::ReverseFlow);
Expand Down Expand Up @@ -1166,6 +1173,11 @@ void FlowEntry::GetVrfAssignAcl() {
return;
}

// Skip VrfTranslate rules for l2-flows
if (l3_flow() == false) {
return;
}

if (data_.intf_entry->type() != Interface::VM_INTERFACE) {
return;
}
Expand Down
9 changes: 6 additions & 3 deletions src/vnsw/agent/pkt/flow_entry.h
Expand Up @@ -422,6 +422,7 @@ class FlowEntry {
SHORT_NO_MIRROR_ENTRY,
SHORT_SAME_FLOW_RFLOW_KEY,
SHORT_NO_SRC_ROUTE_L2RPF,
SHORT_INACTIVE_NH,
SHORT_MAX
};

Expand Down Expand Up @@ -485,18 +486,19 @@ class FlowEntry {

void Reset(const FlowKey &k);
void Reset();
void SetKey(const FlowKey &k);

// Copy data fields from rhs
void Copy(FlowEntry *rhs, bool update);

void InitFwdFlow(const PktFlowInfo *info, const PktInfo *pkt,
const PktControlInfo *ctrl,
const PktControlInfo *rev_ctrl, FlowEntry *rflow,
Agent *agent);
Agent *agent, bool l3_flow);
void InitRevFlow(const PktFlowInfo *info, const PktInfo *pkt,
const PktControlInfo *ctrl,
const PktControlInfo *rev_ctrl, FlowEntry *rflow,
Agent *agent);
Agent *agent, bool l3_flow);
void InitAuditFlow(uint32_t flow_idx, uint8_t gen_id);
static void Init();

Expand Down Expand Up @@ -670,7 +672,8 @@ class FlowEntry {
bool SetEcmpRpfNH(FlowTable*, uint32_t);
bool SetRpfNHState(FlowTable*, const NextHop*);
bool InitFlowCmn(const PktFlowInfo *info, const PktControlInfo *ctrl,
const PktControlInfo *rev_ctrl, FlowEntry *rflow);
const PktControlInfo *rev_ctrl, FlowEntry *rflow,
bool l3_flow);
void GetSourceRouteInfo(const AgentRoute *rt);
void GetDestRouteInfo(const AgentRoute *rt);
void UpdateRpf();
Expand Down
191 changes: 154 additions & 37 deletions src/vnsw/agent/pkt/pkt_flow_info.cc
Expand Up @@ -286,40 +286,44 @@ static bool NhDecode(const NextHop *nh, const PktInfo *pkt, PktFlowInfo *info,
// have MPLS label. The MPLS label can point to
// 1. In case of non-ECMP, label will points to local interface
// 2. In case of ECMP, label will point to ECMP of local-composite members
// Setup the NH for reverse flow appropriately
case NextHop::TUNNEL: {
if (pkt->l3_forwarding) {
const InetUnicastRouteEntry *rt =
static_cast<const InetUnicastRouteEntry *>(in->rt_);
if (rt != NULL && rt->GetLocalNextHop()) {
const NextHop *local_nh = rt->GetLocalNextHop();
out->nh_ = local_nh->id();
if (local_nh->GetType() == NextHop::INTERFACE) {
const Interface *local_intf =
static_cast<const InterfaceNH*>(local_nh)->GetInterface();
//Get policy enabled nexthop only for
//vm interface, in case of vgw or service interface in
//transparent mode we should still
//use policy disabled interface
if (local_intf &&
local_intf->type() == Interface::VM_INTERFACE) {
if (local_nh->IsActive()) {
out->nh_ = local_intf->flow_key_nh()->id();
} else {
LogError(pkt, "Invalid or Inactive ifindex");
info->short_flow = true;
info->short_flow_reason =
FlowEntry::SHORT_UNAVIALABLE_INTERFACE;
}
}
}
} else {
out->nh_ = in->nh_;
}
} else {
// Bridged flow. ECMP not supported for L2 flows
out->nh_ = in->nh_;
}
// out->intf_ is invalid for packets going out on tunnel. Reset it.
out->intf_ = NULL;

// Packet going out on tunnel. Assume NH in reverse flow is same as
// that of forward flow. It can be over-written down if route for
// source-ip is ECMP
out->nh_ = in->nh_;

// The NH in reverse flow can change only if ECMP-NH is used. There is
// no ECMP for layer2 flows
if (pkt->l3_forwarding == false) {
break;
}

// If source-ip is in ECMP, reverse flow would use ECMP-NH as key
const InetUnicastRouteEntry *rt =
dynamic_cast<const InetUnicastRouteEntry *>(in->rt_);
if (rt == NULL) {
break;
}

// Get only local-NH from route
const NextHop *local_nh = rt->GetLocalNextHop();
if (local_nh->IsActive() == false) {
LogError(pkt, "Invalid or Inactive local nexthop ");
info->short_flow = true;
info->short_flow_reason = FlowEntry::SHORT_UNAVIALABLE_INTERFACE;
break;
}

// Change NH in reverse flow if route points to composite-NH
const CompositeNH *comp_nh = dynamic_cast<const CompositeNH *>
(local_nh);
if (comp_nh != NULL) {
out->nh_ = comp_nh->id();
}
break;
}

Expand Down Expand Up @@ -1054,6 +1058,10 @@ void PktFlowInfo::FloatingIpSNat(const PktInfo *pkt, PktControlInfo *in,
bool PktFlowInfo::VrfTranslate(const PktInfo *pkt, PktControlInfo *in,
PktControlInfo *out, const IpAddress &src_ip,
bool nat_flow) {
// Skip VrfTranslate rules for l2-flows
if (l3_flow == false)
return true;

const Interface *intf = NULL;
if (ingress) {
intf = in->intf_;
Expand Down Expand Up @@ -1699,6 +1707,105 @@ static bool ShouldSwapFlows(const PktFlowInfo *info, const PktInfo *pkt,
return false;
}

// We want to support a scenario where layer-2 flow is created for forward
// flow and reverse packet is received as layer-3 packet. In this case, we
// want to stitch the flows created for layer-2 and layer-3 flows
//
// While setting up layer-3 flow check if there was a layer-2 flow created for
// 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
//
// Notations:
// ----------
// Layer-2 flows are denoted as L2-Fwd-Flow and L2-Rev-Flow
// Layer-2 flows are denoted as L3-Fwd-Flow and L3-Rev-Flow
//
// Local Flow:
// -----------
// Both L2 and L3 Flows are created with interface-nh in key even if source or
// destination is ECMP.
//
// FlowTable::Add ensures layer-2 flows is re-used since keys match
//
// Egress Flow:
// ------------
// Layer-2 flow would be creaed with interface-nh as key
// Reverse flow *will* be created with interface-nh as key
//
// FlowTable::Add ensures layer-2 flows is re-used since keys match
//
// 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
//
// 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
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;
}

void PktFlowInfo::Add(const PktInfo *pkt, PktControlInfo *in,
PktControlInfo *out) {
bool update = false;
Expand Down Expand Up @@ -1756,6 +1863,7 @@ void PktFlowInfo::Add(const PktInfo *pkt, PktControlInfo *in,
rflow = FlowEntry::Allocate(rkey, flow_table);
}

// Should we swap forward/reverse flows?
bool swap_flows = false;
// If this is message processing or ECMP resolution, then retain forward
// and reverse flows
Expand All @@ -1766,10 +1874,19 @@ void PktFlowInfo::Add(const PktInfo *pkt, PktControlInfo *in,
swap_flows = true;
}

// It is possible that we already have a L2 flow with same 5-tuple. Stich
// current flow with old L2 flow in that case
bool rflow_l3_flow = l3_flow;
if (StitchL2Flow(agent, this, pkt, flow, rflow)) {
swap_flows = true;
rflow_l3_flow = false;
}

tcp_ack = pkt->tcp_ack;
flow->InitFwdFlow(this, pkt, in, out, rflow.get(), agent);
flow->InitFwdFlow(this, pkt, in, out, rflow.get(), agent, l3_flow);
if (rflow != NULL) {
rflow->InitRevFlow(this, pkt, out, in, flow.get(), agent);
rflow->InitRevFlow(this, pkt, out, in, flow.get(), agent,
rflow_l3_flow);
}

flow->GetPolicyInfo();
Expand Down Expand Up @@ -1876,8 +1993,8 @@ void PktFlowInfo::UpdateFipStatsInfo
void PktFlowInfo::GetEcmpCompositeAffinityNh() {
// Pick the first member in ECMP by default
out_component_nh_idx = 0;
if (flow_entry->IsForwardFlow())
return;
//if (flow_entry->IsForwardFlow())
// return;

// Get reverse-flow. We will try to setup ECMP member such that packet is
// forwarded to origin of reverse flow
Expand Down Expand Up @@ -1931,7 +2048,7 @@ void PktFlowInfo::RewritePktInfo(uint32_t flow_index) {
flow_entry = flow_table->Find(key);
if (!flow_entry) {
std::ostringstream ostr;
ostr << "ECMP Resolve: unable to find flow index " << flow_index;
ostr << "ECMP Resolve: Flow not present in table " << flow_index;
PKTFLOW_TRACE(Err,ostr.str());
return;
}
Expand Down
1 change: 1 addition & 0 deletions src/vnsw/agent/pkt/pkt_flow_info.h
Expand Up @@ -57,6 +57,7 @@ class PktFlowInfo {
alias_ip_flow(false), ttl(0), ecmp_component_affinity_nh(NULL) {
}

FlowTable *get_flow_table() const { return flow_table; }
static bool ComputeDirection(const Interface *intf);
void CheckLinkLocal(const PktInfo *pkt);
void LinkLocalServiceFromVm(const PktInfo *pkt, PktControlInfo *in,
Expand Down
2 changes: 2 additions & 0 deletions src/vnsw/agent/pkt/test/SConscript
Expand Up @@ -47,6 +47,8 @@ test_pkt_parse = AgentEnv.MakeTestCmd(env, 'test_pkt_parse', pkt_flaky_test_suit
test_flowtable = AgentEnv.MakeTestCmd(env, 'test_flowtable', pkt_test_suite)
test_pkt_fip = AgentEnv.MakeTestCmd(env, 'test_pkt_fip', pkt_flaky_test_suite)
test_ecmp = AgentEnv.MakeTestCmd(env, 'test_ecmp', pkt_flaky_test_suite)
test_ecmp_bridge_route = AgentEnv.MakeTestCmd(env, 'test_ecmp_bridge_route',
pkt_test_suite)
test_flow_scale = AgentEnv.MakeTestCmd(env, 'test_flow_scale', pkt_flaky_test_suite)
test_flow_freelist = AgentEnv.MakeTestCmd(env, 'test_flow_freelist', pkt_test_suite)
test_sg_flow = AgentEnv.MakeTestCmd(env, 'test_sg_flow', pkt_flaky_test_suite)
Expand Down

0 comments on commit 08dfca5

Please sign in to comment.