diff --git a/orchagent/p4orch/acl_table_manager.cpp b/orchagent/p4orch/acl_table_manager.cpp index 456c2f04d253..312f54c51cac 100644 --- a/orchagent/p4orch/acl_table_manager.cpp +++ b/orchagent/p4orch/acl_table_manager.cpp @@ -36,16 +36,15 @@ AclTableManager::AclTableManager(P4OidMapper *p4oidMapper, ResponsePublisherInte SWSS_LOG_ENTER(); assert(p4oidMapper != nullptr); - // Create the default UDF match - auto status = createDefaultUdfMatch(); - if (!status.ok()) - { - SWSS_LOG_ERROR("Failed to create ACL UDF default match : %s", status.message().c_str()); - } } AclTableManager::~AclTableManager() { + sai_object_id_t udf_match_oid; + if (!m_p4OidMapper->getOID(SAI_OBJECT_TYPE_UDF_MATCH, P4_UDF_MATCH_DEFAULT, &udf_match_oid)) + { + return; + } auto status = removeDefaultUdfMatch(); if (!status.ok()) { @@ -465,8 +464,11 @@ ReturnCode AclTableManager::createUdf(const P4UdfField &udf_field) sai_object_id_t udf_match_oid; if (!m_p4OidMapper->getOID(SAI_OBJECT_TYPE_UDF_MATCH, P4_UDF_MATCH_DEFAULT, &udf_match_oid)) { - return ReturnCode(StatusCode::SWSS_RC_NOT_FOUND) - << "UDF default match " << QuotedVar(P4_UDF_MATCH_DEFAULT) << " does not exist"; + // Create the default UDF match + LOG_AND_RETURN_IF_ERROR(createDefaultUdfMatch() + << "Failed to create ACL UDF default match " + << QuotedVar(P4_UDF_MATCH_DEFAULT)); + m_p4OidMapper->getOID(SAI_OBJECT_TYPE_UDF_MATCH, P4_UDF_MATCH_DEFAULT, &udf_match_oid); } std::vector udf_attrs; sai_attribute_t udf_attr; diff --git a/orchagent/p4orch/tests/acl_manager_test.cpp b/orchagent/p4orch/tests/acl_manager_test.cpp index 64ba37e5a3d3..55abf24606de 100644 --- a/orchagent/p4orch/tests/acl_manager_test.cpp +++ b/orchagent/p4orch/tests/acl_manager_test.cpp @@ -834,8 +834,6 @@ class AclManagerTest : public ::testing::Test Truly(std::bind(MatchSaiSwitchAttrByAclStage, SAI_SWITCH_ATTR_PRE_INGRESS_ACL, kAclGroupLookupOid, std::placeholders::_1)))) .WillRepeatedly(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) - .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); std::vector p4_tables; gP4Orch = new P4Orch(gAppDb, p4_tables, gVrfOrch, copp_orch_); acl_table_manager_ = gP4Orch->getAclTableManager(); @@ -860,6 +858,8 @@ class AclManagerTest : public ::testing::Test .WillOnce(DoAll(SetArgPointee<0>(kAclTableIngressOid), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_acl_, create_acl_table_group_member(_, _, _, _)) .WillOnce(DoAll(SetArgPointee<0>(kAclGroupMemberIngressOid), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)) .Times(3) .WillRepeatedly(DoAll(SetArgPointee<0>(kUdfGroupOid1), Return(SAI_STATUS_SUCCESS))); @@ -1156,6 +1156,8 @@ TEST_F(AclManagerTest, CreateIngressPuntTableFailsWhenCapabilityExceeds) auto app_db_entry = getDefaultAclTableDefAppDbEntry(); sai_object_id_t user_defined_trap_oid = gUserDefinedTrapStartOid; AddDefaultUserTrapsSaiCalls(&user_defined_trap_oid); + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)) .WillRepeatedly(DoAll(SetArgPointee<0>(kUdfGroupOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf(_, _, _, _)).Times(3).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); @@ -1170,6 +1172,8 @@ TEST_F(AclManagerTest, CreateIngressPuntTableFailsWhenFailedToCreateTableGroupMe auto app_db_entry = getDefaultAclTableDefAppDbEntry(); sai_object_id_t user_defined_trap_oid = gUserDefinedTrapStartOid; AddDefaultUserTrapsSaiCalls(&user_defined_trap_oid); + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)) .WillRepeatedly(DoAll(SetArgPointee<0>(kUdfGroupOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf(_, _, _, _)).Times(3).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); @@ -1187,6 +1191,8 @@ TEST_F(AclManagerTest, CreateIngressPuntTableRaisesCriticalStateWhenAclTableReco auto app_db_entry = getDefaultAclTableDefAppDbEntry(); sai_object_id_t user_defined_trap_oid = gUserDefinedTrapStartOid; AddDefaultUserTrapsSaiCalls(&user_defined_trap_oid); + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)) .WillRepeatedly(DoAll(SetArgPointee<0>(kUdfGroupOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf(_, _, _, _)).Times(3).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); @@ -1205,6 +1211,8 @@ TEST_F(AclManagerTest, CreateIngressPuntTableRaisesCriticalStateWhenUdfGroupReco auto app_db_entry = getDefaultAclTableDefAppDbEntry(); sai_object_id_t user_defined_trap_oid = gUserDefinedTrapStartOid; AddDefaultUserTrapsSaiCalls(&user_defined_trap_oid); + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)) .WillRepeatedly(DoAll(SetArgPointee<0>(kUdfGroupOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf(_, _, _, _)).Times(3).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); @@ -1223,6 +1231,8 @@ TEST_F(AclManagerTest, CreateIngressPuntTableRaisesCriticalStateWhenUdfRecoveryF auto app_db_entry = getDefaultAclTableDefAppDbEntry(); sai_object_id_t user_defined_trap_oid = gUserDefinedTrapStartOid; AddDefaultUserTrapsSaiCalls(&user_defined_trap_oid); + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)) .WillRepeatedly(DoAll(SetArgPointee<0>(kUdfGroupOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf(_, _, _, _)).Times(3).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); @@ -1244,6 +1254,8 @@ TEST_F(AclManagerTest, CreateIngressPuntTableFailsWhenFailedToCreateUdf) AddDefaultUserTrapsSaiCalls(&user_defined_trap_oid); // Fail to create the first UDF, and success to remove the first UDF // group + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)).WillOnce(Return(SAI_STATUS_SUCCESS)); EXPECT_CALL(mock_sai_udf_, create_udf(_, _, _, _)).WillOnce(Return(SAI_STATUS_FAILURE)); EXPECT_CALL(mock_sai_udf_, remove_udf_group(_)).WillOnce(Return(SAI_STATUS_SUCCESS)); @@ -2099,6 +2111,8 @@ TEST_F(AclManagerTest, DrainRuleTuplesToProcessSetRequestSucceeds) .WillOnce(DoAll(SetArgPointee<0>(kAclTableIngressOid), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_acl_, create_acl_table_group_member(_, _, _, _)) .WillOnce(DoAll(SetArgPointee<0>(kAclGroupMemberIngressOid), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)) .WillRepeatedly(DoAll(SetArgPointee<0>(kUdfGroupOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf(_, _, _, _)).Times(3).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); @@ -4055,6 +4069,8 @@ TEST_F(AclManagerTest, DoAclCounterStatsTaskSucceeds) .WillOnce(DoAll(SetArgPointee<0>(kAclTableIngressOid), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_acl_, create_acl_table_group_member(_, _, _, _)) .WillOnce(DoAll(SetArgPointee<0>(kAclGroupMemberIngressOid), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) + .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf_group(_, _, _, _)) .WillRepeatedly(DoAll(SetArgPointee<0>(kUdfGroupOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_udf_, create_udf(_, _, _, _)).Times(3).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); diff --git a/orchagent/p4orch/tests/wcmp_manager_test.cpp b/orchagent/p4orch/tests/wcmp_manager_test.cpp index 73cf34be25fb..b2cc68aaaa70 100644 --- a/orchagent/p4orch/tests/wcmp_manager_test.cpp +++ b/orchagent/p4orch/tests/wcmp_manager_test.cpp @@ -12,7 +12,6 @@ #include "mock_sai_next_hop_group.h" #include "mock_sai_serialize.h" #include "mock_sai_switch.h" -#include "mock_sai_udf.h" #include "p4oidmapper.h" #include "p4orch.h" #include "p4orch/p4orch_util.h" @@ -31,7 +30,6 @@ extern sai_object_id_t gSwitchId; extern sai_next_hop_group_api_t *sai_next_hop_group_api; extern sai_hostif_api_t *sai_hostif_api; extern sai_switch_api_t *sai_switch_api; -extern sai_udf_api_t *sai_udf_api; extern sai_object_id_t gSwitchId; extern sai_acl_api_t *sai_acl_api; @@ -68,7 +66,6 @@ const std::string kWcmpGroupKey1 = KeyGenerator::generateWcmpGroupKey(kWcmpGroup const std::string kNexthopKey1 = KeyGenerator::generateNextHopKey(kNexthopId1); const std::string kNexthopKey2 = KeyGenerator::generateNextHopKey(kNexthopId2); const std::string kNexthopKey3 = KeyGenerator::generateNextHopKey(kNexthopId3); -constexpr sai_object_id_t kUdfMatchOid1 = 5001; // Matches the next hop group type sai_attribute_t argument. bool MatchSaiNextHopGroupAttribute(const sai_attribute_t *attr) @@ -154,7 +151,6 @@ class WcmpManagerTest : public ::testing::Test EXPECT_CALL(mock_sai_switch_, set_switch_attribute(Eq(gSwitchId), _)) .WillRepeatedly(Return(SAI_STATUS_SUCCESS)); EXPECT_CALL(mock_sai_acl_, remove_acl_table_group(_)).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_udf_, remove_udf_match(_)).WillRepeatedly(Return(SAI_STATUS_SUCCESS)); delete gP4Orch; delete copp_orch_; } @@ -167,7 +163,6 @@ class WcmpManagerTest : public ::testing::Test mock_sai_hostif = &mock_sai_hostif_; mock_sai_serialize = &mock_sai_serialize_; mock_sai_acl = &mock_sai_acl_; - mock_sai_udf = &mock_sai_udf_; sai_next_hop_group_api->create_next_hop_group = create_next_hop_group; sai_next_hop_group_api->remove_next_hop_group = remove_next_hop_group; @@ -181,8 +176,6 @@ class WcmpManagerTest : public ::testing::Test sai_switch_api->set_switch_attribute = mock_set_switch_attribute; sai_acl_api->create_acl_table_group = create_acl_table_group; sai_acl_api->remove_acl_table_group = remove_acl_table_group; - sai_udf_api->create_udf_match = create_udf_match; - sai_udf_api->remove_udf_match = remove_udf_match; } void setUpP4Orch() @@ -194,9 +187,6 @@ class WcmpManagerTest : public ::testing::Test copp_orch_ = new CoppOrch(gAppDb, APP_COPP_TABLE_NAME); // init P4 orch - EXPECT_CALL(mock_sai_udf_, create_udf_match(_, _, _, _)) - .WillOnce(DoAll(SetArgPointee<0>(kUdfMatchOid1), Return(SAI_STATUS_SUCCESS))); - std::vector p4_tables; gP4Orch = new P4Orch(gAppDb, p4_tables, gVrfOrch, copp_orch_); } @@ -301,7 +291,6 @@ class WcmpManagerTest : public ::testing::Test StrictMock mock_sai_hostif_; StrictMock mock_sai_serialize_; StrictMock mock_sai_acl_; - StrictMock mock_sai_udf_; P4OidMapper *p4_oid_mapper_; WcmpManager *wcmp_group_manager_; CoppOrch *copp_orch_; diff --git a/tests/p4rt/test_l3.py b/tests/p4rt/test_l3.py index 4156576bc2a9..bbe7d0765335 100644 --- a/tests/p4rt/test_l3.py +++ b/tests/p4rt/test_l3.py @@ -262,7 +262,7 @@ def test_IPv4RouteWithNexthopAddUpdateDeletePass(self, dvs, testlog): # Verify that P4RT key to OID count is same as the original count. status, fvs = key_to_oid_helper.get_db_info() - assert status == True + assert status == False assert len(fvs) == len(original_key_oid_info) # Query application database for route entries. @@ -651,7 +651,7 @@ def test_IPv6WithWcmpRouteAddUpdateDeletePass(self, dvs, testlog): # Verify that P4RT key to OID count is same as original count. status, fvs = key_to_oid_helper.get_db_info() - assert status == True + assert status == False assert len(fvs) == len(original_key_oid_info) # Query application database for route entries. @@ -1148,7 +1148,7 @@ def test_PruneAndRestoreNextHop(self, dvs, testlog): # Verify that P4RT key to OID count is same as the original count. status, fvs = key_to_oid_helper.get_db_info() - assert status == True + assert status == False assert len(fvs) == len(original_key_oid_info) def test_PruneNextHopOnWarmBoot(self, dvs, testlog): @@ -1386,7 +1386,7 @@ def test_PruneNextHopOnWarmBoot(self, dvs, testlog): # Verify that P4RT key to OID count is same as the original count. status, fvs = key_to_oid_helper.get_db_info() - assert status == True + assert status == False assert len(fvs) == len(original_key_oid_info) def test_CreateWcmpMemberForOperUpWatchportOnly(self, dvs, testlog): @@ -1620,7 +1620,7 @@ def test_CreateWcmpMemberForOperUpWatchportOnly(self, dvs, testlog): # Verify that P4RT key to OID count is same as the original count. status, fvs = key_to_oid_helper.get_db_info() - assert status == True + assert status == False assert len(fvs) == len(original_key_oid_info) def test_RemovePrunedWcmpGroupMember(self, dvs, testlog): @@ -1841,5 +1841,5 @@ def test_RemovePrunedWcmpGroupMember(self, dvs, testlog): # Verify that P4RT key to OID count is same as the original count. status, fvs = key_to_oid_helper.get_db_info() - assert status == True + assert status == False assert len(fvs) == len(original_key_oid_info) diff --git a/tests/p4rt/test_p4rt_acl.py b/tests/p4rt/test_p4rt_acl.py index 89015fc9d5ee..52989e5b7225 100644 --- a/tests/p4rt/test_p4rt_acl.py +++ b/tests/p4rt/test_p4rt_acl.py @@ -257,8 +257,17 @@ def test_AclRulesAddUpdateDelPass(self, dvs, testlog): assert status == True util.verify_attr(fvs, attr_list) + asic_udf_matches = util.get_keys( + self._p4rt_udf_match_obj.asic_db, self._p4rt_udf_match_obj.ASIC_DB_TBL_NAME + ) + # query ASIC database for default UDF wildcard match - udf_match_asic_db_key = original_asic_udf_matches[0] + udf_match_asic_db_keys = [ + key for key in asic_udf_matches if key not in original_asic_udf_matches + ] + + assert len(udf_match_asic_db_keys) == 1 + udf_match_asic_db_key = udf_match_asic_db_keys[0] (status, fvs) = util.get_key( self._p4rt_udf_match_obj.asic_db,