diff --git a/fboss/agent/hw/sai/hw_test/SaiAclTableGroupTests.cpp b/fboss/agent/hw/sai/hw_test/SaiAclTableGroupTests.cpp index 7fd9fce0b1ccb..2e1f139837c9f 100644 --- a/fboss/agent/hw/sai/hw_test/SaiAclTableGroupTests.cpp +++ b/fboss/agent/hw/sai/hw_test/SaiAclTableGroupTests.cpp @@ -356,7 +356,10 @@ class SaiAclTableGroupTest : public HwTest { // Table 2: Create TTL acl Table follwed by entry. // This utlity call adds the TTL Acl entry as well. - utility::addTtlAclTable(newCfg, 2 /* priority */); + utility::addTtlAclTable( + newCfg, + 2 /* priority */, + isSupported(HwAsic::Feature::ACL_BYTE_COUNTER)); applyNewConfig(*newCfg); } @@ -403,7 +406,10 @@ class SaiAclTableGroupTest : public HwTest { case tableAddType::table2: utility::addAclTableGroup(&newCfg, kAclStage(), kAclTableGroup()); // Add Table 2: TtlTable - utility::addTtlAclTable(&newCfg, 2 /* priority */); + utility::addTtlAclTable( + &newCfg, + 2 /* priority */, + isSupported(HwAsic::Feature::ACL_BYTE_COUNTER)); break; case tableAddType::tableBoth: addTwoAclTables(&newCfg); @@ -495,7 +501,11 @@ class SaiAclTableGroupTest : public HwTest { utility::addAclTableGroup(&newCfg, kAclStage(), kAclTableGroup()); addAclTable3WithEntry(&newCfg, addExtraQualifier, false); - utility::addTtlAclTable(&newCfg, 2 /* priority */, addExtraQualifier); + utility::addTtlAclTable( + &newCfg, + 2 /* priority */, + isSupported(HwAsic::Feature::ACL_BYTE_COUNTER), + addExtraQualifier); return newCfg; } diff --git a/fboss/agent/test/agent_hw_tests/AgentAclCounterTests.cpp b/fboss/agent/test/agent_hw_tests/AgentAclCounterTests.cpp index 728cafd5ffd4f..310b09e69457f 100644 --- a/fboss/agent/test/agent_hw_tests/AgentAclCounterTests.cpp +++ b/fboss/agent/test/agent_hw_tests/AgentAclCounterTests.cpp @@ -490,6 +490,10 @@ class AgentAclCounterTest : public AgentHwTest { auto* acl = &aclEntry; auto l3Asics = getAgentEnsemble()->getL3Asics(); auto asic = checkSameAndGetAsicForTesting(l3Asics); + std::vector counterTypes{cfg::CounterType::PACKETS}; + if (asic->isSupported(HwAsic::Feature::ACL_BYTE_COUNTER)) { + counterTypes.push_back(cfg::CounterType::BYTES); + } bool isSai = getAgentEnsemble()->isSai(); switch (aclType) { case AclType::TCP_TTLD: @@ -523,8 +527,6 @@ class AgentAclCounterTest : public AgentHwTest { // and v6 traffic. The test sends IPv6 traffic, so the IPv6 copy // keeps the canonical name/counter (the one verifyAclType checks); // the IPv4 copy gets a "-v4" suffix. - std::vector counterTypes{ - cfg::CounterType::PACKETS, cfg::CounterType::BYTES}; auto addSrcPortEntry = [&](const std::string& name, const std::string& counter, cfg::EtherType etherType, @@ -595,9 +597,7 @@ class AgentAclCounterTest : public AgentHwTest { } utility::addAcl(config, aclEntry, cfg::AclStage::INGRESS); - std::vector setCounterTypes{ - cfg::CounterType::PACKETS, cfg::CounterType::BYTES}; - utility::addAclStat(config, aclName, counterName, setCounterTypes); + utility::addAclStat(config, aclName, counterName, counterTypes); } std::unique_ptr helper_; diff --git a/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTests.cpp b/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTests.cpp index 55b5cc8fa9ea7..5e04017493053 100644 --- a/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTests.cpp +++ b/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTests.cpp @@ -290,7 +290,10 @@ class AgentAclTableGroupTest : public AgentHwTest { void addTwoAclTables(cfg::SwitchConfig* newCfg) { utility::addAclTableGroup(newCfg, kAclStage(), kAclTableGroup()); addAclTable3WithEntry(newCfg); - utility::addTtlAclTable(newCfg, 2 /* priority */); + utility::addTtlAclTable( + newCfg, + 2 /* priority */, + isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)); applyNewConfig(*newCfg); } @@ -325,7 +328,10 @@ class AgentAclTableGroupTest : public AgentHwTest { break; case tableAddType::table2: utility::addAclTableGroup(&newCfg, kAclStage(), kAclTableGroup()); - utility::addTtlAclTable(&newCfg, 2 /* priority */); + utility::addTtlAclTable( + &newCfg, + 2 /* priority */, + isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)); break; case tableAddType::tableBoth: addTwoAclTables(&newCfg); @@ -415,7 +421,11 @@ class AgentAclTableGroupTest : public AgentHwTest { auto newCfg = initialConfig(ensemble); utility::addAclTableGroup(&newCfg, kAclStage(), kAclTableGroup()); addAclTable3WithEntry(&newCfg, addExtraQualifier, false); - utility::addTtlAclTable(&newCfg, 2 /* priority */, addExtraQualifier); + utility::addTtlAclTable( + &newCfg, + 2 /* priority */, + isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER), + addExtraQualifier); return newCfg; } @@ -671,7 +681,10 @@ TEST_F(AgentAclTableGroupTest, AddTwoTablesDeleteAddSecond) { auto newCfg = initialConfig(ensemble); addTwoAclTables(&newCfg); deleteTtlAclTable(&newCfg); - utility::addTtlAclTable(&newCfg, 2 /* priority */); + utility::addTtlAclTable( + &newCfg, + 2 /* priority */, + isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)); applyNewConfig(newCfg); }; diff --git a/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTrafficTests.cpp b/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTrafficTests.cpp index 46c8b3e4f77d5..5a6548b1f4a0e 100644 --- a/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTrafficTests.cpp +++ b/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTrafficTests.cpp @@ -296,7 +296,10 @@ class AgentAclTableGroupTrafficTest : public AgentHwTest { 1 /*priority*/, addAllQualifiers, getAgentEnsemble()->isSai()); - utility::addTtlAclTable(&newCfg, 2 /*priority*/); + utility::addTtlAclTable( + &newCfg, + 2 /*priority*/, + isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)); applyNewConfig(newCfg); utility::EcmpSetupAnyNPorts6 ecmpHelper( @@ -403,7 +406,8 @@ class AgentAclTableGroupTrafficTest : public AgentHwTest { 1 /*priority*/, addAllQualifiers, getAgentEnsemble()->isSai()); - utility::addTtlAclTable(&newCfg, 2); + utility::addTtlAclTable( + &newCfg, 2, isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)); applyNewConfig(newCfg); utility::EcmpSetupAnyNPorts6 ecmpHelper( diff --git a/fboss/agent/test/agent_hw_tests/AgentHwAclStatTests.cpp b/fboss/agent/test/agent_hw_tests/AgentHwAclStatTests.cpp index 72b4db1bcc85f..5764f00b38423 100644 --- a/fboss/agent/test/agent_hw_tests/AgentHwAclStatTests.cpp +++ b/fboss/agent/test/agent_hw_tests/AgentHwAclStatTests.cpp @@ -182,6 +182,10 @@ TEST_F(AgentHwAclStatTest, AclStatCreateMultiple) { } TEST_F(AgentHwAclStatCounterTypeTest, AclStatChangeCounterType) { + if (!isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)) { + GTEST_SKIP(); + } + auto setup = [=, this]() { auto& ensemble = *getAgentEnsemble(); auto newCfg = initialConfig(ensemble); diff --git a/fboss/agent/test/agent_hw_tests/AgentQueuePerHostTests.cpp b/fboss/agent/test/agent_hw_tests/AgentQueuePerHostTests.cpp index 56dbd2e835ba1..c3f163534f44a 100644 --- a/fboss/agent/test/agent_hw_tests/AgentQueuePerHostTests.cpp +++ b/fboss/agent/test/agent_hw_tests/AgentQueuePerHostTests.cpp @@ -440,11 +440,13 @@ class AgentQueuePerHostTest : public AgentHwTest { auto ttlAclName = utility::getQueuePerHostTtlAclName(); auto ttlCounterName = utility::getQueuePerHostTtlCounterName(); + const auto aclByteCounterSupported = + isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER); for (bool frontPanel : {false, true}) { auto packetsBefore = utility::getAclInOutPackets(getSw(), ttlCounterName); - - auto bytesBefore = - utility::getAclInOutPackets(getSw(), ttlCounterName, true); + auto bytesBefore = aclByteCounterSupported + ? utility::getAclInOutPackets(getSw(), ttlCounterName, true) + : 0; auto dstIP = getIpToMacAndClassID().begin()->first; sendPacket(dstIP, frontPanel, 64 /* ttl < 128 */); @@ -453,9 +455,9 @@ class AgentQueuePerHostTest : public AgentHwTest { WITH_RETRIES({ auto packetsAfter = utility::getAclInOutPackets(getSw(), ttlCounterName); - - auto bytesAfter = - utility::getAclInOutPackets(getSw(), ttlCounterName, true); + auto bytesAfter = aclByteCounterSupported + ? utility::getAclInOutPackets(getSw(), ttlCounterName, true) + : 0; XLOG(DBG2) << "verify send packets " << (frontPanel ? "out of port" : "switched") << "\n" @@ -466,7 +468,7 @@ class AgentQueuePerHostTest : public AgentHwTest { // counts ttl >= 128 packet only EXPECT_EVENTUALLY_EQ(packetsAfter - packetsBefore, 1); - if (isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)) { + if (aclByteCounterSupported) { if (frontPanel) { EXPECT_EVENTUALLY_EQ(bytesAfter - bytesBefore, packetSize); } diff --git a/fboss/agent/test/utils/AclTestUtils.cpp b/fboss/agent/test/utils/AclTestUtils.cpp index 3dce1b63266ae..943c68068896e 100644 --- a/fboss/agent/test/utils/AclTestUtils.cpp +++ b/fboss/agent/test/utils/AclTestUtils.cpp @@ -667,12 +667,12 @@ std::vector getAclCounterTypes( // one of the two is enabled. FBOSS use case does not require enabling // only one, but always enables both packets and bytes counters. Thus, // enable both in the test. Reference: CS00012271364 - if (asic->isSupported( + if (!asic->isSupported(HwAsic::Feature::ACL_BYTE_COUNTER) || + asic->isSupported( HwAsic::Feature::SEPARATE_BYTE_AND_PACKET_ACL_COUNTER)) { return {cfg::CounterType::PACKETS}; - } else { - return {cfg::CounterType::BYTES, cfg::CounterType::PACKETS}; } + return {cfg::CounterType::BYTES, cfg::CounterType::PACKETS}; } uint64_t getAclInOutPackets( diff --git a/fboss/agent/test/utils/QueuePerHostTestUtils.cpp b/fboss/agent/test/utils/QueuePerHostTestUtils.cpp index 43a97df64473c..670d372bd7270 100644 --- a/fboss/agent/test/utils/QueuePerHostTestUtils.cpp +++ b/fboss/agent/test/utils/QueuePerHostTestUtils.cpp @@ -390,12 +390,15 @@ void deleteTtlCounters(cfg::SwitchConfig* config) { void addTtlAclEntry( cfg::SwitchConfig* config, - const std::string& aclTableName) { + const std::string& aclTableName, + bool aclByteCounterSupported) { cfg::Ttl ttl; std::tie(*ttl.value(), *ttl.mask()) = std::make_tuple(0x80, 0x80); auto ttlCounterName = getQueuePerHostTtlCounterName(); - std::vector counterTypes{ - cfg::CounterType::PACKETS, cfg::CounterType::BYTES}; + std::vector counterTypes{cfg::CounterType::PACKETS}; + if (aclByteCounterSupported) { + counterTypes.push_back(cfg::CounterType::BYTES); + } utility::addTrafficCounter(config, ttlCounterName, counterTypes); cfg::AclEntry ttlAcl{}; @@ -403,17 +406,15 @@ void addTtlAclEntry( ttlAcl.ttl() = ttl; ttlAcl.actionType() = cfg::AclActionType::PERMIT; utility::addAclEntry(config, ttlAcl, aclTableName); - std::vector setCounterTypes{ - cfg::CounterType::PACKETS, cfg::CounterType::BYTES}; - utility::addAclStat( - config, getQueuePerHostTtlAclName(), ttlCounterName, setCounterTypes); + config, getQueuePerHostTtlAclName(), ttlCounterName, counterTypes); } // Utility to add TTL ACL table to a multi acl table group void addTtlAclTable( cfg::SwitchConfig* config, int16_t priority, + bool aclByteCounterSupported, bool addExtraQualifier) { std::vector qualifiers = { cfg::AclTableQualifier::TTL, cfg::AclTableQualifier::DSCP}; @@ -432,7 +433,7 @@ void addTtlAclTable( cfg::AclTableActionType::COUNTER}, qualifiers); - addTtlAclEntry(config, getTtlAclTableName()); + addTtlAclEntry(config, getTtlAclTableName(), aclByteCounterSupported); } void deleteQueuePerHostMatchers(cfg::SwitchConfig* config) { diff --git a/fboss/agent/test/utils/QueuePerHostTestUtils.h b/fboss/agent/test/utils/QueuePerHostTestUtils.h index a7e96775808c6..29f5a3f2e4b0c 100644 --- a/fboss/agent/test/utils/QueuePerHostTestUtils.h +++ b/fboss/agent/test/utils/QueuePerHostTestUtils.h @@ -65,10 +65,14 @@ void updateRoutesClassID( std::optional>& routePrefix2ClassID, RouteUpdateWrapper* updater); -void addTtlAclEntry(cfg::SwitchConfig* config, const std::string& aclTableName); +void addTtlAclEntry( + cfg::SwitchConfig* config, + const std::string& aclTableName, + bool aclByteCounterSupported); void addTtlAclTable( cfg::SwitchConfig* config, int16_t priority, + bool aclByteCounterSupported, bool addExtraQualifier = false); void deleteTtlCounters(cfg::SwitchConfig* config); void addQueuePerHostAclEntry(