Skip to content

Commit

Permalink
fix AgentDscpMarkingTest.VerifyDscpMarking on J3
Browse files Browse the repository at this point in the history
Summary:
As titled, fix AgentDscpMarkingTest.VerifyDscpMarking on J3 by:
1) use interface mac when creating L3 ecmp loop, otherwise, packets would be dropped during the second loop on J3.
2) change DSCP counter ACL action type to DENY. Otherwise, packets would be matched and increment counter for ttl times.
3) modified test verification part "Inject a pkt with dscp = ICP DSCP", since packets now will only hit counter ACL once. Note that, this verification is not that useful in my opinion, since the DSCP re-write ACL that the is focus of this test will never be hit anyway.

Reviewed By: nivinl

Differential Revision:
D61146672

Privacy Context Container: L1125642

fbshipit-source-id: 966cc3cff110422e6239d42dea3b6dfca817fa28
  • Loading branch information
daiwei1983 authored and facebook-github-bot committed Aug 14, 2024
1 parent 202ef16 commit 51e390e
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 15 deletions.
30 changes: 21 additions & 9 deletions fboss/agent/test/agent_hw_tests/AgentDscpMarkingTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ class AgentDscpMarkingTest : public AgentHwTest {
auto l3Asics = ensemble.getL3Asics();
auto asic = utility::checkSameAndGetAsic(l3Asics);
utility::addOlympicQosMaps(cfg, l3Asics);
utility::addDscpCounterAcl(asic, &cfg);
// drop packets are match to avoid packets matching multiple times in L3
// loop case
utility::addDscpCounterAcl(asic, &cfg, cfg::AclActionType::DENY);
utility::addDscpMarkingAcls(asic, &cfg, ensemble.isSai());
return cfg;
}
Expand All @@ -68,20 +70,26 @@ class AgentDscpMarkingTest : public AgentHwTest {
*
* Inject a pkt with dscp = ICP DSCP, l4SrcPort matching ACL2:
* - The packet will match ACL1, thus counter incremented.
* - Packet egress via front panel port which is in loopback mode.
* - Thus, packet gets looped back.
* - Hits ACL1 again, and thus counter incremented twice.
* - Packet dropped after hitting ACL1
*/

auto setup = [=, this]() {
utility::EcmpSetupAnyNPorts6 ecmpHelper(
getProgrammedState(), RouterID(0), {cfg::PortType::INTERFACE_PORT});
getProgrammedState(),
utility::getFirstInterfaceMac(getProgrammedState()),
RouterID(0),
false,
{cfg::PortType::INTERFACE_PORT});
resolveNeigborAndProgramRoutes(ecmpHelper, kEcmpWidth);
};

auto verify = [=, this]() {
utility::EcmpSetupAnyNPorts6 ecmpHelper(
getProgrammedState(), RouterID(0), {cfg::PortType::INTERFACE_PORT});
getProgrammedState(),
utility::getFirstInterfaceMac(getProgrammedState()),
RouterID(0),
false,
{cfg::PortType::INTERFACE_PORT});
auto portId = ecmpHelper.ecmpPortDescriptorAt(0).phyPortID();
auto portStatsBefore = getLatestPortStats(portId);

Expand Down Expand Up @@ -136,9 +144,9 @@ class AgentDscpMarkingTest : public AgentHwTest {
// See detailed comment block at the beginning of this function
EXPECT_EVENTUALLY_EQ(
afterAclInOutPkts2 - afterAclInOutPkts,
(2 /* ACL hit twice */ * 2 /* l4SrcPort, l4DstPort */ *
(1 /* ACL hit once */ * 2 /* l4SrcPort, l4DstPort */ *
utility::kUdpPorts().size()) +
(2 /* ACL hit twice */ * 2 /* l4SrcPort, l4DstPort */ *
(1 /* ACL hit once */ * 2 /* l4SrcPort, l4DstPort */ *
utility::kTcpPorts().size()));
});
}
Expand Down Expand Up @@ -215,7 +223,11 @@ class AgentDscpMarkingTest : public AgentHwTest {
// ingressed on the port, and be properly queued.
if (frontPanel) {
utility::EcmpSetupAnyNPorts6 ecmpHelper(
getProgrammedState(), RouterID(0), {cfg::PortType::INTERFACE_PORT});
getProgrammedState(),
utility::getFirstInterfaceMac(getProgrammedState()),
RouterID(0),
false,
{cfg::PortType::INTERFACE_PORT});
auto outPort = ecmpHelper.ecmpPortDescriptorAt(kEcmpWidth).phyPortID();
getSw()->sendPacketOutOfPortAsync(std::move(txPacket), outPort);
} else {
Expand Down
8 changes: 6 additions & 2 deletions fboss/agent/test/utils/DscpMarkingUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,14 @@ void addDscpMarkingAcls(
hwAsic, config, IP_PROTO::IP_PROTO_TCP, kTcpPorts(), isSai);
}

void addDscpCounterAcl(const HwAsic* hwAsic, cfg::SwitchConfig* config) {
void addDscpCounterAcl(
const HwAsic* hwAsic,
cfg::SwitchConfig* config,
cfg::AclActionType actionType) {
// Create ACL to count the number of packets with DSCP == ICP
utility::addDscpAclToCfg(
auto acl = utility::addDscpAclToCfg(
hwAsic, config, kDscpCounterAclName(), utility::kIcpDscp());
acl->actionType() = actionType;
std::vector<cfg::CounterType> counterTypes{cfg::CounterType::PACKETS};
utility::addTrafficCounter(config, kCounterName(), counterTypes);
cfg::MatchAction matchAction = cfg::MatchAction();
Expand Down
5 changes: 4 additions & 1 deletion fboss/agent/test/utils/DscpMarkingUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ void addDscpMarkingAcls(
const HwAsic* hwAsic,
cfg::SwitchConfig* config,
bool isSai);
void addDscpCounterAcl(const HwAsic* hwAsic, cfg::SwitchConfig* config);
void addDscpCounterAcl(
const HwAsic* hwAsic,
cfg::SwitchConfig* config,
cfg::AclActionType actionType);
void addDscpMarkingAclTable(cfg::SwitchConfig* config, bool isSai);
void addDscpAclEntryWithCounter(
cfg::SwitchConfig* config,
Expand Down
4 changes: 2 additions & 2 deletions fboss/agent/test/utils/TrafficPolicyTestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
DECLARE_bool(enable_acl_table_group);

namespace facebook::fboss::utility {
void addDscpAclToCfg(
cfg::AclEntry* addDscpAclToCfg(
const HwAsic* hwAsic,
cfg::SwitchConfig* config,
const std::string& aclName,
Expand All @@ -25,7 +25,7 @@ void addDscpAclToCfg(
acl.dscp() = dscp;
utility::addEtherTypeToAcl(hwAsic, &acl, cfg::EtherType::IPv6);

utility::addAclEntry(config, acl, utility::kDefaultAclTable());
return utility::addAclEntry(config, acl, utility::kDefaultAclTable());
}

void addL4SrcPortAclToCfg(
Expand Down
2 changes: 1 addition & 1 deletion fboss/agent/test/utils/TrafficPolicyTestUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*/

namespace facebook::fboss::utility {
void addDscpAclToCfg(
cfg::AclEntry* addDscpAclToCfg(
const HwAsic* hwAsic,
cfg::SwitchConfig* config,
const std::string& aclName,
Expand Down

0 comments on commit 51e390e

Please sign in to comment.