Clear PRBS stats API for CMIS modules

Summary: Added implementation for clearing PRBS stats in HW for CMIS modules Reviewed By: xanabani Differential Revision: D63242442 Privacy Context Container: L1125642 fbshipit-source-id: 0c82da330535b2b0628665850633dc0bee1df154
facebook · Sep 25, 2024 · 482c6d9 · 482c6d9
1 parent 7283404
commit 482c6d9
Show file tree

Hide file tree

Showing 10 changed files with 75 additions and 21 deletions.
diff --git 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
@@ -2553,14 +2553,18 @@ 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) {
     auto lockedTransceivers = transceivers_.rlock();
     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
@@ -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<phy::PrbsLaneStats>& 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<phy::PrbsLaneStats>& 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
@@ -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
@@ -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
@@ -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
@@ -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<std::mutex> 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
@@ -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
@@ -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
@@ -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<uint8_t> configuredHostLanes(
       uint8_t hostStartLane) const override;