From 2f01b11a9661a1c005af7ffbde147f049b0f93e8 Mon Sep 17 00:00:00 2001 From: marif-nexthop Date: Fri, 30 Oct 2026 18:54:24 +0000 Subject: [PATCH] skip ports without PRBS state in fboss2 show interface prbs state --- .../prbs/state/CmdShowInterfacePrbsState.cpp | 38 ++++- .../fboss2/test/CmdShowInterfacePrbsTest.cpp | 146 +++++++++++++++++- fboss/cli/fboss2/test/MockClients.h | 8 + 3 files changed, 183 insertions(+), 9 deletions(-) diff --git a/fboss/cli/fboss2/commands/show/interface/prbs/state/CmdShowInterfacePrbsState.cpp b/fboss/cli/fboss2/commands/show/interface/prbs/state/CmdShowInterfacePrbsState.cpp index 848a09b50e5db..44aaaefce8681 100644 --- a/fboss/cli/fboss2/commands/show/interface/prbs/state/CmdShowInterfacePrbsState.cpp +++ b/fboss/cli/fboss2/commands/show/interface/prbs/state/CmdShowInterfacePrbsState.cpp @@ -12,7 +12,9 @@ #include "fboss/lib/phy/gen-cpp2/phy_types.h" #include +#include #include +#include #include "fboss/agent/if/gen-cpp2/common_types.h" #include "thrift/lib/cpp/util/EnumUtils.h" @@ -23,12 +25,13 @@ namespace { CmdShowInterfacePrbsStateTraits::RetType createModel( const HostInfo& hostInfo, const std::vector& queriedIfs, + bool ifsExplicitlyQueried, const std::vector& components, const std::map< phy::PortComponent, std::map>& allPrbsStates); -prbs::InterfacePrbsState getPrbsState( +std::optional getPrbsState( const std::map< phy::PortComponent, std::map>& allPrbsStates, @@ -59,10 +62,19 @@ CmdShowInterfacePrbsStateTraits::RetType CmdShowInterfacePrbsState::queryClient( for (const auto& port : portEntries_) { allIfs.push_back(port.second.name().value()); } - return createModel(hostInfo, allIfs, queriedPrbsComponents, allPrbsStates); + return createModel( + hostInfo, + allIfs, + false /* ifsExplicitlyQueried */, + queriedPrbsComponents, + allPrbsStates); } else { return createModel( - hostInfo, queriedIfs, queriedPrbsComponents, allPrbsStates); + hostInfo, + queriedIfs, + true /* ifsExplicitlyQueried */, + queriedPrbsComponents, + allPrbsStates); } } @@ -124,6 +136,7 @@ namespace { CmdShowInterfacePrbsStateTraits::RetType createModel( const HostInfo& /* hostInfo */, const std::vector& queriedIfs, + bool ifsExplicitlyQueried, const std::vector& components, const std::map< phy::PortComponent, @@ -132,20 +145,30 @@ CmdShowInterfacePrbsStateTraits::RetType createModel( for (const auto& intf : queriedIfs) { cli::PrbsStateInterfaceEntry intfEntry; for (const auto& component : components) { + // Each service reports only the ports that support PRBS on that + // component (e.g. no recycle/eventor ports, no transceiver components + // on empty cages), so a missing entry means "not applicable". + auto prbsState = getPrbsState(allPrbsStates, intf, component); + if (!prbsState) { + continue; + } cli::PrbsStateComponentEntry entry; entry.interfaceName() = intf; entry.component() = component; - entry.prbsState() = getPrbsState(allPrbsStates, intf, component); + entry.prbsState() = *prbsState; intfEntry.componentEntries()->push_back(entry); } if (intfEntry.componentEntries()->size()) { model.interfaceEntries()->push_back(intfEntry); + } else if (ifsExplicitlyQueried) { + throw std::runtime_error( + fmt::format("Could not find interface in PRBS states: {}", intf)); } } return model; } -prbs::InterfacePrbsState getPrbsState( +std::optional getPrbsState( const std::map< phy::PortComponent, std::map>& allPrbsStates, @@ -157,11 +180,10 @@ prbs::InterfacePrbsState getPrbsState( "Could not find component in PRBS states: " + std::to_string((int)component)); } - auto componentPrbsStates = allPrbsStatesItr->second; + const auto& componentPrbsStates = allPrbsStatesItr->second; auto componentPrbsStatesItr = componentPrbsStates.find(interface); if (componentPrbsStatesItr == componentPrbsStates.end()) { - throw std::runtime_error( - "Could not find interface in PRBS states: " + interface); + return std::nullopt; } return componentPrbsStatesItr->second; } diff --git a/fboss/cli/fboss2/test/CmdShowInterfacePrbsTest.cpp b/fboss/cli/fboss2/test/CmdShowInterfacePrbsTest.cpp index 709a469c87cb9..7605e68800627 100644 --- a/fboss/cli/fboss2/test/CmdShowInterfacePrbsTest.cpp +++ b/fboss/cli/fboss2/test/CmdShowInterfacePrbsTest.cpp @@ -1,14 +1,89 @@ // (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. +#include #include #include "fboss/cli/fboss2/commands/show/interface/prbs/capabilities/CmdShowInterfacePrbsCapabilities.h" #include "fboss/cli/fboss2/commands/show/interface/prbs/state/CmdShowInterfacePrbsState.h" #include "fboss/cli/fboss2/test/CmdHandlerTestBase.h" +using namespace ::testing; + namespace facebook::fboss { -class CmdShowInterfacePrbsTestFixture : public CmdHandlerTestBase {}; +namespace { + +using PrbsStates = std::map; + +prbs::InterfacePrbsState makePrbsState(bool enabled) { + prbs::InterfacePrbsState state; + state.polynomial() = prbs::PrbsPolynomial::PRBS31; + state.generatorEnabled() = enabled; + state.checkerEnabled() = enabled; + return state; +} + +std::map makePortInfos( + const std::vector& names) { + std::map ports; + int32_t portId = 1; + for (const auto& name : names) { + PortInfoThrift port; + port.portId() = portId; + port.name() = name; + ports[portId++] = port; + } + return ports; +} + +std::map> componentsByInterface( + const cli::ShowPrbsStateModel& model) { + std::map> components; + for (const auto& intfEntry : *model.interfaceEntries()) { + for (const auto& entry : *intfEntry.componentEntries()) { + components[*entry.interfaceName()].push_back(*entry.component()); + } + } + return components; +} + +} // namespace + +class CmdShowInterfacePrbsTestFixture : public CmdHandlerTestBase { + public: + // One hw switch reporting `asicStates`; qsfp_service reporting + // `transceiverLineStates` for TRANSCEIVER_LINE, nothing for + // TRANSCEIVER_SYSTEM, and rejecting the gearbox components like a platform + // without an external PHY. + void setupPrbsServices( + const PrbsStates& asicStates, + const PrbsStates& transceiverLineStates) { + setupMockedAgentServer(); + setupMockedHwAgentServer(); + + EXPECT_CALL(getMockAgent(), getMultiSwitchRunState(_)) + .WillRepeatedly([](MultiSwitchRunState& runState) { + runState.hwIndexToRunState()->insert({0, SwitchRunState::CONFIGURED}); + }); + EXPECT_CALL( + getMockHwAgent(), + getAllInterfacePrbsStates(_, phy::PortComponent::ASIC)) + .WillOnce([asicStates](PrbsStates& out, phy::PortComponent) { + out = asicStates; + }); + EXPECT_CALL(getQsfpService(), getAllInterfacePrbsStates(_, _)) + .WillRepeatedly([transceiverLineStates]( + PrbsStates& out, phy::PortComponent component) { + if (component == phy::PortComponent::GB_SYSTEM || + component == phy::PortComponent::GB_LINE) { + throw thrift::FbossBaseError("Unsupported component"); + } + if (component == phy::PortComponent::TRANSCEIVER_LINE) { + out = transceiverLineStates; + } + }); + } +}; TEST_F(CmdShowInterfacePrbsTestFixture, wikiDocHooksCapabilities) { EXPECT_FALSE(CmdShowInterfacePrbsCapabilitiesTraits::description().empty()); @@ -24,4 +99,73 @@ TEST_F(CmdShowInterfacePrbsTestFixture, wikiDocHooksState) { CmdShowInterfacePrbsState::sampleModel().interfaceEntries()->empty()); } +TEST_F( + CmdShowInterfacePrbsTestFixture, + queryClientSkipsPortsWithoutPrbsStateWhenListingAll) { + // The hw agent reports no recycle or eventor ports; qsfp_service reports + // the recycle port but not the eventor port. + setupPrbsServices( + {{"eth1/1/1", makePrbsState(true)}}, + {{"eth1/1/1", makePrbsState(false)}, + {"rcy1/1/1177", makePrbsState(false)}}); + EXPECT_CALL(getMockAgent(), getAllPortInfo(_)) + .WillOnce([](std::map& out) { + out = makePortInfos({"eth1/1/1", "rcy1/1/1177", "evt1/1/1553"}); + }); + + auto model = CmdShowInterfacePrbsState().queryClient(localhost(), {}, {}); + + EXPECT_THAT( + componentsByInterface(model), + ElementsAre( + Pair( + "eth1/1/1", + ElementsAre( + phy::PortComponent::ASIC, + phy::PortComponent::TRANSCEIVER_LINE)), + Pair( + "rcy1/1/1177", + ElementsAre(phy::PortComponent::TRANSCEIVER_LINE)))); + const auto& asicEntry = + model.interfaceEntries()->at(0).componentEntries()->at(0); + EXPECT_TRUE(*asicEntry.prbsState()->generatorEnabled()); + EXPECT_EQ(*asicEntry.prbsState()->polynomial(), prbs::PrbsPolynomial::PRBS31); +} + +TEST_F( + CmdShowInterfacePrbsTestFixture, + queryClientShowsReportedComponentsForQueriedPort) { + setupPrbsServices( + {{"eth1/1/1", makePrbsState(false)}}, + {{"rcy1/1/1177", makePrbsState(false)}}); + EXPECT_CALL(getMockAgent(), getAllPortInfo(_)).Times(0); + + auto model = + CmdShowInterfacePrbsState().queryClient(localhost(), {"rcy1/1/1177"}, {}); + + EXPECT_THAT( + componentsByInterface(model), + ElementsAre(Pair( + "rcy1/1/1177", ElementsAre(phy::PortComponent::TRANSCEIVER_LINE)))); +} + +TEST_F( + CmdShowInterfacePrbsTestFixture, + queryClientThrowsForQueriedPortWithoutPrbsState) { + setupPrbsServices( + {{"eth1/1/1", makePrbsState(false)}}, + {{"eth1/1/1", makePrbsState(false)}}); + EXPECT_CALL(getMockAgent(), getAllPortInfo(_)).Times(0); + + try { + CmdShowInterfacePrbsState().queryClient( + localhost(), {"eth1/1/1", "eth1/99/1"}, {}); + ADD_FAILURE() << "expected queryClient to throw for eth1/99/1"; + } catch (const std::runtime_error& e) { + EXPECT_THAT( + e.what(), + HasSubstr("Could not find interface in PRBS states: eth1/99/1")); + } +} + } // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/MockClients.h b/fboss/cli/fboss2/test/MockClients.h index ec22b7f9cdab9..5a3913f6b82f7 100644 --- a/fboss/cli/fboss2/test/MockClients.h +++ b/fboss/cli/fboss2/test/MockClients.h @@ -176,6 +176,10 @@ class MockFbossHwCtrlAgent MOCK_METHOD(void, getProgrammedState, (SwitchState)); MOCK_METHOD(BootType, getBootType, ()); MOCK_METHOD(void, triggerCableLengthMeasurement, (Ports)); + MOCK_METHOD( + void, + getAllInterfacePrbsStates, + ((std::map&), phy::PortComponent)); }; class MockFbossQsfpService @@ -209,6 +213,10 @@ class MockFbossQsfpService void, setPortAdminState, (std::unique_ptr, phy::PortComponent, bool)); + MOCK_METHOD( + void, + getAllInterfacePrbsStates, + ((std::map&), phy::PortComponent)); }; #ifdef IS_OSS