From cd8d2939c70b69319d8aa5b75a20e3576f2a4470 Mon Sep 17 00:00:00 2001 From: Naveen N Date: Fri, 14 Oct 2016 14:47:21 +0530 Subject: [PATCH] * Handle update of interface VRF assign rule 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 --- src/vnsw/agent/oper/vm_interface.cc | 78 ++----------------- src/vnsw/agent/oper/vm_interface.h | 7 +- .../agent/pkt/test/test_vrf_assign_acl.cc | 51 ++++++++++++ 3 files changed, 61 insertions(+), 75 deletions(-) diff --git a/src/vnsw/agent/oper/vm_interface.cc b/src/vnsw/agent/oper/vm_interface.cc index 273feaeff9e..cba0f44a3aa 100644 --- a/src/vnsw/agent/oper/vm_interface.cc +++ b/src/vnsw/agent/oper/vm_interface.cc @@ -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(get_table())->agent(); //Erase all delete marked entry @@ -4858,12 +4801,12 @@ 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_) { } @@ -4871,7 +4814,7 @@ 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) { } @@ -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) { @@ -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) { diff --git a/src/vnsw/agent/oper/vm_interface.h b/src/vnsw/agent/oper/vm_interface.h index 00507eee068..5a89b01bc4e 100644 --- a/src/vnsw/agent/oper/vm_interface.h +++ b/src/vnsw/agent/oper/vm_interface.h @@ -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 VrfAssignRuleSet; diff --git a/src/vnsw/agent/pkt/test/test_vrf_assign_acl.cc b/src/vnsw/agent/pkt/test/test_vrf_assign_acl.cc index ae02567163c..c35d2691253 100644 --- a/src/vnsw/agent/pkt/test/test_vrf_assign_acl.cc +++ b/src/vnsw/agent/pkt/test/test_vrf_assign_acl.cc @@ -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() { @@ -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)); @@ -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)); @@ -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