Skip to content

Commit

Permalink
Revert "Detect vrouter evicted flows during aging and remove them fro…
Browse files Browse the repository at this point in the history
…m agent."

This reverts commit af647a2.

Change-Id: I995d42eb00b435ac6fc2a864a27beeb5cb28ee09
Closes-Bug: #1541033
  • Loading branch information
praveenkv committed Feb 4, 2016
1 parent d2f83af commit af3b93d
Show file tree
Hide file tree
Showing 21 changed files with 56 additions and 156 deletions.
1 change: 0 additions & 1 deletion src/ksync/ksync_sock.h
Expand Up @@ -246,7 +246,6 @@ class KSyncSock {
int SendBulkMessage(KSyncBulkSandeshContext *bulk_context, uint32_t seqno);
bool TryAddToBulk(KSyncBulkSandeshContext *bulk_context, IoContext *ioc);
void OnEmptyQueue(bool done);
int tx_count() const { return tx_count_; }

// Start Ksync Asio operations
static void Start(bool read_inline);
Expand Down
14 changes: 0 additions & 14 deletions src/ksync/ksync_sock_user.cc
Expand Up @@ -639,20 +639,6 @@ void KSyncSockTypeMap::IncrFlowStats(int idx, int pkts, int bytes) {
}
}

void KSyncSockTypeMap::SetEvictedFlag(int idx) {
vr_flow_entry *f = &flow_table_[idx];
if (f->fe_flags & VR_FLOW_FLAG_ACTIVE) {
f->fe_flags |= VR_FLOW_FLAG_EVICTED;
}
}

void KSyncSockTypeMap::ResetEvictedFlag(int idx) {
vr_flow_entry *f = &flow_table_[idx];
if (f->fe_flags & VR_FLOW_FLAG_ACTIVE) {
f->fe_flags &= ~VR_FLOW_FLAG_EVICTED;
}
}

