Skip to content

Commit

Permalink
Merge "Revert "Detect vrouter evicted flows during aging and remove t…
Browse files Browse the repository at this point in the history
…hem from agent.""
  • Loading branch information
Zuul authored and opencontrail-ci-admin committed Feb 5, 2016
2 parents 424ab14 + af3b93d commit 51a131f
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 51a131f

Please sign in to comment.