Skip to content

Commit 7dd1b5e

Browse files
committed
Gate ACL byte counters by ASIC capability
1 parent 54963da commit 7dd1b5e

9 files changed

Lines changed: 71 additions & 33 deletions

fboss/agent/hw/sai/hw_test/SaiAclTableGroupTests.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,10 @@ class SaiAclTableGroupTest : public HwTest {
356356

357357
// Table 2: Create TTL acl Table follwed by entry.
358358
// This utlity call adds the TTL Acl entry as well.
359-
utility::addTtlAclTable(newCfg, 2 /* priority */);
359+
utility::addTtlAclTable(
360+
newCfg,
361+
2 /* priority */,
362+
isSupported(HwAsic::Feature::ACL_BYTE_COUNTER));
360363
applyNewConfig(*newCfg);
361364
}
362365

@@ -403,7 +406,10 @@ class SaiAclTableGroupTest : public HwTest {
403406
case tableAddType::table2:
404407
utility::addAclTableGroup(&newCfg, kAclStage(), kAclTableGroup());
405408
// Add Table 2: TtlTable
406-
utility::addTtlAclTable(&newCfg, 2 /* priority */);
409+
utility::addTtlAclTable(
410+
&newCfg,
411+
2 /* priority */,
412+
isSupported(HwAsic::Feature::ACL_BYTE_COUNTER));
407413
break;
408414
case tableAddType::tableBoth:
409415
addTwoAclTables(&newCfg);
@@ -495,7 +501,11 @@ class SaiAclTableGroupTest : public HwTest {
495501

496502
utility::addAclTableGroup(&newCfg, kAclStage(), kAclTableGroup());
497503
addAclTable3WithEntry(&newCfg, addExtraQualifier, false);
498-
utility::addTtlAclTable(&newCfg, 2 /* priority */, addExtraQualifier);
504+
utility::addTtlAclTable(
505+
&newCfg,
506+
2 /* priority */,
507+
isSupported(HwAsic::Feature::ACL_BYTE_COUNTER),
508+
addExtraQualifier);
499509

500510
return newCfg;
501511
}

fboss/agent/test/agent_hw_tests/AgentAclCounterTests.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,10 @@ class AgentAclCounterTest : public AgentHwTest {
490490
auto* acl = &aclEntry;
491491
auto l3Asics = getAgentEnsemble()->getL3Asics();
492492
auto asic = checkSameAndGetAsicForTesting(l3Asics);
493+
std::vector<cfg::CounterType> counterTypes{cfg::CounterType::PACKETS};
494+
if (asic->isSupported(HwAsic::Feature::ACL_BYTE_COUNTER)) {
495+
counterTypes.push_back(cfg::CounterType::BYTES);
496+
}
493497
bool isSai = getAgentEnsemble()->isSai();
494498
switch (aclType) {
495499
case AclType::TCP_TTLD:
@@ -523,8 +527,6 @@ class AgentAclCounterTest : public AgentHwTest {
523527
// and v6 traffic. The test sends IPv6 traffic, so the IPv6 copy
524528
// keeps the canonical name/counter (the one verifyAclType checks);
525529
// the IPv4 copy gets a "-v4" suffix.
526-
std::vector<cfg::CounterType> counterTypes{
527-
cfg::CounterType::PACKETS, cfg::CounterType::BYTES};
528530
auto addSrcPortEntry = [&](const std::string& name,
529531
const std::string& counter,
530532
cfg::EtherType etherType,
@@ -595,9 +597,7 @@ class AgentAclCounterTest : public AgentHwTest {
595597
}
596598
utility::addAcl(config, aclEntry, cfg::AclStage::INGRESS);
597599

598-
std::vector<cfg::CounterType> setCounterTypes{
599-
cfg::CounterType::PACKETS, cfg::CounterType::BYTES};
600-
utility::addAclStat(config, aclName, counterName, setCounterTypes);
600+
utility::addAclStat(config, aclName, counterName, counterTypes);
601601
}
602602

603603
std::unique_ptr<utility::EcmpSetupAnyNPorts6> helper_;

fboss/agent/test/agent_hw_tests/AgentAclTableGroupTests.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,10 @@ class AgentAclTableGroupTest : public AgentHwTest {
290290
void addTwoAclTables(cfg::SwitchConfig* newCfg) {
291291
utility::addAclTableGroup(newCfg, kAclStage(), kAclTableGroup());
292292
addAclTable3WithEntry(newCfg);
293-
utility::addTtlAclTable(newCfg, 2 /* priority */);
293+
utility::addTtlAclTable(
294+
newCfg,
295+
2 /* priority */,
296+
isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER));
294297
applyNewConfig(*newCfg);
295298
}
296299

@@ -325,7 +328,10 @@ class AgentAclTableGroupTest : public AgentHwTest {
325328
break;
326329
case tableAddType::table2:
327330
utility::addAclTableGroup(&newCfg, kAclStage(), kAclTableGroup());
328-
utility::addTtlAclTable(&newCfg, 2 /* priority */);
331+
utility::addTtlAclTable(
332+
&newCfg,
333+
2 /* priority */,
334+
isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER));
329335
break;
330336
case tableAddType::tableBoth:
331337
addTwoAclTables(&newCfg);
@@ -415,7 +421,11 @@ class AgentAclTableGroupTest : public AgentHwTest {
415421
auto newCfg = initialConfig(ensemble);
416422
utility::addAclTableGroup(&newCfg, kAclStage(), kAclTableGroup());
417423
addAclTable3WithEntry(&newCfg, addExtraQualifier, false);
418-
utility::addTtlAclTable(&newCfg, 2 /* priority */, addExtraQualifier);
424+
utility::addTtlAclTable(
425+
&newCfg,
426+
2 /* priority */,
427+
isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER),
428+
addExtraQualifier);
419429
return newCfg;
420430
}
421431

@@ -671,7 +681,10 @@ TEST_F(AgentAclTableGroupTest, AddTwoTablesDeleteAddSecond) {
671681
auto newCfg = initialConfig(ensemble);
672682
addTwoAclTables(&newCfg);
673683
deleteTtlAclTable(&newCfg);
674-
utility::addTtlAclTable(&newCfg, 2 /* priority */);
684+
utility::addTtlAclTable(
685+
&newCfg,
686+
2 /* priority */,
687+
isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER));
675688
applyNewConfig(newCfg);
676689
};
677690

fboss/agent/test/agent_hw_tests/AgentAclTableGroupTrafficTests.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,10 @@ class AgentAclTableGroupTrafficTest : public AgentHwTest {
296296
1 /*priority*/,
297297
addAllQualifiers,
298298
getAgentEnsemble()->isSai());
299-
utility::addTtlAclTable(&newCfg, 2 /*priority*/);
299+
utility::addTtlAclTable(
300+
&newCfg,
301+
2 /*priority*/,
302+
isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER));
300303
applyNewConfig(newCfg);
301304

302305
utility::EcmpSetupAnyNPorts6 ecmpHelper(
@@ -403,7 +406,8 @@ class AgentAclTableGroupTrafficTest : public AgentHwTest {
403406
1 /*priority*/,
404407
addAllQualifiers,
405408
getAgentEnsemble()->isSai());
406-
utility::addTtlAclTable(&newCfg, 2);
409+
utility::addTtlAclTable(
410+
&newCfg, 2, isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER));
407411
applyNewConfig(newCfg);
408412

409413
utility::EcmpSetupAnyNPorts6 ecmpHelper(

fboss/agent/test/agent_hw_tests/AgentHwAclStatTests.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ TEST_F(AgentHwAclStatTest, AclStatCreateMultiple) {
182182
}
183183

184184
TEST_F(AgentHwAclStatCounterTypeTest, AclStatChangeCounterType) {
185+
if (!isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)) {
186+
GTEST_SKIP();
187+
}
188+
185189
auto setup = [=, this]() {
186190
auto& ensemble = *getAgentEnsemble();
187191
auto newCfg = initialConfig(ensemble);

fboss/agent/test/agent_hw_tests/AgentQueuePerHostTests.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -440,11 +440,13 @@ class AgentQueuePerHostTest : public AgentHwTest {
440440
auto ttlAclName = utility::getQueuePerHostTtlAclName();
441441
auto ttlCounterName = utility::getQueuePerHostTtlCounterName();
442442

443+
const auto aclByteCounterSupported =
444+
isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER);
443445
for (bool frontPanel : {false, true}) {
444446
auto packetsBefore = utility::getAclInOutPackets(getSw(), ttlCounterName);
445-
446-
auto bytesBefore =
447-
utility::getAclInOutPackets(getSw(), ttlCounterName, true);
447+
auto bytesBefore = aclByteCounterSupported
448+
? utility::getAclInOutPackets(getSw(), ttlCounterName, true)
449+
: 0;
448450

449451
auto dstIP = getIpToMacAndClassID<AddrT>().begin()->first;
450452
sendPacket(dstIP, frontPanel, 64 /* ttl < 128 */);
@@ -453,9 +455,9 @@ class AgentQueuePerHostTest : public AgentHwTest {
453455
WITH_RETRIES({
454456
auto packetsAfter =
455457
utility::getAclInOutPackets(getSw(), ttlCounterName);
456-
457-
auto bytesAfter =
458-
utility::getAclInOutPackets(getSw(), ttlCounterName, true);
458+
auto bytesAfter = aclByteCounterSupported
459+
? utility::getAclInOutPackets(getSw(), ttlCounterName, true)
460+
: 0;
459461

460462
XLOG(DBG2) << "verify send packets "
461463
<< (frontPanel ? "out of port" : "switched") << "\n"
@@ -466,7 +468,7 @@ class AgentQueuePerHostTest : public AgentHwTest {
466468

467469
// counts ttl >= 128 packet only
468470
EXPECT_EVENTUALLY_EQ(packetsAfter - packetsBefore, 1);
469-
if (isSupportedOnAllAsics(HwAsic::Feature::ACL_BYTE_COUNTER)) {
471+
if (aclByteCounterSupported) {
470472
if (frontPanel) {
471473
EXPECT_EVENTUALLY_EQ(bytesAfter - bytesBefore, packetSize);
472474
}

fboss/agent/test/utils/AclTestUtils.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -667,12 +667,12 @@ std::vector<cfg::CounterType> getAclCounterTypes(
667667
// one of the two is enabled. FBOSS use case does not require enabling
668668
// only one, but always enables both packets and bytes counters. Thus,
669669
// enable both in the test. Reference: CS00012271364
670-
if (asic->isSupported(
670+
if (!asic->isSupported(HwAsic::Feature::ACL_BYTE_COUNTER) ||
671+
asic->isSupported(
671672
HwAsic::Feature::SEPARATE_BYTE_AND_PACKET_ACL_COUNTER)) {
672673
return {cfg::CounterType::PACKETS};
673-
} else {
674-
return {cfg::CounterType::BYTES, cfg::CounterType::PACKETS};
675674
}
675+
return {cfg::CounterType::BYTES, cfg::CounterType::PACKETS};
676676
}
677677

678678
uint64_t getAclInOutPackets(

fboss/agent/test/utils/QueuePerHostTestUtils.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -390,30 +390,31 @@ void deleteTtlCounters(cfg::SwitchConfig* config) {
390390

391391
void addTtlAclEntry(
392392
cfg::SwitchConfig* config,
393-
const std::string& aclTableName) {
393+
const std::string& aclTableName,
394+
bool aclByteCounterSupported) {
394395
cfg::Ttl ttl;
395396
std::tie(*ttl.value(), *ttl.mask()) = std::make_tuple(0x80, 0x80);
396397
auto ttlCounterName = getQueuePerHostTtlCounterName();
397-
std::vector<cfg::CounterType> counterTypes{
398-
cfg::CounterType::PACKETS, cfg::CounterType::BYTES};
398+
std::vector<cfg::CounterType> counterTypes{cfg::CounterType::PACKETS};
399+
if (aclByteCounterSupported) {
400+
counterTypes.push_back(cfg::CounterType::BYTES);
401+
}
399402
utility::addTrafficCounter(config, ttlCounterName, counterTypes);
400403

401404
cfg::AclEntry ttlAcl{};
402405
ttlAcl.name() = getQueuePerHostTtlAclName();
403406
ttlAcl.ttl() = ttl;
404407
ttlAcl.actionType() = cfg::AclActionType::PERMIT;
405408
utility::addAclEntry(config, ttlAcl, aclTableName);
406-
std::vector<cfg::CounterType> setCounterTypes{
407-
cfg::CounterType::PACKETS, cfg::CounterType::BYTES};
408-
409409
utility::addAclStat(
410-
config, getQueuePerHostTtlAclName(), ttlCounterName, setCounterTypes);
410+
config, getQueuePerHostTtlAclName(), ttlCounterName, counterTypes);
411411
}
412412

413413
// Utility to add TTL ACL table to a multi acl table group
414414
void addTtlAclTable(
415415
cfg::SwitchConfig* config,
416416
int16_t priority,
417+
bool aclByteCounterSupported,
417418
bool addExtraQualifier) {
418419
std::vector<cfg::AclTableQualifier> qualifiers = {
419420
cfg::AclTableQualifier::TTL, cfg::AclTableQualifier::DSCP};
@@ -432,7 +433,7 @@ void addTtlAclTable(
432433
cfg::AclTableActionType::COUNTER},
433434
qualifiers);
434435

435-
addTtlAclEntry(config, getTtlAclTableName());
436+
addTtlAclEntry(config, getTtlAclTableName(), aclByteCounterSupported);
436437
}
437438

438439
void deleteQueuePerHostMatchers(cfg::SwitchConfig* config) {

fboss/agent/test/utils/QueuePerHostTestUtils.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,14 @@ void updateRoutesClassID(
6565
std::optional<cfg::AclLookupClass>>& routePrefix2ClassID,
6666
RouteUpdateWrapper* updater);
6767

68-
void addTtlAclEntry(cfg::SwitchConfig* config, const std::string& aclTableName);
68+
void addTtlAclEntry(
69+
cfg::SwitchConfig* config,
70+
const std::string& aclTableName,
71+
bool aclByteCounterSupported);
6972
void addTtlAclTable(
7073
cfg::SwitchConfig* config,
7174
int16_t priority,
75+
bool aclByteCounterSupported,
7276
bool addExtraQualifier = false);
7377
void deleteTtlCounters(cfg::SwitchConfig* config);
7478
void addQueuePerHostAclEntry(

0 commit comments

Comments
 (0)