Skip to content

[Nexthop][m4340nhp] Fix ACL tests and trap ACLs on split-ACL DNX ASICs #1887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marif-nexthop wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Nexthop][m4340nhp] Fix ACL tests and trap ACLs on split-ACL DNX ASICs #1887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions fboss/agent/hw/sai/hw_test/HwTestAclUtilsThriftHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,15 @@ bool HwTestThriftHandler::isAclEntrySame(
auto tableName = aclTableName ? getActualAclTableName(*aclTableName)
: getActualAclTableName(std::nullopt);
auto aclTableHandle = aclTableManager.getAclTableHandle(tableName);
if (!aclTableHandle) {
throw FbossError("ACL table ", tableName, " not found");
}
auto aclEntryHandle = aclTableManager.getAclEntryHandle(
aclTableHandle, swAcl->getPriority(), swAcl->getID());
if (!aclEntryHandle) {
throw FbossError(
"ACL entry ", swAcl->getID(), " not found in table ", tableName);
}
auto aclEntryId = aclEntryHandle->aclEntry->adapterKey();

auto aclFieldPriorityExpected =
Expand Down
19 changes: 16 additions & 3 deletions fboss/agent/test/agent_hw_tests/AgentHwAclQualifierTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,19 @@ class AgentHwAclQualifierTest : public AgentHwTest {
}
};

// With ACL table groups, Q4D/J4 split the default ACL into IPv4 and
// IPv6 tables and ICMPv6 entries land in the IPv6 one; with a single table
// every entry is in the default table.
std::string icmpv6AclTable(const AgentEnsemble* ensemble) {
auto asicType = ensemble->getL3Asics().front()->getAsicType();
if (FLAGS_enable_acl_table_group &&
(asicType == cfg::AsicType::ASIC_TYPE_QUMRAN4D ||
asicType == cfg::AsicType::ASIC_TYPE_JERICHO4)) {
return utility::kIpv6AclTable();
}
return utility::kDefaultAclTable();
}

TEST_F(AgentHwAclQualifierTest, AclIp4TcpQualifiers) {
auto setup = [=, this]() {
auto& ensemble = *getAgentEnsemble();
Expand Down Expand Up @@ -676,7 +689,7 @@ TEST_F(AgentHwAclQualifierTest, AclEmptyCodeIcmp) {
auto l3Asics = ensemble.getL3Asics();
auto asic = checkSameAndGetAsicForTesting(l3Asics);
if (asic->isSupported(HwAsic::Feature::ACL_ENTRY_ETHER_TYPE)) {
acl.etherType() = cfg::EtherType::IPv4;
acl.etherType() = cfg::EtherType::IPv6;
}
acl.proto() = 58;
acl.icmpType() = 3;
Expand Down Expand Up @@ -705,8 +718,8 @@ TEST_F(AgentHwAclQualifierTest, AclEmptyCodeIcmp) {
if (this->getAsicVendor() == HwAsic::AsicVendor::ASIC_VENDOR_CHENAB) {
EXPECT_TRUE(client->sync_isAclEntrySame(aclThrift, "icmp_acl_table"));
} else {
EXPECT_TRUE(
client->sync_isAclEntrySame(aclThrift, utility::kDefaultAclTable()));
EXPECT_TRUE(client->sync_isAclEntrySame(
aclThrift, icmpv6AclTable(getAgentEnsemble())));
}
};

Expand Down
25 changes: 21 additions & 4 deletions fboss/agent/test/utils/TrapPacketUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@

namespace facebook::fboss::utility {

namespace {
// These DNX ASICs split the default ingress ACL into an IPv4 table and an IPv6
// table, so IPv6 entries go in the IPv6 table.
bool hasSeparateIpv6AclTable(const HwAsic* asic) {
return asic->getAsicType() == cfg::AsicType::ASIC_TYPE_QUMRAN4D ||
asic->getAsicType() == cfg::AsicType::ASIC_TYPE_JERICHO4;
}
} // namespace

void addTrapPacketAcl(
const HwAsic* asic,
cfg::SwitchConfig* config,
Expand Down Expand Up @@ -53,8 +62,7 @@ void addTrapPacketAcl(
trafficPolicy.matchToAction()->push_back(match2Action);
};

if (asic->getAsicType() == cfg::AsicType::ASIC_TYPE_QUMRAN4D ||
asic->getAsicType() == cfg::AsicType::ASIC_TYPE_JERICHO4) {
if (hasSeparateIpv6AclTable(asic)) {
// Q4D/J4 (DNX) reject FIELD_SRC_PORT + FIELD_ACL_IP_TYPE=NON_IP. Per
// Broadcom, install the source-port entry in both ACL tables: an
// etherType=IPv4 copy in the default table and an etherType=IPv6 copy in
Expand Down Expand Up @@ -124,7 +132,12 @@ void addTrapPacketAcl(
prefix.first.isV6() ? cfg::EtherType::IPv6 : cfg::EtherType::IPv4;
}
entry.actionType() = cfg::AclActionType::PERMIT;
utility::addAclEntry(config, entry, utility::kDefaultAclTable());
utility::addAclEntry(
config,
entry,
prefix.first.isV6() && hasSeparateIpv6AclTable(asic)
? utility::kIpv6AclTable()
: utility::kDefaultAclTable());

cfg::MatchAction action;
action.toCpuAction() = toCpuAction;
Expand Down Expand Up @@ -179,7 +192,11 @@ void addTrapPacketAcl(
entry.etherType() = cfg::EtherType::IPv6;
}
entry.actionType() = cfg::AclActionType::PERMIT;
utility::addAclEntry(config, entry, utility::kDefaultAclTable());
utility::addAclEntry(
config,
entry,
hasSeparateIpv6AclTable(asic) ? utility::kIpv6AclTable()
: utility::kDefaultAclTable());

cfg::MatchAction action;
action.sendToQueue() = cfg::QueueMatchAction();
Expand Down
Loading