Skip to content

Commit

Permalink
Fix p4orch tests after SAI update (#3337)
Browse files Browse the repository at this point in the history
* Make sure p4orch tests always run, not just when gcov is enabled
* Update JSON fields in p4orch for latest SAI
* Disable the disabled-optimization for 2 ACL Manager tests in p4orch

2 test functions in acl_manager_test.cpp for p4orch are near 400 lines,
with macro expansion almost certainly adding a number of lines. On armhf
and arm64, GCC is complaining that it ran out of memory to do
optimizations here (specifically, keep track of const and copies).
  • Loading branch information
saiarcot895 authored Oct 22, 2024
1 parent 90fcead commit e71eb2d
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 33 deletions.
2 changes: 0 additions & 2 deletions orchagent/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ INCLUDES = -I $(top_srcdir)/lib \
-I pbh \
-I nhg

if GCOV_ENABLED
SUBDIRS = p4orch/tests
endif

CFLAGS_SAI = -I /usr/include/sai

Expand Down
33 changes: 5 additions & 28 deletions orchagent/p4orch/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ INCLUDES = -I $(top_srcdir) -I $(ORCHAGENT_DIR) -I $(P4ORCH_DIR) -I $(top_srcdir

CFLAGS_SAI = -I /usr/include/sai

TESTS = p4orch_tests p4orch_tests_asan p4orch_tests_tsan p4orch_tests_usan
TESTS = p4orch_tests

noinst_PROGRAMS = p4orch_tests p4orch_tests_asan p4orch_tests_tsan p4orch_tests_usan
noinst_PROGRAMS = p4orch_tests

if DEBUG
DBGFLAGS = -ggdb -DDEBUG
Expand All @@ -16,11 +16,6 @@ endif

CFLAGS_GTEST =
LDADD_GTEST = -lgtest -lgtest_main -lgmock -lgmock_main
CFLAGS_COVERAGE = --coverage -fprofile-arcs -ftest-coverage
LDADD_COVERAGE = -lgcov
CFLAGS_ASAN = -fsanitize=address
CFLAGS_TSAN = -fsanitize=thread
CFLAGS_USAN = -fsanitize=undefined

p4orch_tests_SOURCES = $(ORCHAGENT_DIR)/orch.cpp \
$(ORCHAGENT_DIR)/vrforch.cpp \
Expand Down Expand Up @@ -83,24 +78,6 @@ p4orch_tests_SOURCES = $(ORCHAGENT_DIR)/orch.cpp \
mock_sai_switch.cpp \
mock_sai_udf.cpp

p4orch_tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_COVERAGE) $(CFLAGS_SAI)
p4orch_tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_COVERAGE) $(CFLAGS_SAI)
p4orch_tests_LDADD = $(LDADD_GTEST) $(LDADD_COVERAGE) -lpthread -lsairedis -lswsscommon -lsaimeta -lsaimetadata -lzmq

p4orch_tests_asan_SOURCES = $(p4orch_tests_SOURCES)
p4orch_tests_asan_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_ASAN) $(CFLAGS_SAI)
p4orch_tests_asan_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_ASAN) $(CFLAGS_SAI)
p4orch_tests_asan_LDFLAGS = $(CFLAGS_ASAN)
p4orch_tests_asan_LDADD = $(LDADD_GTEST) -lpthread -lsairedis -lswsscommon -lsaimeta -lsaimetadata -lzmq

p4orch_tests_tsan_SOURCES = $(p4orch_tests_SOURCES)
p4orch_tests_tsan_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_TSAN) $(CFLAGS_SAI)
p4orch_tests_tsan_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_TSAN) $(CFLAGS_SAI)
p4orch_tests_tsan_LDFLAGS = $(CFLAGS_TSAN)
p4orch_tests_tsan_LDADD = $(LDADD_GTEST) -lpthread -lsairedis -lswsscommon -lsaimeta -lsaimetadata -lzmq