void KSyncSockTypeMap::SetTcpFlag(int idx, uint32_t flags) {
vr_flow_entry *f = &flow_table_[idx];
if (f->fe_flags & VR_FLOW_FLAG_ACTIVE) {
Expand Down
2 changes: 0 additions & 2 deletions src/ksync/ksync_sock_user.h
Expand Up @@ -176,8 +176,6 @@ class KSyncSockTypeMap : public KSyncSock {
static void SetFlowEntry(vr_flow_req *req, bool set);
static void IncrFlowStats(int idx, int pkts, int bytes);
static void SetTcpFlag(int idx, uint32_t flags);
static void SetEvictedFlag(int idx);
static void ResetEvictedFlag(int idx);
static void SetOFlowStats(int idx, uint8_t pkts, uint16_t bytes);
static void SetFlowTcpFlags(int idx, uint16_t flags);;
static void FlowNatResponse(uint32_t seq_num, vr_flow_req *req);
Expand Down
1 change: 0 additions & 1 deletion src/vnsw/agent/pkt/flow_entry.cc
Expand Up @@ -348,7 +348,6 @@ void FlowEntry::Reset() {
sg_rule_uuid_= FlowPolicyStateStr.at(NOT_EVALUATED);
ksync_index_entry_ = std::auto_ptr<KSyncFlowIndexEntry>
(new KSyncFlowIndexEntry());
vrouter_evicted_ = false;
}

void FlowEntry::Reset(const FlowKey &k) {
Expand Down
3 changes: 0 additions & 3 deletions src/vnsw/agent/pkt/flow_entry.h
Expand Up @@ -483,8 +483,6 @@ class FlowEntry {
void set_fsc(FlowStatsCollector *fsc) {
fsc_ = fsc;
}
void set_vrouter_evicted(bool value) { vrouter_evicted_ = value; }
bool vrouter_evicted() const { return vrouter_evicted_; }
static std::string DropReasonStr(uint16_t reason);
private:
friend class FlowTable;
Expand Down Expand Up @@ -530,7 +528,6 @@ class FlowEntry {
tbb::mutex mutex_;
boost::intrusive::list_member_hook<> free_list_node_;
FlowStatsCollector *fsc_;
bool vrouter_evicted_;
// IMPORTANT: Remember to update Reset() routine if new fields are added
// IMPORTANT: Remember to update Copy() routine if new fields are added

Expand Down
36 changes: 16 additions & 20 deletions src/vnsw/agent/pkt/flow_event.h
Expand Up @@ -58,84 +58,82 @@ class FlowEvent {

FlowEvent() :
event_(INVALID), flow_(NULL), pkt_info_(), db_entry_(NULL),
gen_id_(0), del_rev_flow_(false), vrouter_evicted_(false),
gen_id_(0), del_rev_flow_(false),
flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL),
ksync_event_() {
}

FlowEvent(Event event) :
event_(event), flow_(NULL), pkt_info_(), db_entry_(NULL),
gen_id_(0), del_rev_flow_(false), vrouter_evicted_(false),
ksync_entry_(NULL), ksync_event_() {
gen_id_(0), del_rev_flow_(false), ksync_entry_(NULL), ksync_event_() {
}

FlowEvent(Event event, FlowEntry *flow) :
event_(event), flow_(flow), pkt_info_(), db_entry_(NULL),
gen_id_(0), del_rev_flow_(false), vrouter_evicted_(false),
gen_id_(0), del_rev_flow_(false),
flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL),
ksync_event_() {
}

FlowEvent(Event event, FlowEntry *flow, uint32_t flow_handle) :
event_(event), flow_(flow), pkt_info_(), db_entry_(NULL),
gen_id_(0), del_rev_flow_(false), vrouter_evicted_(false),
flow_handle_(flow_handle), ksync_entry_(NULL), ksync_event_() {
gen_id_(0), del_rev_flow_(false), flow_handle_(flow_handle),
ksync_entry_(NULL), ksync_event_() {
}

FlowEvent(Event event, FlowEntry *flow, const DBEntry *db_entry) :
event_(event), flow_(flow), pkt_info_(), db_entry_(db_entry),
gen_id_(0), del_rev_flow_(false), vrouter_evicted_(false),
gen_id_(0), del_rev_flow_(false),
flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL),
ksync_event_() {
}

FlowEvent(Event event, const DBEntry *db_entry, uint32_t gen_id) :
event_(event), flow_(NULL), pkt_info_(), db_entry_(db_entry),
gen_id_(gen_id), del_rev_flow_(false), vrouter_evicted_(false),
gen_id_(gen_id), del_rev_flow_(false),
flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL),
ksync_event_() {
}

FlowEvent(Event event, const FlowKey &key, bool del_rev_flow, bool evict) :
FlowEvent(Event event, const FlowKey &key, bool del_rev_flow) :
event_(event), flow_(NULL), pkt_info_(), db_entry_(NULL),
gen_id_(0), flow_key_(key), del_rev_flow_(del_rev_flow),
vrouter_evicted_(evict), flow_handle_(FlowEntry::kInvalidFlowHandle),
ksync_entry_(NULL), ksync_event_() {
flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL),
ksync_event_() {
}

FlowEvent(Event event, PktInfoPtr pkt_info) :
event_(event), flow_(NULL), pkt_info_(pkt_info), db_entry_(NULL),
gen_id_(0), flow_key_(), del_rev_flow_(), vrouter_evicted_(false),
gen_id_(0), flow_key_(), del_rev_flow_(),
flow_handle_(FlowEntry::kInvalidFlowHandle),
ksync_entry_(NULL), ksync_event_() {
}

FlowEvent(KSyncEntry *entry, KSyncEntry::KSyncEvent event) :
event_(KSYNC_EVENT), flow_(NULL), pkt_info_(), db_entry_(NULL),
gen_id_(0), flow_key_(), del_rev_flow_(), vrouter_evicted_(false),
gen_id_(0), flow_key_(), del_rev_flow_(),
flow_handle_(FlowEntry::kInvalidFlowHandle),
ksync_entry_(entry), ksync_event_(event) {
}

FlowEvent(KSyncEntry *entry, uint32_t flow_handle) :
event_(FLOW_HANDLE_UPDATE), flow_(NULL), pkt_info_(),
db_entry_(NULL), gen_id_(0), flow_key_(), del_rev_flow_(),
vrouter_evicted_(false), flow_handle_(flow_handle), ksync_entry_(entry),
ksync_event_() {
flow_handle_(flow_handle), ksync_entry_(entry), ksync_event_() {
}

FlowEvent(KSyncEntry *entry) :
event_(KSYNC_VROUTER_ERROR), flow_(NULL), pkt_info_(),
db_entry_(NULL), gen_id_(0), flow_key_(), del_rev_flow_(),
vrouter_evicted_(false), flow_handle_(FlowEntry::kInvalidFlowHandle),
ksync_entry_(entry), ksync_event_() {
flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(entry),
ksync_event_() {
}

FlowEvent(const FlowEvent &rhs) :
event_(rhs.event_), flow_(rhs.flow()), pkt_info_(rhs.pkt_info_),
db_entry_(rhs.db_entry_), gen_id_(rhs.gen_id_),
flow_key_(rhs.flow_key_), del_rev_flow_(rhs.del_rev_flow_),
vrouter_evicted_(rhs.vrouter_evicted_), flow_handle_(rhs.flow_handle_),
flow_handle_(rhs.flow_handle_),
ksync_entry_(rhs.ksync_entry_), ksync_event_(rhs.ksync_event_) {
}

Expand All @@ -149,7 +147,6 @@ class FlowEvent {
uint32_t gen_id() const { return gen_id_; }
const FlowKey &get_flow_key() const { return flow_key_; }
bool get_del_rev_flow() const { return del_rev_flow_; }
bool get_vrouter_evicted() const { return vrouter_evicted_; }
PktInfoPtr pkt_info() const { return pkt_info_; }
uint32_t flow_handle() const { return flow_handle_; }

Expand All @@ -163,7 +160,6 @@ class FlowEvent {
uint32_t gen_id_;
FlowKey flow_key_;
bool del_rev_flow_;
bool vrouter_evicted_;
uint32_t flow_handle_;
KSyncEntry::KSyncEntryPtr ksync_entry_;
KSyncEntry::KSyncEvent ksync_event_;
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/pkt/flow_mgmt.cc
Expand Up @@ -259,7 +259,7 @@ bool BgpAsAServiceFlowMgmtEntry::NonOperEntryDelete(FlowMgmtManager *mgr,

Tree::iterator it = tree_.begin();
while (it != tree_.end()) {
FlowEvent flow_resp(event, (*it)->key(), true, false);
FlowEvent flow_resp(event, (*it)->key(), true);
flow_resp.set_flow(*it);
mgr->EnqueueFlowEvent(flow_resp);
it++;
Expand Down
11 changes: 4 additions & 7 deletions src/vnsw/agent/pkt/flow_proto.cc
Expand Up @@ -259,8 +259,7 @@ bool FlowProto::FlowEventHandler(const FlowEvent &req, FlowTable *table) {

case FlowEvent::DELETE_FLOW: {
FlowTable *table = GetFlowTable(req.get_flow_key());
table->Delete(req.get_flow_key(), req.get_del_rev_flow(),
req.get_vrouter_evicted());
table->Delete(req.get_flow_key(), req.get_del_rev_flow());
break;
}

Expand Down Expand Up @@ -355,10 +354,8 @@ bool FlowProto::FlowEventHandler(const FlowEvent &req, FlowTable *table) {
return true;
}

void FlowProto::DeleteFlowRequest(const FlowKey &flow_key, bool del_rev_flow,
bool evicted) {
EnqueueFlowEvent(FlowEvent(FlowEvent::DELETE_FLOW, flow_key, del_rev_flow,
evicted));
void FlowProto::DeleteFlowRequest(const FlowKey &flow_key, bool del_rev_flow) {
EnqueueFlowEvent(FlowEvent(FlowEvent::DELETE_FLOW, flow_key, del_rev_flow));
return;
}

Expand All @@ -380,7 +377,7 @@ void FlowProto::CreateAuditEntry(FlowEntry *flow) {


void FlowProto::GrowFreeListRequest(const FlowKey &key) {
EnqueueFlowEvent(FlowEvent(FlowEvent::GROW_FREE_LIST, key, false, false));
EnqueueFlowEvent(FlowEvent(FlowEvent::GROW_FREE_LIST, key, false));
return;
}

Expand Down
3 changes: 1 addition & 2 deletions src/vnsw/agent/pkt/flow_proto.h
Expand Up @@ -62,8 +62,7 @@ class FlowProto : public Proto {

void EnqueueEvent(const FlowEvent &event, FlowTable *table);
void EnqueueFlowEvent(const FlowEvent &event);
void DeleteFlowRequest(const FlowKey &flow_key, bool del_rev_flow,
bool evicted);
void DeleteFlowRequest(const FlowKey &flow_key, bool del_rev_flow);
void EvictFlowRequest(FlowEntry *flow, uint32_t flow_handle);
void RetryIndexAcquireRequest(FlowEntry *flow, uint32_t flow_handle);
void CreateAuditEntry(FlowEntry *flow);
Expand Down
11 changes: 4 additions & 7 deletions src/vnsw/agent/pkt/flow_table.cc
Expand Up @@ -263,8 +263,7 @@ void FlowTable::DeleteInternal(FlowEntryMap::iterator &it, uint64_t time) {
agent_->stats()->UpdateFlowDelMinMaxStats(time);
}

bool FlowTable::Delete(const FlowKey &key, bool del_reverse_flow,
bool vrouter_evicted) {
bool FlowTable::Delete(const FlowKey &key, bool del_reverse_flow) {
FlowEntryMap::iterator it;
FlowEntry *fe;

Expand All @@ -273,7 +272,6 @@ bool FlowTable::Delete(const FlowKey &key, bool del_reverse_flow,
return false;
}
fe = it->second;
fe->set_vrouter_evicted(vrouter_evicted);

FlowEntry *reverse_flow = NULL;
if (del_reverse_flow) {
Expand All @@ -290,7 +288,6 @@ bool FlowTable::Delete(const FlowKey &key, bool del_reverse_flow,

it = flow_entry_map_.find(reverse_flow->key());
if (it != flow_entry_map_.end()) {
it->second->set_vrouter_evicted(vrouter_evicted);
DeleteInternal(it, time);
return true;
}
Expand All @@ -308,7 +305,7 @@ void FlowTable::DeleteAll() {
it->second == entry->reverse_flow_entry()) {
++it;
}
Delete(entry->key(), true, false);
Delete(entry->key(), true);
}
}

Expand Down Expand Up @@ -369,7 +366,7 @@ void FlowTable::UpdateReverseFlow(FlowEntry *flow, FlowEntry *rflow) {
//same reverse flow as its is nat'd with fabric sip/dip.
//To avoid this delete old flow and dont let new flow to be short flow.
if (rflow_rev) {
Delete(rflow_rev->key(), false, false);
Delete(rflow_rev->key(), false);
rflow_rev = NULL;
}
}
Expand Down Expand Up @@ -671,7 +668,7 @@ void FlowTable::RevaluateFlow(FlowEntry *flow) {
// Handle deletion of a Route. Flow management module has identified that route
// must be deleted
void FlowTable::DeleteMessage(FlowEntry *flow) {
Delete(flow->key(), true, false);
Delete(flow->key(), true);
DeleteFlowInfo(flow);
}

Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/pkt/flow_table.h
Expand Up @@ -163,7 +163,7 @@ class FlowTable {
bool IsEvictedFlow(const FlowKey &key);
void Add(FlowEntry *flow, FlowEntry *rflow);
void Update(FlowEntry *flow, FlowEntry *rflow);
bool Delete(const FlowKey &key, bool del_reverse_flow, bool evicted);
bool Delete(const FlowKey &key, bool del_reverse_flow);
bool Delete(const FlowKey &flow_key);
void DeleteAll();
// Test code only used method
Expand Down
57 changes: 4 additions & 53 deletions src/vnsw/agent/pkt/test/test_flow_eviction.cc
Expand Up @@ -9,7 +9,6 @@
#include "ksync/ksync_sock_user.h"
#include "oper/tunnel_nh.h"
#include "pkt/flow_mgmt.h"
#include "uve/test/test_uve_util.h"
#include <algorithm>

#define vm1_ip "1.1.1.1"
Expand All @@ -24,7 +23,7 @@ struct PortInfo input[] = {

class FlowEvictionTest : public ::testing::Test {
public:
FlowEvictionTest() : peer_(NULL), agent_(Agent::GetInstance()), util_() {
FlowEvictionTest() : peer_(NULL), agent_(Agent::GetInstance()) {
flow_proto_ = agent_->pkt()->get_flow_proto();
flow_mgmt_ = agent_->pkt()->flow_mgmt_manager();
eth = EthInterfaceGet("vnet0");
Expand Down Expand Up @@ -106,7 +105,6 @@ class FlowEvictionTest : public ::testing::Test {
char router_id_[80];
InetUnicastAgentRouteTable *inet4_table_;
KSyncSockTypeMap *ksync_sock_;
TestUveUtil util_;
};

// New flow no-eviction
Expand Down Expand Up @@ -419,7 +417,7 @@ TEST_F(FlowEvictionTest, Delete_Evicted_Flow_1) {
EXPECT_TRUE(FlowGet(vrf_id, remote_vm1_ip, vm1_ip, 2, 0, 0,
vif0->flow_key_nh()->id()) == false);

flow_proto_->DeleteFlowRequest(key, true, false);
flow_proto_->DeleteFlowRequest(key, true);
client->WaitForIdle();

// New flow should be present
Expand All @@ -443,7 +441,7 @@ TEST_F(FlowEvictionTest, Delete_Evicted_Flow_2) {
EXPECT_TRUE(flow != NULL);

// Generate delete request followed by flow-evict
flow_proto_->DeleteFlowRequest(flow->key(), true, false);
flow_proto_->DeleteFlowRequest(flow->key(), true);
// Generate a flow that evicts flow created above
TxIpMplsPacket(eth->id(), remote_compute, router_id_, vif0->label(),
remote_vm1_ip, vm1_ip, 1, 1);
Expand Down Expand Up @@ -476,7 +474,7 @@ TEST_F(FlowEvictionTest, Delete_Index_Unassigned_Flow_1) {
EXPECT_TRUE(flow != NULL);

FlowKey key = flow->key();
flow_proto_->DeleteFlowRequest(key, true, false);
flow_proto_->DeleteFlowRequest(key, true);
client->WaitForIdle();

ksync_sock_->DisableReceiveQueue(false);
Expand All @@ -486,53 +484,6 @@ TEST_F(FlowEvictionTest, Delete_Index_Unassigned_Flow_1) {
vif0->flow_key_nh()->id()) == NULL);
}

TEST_F(FlowEvictionTest, AgeOutVrouterEvictedFlow) {
TxIpMplsPacket(eth->id(), remote_compute, router_id_, vif0->label(),
remote_vm1_ip, vm1_ip, 1, 1);
client->WaitForIdle();

uint32_t vrf_id = vif0->vrf_id();
FlowEntry *flow = FlowGet(vrf_id, remote_vm1_ip, vm1_ip, 1, 0, 0,
vif0->flow_key_nh()->id());
EXPECT_TRUE(flow != NULL);
uint32_t flow_handle = flow->flow_handle();
EXPECT_TRUE(flow_handle != FlowEntry::kInvalidFlowHandle);
KSyncSock *sock = KSyncSock::Get(0);

//Fetch the delete_count_ for delete enqueues
uint32_t delete_count = agent_->GetFlowProto()->flow_stats()->delete_count_;
uint32_t tx_count = sock->tx_count();

//Invoke FlowStatsCollector to enqueue delete for evicted flow.
util_.EnqueueFlowStatsCollectorTask();
client->WaitForIdle();

//Verify that delete count has not been modified
EXPECT_EQ(delete_count, agent_->GetFlowProto()->flow_stats()->
delete_count_);

//Set Evicted flag for the flow
KSyncSockTypeMap::SetEvictedFlag(flow_handle);

//Invoke FlowStatsCollector to enqueue delete for evicted flow.
util_.EnqueueFlowStatsCollectorTask();
client->WaitForIdle();

//Verify that delete count has been increased by 1
EXPECT_EQ((delete_count + 1), agent_->GetFlowProto()->flow_stats()->
delete_count_);

//Verify that evicted flows have been removed
WAIT_FOR(100, 100, (flow_proto_->FlowCount() == 0));

//Verify that tx_count is unchanged after deletion of evicted flows
//because agent does not sent any message to vrouter for evicted flows
EXPECT_EQ(tx_count, sock->tx_count());

//Reset the Evicted flag set by this test-case
KSyncSockTypeMap::ResetEvictedFlag(flow_handle);
}

int main(int argc, char *argv[]) {
int ret = 0;

Expand Down

0 comments on commit af3b93d

Please sign in to comment.