Skip to content

Commit

Permalink
* Handle update of interface VRF assign rule
Browse files Browse the repository at this point in the history
Match condition were compared in interface VRF assign rule
and sorted, this is not needed as ID of rule will be the key
and match condition should be just treated as data.
Changing the same.

Change-Id: I695f5c31672c874caac91e3803b098a8beb93898
Closes-bug:#1633383
  • Loading branch information
naveen-n committed Oct 22, 2016
1 parent 3a0aef2 commit cd8d293
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 75 deletions.
78 changes: 7 additions & 71 deletions src/vnsw/agent/oper/vm_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3226,63 +3226,6 @@ void VmInterface::DeleteAllowedAddressPair(bool l2) {
}
}

static bool CompareAddressType(const AddressType &lhs,
const AddressType &rhs) {
if (lhs.subnet.ip_prefix != rhs.subnet.ip_prefix) {
return false;
}

if (lhs.subnet.ip_prefix_len != rhs.subnet.ip_prefix_len) {
return false;
}

if (lhs.virtual_network != rhs.virtual_network) {
return false;
}

if (lhs.security_group != rhs.security_group) {
return false;
}
return true;
}

static bool ComparePortType(const PortType &lhs,
const PortType &rhs) {
if (lhs.start_port != rhs.start_port) {
return false;
}

if (lhs.end_port != rhs.end_port) {
return false;
}
return true;
}

static bool CompareMatchConditionType(const MatchConditionType &lhs,
const MatchConditionType &rhs) {
if (lhs.protocol != rhs.protocol) {
return lhs.protocol < rhs.protocol;
}

if (!CompareAddressType(lhs.src_address, rhs.src_address)) {
return false;
}

if (!ComparePortType(lhs.src_port, rhs.src_port)) {
return false;
}

if (!CompareAddressType(lhs.dst_address, rhs.dst_address)) {
return false;
}

if (!ComparePortType(lhs.dst_port, rhs.dst_port)) {
return false;
}

return true;
}

