From c42cbffe832b2f047231164a8723e81e60969f6f Mon Sep 17 00:00:00 2001 From: Saikrishna Arcot Date: Tue, 22 Oct 2024 15:33:57 -0700 Subject: [PATCH] Fix p4orch tests after SAI update (#3337) * 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). --- orchagent/Makefile.am | 2 -- orchagent/p4orch/tests/Makefile.am | 33 +++----------------- orchagent/p4orch/tests/acl_manager_test.cpp | 8 +++++ orchagent/p4orch/tests/wcmp_manager_test.cpp | 6 ++-- 4 files changed, 16 insertions(+), 33 deletions(-) diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 5e24c7a6a6..b073932f43 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -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 diff --git a/orchagent/p4orch/tests/Makefile.am b/orchagent/p4orch/tests/Makefile.am index 2c7301311f..74e3d7aa5c 100644 --- a/orchagent/p4orch/tests/Makefile.am +++ b/orchagent/p4orch/tests/Makefile.am @@ -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 @@ -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 \ @@ -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 diff --git a/orchagent/p4orch/tests/acl_manager_test.cpp b/orchagent/p4orch/tests/acl_manager_test.cpp index 13856c07ea..18128c2705 100644 --- a/orchagent/p4orch/tests/acl_manager_test.cpp +++ b/orchagent/p4orch/tests/acl_manager_test.cpp @@ -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()); @@ -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()); @@ -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()); @@ -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()); diff --git a/orchagent/p4orch/tests/wcmp_manager_test.cpp b/orchagent/p4orch/tests/wcmp_manager_test.cpp index 088264bba4..9e0c575cfd 100644 --- a/orchagent/p4orch/tests/wcmp_manager_test.cpp +++ b/orchagent/p4orch/tests/wcmp_manager_test.cpp @@ -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); @@ -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, @@ -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);