p4orch_tests_usan_SOURCES = $(p4orch_tests_SOURCES)
p4orch_tests_usan_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_USAN) $(CFLAGS_SAI)
p4orch_tests_usan_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_USAN) $(CFLAGS_SAI)
p4orch_tests_usan_LDFLAGS = $(CFLAGS_USAN)
p4orch_tests_usan_LDADD = $(LDADD_GTEST) -lpthread -lsairedis -lswsscommon -lsaimeta -lsaimetadata -lzmq
p4orch_tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(CFLAGS_ASAN)
p4orch_tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(CFLAGS_ASAN)
p4orch_tests_LDADD = $(LDADD_GTEST) $(LDFLAGS_ASAN) -lpthread -lsairedis -lswsscommon -lsaimeta -lsaimetadata -lzmq
8 changes: 8 additions & 0 deletions orchagent/p4orch/tests/acl_manager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2849,6 +2849,8 @@ TEST_F(AclManagerTest, AclRuleWithColorPacketActionsButNoRateLimit)
acl_rule->action_fvs[SAI_ACL_ENTRY_ATTR_ACTION_SET_USER_TRAP_ID].aclaction.parameter.oid);
}

#pragma GCC diagnostic warning "-Wdisabled-optimization"

TEST_F(AclManagerTest, AclRuleWithValidAction)
{
ASSERT_NO_FATAL_FAILURE(AddDefaultIngressTable());
Expand Down Expand Up @@ -3201,6 +3203,8 @@ TEST_F(AclManagerTest, AclRuleWithValidAction)
EXPECT_EQ(nullptr, GetAclRule(kAclIngressTableName, acl_rule_key));
}

#pragma GCC diagnostic pop

TEST_F(AclManagerTest, AclRuleWithVrfAction)
{
ASSERT_NO_FATAL_FAILURE(AddDefaultIngressTable());
Expand Down Expand Up @@ -3410,6 +3414,8 @@ TEST_F(AclManagerTest, AclRuleWithIpTypeBitEncoding)
ASSERT_EQ(nullptr, acl_rule);
}

#pragma GCC diagnostic warning "-Wdisabled-optimization"

TEST_F(AclManagerTest, UpdateAclRuleWithActionMeterChange)
{
ASSERT_NO_FATAL_FAILURE(AddDefaultIngressTable());
Expand Down Expand Up @@ -3834,6 +3840,8 @@ TEST_F(AclManagerTest, UpdateAclRuleWithActionMeterChange)
acl_rule->action_fvs[SAI_ACL_ENTRY_ATTR_ACTION_SET_USER_TRAP_ID].aclaction.parameter.oid);
}

#pragma GCC diagnostic pop

TEST_F(AclManagerTest, UpdateAclRuleWithVrfActionChange)
{
ASSERT_NO_FATAL_FAILURE(AddDefaultIngressTable());
Expand Down
6 changes: 3 additions & 3 deletions orchagent/p4orch/tests/wcmp_manager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2291,7 +2291,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangetoOperDownSucceeds)
// Verify that the next hop member associated with the port is pruned.
std::string op = "port_state_change";
std::string data = "[{\"port_id\":\"oid:0x56789abcdff\",\"port_state\":\"SAI_PORT_OPER_"
"STATUS_DOWN\"}]";
"STATUS_DOWN\",\"port_error_status\":\"0\"}]";
EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1)))
.WillOnce(Return(SAI_STATUS_SUCCESS));
HandlePortStatusChangeNotification(op, data);
Expand All @@ -2314,7 +2314,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangeToOperUpSucceeds)
// restored.
std::string op = "port_state_change";
std::string data = "[{\"port_id\":\"oid:0x112233\",\"port_state\":\"SAI_PORT_OPER_"
"STATUS_UP\"}]";
"STATUS_UP\",\"port_error_status\":\"0\"}]";
EXPECT_CALL(mock_sai_next_hop_group_,
create_next_hop_group_member(_, Eq(gSwitchId), Eq(3),
Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2,
Expand All @@ -2339,7 +2339,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangeFromOperUnknownToDownPrunesMemberOnl
// Verify that the pruned next hop member is not pruned again.
std::string op = "port_state_change";
std::string data = "[{\"port_id\":\"oid:0x56789abcfff\",\"port_state\":\"SAI_PORT_OPER_"
"STATUS_DOWN\"}]";
"STATUS_DOWN\",\"port_error_status\":\"0\"}]";
HandlePortStatusChangeNotification(op, data);
EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1));
EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned);
Expand Down

0 comments on commit e71eb2d

Please sign in to comment.