void VmInterface::UpdateVrfAssignRule() {
Agent *agent = static_cast<InterfaceTable *>(get_table())->agent();
//Erase all delete marked entry
Expand Down Expand Up @@ -4858,20 +4801,20 @@ VmInterface::hc_instance_set() const {
// VRF assign rule routines
////////////////////////////////////////////////////////////////////////////
VmInterface::VrfAssignRule::VrfAssignRule():
ListEntry(), id_(0), vrf_name_(" "), vrf_(NULL, this), ignore_acl_(false) {
ListEntry(), id_(0), vrf_name_(" "), ignore_acl_(false) {
}

VmInterface::VrfAssignRule::VrfAssignRule(const VrfAssignRule &rhs):
ListEntry(rhs.installed_, rhs.del_pending_), id_(rhs.id_),
vrf_name_(rhs.vrf_name_), vrf_(rhs.vrf_, this), ignore_acl_(rhs.ignore_acl_),
vrf_name_(rhs.vrf_name_), ignore_acl_(rhs.ignore_acl_),
match_condition_(rhs.match_condition_) {
}

VmInterface::VrfAssignRule::VrfAssignRule(uint32_t id,
const autogen::MatchConditionType &match_condition,
const std::string &vrf_name,
bool ignore_acl):
ListEntry(), id_(id), vrf_name_(vrf_name), vrf_(NULL, this),
ListEntry(), id_(id), vrf_name_(vrf_name),
ignore_acl_(ignore_acl), match_condition_(match_condition) {
}

Expand All @@ -4884,17 +4827,7 @@ bool VmInterface::VrfAssignRule::operator() (const VrfAssignRule &lhs,
}

bool VmInterface::VrfAssignRule::IsLess(const VrfAssignRule *rhs) const {
if (id_ != rhs->id_) {
return id_ < rhs->id_;
}
if (vrf_name_ != rhs->vrf_name_) {
return vrf_name_ < rhs->vrf_name_;
}
if (ignore_acl_ != rhs->ignore_acl_) {
return ignore_acl_ < rhs->ignore_acl_;
}

return CompareMatchConditionType(match_condition_, rhs->match_condition_);
return id_ < rhs->id_;
}

void VmInterface::VrfAssignRuleList::Insert(const VrfAssignRule *rhs) {
Expand All @@ -4904,6 +4837,9 @@ void VmInterface::VrfAssignRuleList::Insert(const VrfAssignRule *rhs) {
void VmInterface::VrfAssignRuleList::Update(const VrfAssignRule *lhs,
const VrfAssignRule *rhs) {
lhs->set_del_pending(false);
lhs->match_condition_ = rhs->match_condition_;
lhs->ignore_acl_ = rhs->ignore_acl_;
lhs->vrf_name_ = rhs->vrf_name_;
}

void VmInterface::VrfAssignRuleList::Remove(VrfAssignRuleSet::iterator &it) {
Expand Down
7 changes: 3 additions & 4 deletions src/vnsw/agent/oper/vm_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,9 @@ class VmInterface : public Interface {
bool IsLess(const VrfAssignRule *rhs) const;

const uint32_t id_;
const std::string vrf_name_;
const VrfEntryRef vrf_;
bool ignore_acl_;
autogen::MatchConditionType match_condition_;
mutable std::string vrf_name_;
mutable bool ignore_acl_;
mutable autogen::MatchConditionType match_condition_;
};
typedef std::set<VrfAssignRule, VrfAssignRule> VrfAssignRuleSet;

Expand Down
51 changes: 51 additions & 0 deletions src/vnsw/agent/pkt/test/test_vrf_assign_acl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,28 @@ struct PortInfo input[] = {
{"intf2", 2, "1.1.1.2", "00:00:00:01:01:02", 1, 2},
};

IpamInfo ipam_info1[] = {
{"1.1.1.0", 24, "1.1.1.10", true},
};

struct PortInfo input1[] = {
{"intf3", 3, "2.1.1.1", "00:00:00:01:01:01", 2, 3},
{"intf4", 4, "2.1.1.2", "00:00:00:01:01:02", 2, 4},
};

IpamInfo ipam_info2[] = {
{"2.1.1.0", 24, "2.1.1.10", true},
};

struct PortInfo input2[] = {
{"intf5", 5, "2.1.1.1", "00:00:00:01:01:01", 3, 5},
{"intf6", 6, "2.1.1.2", "00:00:00:01:01:02", 3, 6},
};

IpamInfo ipam_info3[] = {
{"2.1.1.0", 24, "2.1.1.10", true},
};

class TestVrfAssignAclFlow : public ::testing::Test {
protected:
virtual void SetUp() {
Expand All @@ -34,6 +46,12 @@ class TestVrfAssignAclFlow : public ::testing::Test {
CreateVmportFIpEnv(input1, 2);
CreateVmportFIpEnv(input2, 2);
client->WaitForIdle();

AddIPAM("default-project:vn1", ipam_info1, 1);
AddIPAM("default-project:vn2", ipam_info2, 1);
AddIPAM("default-project:vn3", ipam_info3, 1);
client->WaitForIdle();

EXPECT_TRUE(VmPortActive(1));
EXPECT_TRUE(VmPortActive(2));
EXPECT_TRUE(VmPortActive(3));
Expand Down Expand Up @@ -77,6 +95,9 @@ class TestVrfAssignAclFlow : public ::testing::Test {
DeleteVmportFIpEnv(input, 2, true);
DeleteVmportFIpEnv(input1, 2, true);
DeleteVmportFIpEnv(input2, 2, true);
DelIPAM("default-project:vn1");
DelIPAM("default-project:vn2");
DelIPAM("default-project:vn3");
client->WaitForIdle();
WAIT_FOR(1000, 1000, (0U == flow_proto_->FlowCount()));
EXPECT_FALSE(VmPortFindRetDel(1));
Expand Down Expand Up @@ -389,6 +410,36 @@ TEST_F(TestVrfAssignAclFlow, VrfAssignAcl9) {
EXPECT_TRUE(fe->is_flags_set(FlowEntry::ShortFlow) == true);
}

//Modify ACL and check if new flow is set with proper action
TEST_F(TestVrfAssignAclFlow, VrfAssignAcl10) {
AddAddressVrfAssignAcl("intf1", 1, "1.1.1.0", "2.1.1.0", 6, 1, 65535,
1, 65535, "default-project:vn2:vn2", "true");

TestFlow flow[] = {
{ TestFlowPkt(Address::INET, "1.1.1.1", "2.1.1.1", IPPROTO_TCP, 10, 20,
"default-project:vn1:vn1", VmPortGet(1)->id()),
{
new VerifyVn("default-project:vn1", "default-project:vn2"),
new VerifyAction((1 << TrafficAction::PASS) |
(1 << TrafficAction::VRF_TRANSLATE),
(1 << TrafficAction::PASS))
}
}
};
CreateFlow(flow, 1);

AddAddressVrfAssignAcl("intf1", 1, "2.1.1.0", "2.1.1.0", 6, 1, 65535,
1, 65535, "default-project:vn2:vn2", "true");
TestFlow flow2[] = {
{ TestFlowPkt(Address::INET, "1.1.1.1", "2.1.1.1", IPPROTO_TCP, 10, 20,
"default-project:vn1:vn1", VmPortGet(1)->id()),
{
new ShortFlow()
}
}
};
CreateFlow(flow2, 1);
}
//Add an VRF translate ACL to send all ssh traffic to "2.1.1.1"
//via default-project:vn2 and also add mirror ACL for VN1
//Verify that final action has mirror action also
Expand Down

0 comments on commit cd8d293

Please sign in to comment.