From 3a63721351f0af5fa3f6b05a1268509b96e4faf0 Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 15 Nov 2024 15:57:20 -0800 Subject: [PATCH 1/3] [ACL] Add Tunnel Next Hop redirect support Signed-off-by: Vivek Reddy --- orchagent/aclorch.cpp | 74 ++++++- orchagent/aclorch.h | 18 ++ orchagent/vxlanorch.cpp | 8 +- tests/mock_tests/Makefile.am | 1 + tests/mock_tests/aclorch_rule_ut.cpp | 294 +++++++++++++++++++++++++++ tests/mock_tests/check.h | 96 +++++++-- 6 files changed, 458 insertions(+), 33 deletions(-) create mode 100644 tests/mock_tests/aclorch_rule_ut.cpp diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 570d40825c..5718d16fc3 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -11,6 +11,7 @@ #include "timer.h" #include "crmorch.h" #include "sai_serialize.h" +#include "directory.h" using namespace std; using namespace swss; @@ -30,6 +31,7 @@ extern PortsOrch* gPortsOrch; extern CrmOrch *gCrmOrch; extern SwitchOrch *gSwitchOrch; extern string gMySwitchType; +extern Directory gDirectory; #define MIN_VLAN_ID 1 // 0 is a reserved VLAN ID #define MAX_VLAN_ID 4095 // 4096 is a reserved VLAN ID @@ -617,6 +619,35 @@ bool AclTableRangeMatch::validateAclRuleMatch(const AclRule& rule) const return true; } +void AclRule::TunnelNH::load(const std::string& target) +{ + parse(target); + + VxlanTunnelOrch* vxlan_orch = gDirectory.get(); + /* Only the first call creates the SAI object, further calls just increment the ref count */ + oid = vxlan_orch->createNextHopTunnel(tunnel_name, endpoint_ip, mac, vni); +} + +void AclRule::TunnelNH::parse(const std::string& target) +{ + /* Supported Format: endpoint_ip@tunnel_name */ + auto at_pos = target.find('@'); + if (at_pos == std::string::npos) + { + throw std::logic_error("Invalid format for Tunnel Next Hop"); + } + + endpoint_ip = swss::IpAddress(target.substr(0, at_pos)); + tunnel_name = target.substr(at_pos + 1); +} + +void AclRule::TunnelNH::clear() +{ + oid = SAI_NULL_OBJECT_ID; + VxlanTunnelOrch* vxlan_orch = gDirectory.get(); + vxlan_orch->removeNextHopTunnel(tunnel_name, endpoint_ip, mac, vni); +} + string AclTableType::getName() const { return m_name; @@ -1308,7 +1339,17 @@ void AclRule::decreaseNextHopRefCount() } m_redirect_target_next_hop_group.clear(); } - + if (m_redirect_target_tun_nh.oid != SAI_NULL_OBJECT_ID) + { + try + { + m_redirect_target_tun_nh.clear(); + } + catch (const std::runtime_error& e) + { + SWSS_LOG_ERROR("Failed to remove tunnel nh reference %s, ACL Rule: %s", e.what(), m_id.c_str()); + } + } return; } @@ -2001,21 +2042,38 @@ sai_object_id_t AclRulePacket::getRedirectObjectId(const string& redirect_value) try { NextHopKey nh(target); - if (!m_pAclOrch->m_neighOrch->hasNextHop(nh)) + if (m_pAclOrch->m_neighOrch->hasNextHop(nh)) { - SWSS_LOG_ERROR("ACL Redirect action target next hop ip: '%s' doesn't exist on the switch", nh.to_string().c_str()); - return SAI_NULL_OBJECT_ID; + m_redirect_target_next_hop = target; + m_pAclOrch->m_neighOrch->increaseNextHopRefCount(nh); + return m_pAclOrch->m_neighOrch->getNextHopId(nh); } - - m_redirect_target_next_hop = target; - m_pAclOrch->m_neighOrch->increaseNextHopRefCount(nh); - return m_pAclOrch->m_neighOrch->getNextHopId(nh); } catch (...) { // no error, just try next variant } + // Try to parse if this is a tunnel nexthop. + try + { + m_redirect_target_tun_nh.load(target); + if (SAI_NULL_OBJECT_ID != m_redirect_target_tun_nh.oid) + { + SWSS_LOG_INFO("Tunnel Next Hop Found: oid:%ld, target: %s", m_redirect_target_tun_nh.oid, target.c_str()); + return m_redirect_target_tun_nh.oid; + } + } + catch (std::logic_error& e) + { + // no error, just try next variant + } + catch (const std::runtime_error& e) + { + SWSS_LOG_ERROR("Failed to create/fetch tunnel next hop, %s, err: %s", target.c_str(), e.what()); + return SAI_NULL_OBJECT_ID; + } + // try to parse nh group the set of try { diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 9e27be2ac0..beb389260e 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -15,6 +15,7 @@ #include "mirrororch.h" #include "dtelorch.h" #include "observer.h" +#include "vxlanorch.h" #include "flex_counter_manager.h" #include "acltable.h" @@ -286,6 +287,22 @@ class AclTable; class AclRule { public: + struct TunnelNH + { + TunnelNH() = default; + ~TunnelNH() = default; + + void load(const std::string& target); + void parse(const std::string& target); + void clear(); + + std::string tunnel_name; + swss::IpAddress endpoint_ip; + swss::MacAddress mac; + uint32_t vni = 0; + sai_object_id_t oid = SAI_NULL_OBJECT_ID; + }; + AclRule(AclOrch *pAclOrch, string rule, string table, bool createCounter = true); virtual bool validateAddPriority(string attr_name, string attr_value); virtual bool validateAddMatch(string attr_name, string attr_value); @@ -359,6 +376,7 @@ class AclRule map m_matches; string m_redirect_target_next_hop; string m_redirect_target_next_hop_group; + AclRule::TunnelNH m_redirect_target_tun_nh; vector m_rangeConfig; vector m_ranges; diff --git a/orchagent/vxlanorch.cpp b/orchagent/vxlanorch.cpp index 05a2d3e603..5fcc35b6b6 100644 --- a/orchagent/vxlanorch.cpp +++ b/orchagent/vxlanorch.cpp @@ -1329,10 +1329,6 @@ VxlanTunnelOrch::createNextHopTunnel(string tunnelName, IpAddress& ipAddr, return SAI_NULL_OBJECT_ID; } - SWSS_LOG_NOTICE("NH tunnel create for %s, ip %s, mac %s, vni %d", - tunnelName.c_str(), ipAddr.to_string().c_str(), - macAddress.to_string().c_str(), vni); - auto tunnel_obj = getVxlanTunnel(tunnelName); sai_object_id_t nh_id, tunnel_id = tunnel_obj->getTunnelId(); @@ -1342,6 +1338,10 @@ VxlanTunnelOrch::createNextHopTunnel(string tunnelName, IpAddress& ipAddr, return nh_id; } + SWSS_LOG_NOTICE("NH tunnel create for %s, ip %s, mac %s, vni %d", + tunnelName.c_str(), ipAddr.to_string().c_str(), + macAddress.to_string().c_str(), vni); + sai_ip_address_t host_ip; swss::copy(host_ip, ipAddr); diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 126f4e88c5..3f4752ef74 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -26,6 +26,7 @@ LDADD_GTEST = -L/usr/src/gtest tests_INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I$(top_srcdir)/cfgmgr -I$(top_srcdir)/orchagent -I$(P4_ORCH_DIR)/tests -I$(DASH_ORCH_DIR) -I$(top_srcdir)/warmrestart tests_SOURCES = aclorch_ut.cpp \ + aclorch_rule_ut.cpp \ portsorch_ut.cpp \ routeorch_ut.cpp \ qosorch_ut.cpp \ diff --git a/tests/mock_tests/aclorch_rule_ut.cpp b/tests/mock_tests/aclorch_rule_ut.cpp new file mode 100644 index 0000000000..cd035ae8fc --- /dev/null +++ b/tests/mock_tests/aclorch_rule_ut.cpp @@ -0,0 +1,294 @@ +#include "ut_helper.h" +#include "mock_orchagent_main.h" +#include "mock_sai_api.h" +#include "mock_orch_test.h" +#include "check.h" + +EXTERN_MOCK_FNS + +/* + This test provides a framework to mock create_acl_entry & remove_acl_entry API's +*/ +namespace aclorch_rule_test +{ + DEFINE_SAI_GENERIC_API_MOCK(acl, acl_entry); + /* To mock Redirect Action functionality */ + DEFINE_SAI_GENERIC_API_MOCK(next_hop, next_hop); + + using namespace ::testing; + using namespace std; + using namespace saimeta; + using namespace swss; + using namespace mock_orch_test; + + struct SaiMockState + { + /* Add extra attributes on demand */ + vector create_attrs; + sai_status_t create_status = SAI_STATUS_SUCCESS; + sai_status_t remove_status = SAI_STATUS_SUCCESS; + sai_object_id_t remove_oid; + sai_object_id_t create_oid; + + sai_status_t handleCreate(sai_object_id_t *sai, sai_object_id_t switch_id, uint32_t attr_count, const sai_attribute_t *attr_list) + { + *sai = create_oid; + create_attrs.clear(); + for (uint32_t i = 0; i < attr_count; ++i) + { + create_attrs.emplace_back(attr_list[i]); + } + return create_status; + } + + sai_status_t handleRemove(sai_object_id_t oid) + { + EXPECT_EQ(oid, remove_oid); + return remove_status; + } + }; + + struct AclOrchRuleTest : public MockOrchTest + { + unique_ptr aclMockState; + + void PostSetUp() override + { + INIT_SAI_API_MOCK(acl); + INIT_SAI_API_MOCK(next_hop); + MockSaiApis(); + + aclMockState = make_unique(); + /* Port init done is a pre-req for Aclorch */ + auto consumer = unique_ptr(new Consumer( + new swss::ConsumerStateTable(m_app_db.get(), APP_PORT_TABLE_NAME, 1, 1), gPortsOrch, APP_PORT_TABLE_NAME)); + consumer->addToSync({ { "PortInitDone", EMPTY_PREFIX, { { "", "" } } } }); + static_cast(gPortsOrch)->doTask(*consumer.get()); + } + + void PreTearDown() override + { + aclMockState.reset(); + RestoreSaiApis(); + } + + void doAclTableTypeTask(const deque &entries) + { + auto consumer = unique_ptr(new Consumer( + new swss::ConsumerStateTable(m_config_db.get(), CFG_ACL_TABLE_TYPE_TABLE_NAME, 1, 1), + gAclOrch, CFG_ACL_TABLE_TYPE_TABLE_NAME)); + consumer->addToSync(entries); + static_cast(gAclOrch)->doTask(*consumer); + } + + void doAclTableTask(const deque &entries) + { + auto consumer = unique_ptr(new Consumer( + new swss::ConsumerStateTable(m_config_db.get(), CFG_ACL_TABLE_TABLE_NAME, 1, 1), + gAclOrch, CFG_ACL_TABLE_TABLE_NAME)); + consumer->addToSync(entries); + static_cast(gAclOrch)->doTask(*consumer); + } + + void doAclRuleTask(const deque &entries) + { + auto consumer = unique_ptr(new Consumer( + new swss::ConsumerStateTable(m_config_db.get(), CFG_ACL_RULE_TABLE_NAME, 1, 1), + gAclOrch, CFG_ACL_RULE_TABLE_NAME)); + consumer->addToSync(entries); + static_cast(gAclOrch)->doTask(*consumer); + } + }; + + struct AclRedirectActionTest : public AclOrchRuleTest + { + string acl_table_type = "TEST_ACL_TABLE_TYPE"; + string acl_table = "TEST_ACL_TABLE"; + string acl_rule = "TEST_ACL_RULE"; + + string mock_tunnel_name = "tunnel0"; + string mock_invalid_tunnel_name = "tunnel1"; + string mock_src_ip = "20.0.0.1"; + string mock_nh_ip_str = "20.0.0.3"; + string mock_invalid_nh_ip_str = "20.0.0.4"; + sai_object_id_t nh_oid = 0x400000000064d; + + void PostSetUp() override + { + AclOrchRuleTest::PostSetUp(); + + /* Create a tunnel */ + auto consumer = unique_ptr(new Consumer( + new swss::ConsumerStateTable(m_app_db.get(), APP_VXLAN_TUNNEL_TABLE_NAME, 1, 1), + m_VxlanTunnelOrch, APP_VXLAN_TUNNEL_TABLE_NAME)); + + consumer->addToSync( + deque( + { + { + mock_tunnel_name, + SET_COMMAND, + { + { "src_ip", mock_src_ip } + } + } + } + )); + static_cast(m_VxlanTunnelOrch)->doTask(*consumer.get()); + + populateAclTale(); + setDefaultMockState(); + } + + void PreTearDown() override + { + AclOrchRuleTest::PreTearDown(); + + /* Delete the Tunnel Object */ + auto consumer = unique_ptr(new Consumer( + new swss::ConsumerStateTable(m_app_db.get(), APP_VXLAN_TUNNEL_TABLE_NAME, 1, 1), + m_VxlanTunnelOrch, APP_VXLAN_TUNNEL_TABLE_NAME)); + + consumer->addToSync( + deque( + { + { + mock_tunnel_name, + DEL_COMMAND, + { } + } + } + )); + static_cast(m_VxlanTunnelOrch)->doTask(*consumer.get()); + } + + void createTunnelNH(string ip) + { + IpAddress mock_nh_ip(ip); + ASSERT_EQ(m_VxlanTunnelOrch->createNextHopTunnel(mock_tunnel_name, mock_nh_ip, MacAddress()), nh_oid); + } + + void populateAclTale() + { + /* Create a Table type and Table */ + doAclTableTypeTask({ + { + acl_table_type, + SET_COMMAND, + { + { ACL_TABLE_TYPE_MATCHES, MATCH_DST_IP }, + { ACL_TABLE_TYPE_ACTIONS, ACTION_REDIRECT_ACTION } + } + } + }); + doAclTableTask({ + { + acl_table, + SET_COMMAND, + { + { ACL_TABLE_TYPE, acl_table_type }, + { ACL_TABLE_STAGE, STAGE_INGRESS }, + } + } + }); + } + + void addTunnelNhRule(string ip, string tunnel_name) + { + /* Create a rule */ + doAclRuleTask({ + { + acl_table + "|" + acl_rule, + SET_COMMAND, + { + { RULE_PRIORITY, "9999" }, + { MATCH_DST_IP, "10.0.0.1/24" }, + { ACTION_REDIRECT_ACTION, ip + "@" + tunnel_name } + } + } + }); + } + + void delTunnelNhRule() + { + doAclRuleTask( + { + { + acl_table + "|" + acl_rule, + DEL_COMMAND, + { } + } + }); + } + + void setDefaultMockState() + { + aclMockState->create_status = SAI_STATUS_SUCCESS; + aclMockState->remove_status = SAI_STATUS_SUCCESS; + aclMockState->create_oid = nh_oid; + aclMockState->remove_oid = nh_oid; + } + }; + + TEST_F(AclRedirectActionTest, TunnelNH) + { + EXPECT_CALL(*mock_sai_next_hop_api, create_next_hop).WillOnce(DoAll(SetArgPointee<0>(nh_oid), + Return(SAI_STATUS_SUCCESS) + )); + EXPECT_CALL(*mock_sai_acl_api, create_acl_entry).WillOnce(testing::Invoke(aclMockState.get(), &SaiMockState::handleCreate)); + addTunnelNhRule(mock_nh_ip_str, mock_tunnel_name); + + /* Verify SAI attributes and if the rule is created */ + SaiAttributeList attr_list(SAI_OBJECT_TYPE_ACL_ENTRY, vector({ + { "SAI_ACL_ENTRY_ATTR_TABLE_ID", sai_serialize_object_id(gAclOrch->getTableById(acl_table)) }, + { "SAI_ACL_ENTRY_ATTR_PRIORITY", "9999" }, + { "SAI_ACL_ENTRY_ATTR_ADMIN_STATE", "true" }, + { "SAI_ACL_ENTRY_ATTR_ACTION_COUNTER", "oid:0xfffffffffff"}, + { "SAI_ACL_ENTRY_ATTR_FIELD_DST_IP", "10.0.0.1&mask:255.255.255.0"}, + { "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT", sai_serialize_object_id(nh_oid) } + }), false); + vector skip_list = {false, false, false, true, false, false}; /* skip checking counter */ + ASSERT_TRUE(Check::AttrListSubset(SAI_OBJECT_TYPE_ACL_ENTRY, aclMockState->create_attrs, attr_list, skip_list)); + ASSERT_TRUE(gAclOrch->getAclRule(acl_table, acl_rule)); + + /* ACLRule is deleted along with Nexthop */ + EXPECT_CALL(*mock_sai_next_hop_api, remove_next_hop).Times(1).WillOnce(Return(SAI_STATUS_SUCCESS)); + EXPECT_CALL(*mock_sai_acl_api, remove_acl_entry).WillOnce(testing::Invoke(aclMockState.get(), &SaiMockState::handleRemove)); + delTunnelNhRule(); + ASSERT_FALSE(gAclOrch->getAclRule(acl_table, acl_rule)); + } + + TEST_F(AclRedirectActionTest, TunnelNH_ExistingNhObject) + { + EXPECT_CALL(*mock_sai_next_hop_api, create_next_hop).WillOnce(DoAll(SetArgPointee<0>(nh_oid), + Return(SAI_STATUS_SUCCESS) + )); + EXPECT_CALL(*mock_sai_acl_api, create_acl_entry).WillOnce(testing::Invoke(aclMockState.get(), &SaiMockState::handleCreate)); + createTunnelNH(mock_nh_ip_str); + addTunnelNhRule(mock_nh_ip_str, mock_tunnel_name); + ASSERT_TRUE(gAclOrch->getAclRule(acl_table, acl_rule)); + + /* ACL Rule is deleted but nexthop is not deleted */ + EXPECT_CALL(*mock_sai_acl_api, remove_acl_entry).WillOnce(testing::Invoke(aclMockState.get(), &SaiMockState::handleRemove)); + EXPECT_CALL(*mock_sai_next_hop_api, remove_next_hop).Times(0); + delTunnelNhRule(); + ASSERT_FALSE(gAclOrch->getAclRule(acl_table, acl_rule)); + } + + TEST_F(AclRedirectActionTest, TunnelNH_InvalidTunnel) + { + EXPECT_CALL(*mock_sai_acl_api, create_acl_entry).Times(0); + addTunnelNhRule(mock_nh_ip_str, mock_invalid_tunnel_name); + ASSERT_FALSE(gAclOrch->getAclRule(acl_table, acl_rule)); + } + + TEST_F(AclRedirectActionTest, TunnelNH_InvalidNextHop) + { + EXPECT_CALL(*mock_sai_next_hop_api, create_next_hop).WillOnce( + Return(SAI_STATUS_FAILURE) /* create next hop fails */ + ); + EXPECT_CALL(*mock_sai_acl_api, create_acl_entry).Times(0); + addTunnelNhRule(mock_invalid_nh_ip_str, mock_tunnel_name); + ASSERT_FALSE(gAclOrch->getAclRule(acl_table, acl_rule)); + } +} diff --git a/tests/mock_tests/check.h b/tests/mock_tests/check.h index d1b095562d..a13f2abd3c 100644 --- a/tests/mock_tests/check.h +++ b/tests/mock_tests/check.h @@ -42,40 +42,94 @@ struct Check std::cerr << "Expected: " << meta->attridname << "\n"; } } - continue; } - const int MAX_BUF_SIZE = 0x4000; - std::string act_str; - std::string exp_str; + const sai_attribute_t* act = &act_attr_list[i]; + const sai_attribute_t* exp = &exp_attr_list.get_attr_list()[i]; + if (!Check::AttrValue(objecttype, id, act, exp)) + { + return false; + } + } + + return true; + } + + static bool AttrValue(sai_object_type_t objecttype, sai_attr_id_t id, const sai_attribute_t* act, const sai_attribute_t* exp) + { + auto meta = sai_metadata_get_attr_metadata(objecttype, id); + assert(meta != nullptr); + + const int MAX_BUF_SIZE = 0x4000; + std::vector act_buf(MAX_BUF_SIZE); + std::vector exp_buf(MAX_BUF_SIZE); + + act_buf.reserve(MAX_BUF_SIZE); + exp_buf.reserve(MAX_BUF_SIZE); + + auto act_len = sai_serialize_attribute_value(act_buf.data(), meta, &act->value); + auto exp_len = sai_serialize_attribute_value(exp_buf.data(), meta, &exp->value); - act_str.reserve(MAX_BUF_SIZE); - exp_str.reserve(MAX_BUF_SIZE); + assert(act_len < act_str.size()); + assert(act_len < exp_str.size()); - auto act_len = sai_serialize_attribute_value(&act_str[0], meta, &act_attr_list[i].value); - auto exp_len = sai_serialize_attribute_value(&exp_str[0], meta, &exp_attr_list.get_attr_list()[i].value); + act_buf.resize(act_len); + exp_buf.resize(exp_len); - assert(act_len < act_str.size()); - assert(act_len < exp_str.size()); + std::string act_str(act_buf.begin(), act_buf.end()); + std::string exp_str(exp_buf.begin(), exp_buf.end()); + + if (act_len != exp_len) + { + std::cerr << "AttrValue length failed\n"; + std::cerr << "Actual: " << act_len << "," << act_str << "\n"; + std::cerr << "Expected: " << exp_len << "," << exp_str << "\n"; + return false; + } - if (act_len != exp_len) + if (act_str != exp_str) + { + std::cerr << "AttrValue string failed\n"; + std::cerr << "Actual: " << act_str << "\n"; + std::cerr << "Expected: " << exp_str << "\n"; + return false; + } + return true; + } + + static bool AttrListSubset(sai_object_type_t objecttype, const std::vector &act_attr_list, + saimeta::SaiAttributeList &exp_attr_list, const std::vector skip_check) + { + /* + Size of attributes should be equal and in the same order. + If the validation has to be skipped for certain attributes populate the skip_check. + */ + if (act_attr_list.size() != exp_attr_list.get_attr_count()) + { + std::cerr << "AttrListSubset size mismatch\n"; + return false; + } + if (act_attr_list.size() != skip_check.size()) + { + std::cerr << "AttrListSubset size mismatch\n"; + return false; + } + + for (uint32_t i = 0; i < exp_attr_list.get_attr_count(); ++i) + { + if (skip_check[i]) { - std::cerr << "AttrListEq failed\n"; - std::cerr << "Actual: " << act_str << "\n"; - std::cerr << "Expected: " << exp_str << "\n"; - return false; + continue; } - - if (act_str != exp_str) + sai_attr_id_t id = exp_attr_list.get_attr_list()[i].id; + const sai_attribute_t* act = &act_attr_list[i]; + const sai_attribute_t* exp = &exp_attr_list.get_attr_list()[i]; + if (!Check::AttrValue(objecttype, id, act, exp)) { - std::cerr << "AttrListEq failed\n"; - std::cerr << "Actual: " << act_str << "\n"; - std::cerr << "Expected: " << exp_str << "\n"; return false; } } - return true; } }; From a81dbf0e9ca9bc17f86e9171ca94798a70a0f81d Mon Sep 17 00:00:00 2001 From: Vivek Reddy Date: Thu, 28 Nov 2024 09:34:18 +0000 Subject: [PATCH 2/3] Fix wrong API invocation bug in SAI mocking infra Signed-off-by: Vivek Reddy --- tests/mock_tests/aclorch_rule_ut.cpp | 2 ++ tests/mock_tests/mock_sai_api.h | 9 +++++++++ tests/mock_tests/mux_rollback_ut.cpp | 4 ++++ 3 files changed, 15 insertions(+) diff --git a/tests/mock_tests/aclorch_rule_ut.cpp b/tests/mock_tests/aclorch_rule_ut.cpp index cd035ae8fc..4e24a5651a 100644 --- a/tests/mock_tests/aclorch_rule_ut.cpp +++ b/tests/mock_tests/aclorch_rule_ut.cpp @@ -70,6 +70,8 @@ namespace aclorch_rule_test { aclMockState.reset(); RestoreSaiApis(); + DEINIT_SAI_API_MOCK(next_hop); + DEINIT_SAI_API_MOCK(acl); } void doAclTableTypeTask(const deque &entries) diff --git a/tests/mock_tests/mock_sai_api.h b/tests/mock_tests/mock_sai_api.h index 58e3c9da23..2903a95fde 100644 --- a/tests/mock_tests/mock_sai_api.h +++ b/tests/mock_tests/mock_sai_api.h @@ -11,6 +11,7 @@ To mock a particular SAI API: 3. In the SetUp method of the test class, call INIT_SAI_API_MOCK for each SAI API you want to mock. 4. In the SetUp method of the test class, call MockSaiApis. 5. In the TearDown method of the test class, call RestoreSaiApis. +6. After RestoreSaiApis, call DEINIT_SAI_API_MOCK */ using ::testing::Return; @@ -168,6 +169,14 @@ The macro DEFINE_SAI_API_MOCK will perform the steps to mock the SAI API for the apply_mock_fns.insert(&apply_sai_##sai_object_type##_api_mock); \ remove_mock_fns.insert(&remove_sai_##sai_object_type##_api_mock); +/* + Call this after RestoreSaiApis to clear the mock_fns + Required when same SAI_API is being mocked in multiple files eg: acl API in multiple tests +*/ +#define DEINIT_SAI_API_MOCK(sai_object_type) \ + apply_mock_fns.erase(&apply_sai_##sai_object_type##_api_mock); \ + remove_mock_fns.erase(&remove_sai_##sai_object_type##_api_mock); + void MockSaiApis(); void RestoreSaiApis(); #endif \ No newline at end of file diff --git a/tests/mock_tests/mux_rollback_ut.cpp b/tests/mock_tests/mux_rollback_ut.cpp index 52aa29d24e..6b5eaee073 100644 --- a/tests/mock_tests/mux_rollback_ut.cpp +++ b/tests/mock_tests/mux_rollback_ut.cpp @@ -156,6 +156,10 @@ namespace mux_rollback_test void PreTearDown() override { RestoreSaiApis(); + DEINIT_SAI_API_MOCK(next_hop); + DEINIT_SAI_API_MOCK(acl); + DEINIT_SAI_API_MOCK(route); + DEINIT_SAI_API_MOCK(neighbor); gNeighOrch->gNeighBulker.create_entries = old_create_neighbor_entries; gNeighOrch->gNeighBulker.remove_entries = old_remove_neighbor_entries; m_MuxCable->nbr_handler_->gRouteBulker.create_entries = old_create_route_entries; From 03c70410ad70b4cfd2e391bf0bd5e5a0f055b275 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Date: Thu, 28 Nov 2024 21:50:56 +0000 Subject: [PATCH 3/3] Fix armhf build failure Signed-off-by: Vivek Reddy --- orchagent/aclorch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 5718d16fc3..abe7290e09 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2060,7 +2060,7 @@ sai_object_id_t AclRulePacket::getRedirectObjectId(const string& redirect_value) m_redirect_target_tun_nh.load(target); if (SAI_NULL_OBJECT_ID != m_redirect_target_tun_nh.oid) { - SWSS_LOG_INFO("Tunnel Next Hop Found: oid:%ld, target: %s", m_redirect_target_tun_nh.oid, target.c_str()); + SWSS_LOG_INFO("Tunnel Next Hop Found: oid:0x%" PRIx64 ", target: %s", m_redirect_target_tun_nh.oid, target.c_str()); return m_redirect_target_tun_nh.oid; } }