diff --git a/fboss/agent/test/link_tests/PrbsTest.cpp b/fboss/agent/test/link_tests/PrbsTest.cpp index d51a62689f042..ab31cd4125512 100644 --- a/fboss/agent/test/link_tests/PrbsTest.cpp +++ b/fboss/agent/test/link_tests/PrbsTest.cpp @@ -435,7 +435,8 @@ class PrbsTest : public LinkTest { client->sync_getInterfacePrbsStats(stats, interfaceName, component); EXPECT_FALSE(stats.get_laneStats().empty()); for (const auto& laneStat : stats.get_laneStats()) { - EXPECT_EQ(laneStat.get_locked(), prbsEnabled); + // Don't check lock status because clear would have cleared it too and + // we may not have had an update of stats yet EXPECT_EQ(laneStat.get_numLossOfLock(), 0); EXPECT_LT(laneStat.get_timeSinceLastClear(), timestampBeforeClear); } diff --git a/fboss/qsfp_service/TransceiverManager.cpp b/fboss/qsfp_service/TransceiverManager.cpp index a59b8e737453c..51f6d037ccf75 100644 --- a/fboss/qsfp_service/TransceiverManager.cpp +++ b/fboss/qsfp_service/TransceiverManager.cpp @@ -2553,6 +2553,10 @@ phy::PrbsStats TransceiverManager::getPortPrbsStats( void TransceiverManager::clearPortPrbsStats( PortID portId, phy::PortComponent component) { + auto portName = getPortNameByPortId(portId); + if (!portName.has_value()) { + throw FbossError("Can't find a portName for portId ", portId); + } phy::Side side = prbsComponentToPhySide(component); if (component == phy::PortComponent::TRANSCEIVER_SYSTEM || component == phy::PortComponent::TRANSCEIVER_LINE) { @@ -2560,7 +2564,7 @@ void TransceiverManager::clearPortPrbsStats( if (auto tcvrID = getTransceiverID(portId)) { if (auto it = lockedTransceivers->find(*tcvrID); it != lockedTransceivers->end()) { - it->second->clearTransceiverPrbsStats(side); + it->second->clearTransceiverPrbsStats(*portName, side); } else { throw FbossError("Can't find transceiver ", *tcvrID); } diff --git a/fboss/qsfp_service/module/QsfpModule.cpp b/fboss/qsfp_service/module/QsfpModule.cpp index f9092c8df1fba..58417ad2d751f 100644 --- a/fboss/qsfp_service/module/QsfpModule.cpp +++ b/fboss/qsfp_service/module/QsfpModule.cpp @@ -842,23 +842,29 @@ void QsfpModule::refreshLocked() { } } -void QsfpModule::clearTransceiverPrbsStats(phy::Side side) { +void QsfpModule::clearTransceiverPrbsStats( + const std::string& portName, + phy::Side side) { + QSFP_LOG(INFO, this) << " Clearing prbs stats for port " << portName << " " + << ((side == phy::Side::LINE) ? "Line" : "Host"); auto systemPrbs = systemPrbsStats_.wlock(); auto linePrbs = linePrbsStats_.wlock(); + auto tcvrLanes = getTcvrLanesForPort(portName, side); - auto clearLaneStats = [this](std::vector& laneStats) { - for (auto& laneStat : laneStats) { - laneStat.ber() = 0; - laneStat.maxBer() = 0; - laneStat.snr().reset(); - laneStat.maxSnr().reset(); - laneStat.numLossOfLock() = 0; - laneStat.timeSinceLastClear() = 0; - - QSFP_LOG(INFO, this) << " Lane " << *laneStat.laneId() - << " ber and maxBer cleared"; - } - }; + auto clearLaneStats = + [this, &tcvrLanes](std::vector& laneStats) { + for (auto& laneStat : laneStats) { + auto laneId = *laneStat.laneId(); + if (tcvrLanes.find(laneId) == tcvrLanes.end()) { + continue; + } + laneStat = phy::PrbsLaneStats(); + laneStat.laneId() = laneId; + + QSFP_LOG(INFO, this) + << " Lane " << *laneStat.laneId() << " ber and maxBer cleared"; + } + }; if (side == phy::Side::SYSTEM) { clearLaneStats(*systemPrbs->laneStats()); } else { diff --git a/fboss/qsfp_service/module/QsfpModule.h b/fboss/qsfp_service/module/QsfpModule.h index b290acf997d9a..6941a650c6c4a 100644 --- a/fboss/qsfp_service/module/QsfpModule.h +++ b/fboss/qsfp_service/module/QsfpModule.h @@ -218,7 +218,8 @@ class QsfpModule : public Transceiver { return (*diagsCapability).value().get_prbsLineCapabilities(); } - void clearTransceiverPrbsStats(phy::Side side) override; + void clearTransceiverPrbsStats(const std::string& portName, phy::Side side) + override; SnapshotManager getTransceiverSnapshots() const { // return a copy to avoid needing a lock in the caller diff --git a/fboss/qsfp_service/module/Transceiver.h b/fboss/qsfp_service/module/Transceiver.h index aaa232b78e8b0..a95db351d4180 100644 --- a/fboss/qsfp_service/module/Transceiver.h +++ b/fboss/qsfp_service/module/Transceiver.h @@ -239,7 +239,10 @@ class Transceiver { const std::string& /* portName */, phy::Side /* side */) = 0; - virtual void clearTransceiverPrbsStats(phy::Side side) = 0; + // Clear the PRBS stats for the given port and side + virtual void clearTransceiverPrbsStats( + const std::string& portName, + phy::Side side) = 0; /* * Return true if such Transceiver can support remediation. diff --git a/fboss/qsfp_service/module/cmis/CmisFieldInfo.h b/fboss/qsfp_service/module/cmis/CmisFieldInfo.h index baad49246e222..3fa815412ec4c 100644 --- a/fboss/qsfp_service/module/cmis/CmisFieldInfo.h +++ b/fboss/qsfp_service/module/cmis/CmisFieldInfo.h @@ -265,6 +265,7 @@ enum FieldMasks : uint8_t { VDM_DESCRIPTOR_RESOURCE_MASK = 0x0f, CDB_FW_DOWNLOAD_EPL_SUPPORTED = 0x10, CDB_FW_DOWNLOAD_LPL_EPL_SUPPORTED = 0x11, + BER_CTRL_RESET_STAT_MASK = 0x20, }; enum FieldBitShift : uint8_t { diff --git a/fboss/qsfp_service/module/cmis/CmisModule.cpp b/fboss/qsfp_service/module/cmis/CmisModule.cpp index 18375d12e8d55..91cdada787794 100644 --- a/fboss/qsfp_service/module/cmis/CmisModule.cpp +++ b/fboss/qsfp_service/module/cmis/CmisModule.cpp @@ -3414,6 +3414,37 @@ bool CmisModule::getModuleStateChanged() { return getSettingsValue(CmisField::MODULE_FLAG, MODULE_STATE_CHANGED_MASK); } +void CmisModule::clearTransceiverPrbsStats( + const std::string& portName, + phy::Side side) { + auto clearTransceiverPrbsStatsLambda = [side, portName, this]() { + lock_guard g(qsfpModuleMutex_); + // Read modify write + // Write bit 5 in 13h.177 to 1 and then 0 to reset counters + uint8_t val; + readCmisField(CmisField::BER_CTRL, &val); + val |= BER_CTRL_RESET_STAT_MASK; + writeCmisField(CmisField::BER_CTRL, &val); + val &= ~BER_CTRL_RESET_STAT_MASK; + writeCmisField(CmisField::BER_CTRL, &val); + }; + auto i2cEvb = qsfpImpl_->getI2cEventBase(); + if (!i2cEvb) { + // Certain platforms cannot execute multiple I2C transactions in parallel + // and therefore don't have an I2C evb thread + clearTransceiverPrbsStatsLambda(); + } else { + via(i2cEvb) + .thenValue([clearTransceiverPrbsStatsLambda](auto&&) mutable { + clearTransceiverPrbsStatsLambda(); + }) + .get(); + } + + // Call the base class implementation to clear the common stats + QsfpModule::clearTransceiverPrbsStats(portName, side); +} + /* * setPortPrbsLocked * diff --git a/fboss/qsfp_service/module/cmis/CmisModule.h b/fboss/qsfp_service/module/cmis/CmisModule.h index 7a725d59645ba..7c3f3824c1594 100644 --- a/fboss/qsfp_service/module/cmis/CmisModule.h +++ b/fboss/qsfp_service/module/cmis/CmisModule.h @@ -611,6 +611,9 @@ class CmisModule : public QsfpModule { bool mediaSide); uint8_t datapathResetPendingMask_{0}; + + void clearTransceiverPrbsStats(const std::string& portName, phy::Side side) + override; }; } // namespace fboss diff --git a/fboss/qsfp_service/module/sff/SffModule.cpp b/fboss/qsfp_service/module/sff/SffModule.cpp index a298226c598ce..eb13dcd07cee5 100644 --- a/fboss/qsfp_service/module/sff/SffModule.cpp +++ b/fboss/qsfp_service/module/sff/SffModule.cpp @@ -977,7 +977,9 @@ void SffModule::updateQsfpData(bool allPages) { } } -void SffModule::clearTransceiverPrbsStats(phy::Side side) { +void SffModule::clearTransceiverPrbsStats( + const std::string& portName, + phy::Side side) { // We are asked to clear the prbs stats, therefore reset the bit count // reference points so that the BER calculations get reset too. @@ -1018,7 +1020,7 @@ void SffModule::clearTransceiverPrbsStats(phy::Side side) { } // Call the base class implementation to clear the common stats - QsfpModule::clearTransceiverPrbsStats(side); + QsfpModule::clearTransceiverPrbsStats(portName, side); } phy::PrbsStats SffModule::getPortPrbsStatsSideLocked( diff --git a/fboss/qsfp_service/module/sff/SffModule.h b/fboss/qsfp_service/module/sff/SffModule.h index 4b1b1d4916da5..216a2ac3816f6 100644 --- a/fboss/qsfp_service/module/sff/SffModule.h +++ b/fboss/qsfp_service/module/sff/SffModule.h @@ -257,7 +257,9 @@ class SffModule : public QsfpModule { // no-op } - void clearTransceiverPrbsStats(phy::Side side) override; + void clearTransceiverPrbsStats( + const std::string& /* portName */, + phy::Side side) override; std::vector configuredHostLanes( uint8_t hostStartLane) const override;