Skip to content

Commit

Permalink
Set appropriate alpha values so that traffic loop can build
Browse files Browse the repository at this point in the history
Summary:
The previous alpha value of 1/128 or 1/64 is too small for traffic to build on TH3/TH4 (see P1694647898 and P1694630782). Set it to 1/2 to match production settings.

Also added a small delay between pumpTraffic() and validate() calls, to make sure traffic is actually looping.

Reviewed By: nivinl

Differential Revision: D67075312

fbshipit-source-id: 9220b2323f5447fa719d4d66d12031e921c442bc
  • Loading branch information
maxwindiff authored and facebook-github-bot committed Dec 16, 2024
1 parent 531a2b0 commit ed3124b
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 19 deletions.
55 changes: 39 additions & 16 deletions fboss/agent/test/agent_hw_tests/AgentTrafficPfcTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,21 @@ class AgentTrafficPfcTest : public AgentHwTest {
}

protected:
cfg::MMUScalingFactor getDefaultMmuScalingFactor() {
auto asicType =
utility::checkSameAndGetAsic(getAgentEnsemble()->getL3Asics())
->getAsicType();
switch (asicType) {
case cfg::AsicType::ASIC_TYPE_TOMAHAWK3:
case cfg::AsicType::ASIC_TYPE_TOMAHAWK4:
case cfg::AsicType::ASIC_TYPE_TOMAHAWK5:
return cfg::MMUScalingFactor::ONE_HALF;

default:
return cfg::MMUScalingFactor::ONE_128TH;
}
}

void setupEcmpTraffic(const std::vector<PortID>& portIds) {
utility::EcmpSetupTargetedPorts6 ecmpHelper{
getProgrammedState(), getIntfMac()};
Expand Down Expand Up @@ -393,8 +408,8 @@ class AgentTrafficPfcTest : public AgentHwTest {
void runTestWithCfg(
const int trafficClass,
const int pfcPriority,
const std::map<int, int>& tcToPgOverride = {},
TrafficTestParams testParams = TrafficTestParams{},
const std::map<int, int>& tcToPgOverride,
TrafficTestParams testParams,
std::function<void(
AgentEnsemble* ensemble,
const int pri,
Expand All @@ -410,6 +425,9 @@ class AgentTrafficPfcTest : public AgentHwTest {
// Apply PFC config to all ports
portIdsToConfigure = masterLogicalInterfacePortIds();
}
if (!testParams.buffer.scalingFactor.has_value()) {
testParams.buffer.scalingFactor = getDefaultMmuScalingFactor();
}
utility::setupPfcBuffers(
getAgentEnsemble(),
cfg,
Expand Down Expand Up @@ -448,6 +466,10 @@ class AgentTrafficPfcTest : public AgentHwTest {
};
auto verifyCommon = [&](bool postWb) {
pumpTraffic(trafficClass);
// Sleep for a bit before validation, so that the test will fail if
// traffic doesn't actually build up, instead of passing by luck.
// NOLINTNEXTLINE(facebook-hte-BadCall-sleep)
sleep(2);
// check counters are as expected
validateCounterFn(getAgentEnsemble(), pfcPriority, portIds);
if (testParams.expectDrop) {
Expand Down Expand Up @@ -493,15 +515,11 @@ INSTANTIATE_TEST_SUITE_P(
.scale = true},
TrafficTestParams{
.name = "WithZeroPgHeadRoomCfg",
.buffer =
PfcBufferParams{.pgHeadroom = 0, .scalingFactor = std::nullopt},
.buffer = PfcBufferParams{.pgHeadroom = 0},
.expectDrop = true},
TrafficTestParams{
.name = "WithZeroGlobalHeadRoomCfg",
.buffer =
PfcBufferParams{
.globalHeadroom = 0,
.scalingFactor = std::nullopt},
.buffer = PfcBufferParams{.globalHeadroom = 0},
.expectDrop = true}),
[](const ::testing::TestParamInfo<TrafficTestParams>& info) {
return info.param.name;
Expand All @@ -528,7 +546,11 @@ TEST_P(AgentTrafficPfcGenTest, verifyPfc) {
TEST_F(AgentTrafficPfcTest, verifyPfcWithMapChanges_0) {
const int trafficClass = kLosslessTrafficClass;
const int pfcPriority = 3;
runTestWithCfg(trafficClass, pfcPriority, {{trafficClass, pfcPriority}});
runTestWithCfg(
trafficClass,
pfcPriority,
{{trafficClass, pfcPriority}},
TrafficTestParams{});
}

// intent of this test is to send traffic so that it maps to
Expand All @@ -538,19 +560,21 @@ TEST_F(AgentTrafficPfcTest, verifyPfcWithMapChanges_0) {
TEST_F(AgentTrafficPfcTest, verifyPfcWithMapChanges_1) {
const int trafficClass = 7;
const int pfcPriority = kLosslessPriority;
runTestWithCfg(trafficClass, pfcPriority, {{trafficClass, pfcPriority}});
runTestWithCfg(
trafficClass,
pfcPriority,
{{trafficClass, pfcPriority}},
TrafficTestParams{});
}

TEST_F(AgentTrafficPfcTest, verifyBufferPoolWatermarks) {
const int trafficClass = kLosslessTrafficClass;
const int pfcPriority = kLosslessPriority;
cfg::MMUScalingFactor scalingFactor = cfg::MMUScalingFactor::ONE_64TH;
runTestWithCfg(
trafficClass,
pfcPriority,
{},
TrafficTestParams{
.buffer = PfcBufferParams{.scalingFactor = scalingFactor}},
TrafficTestParams{},
validateBufferPoolWatermarkCounters);
}

Expand All @@ -561,8 +585,7 @@ TEST_F(AgentTrafficPfcTest, verifyIngressPriorityGroupWatermarks) {
trafficClass,
pfcPriority,
{},
TrafficTestParams{
.buffer = PfcBufferParams{.scalingFactor = std::nullopt}},
TrafficTestParams{},
validateIngressPriorityGroupWatermarkCounters);
}

Expand All @@ -576,7 +599,7 @@ class AgentTrafficPfcWatchdogTest : public AgentTrafficPfcTest {
portIds,
kLosslessPgIds,
{},
PfcBufferParams{});
PfcBufferParams{.scalingFactor = cfg::MMUScalingFactor::ONE_128TH});
applyNewConfig(cfg);
setupEcmpTraffic(portIds);
}
Expand Down
5 changes: 2 additions & 3 deletions fboss/agent/test/utils/PfcTestUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "fboss/agent/Utils.h"
#include "fboss/agent/gen-cpp2/switch_config_types.h"
#include "fboss/agent/hw/gen-cpp2/hardware_stats_types.h"
#include "fboss/agent/test/AgentHwTest.h"
#include "fboss/agent/test/AgentEnsemble.h"

namespace facebook::fboss::utility {

Expand All @@ -14,8 +14,7 @@ struct PfcBufferParams {
int globalHeadroom = 5000; // keep this lower than globalShared
int pgLimit = 2200;
int pgHeadroom = 2200; // keep this lower than globalShared
std::optional<facebook::fboss::cfg::MMUScalingFactor> scalingFactor =
facebook::fboss::cfg::MMUScalingFactor::ONE_128TH;
std::optional<facebook::fboss::cfg::MMUScalingFactor> scalingFactor;
int resumeOffset = 1800;
};

Expand Down

0 comments on commit ed3124b

Please sign in to comment.