Skip to content

Commit

Permalink
[P4Orch] Lazy UDF match creation to avoid failure during warm reboot (s…
Browse files Browse the repository at this point in the history
…onic-net#2282)

* [P4Orch] Lazy UDF match creation.
  • Loading branch information
baxia-lan authored May 26, 2022
1 parent d7b5ff7 commit 583236f
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 28 deletions.
18 changes: 10 additions & 8 deletions orchagent/p4orch/acl_table_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
{
Expand Down Expand Up @@ -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<sai_attribute_t> udf_attrs;
sai_attribute_t udf_attr;
Expand Down
20 changes: 18 additions & 2 deletions orchagent/p4orch/tests/acl_manager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> p4_tables;
gP4Orch = new P4Orch(gAppDb, p4_tables, gVrfOrch, copp_orch_);
acl_table_manager_ = gP4Orch->getAclTableManager();
Expand All @@ -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)));
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
11 changes: 0 additions & 11 deletions orchagent/p4orch/tests/wcmp_manager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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_;
}
Expand All @@ -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;
Expand All @@ -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()
Expand All @@ -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<std::string> p4_tables;
gP4Orch = new P4Orch(gAppDb, p4_tables, gVrfOrch, copp_orch_);
}
Expand Down Expand Up @@ -301,7 +291,6 @@ class WcmpManagerTest : public ::testing::Test
StrictMock<MockSaiHostif> mock_sai_hostif_;
StrictMock<MockSaiSerialize> mock_sai_serialize_;
StrictMock<MockSaiAcl> mock_sai_acl_;
StrictMock<MockSaiUdf> mock_sai_udf_;
P4OidMapper *p4_oid_mapper_;
WcmpManager *wcmp_group_manager_;
CoppOrch *copp_orch_;
Expand Down
12 changes: 6 additions & 6 deletions tests/p4rt/test_l3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
11 changes: 10 additions & 1 deletion tests/p4rt/test_p4rt_acl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 583236f

Please sign in to comment.