Skip to content

Commit c5034dd

Browse files
phshaikhmeta-codesync[bot]
authored andcommitted
Program port profile in initialConfig for AgentPortProfileTest
Summary: Fixing port profile tests for Chenab Facebook : On Chenab, `AgentPortProfileTest` crashes the agent instead of failing: ``` mlnx_port_in_use_check: Failed remove port oid 1000100000001 - is a router interface mlnx_remove_port: Failed to remove port 0x1 [port] Failed to remove sai object : PortSaiId(281479271677953): OBJECT IN USE terminate called after throwing an instance of 'facebook::fboss::SaiApiError' ``` Two Chenab properties combine to produce this: - `utility::getInterfaceType()` returns `cfg::InterfaceType::PORT` for `ASIC_VENDOR_CHENAB`, so every router interface is bound to a port rather than to a VLAN. - `SaiPortManager::createOnlyAttributeChanged()` recreates the SAI port whenever `HwLaneList` differs, independent of `SAI_PORT_SPEED_CHANGE` (which Chenab reports as unsupported). Chenab's `changePortByRecreate()` is a bare `removePort()` + `addPort()` and does not chain delete the attached router interface, so `sai_remove_port` is rejected. `setup()` reprogrammed the two test ports from the 400G_4 safe profile to the profile under test, changing the lane count and taking exactly that path. `ConfigUtils.cpp` already records the constraint: "chenab does not support dynamic port profile change, as it may lead to recreation of ports by delete and add". Program the profile as part of `initialConfig()` instead, so the ports come up on it at cold boot and are never recreated, and skip `setup()` on Chenab. Gated on `ASIC_VENDOR_CHENAB` so every other platform keeps the existing `oneL3IntfTwoPortConfig` + `applyNewConfig` path and its warm boot round trip is unaffected. The profile survives warm boot because `applyInitialConfig()` persists the config to `agent.conf` and `AgentEnsemble` reads that same file back on the warm boot run, so the reapplied config is a no-op delta. ___ Differential Revision: D117126187 fbshipit-source-id: 14b260410ede01a4f9112f990f354ab03e19f093
1 parent 599d601 commit c5034dd

1 file changed

Lines changed: 50 additions & 0 deletions

File tree

fboss/agent/test/agent_hw_tests/AgentPortProfileTests.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,43 @@ class AgentPortProfileTest : public AgentHwTest {
4747
initInfo.overrideTransceiverInfo = utility::getTransceiverInfo(Profile);
4848
}
4949

50+
// Chenab router interfaces are port scoped (utility::getInterfaceType returns
51+
// InterfaceType::PORT), and SaiPortManager::changePortByRecreate does not
52+
// chain delete them. Reprogramming a port to a profile with a different lane
53+
// count therefore removes the port while its RIF is still attached, which the
54+
// SDK rejects with OBJECT IN USE. Program the profile as part of the initial
55+
// config instead, so the ports come up on it at cold boot and are never
56+
// recreated.
57+
static bool programProfileInInitialConfig(const AgentEnsemble& ensemble) {
58+
auto l3Asics = ensemble.getHwAsicTable()->getL3Asics();
59+
return !l3Asics.empty() &&
60+
l3Asics.front()->getAsicVendor() ==
61+
HwAsic::AsicVendor::ASIC_VENDOR_CHENAB;
62+
}
63+
64+
cfg::SwitchConfig initialConfig(
65+
const AgentEnsemble& ensemble) const override {
66+
auto config = AgentHwTest::initialConfig(ensemble);
67+
if (!programProfileInInitialConfig(ensemble)) {
68+
return config;
69+
}
70+
auto availablePorts = findAvailablePorts(ensemble);
71+
if (availablePorts.size() < 2) {
72+
// runTest() skips this profile, leave the config as is.
73+
return config;
74+
}
75+
for (const auto& port : {availablePorts[0], availablePorts[1]}) {
76+
utility::configurePortProfile(
77+
ensemble.getPlatformMapping(),
78+
ensemble.supportsAddRemovePort(),
79+
config,
80+
Profile,
81+
utility::getAllPortsInGroup(ensemble.getPlatformMapping(), port),
82+
port);
83+
}
84+
return config;
85+
}
86+
5087
void verifyPort(PortID portID) {
5188
auto port = getProgrammedState()->getPorts()->getNodeIf(portID);
5289
auto switchId =
@@ -185,6 +222,11 @@ class AgentPortProfileTest : public AgentHwTest {
185222
GTEST_SKIP() << "Not enough ports supporting this profile";
186223
}
187224
auto setup = [=, this]() {
225+
if (programProfileInInitialConfig(*getAgentEnsemble())) {
226+
// Profile is already programmed by initialConfig(). Reapplying it here
227+
// would recreate the ports, which Chenab does not support.
228+
return;
229+
}
188230
auto lbMode =
189231
getAgentEnsemble()->getL3Asics().front()->desiredLoopbackModes();
190232
auto config = utility::oneL3IntfTwoPortConfig(
@@ -209,6 +251,14 @@ class AgentPortProfileTest : public AgentHwTest {
209251
auto verify = [=, this]() {
210252
std::vector<PortID> testPorts = {availablePorts[0], availablePorts[1]};
211253
for (auto portID : testPorts) {
254+
// verifyPort() checks the hardware against whatever profile the port
255+
// currently carries, so a port left on its default profile would still
256+
// pass and the test would silently cover the wrong profile. On Chenab
257+
// setup() is a no-op and nothing else asserts the profile was applied,
258+
// so assert it here.
259+
auto port = getProgrammedState()->getPorts()->getNodeIf(portID);
260+
ASSERT_NE(port, nullptr);
261+
EXPECT_EQ(port->getProfileID(), Profile);
212262
verifyPort(portID);
213263
}
214264
// PHY info (line state, PMD lanes, FEC counters) is only available when

0 commit comments

Comments
 (0)