From e583e153e7c5bc14f47ec32107e61f1f23ae2baa Mon Sep 17 00:00:00 2001 From: Vishrant Vasavada Date: Mon, 21 Sep 2026 02:49:40 -0700 Subject: [PATCH] Populate serdes params for G202x in show interface phy Signed-off-by: Vishrant Vasavada --- fboss/agent/hw/sai/switch/SaiPortManager.cpp | 418 +++++++++++++++--- fboss/agent/hw/switch_asics/ChenabAsic.cpp | 1 + fboss/agent/hw/switch_asics/EbroAsic.cpp | 1 + fboss/agent/hw/switch_asics/G202xAsic.cpp | 1 + fboss/agent/hw/switch_asics/HwAsic.h | 5 + fboss/agent/hw/switch_asics/Jericho2Asic.cpp | 1 + fboss/agent/hw/switch_asics/Jericho3Asic.cpp | 1 + fboss/agent/hw/switch_asics/Jericho4Asic.cpp | 1 + fboss/agent/hw/switch_asics/P200Asic.cpp | 1 + fboss/agent/hw/switch_asics/Qumran4DAsic.cpp | 1 + fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp | 1 + fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp | 1 + fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp | 1 + fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp | 1 + fboss/agent/hw/switch_asics/TomahawkAsic.cpp | 1 + .../hw/switch_asics/TomahawkUltra1Asic.cpp | 1 + fboss/agent/hw/switch_asics/YubaAsic.cpp | 1 + .../interface/phy/CmdShowInterfacePhy.cpp | 390 ++++++++-------- .../fboss2/test/CmdShowInterfacePhyTest.cpp | 152 +++++-- fboss/lib/phy/phy.thrift | 23 + 20 files changed, 743 insertions(+), 260 deletions(-) diff --git a/fboss/agent/hw/sai/switch/SaiPortManager.cpp b/fboss/agent/hw/sai/switch/SaiPortManager.cpp index 7b66e7a4aab23..a5951a08745f3 100644 --- a/fboss/agent/hw/sai/switch/SaiPortManager.cpp +++ b/fboss/agent/hw/sai/switch/SaiPortManager.cpp @@ -43,7 +43,7 @@ #include #include -#include +#include // IWYU pragma: keep #if defined(BRCM_SAI_SDK_DNX) || defined(BRCM_SAI_SDK_XGS) #include @@ -2598,6 +2598,9 @@ bool SaiPortManager::rxSerdesParametersSupported() const { #if defined(BRCM_SAI_SDK_GTE_13_0) return platform_->getAsic()->isSupported( HwAsic::Feature::RX_SERDES_PARAMETERS); +#elif defined(TAJO_SDK_GTE_24_8_3001) + return platform_->getAsic()->isSupported( + HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS); #else return false; #endif @@ -4175,12 +4178,22 @@ std::vector SaiPortManager::getSerdesParameters( serdesParams[l].lane() = l; } - // Helper function to get serdes parameters with error handling + // Helper function to get serdes parameters with error handling. The + // attribute is produced by a factory so that, for extension attributes whose + // id is not mapped for this SAI implementation (operator() returns + // std::nullopt), we skip construction entirely -- constructing such an + // attribute would CHECK-fail (fatal) rather than throw SaiApiError. auto getSerdesParam = - [&](const char* paramName, auto attributeType, auto&& setter) { + [&](const char* paramName, auto&& attrFactory, auto&& setter) { + using AttrT = std::remove_cvref_t; + if constexpr (IsSaiExtensionAttribute::value) { + if (!AttrT::optionalExtensionAttributeId().has_value()) { + return; + } + } try { auto values = SaiApiTable::getInstance()->portApi().getAttribute( - serdesSaiPortId, attributeType); + serdesSaiPortId, attrFactory()); for (int l = 0; l < numPmdLanes; l++) { setter(serdesParams[l], values[l]); } @@ -4193,78 +4206,102 @@ std::vector SaiPortManager::getSerdesParameters( // Get all serdes parameters using the helper function getSerdesParam( "RVga", - SaiPortSerdesTraits::Attributes::RVga{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::RVga{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rvga() = val; }); getSerdesParam( "FltM", - SaiPortSerdesTraits::Attributes::FltM{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::FltM{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rxFltM() = val; }); getSerdesParam( "FltS", - SaiPortSerdesTraits::Attributes::FltS{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::FltS{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rxFltS() = val; }); getSerdesParam( "RxPf", - SaiPortSerdesTraits::Attributes::RxPf{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::RxPf{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rxPf() = val; }); #if defined(SAI_VERSION_14_0_EA_ODP) getSerdesParam( "RxPfLfq", - SaiPortSerdesTraits::Attributes::RxPfLfq{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::RxPfLfq{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rxPfLfq() = val; }); getSerdesParam( "RxPfHfq", - SaiPortSerdesTraits::Attributes::RxPfHfq{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::RxPfHfq{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rxPfHfq() = val; }); #endif getSerdesParam( "RxTap2", - SaiPortSerdesTraits::Attributes::RxTap2{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::RxTap2{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rxTap2() = val; }); getSerdesParam( "RxTap1", - SaiPortSerdesTraits::Attributes::RxTap1{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::RxTap1{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rxTap1() = val; }); getSerdesParam( "TpChn2", - SaiPortSerdesTraits::Attributes::TpChn2{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::TpChn2{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.tpChn2() = val; }); getSerdesParam( "TpChn1", - SaiPortSerdesTraits::Attributes::TpChn1{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::TpChn1{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.tpChn1() = val; }); getSerdesParam( "TpChn0", - SaiPortSerdesTraits::Attributes::TpChn0{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::TpChn0{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.tpChn0() = val; }); #if defined(BRCM_SAI_SDK_GTE_13_0) if (platform_->getAsic()->isSupported(HwAsic::Feature::SAI_SERDES_RX_REACH)) { getSerdesParam( "RxReach", - SaiPortSerdesTraits::Attributes::RxReach{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::RxReach{ + std::vector(numPmdLanes)}; + }, [swPortID](auto& param, auto val) { if (val == SAI_PORT_SERDES_REACH_MODE_NR) { param.rxReach() = phy::RxReach::RX_NORMAL_REACH; @@ -4284,12 +4321,166 @@ std::vector SaiPortManager::getSerdesParameters( HwAsic::Feature::SAI_SERDES_PRECODING)) { getSerdesParam( "RxPrecoding", - SaiPortSerdesTraits::Attributes::RxPrecodingAttr{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::RxPrecodingAttr{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.rxPrecoding() = val; }); } #endif +#if defined(TAJO_SDK_GTE_24_8_3001) + // Cisco SiliconOne (G202X) native RX serdes parameters. These live in the + // SAI_PORT_SERDES_ATTR_EXT_RX_* attributes and have no Broadcom equivalent, + // so the BCM-model reads above return nullopt (left unset) on Tajo. + if (platform_->getAsic()->isSupported( + HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS)) { + getSerdesParam( + "RxCtleCode", + [&] { + return SaiPortSerdesTraits::Attributes::RxCtleCode{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxCtleCode() = val; }); + getSerdesParam( + "RxDspMode", + [&] { + return SaiPortSerdesTraits::Attributes::RxDspMode{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxDspMode() = val; }); + getSerdesParam( + "RxAfeTrim", + [&] { + return SaiPortSerdesTraits::Attributes::RxAfeTrim{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxAfeTrim() = val; }); + getSerdesParam( + "RxDiffEncoderEn", + [&] { + return SaiPortSerdesTraits::Attributes::RxDiffEncoderEn{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxDiffEncoderEn() = val; }); + getSerdesParam( + "RxInstgBoost1Start", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgBoost1Start{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgBoost1Start() = val; }); + getSerdesParam( + "RxInstgBoost1Step", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgBoost1Step{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgBoost1Step() = val; }); + getSerdesParam( + "RxInstgBoost1Stop", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgBoost1Stop{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgBoost1Stop() = val; }); + getSerdesParam( + "RxInstgBoost2OrHrStart", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgBoost2OrHrStart{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgBoost2OrHrStart() = val; }); + getSerdesParam( + "RxInstgBoost2OrHrStep", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgBoost2OrHrStep{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgBoost2OrHrStep() = val; }); + getSerdesParam( + "RxInstgBoost2OrHrStop", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgBoost2OrHrStop{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgBoost2OrHrStop() = val; }); + getSerdesParam( + "RxInstgC1Start1p7", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgC1Start1p7{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgC1Start1p7() = val; }); + getSerdesParam( + "RxInstgC1Step1p7", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgC1Step1p7{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgC1Step1p7() = val; }); + getSerdesParam( + "RxInstgC1Stop1p7", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgC1Stop1p7{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgC1Stop1p7() = val; }); + getSerdesParam( + "RxInstgDfeStart1p7", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgDfeStart1p7{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgDfeStart1p7() = val; }); + getSerdesParam( + "RxInstgDfeStep1p7", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgDfeStep1p7{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgDfeStep1p7() = val; }); + getSerdesParam( + "RxInstgDfeStop1p7", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgDfeStop1p7{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgDfeStop1p7() = val; }); + // EXT_RX_INSTG_ENABLE_SCAN. leaba's RX_INSTG_ENABLE_DUMP_SCAN is a + // separate register that FBOSS does not read; the two are not expected to + // agree. + getSerdesParam( + "RxInstgEnableScan", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgEnableScan{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgEnableScan() = val; }); + getSerdesParam( + "RxInstgScanUseSrSettings", + [&] { + return SaiPortSerdesTraits::Attributes::RxInstgScanUseSrSettings{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxInstgScanUseSrSettings() = val; }); + getSerdesParam( + "RxFfeLengthBitmap", + [&] { + return SaiPortSerdesTraits::Attributes::RxFfeLengthBitmap{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxFfeLengthBitmap() = val; }); + getSerdesParam( + "RxFfeLmsDynamicGatingEn", + [&] { + return SaiPortSerdesTraits::Attributes::RxFfeLmsDynamicGatingEn{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.rxFfeLmsDynamicGatingEn() = val; }); + } +#endif + return serdesParams; } @@ -4306,11 +4497,20 @@ std::vector SaiPortManager::getTxSettings( std::vector txSettings(numPmdLanes); + // See getSerdesParameters(): the attribute is produced by a factory so that + // extension attributes whose id is not mapped for this SAI implementation are + // never constructed (construction would CHECK-fail rather than throw). auto getTxParam = - [&](const char* paramName, auto attributeType, auto&& setter) { + [&](const char* paramName, auto&& attrFactory, auto&& setter) { + using AttrT = std::remove_cvref_t; + if constexpr (IsSaiExtensionAttribute::value) { + if (!AttrT::optionalExtensionAttributeId().has_value()) { + return; + } + } try { auto values = SaiApiTable::getInstance()->portApi().getAttribute( - serdesSaiPortId, attributeType); + serdesSaiPortId, attrFactory()); for (int l = 0; l < numPmdLanes; l++) { setter(txSettings[l], values[l]); } @@ -4320,59 +4520,167 @@ std::vector SaiPortManager::getTxSettings( } }; + // On the Cisco SiliconOne (G202X) path the taps go to the i32 fir* fields + // instead of the legacy i16 pre/main/post: the agent programs values up to + // 52428 there, which does not fit in an i16 and wraps negative. Everywhere + // else the i16 fields stay authoritative, so nothing outside Tajo changes. + // This is the same condition the native TX block below uses. + bool wideTaps = false; +#if defined(TAJO_SDK_GTE_24_8_3001) + wideTaps = platform_->getAsic()->isSupported( + HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS); +#endif + getTxParam( "TxFirPre1", - SaiPortSerdesTraits::Attributes::TxFirPre1{ - std::vector(numPmdLanes)}, - [](auto& param, auto val) { param.pre() = static_cast(val); }); + [&] { + return SaiPortSerdesTraits::Attributes::TxFirPre1{ + std::vector(numPmdLanes)}; + }, + [wideTaps](auto& param, auto val) { + if (wideTaps) { + param.firPre1() = static_cast(val); + } else { + param.pre() = static_cast(val); + } + }); getTxParam( "TxFirPre2", - SaiPortSerdesTraits::Attributes::TxFirPre2{ - std::vector(numPmdLanes)}, - [](auto& param, auto val) { param.pre2() = static_cast(val); }); + [&] { + return SaiPortSerdesTraits::Attributes::TxFirPre2{ + std::vector(numPmdLanes)}; + }, + [wideTaps](auto& param, auto val) { + if (wideTaps) { + param.firPre2() = static_cast(val); + } else { + param.pre2() = static_cast(val); + } + }); getTxParam( "TxFirPre3", - SaiPortSerdesTraits::Attributes::TxFirPre3{ - std::vector(numPmdLanes)}, - [](auto& param, auto val) { param.pre3() = static_cast(val); }); + [&] { + return SaiPortSerdesTraits::Attributes::TxFirPre3{ + std::vector(numPmdLanes)}; + }, + [wideTaps](auto& param, auto val) { + if (wideTaps) { + param.firPre3() = static_cast(val); + } else { + param.pre3() = static_cast(val); + } + }); getTxParam( "TxFirMain", - SaiPortSerdesTraits::Attributes::TxFirMain{ - std::vector(numPmdLanes)}, - [](auto& param, auto val) { param.main() = static_cast(val); }); + [&] { + return SaiPortSerdesTraits::Attributes::TxFirMain{ + std::vector(numPmdLanes)}; + }, + [wideTaps](auto& param, auto val) { + if (wideTaps) { + param.firMain() = static_cast(val); + } else { + param.main() = static_cast(val); + } + }); getTxParam( "TxFirPost1", - SaiPortSerdesTraits::Attributes::TxFirPost1{ - std::vector(numPmdLanes)}, - [](auto& param, auto val) { param.post() = static_cast(val); }); + [&] { + return SaiPortSerdesTraits::Attributes::TxFirPost1{ + std::vector(numPmdLanes)}; + }, + [wideTaps](auto& param, auto val) { + if (wideTaps) { + param.firPost1() = static_cast(val); + } else { + param.post() = static_cast(val); + } + }); getTxParam( "TxFirPost2", - SaiPortSerdesTraits::Attributes::TxFirPost2{ - std::vector(numPmdLanes)}, - [](auto& param, auto val) { param.post2() = static_cast(val); }); + [&] { + return SaiPortSerdesTraits::Attributes::TxFirPost2{ + std::vector(numPmdLanes)}; + }, + [wideTaps](auto& param, auto val) { + if (wideTaps) { + param.firPost2() = static_cast(val); + } else { + param.post2() = static_cast(val); + } + }); getTxParam( "TxFirPost3", - SaiPortSerdesTraits::Attributes::TxFirPost3{ - std::vector(numPmdLanes)}, - [](auto& param, auto val) { param.post3() = static_cast(val); }); + [&] { + return SaiPortSerdesTraits::Attributes::TxFirPost3{ + std::vector(numPmdLanes)}; + }, + [wideTaps](auto& param, auto val) { + if (wideTaps) { + param.firPost3() = static_cast(val); + } else { + param.post3() = static_cast(val); + } + }); #if defined(BRCM_SAI_SDK_GTE_13_0) || SAI_API_VERSION >= SAI_VERSION(1, 14, 0) if (platform_->getAsic()->isSupported( HwAsic::Feature::SAI_SERDES_PRECODING)) { getTxParam( "TxPrecoding", - SaiPortSerdesTraits::Attributes::TxPrecodingAttr{ - std::vector(numPmdLanes)}, + [&] { + return SaiPortSerdesTraits::Attributes::TxPrecodingAttr{ + std::vector(numPmdLanes)}; + }, [](auto& param, auto val) { param.precoding() = val; }); } #endif +#if defined(TAJO_SDK_GTE_24_8_3001) + // Cisco SiliconOne (G202X) native TX serdes parameters, read from the + // SAI_PORT_SERDES_ATTR_EXT_TX_* attributes. FFE coefficients 0-4 are not read + // here: the Cisco SAI maps the standard TX FIR taps onto them, so they return + // the same registers as TxFirPre3..TxFirPost1 above. The rest have no + // standard equivalent. + if (platform_->getAsic()->isSupported( + HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS)) { + getTxParam( + "TxDriverSwing", + [&] { + return SaiPortSerdesTraits::Attributes::TxDriverSwing{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.driverSwing() = val; }); + getTxParam( + "TxDigGain", + [&] { + return SaiPortSerdesTraits::Attributes::TxDigGain{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.digGain() = val; }); + getTxParam( + "TxDiffEncoderEn", + [&] { + return SaiPortSerdesTraits::Attributes::TxDiffEncoderEn{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.diffEncoderEn() = val; }); + getTxParam( + "TxLdoBypass", + [&] { + return SaiPortSerdesTraits::Attributes::TxLdoBypass{ + std::vector(numPmdLanes)}; + }, + [](auto& param, auto val) { param.ldoBypass() = val; }); + } +#endif + return txSettings; } diff --git a/fboss/agent/hw/switch_asics/ChenabAsic.cpp b/fboss/agent/hw/switch_asics/ChenabAsic.cpp index 95ec100e25442..ae9e654c4060e 100644 --- a/fboss/agent/hw/switch_asics/ChenabAsic.cpp +++ b/fboss/agent/hw/switch_asics/ChenabAsic.cpp @@ -263,6 +263,7 @@ bool ChenabAsic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::SWITCH_WIDE_LINK_UP_DEBOUNCE: case HwAsic::Feature::ACL_DST_IPV6_WORD_QUALIFIERS: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::MY_MAC: case HwAsic::Feature::ISOLATION_GROUP: diff --git a/fboss/agent/hw/switch_asics/EbroAsic.cpp b/fboss/agent/hw/switch_asics/EbroAsic.cpp index 79de6a516658c..c234a58e9a698 100644 --- a/fboss/agent/hw/switch_asics/EbroAsic.cpp +++ b/fboss/agent/hw/switch_asics/EbroAsic.cpp @@ -280,6 +280,7 @@ bool EbroAsic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::SWITCH_WIDE_LINK_UP_DEBOUNCE: case HwAsic::Feature::ACL_DST_IPV6_WORD_QUALIFIERS: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::MY_MAC: case HwAsic::Feature::ISOLATION_GROUP: diff --git a/fboss/agent/hw/switch_asics/G202xAsic.cpp b/fboss/agent/hw/switch_asics/G202xAsic.cpp index c3f32fa931fff..2946811a20b08 100644 --- a/fboss/agent/hw/switch_asics/G202xAsic.cpp +++ b/fboss/agent/hw/switch_asics/G202xAsic.cpp @@ -69,6 +69,7 @@ bool G202xAsic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::L3_MTU_ERROR_TRAP: case HwAsic::Feature::ACL_COUNTER_LABEL: case HwAsic::Feature::SAI_PORT_SERDES_PROGRAMMING: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::PORT_WRED_COUNTER: case HwAsic::Feature::BRIDGE_PORT_8021Q: case HwAsic::Feature::WARMBOOT: diff --git a/fboss/agent/hw/switch_asics/HwAsic.h b/fboss/agent/hw/switch_asics/HwAsic.h index 6ff69e3ee3e04..86e9e0756f0cf 100644 --- a/fboss/agent/hw/switch_asics/HwAsic.h +++ b/fboss/agent/hw/switch_asics/HwAsic.h @@ -627,6 +627,11 @@ class HwAsic { // Each attribute returns the drop reasons seen since the last read, which // is cleared on read. Counterpart to SWITCH_CUSTOM_DROP_BITMAP_SUPPORT. SWITCH_DROP_REASON_LIST_SUPPORT, + // Read-back of Cisco SiliconOne (Tajo/G202X) native serdes parameters via + // the SAI_PORT_SERDES_ATTR_EXT_* attributes (TX FFE/driver-swing and RX + // instg/ctle/dsp/afe). Distinct from the Broadcom RX_SERDES_PARAMETERS + // feature, which reads a different (BCM-only) attribute set. + TAJO_RX_SERDES_PARAMETERS, // SDK reports the type of a received packet, so the application does // not have to classify the packet itself to identify specific types. RX_PACKET_TYPE, diff --git a/fboss/agent/hw/switch_asics/Jericho2Asic.cpp b/fboss/agent/hw/switch_asics/Jericho2Asic.cpp index 4dac9f557aea8..7e2f8cf05151c 100644 --- a/fboss/agent/hw/switch_asics/Jericho2Asic.cpp +++ b/fboss/agent/hw/switch_asics/Jericho2Asic.cpp @@ -264,6 +264,7 @@ bool Jericho2Asic::isSupported(Feature feature) const { case HwAsic::Feature::HW_PROTECTION_SWITCHOVER_COUNTERS: case HwAsic::Feature::PBR_ACL_BYTE_COUNTER: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::MY_MAC: case HwAsic::Feature::ISOLATION_GROUP: diff --git a/fboss/agent/hw/switch_asics/Jericho3Asic.cpp b/fboss/agent/hw/switch_asics/Jericho3Asic.cpp index 1735a350b84fe..f7604ac0ca1ae 100644 --- a/fboss/agent/hw/switch_asics/Jericho3Asic.cpp +++ b/fboss/agent/hw/switch_asics/Jericho3Asic.cpp @@ -308,6 +308,7 @@ bool Jericho3Asic::isSupported(Feature feature) const { case HwAsic::Feature::XPHY_SERDES_TX_FIR_POST3: case HwAsic::Feature::LAG_TTL_DECREMENT_DISABLE: case HwAsic::Feature::PORT_LATCH_STATUS: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Jericho4Asic.cpp b/fboss/agent/hw/switch_asics/Jericho4Asic.cpp index ca022c4c0502f..51c3f6dc8e883 100644 --- a/fboss/agent/hw/switch_asics/Jericho4Asic.cpp +++ b/fboss/agent/hw/switch_asics/Jericho4Asic.cpp @@ -308,6 +308,7 @@ bool Jericho4Asic::isSupported(Feature feature) const { case HwAsic::Feature::XPHY_SERDES_TX_FIR_POST3: case HwAsic::Feature::LAG_TTL_DECREMENT_DISABLE: case HwAsic::Feature::PORT_LATCH_STATUS: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/P200Asic.cpp b/fboss/agent/hw/switch_asics/P200Asic.cpp index 9d04bc99480b9..f9488a56fa35d 100644 --- a/fboss/agent/hw/switch_asics/P200Asic.cpp +++ b/fboss/agent/hw/switch_asics/P200Asic.cpp @@ -277,6 +277,7 @@ bool P200Asic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::SWITCH_WIDE_LINK_UP_DEBOUNCE: case HwAsic::Feature::ACL_DST_IPV6_WORD_QUALIFIERS: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::MY_MAC: case HwAsic::Feature::ISOLATION_GROUP: diff --git a/fboss/agent/hw/switch_asics/Qumran4DAsic.cpp b/fboss/agent/hw/switch_asics/Qumran4DAsic.cpp index 9b45123402728..a32ffd5d4a4aa 100644 --- a/fboss/agent/hw/switch_asics/Qumran4DAsic.cpp +++ b/fboss/agent/hw/switch_asics/Qumran4DAsic.cpp @@ -301,6 +301,7 @@ bool Qumran4DAsic::isSupported(Feature feature) const { case HwAsic::Feature::HW_PROTECTION_SWITCHOVER_COUNTERS: case HwAsic::Feature::PBR_ACL_BYTE_COUNTER: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::MY_MAC: case HwAsic::Feature::ISOLATION_GROUP: diff --git a/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp index 96fae4b3d86b0..9da3d240e7ffb 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp @@ -258,6 +258,7 @@ bool Tomahawk3Asic::isSupported(Feature feature) const { case HwAsic::Feature::HW_PROTECTION_SWITCHOVER_COUNTERS: case HwAsic::Feature::PBR_ACL_BYTE_COUNTER: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::XPHY_SERDES_TX_FIR_POST3: case HwAsic::Feature::LAG_TTL_DECREMENT_DISABLE: diff --git a/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp index 5e1c9178602cd..0b0b1e7e45827 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp @@ -275,6 +275,7 @@ bool Tomahawk4Asic::isSupported(Feature feature) const { case HwAsic::Feature::PBR_ACL_BYTE_COUNTER: case HwAsic::Feature::ARS_CURRENT_PORT_LOAD: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::ISOLATION_GROUP: case HwAsic::Feature::XPHY_SERDES_TX_FIR_POST3: diff --git a/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp index dde1ca926a02c..ede3c44a90d49 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp @@ -258,6 +258,7 @@ bool Tomahawk5Asic::isSupported(Feature feature) const { case HwAsic::Feature::ACL_DST_IPV6_WORD_QUALIFIERS: case HwAsic::Feature::SLL_HLL_DISCARD_COUNTERS: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::ISOLATION_GROUP: case HwAsic::Feature::XPHY_SERDES_TX_FIR_POST3: diff --git a/fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp index c028acbdbb105..a5b284d749933 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp @@ -264,6 +264,7 @@ bool Tomahawk6Asic::isSupported(Feature feature) const { case HwAsic::Feature::LINK_LAYER_RETRANSMISSION: case HwAsic::Feature::SLL_HLL_DISCARD_COUNTERS: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::ISOLATION_GROUP: case HwAsic::Feature::CSIG: diff --git a/fboss/agent/hw/switch_asics/TomahawkAsic.cpp b/fboss/agent/hw/switch_asics/TomahawkAsic.cpp index 62d85d70ccfda..3981f72452289 100644 --- a/fboss/agent/hw/switch_asics/TomahawkAsic.cpp +++ b/fboss/agent/hw/switch_asics/TomahawkAsic.cpp @@ -256,6 +256,7 @@ bool TomahawkAsic::isSupported(Feature feature) const { case HwAsic::Feature::HW_PROTECTION_SWITCHOVER_COUNTERS: case HwAsic::Feature::PBR_ACL_BYTE_COUNTER: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::ISOLATION_GROUP: case HwAsic::Feature::XPHY_SERDES_TX_FIR_POST3: diff --git a/fboss/agent/hw/switch_asics/TomahawkUltra1Asic.cpp b/fboss/agent/hw/switch_asics/TomahawkUltra1Asic.cpp index 6237a07f3fa43..4bb5bd01fd679 100644 --- a/fboss/agent/hw/switch_asics/TomahawkUltra1Asic.cpp +++ b/fboss/agent/hw/switch_asics/TomahawkUltra1Asic.cpp @@ -259,6 +259,7 @@ bool TomahawkUltra1Asic::isSupported(Feature feature) const { case HwAsic::Feature::HW_PROTECTION_SWITCHOVER_COUNTERS: case HwAsic::Feature::PBR_ACL_BYTE_COUNTER: case HwAsic::Feature::NEXT_HOP_GROUP_MEMBER_MONITORED_OBJECT: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::MY_MAC: case HwAsic::Feature::ISOLATION_GROUP: diff --git a/fboss/agent/hw/switch_asics/YubaAsic.cpp b/fboss/agent/hw/switch_asics/YubaAsic.cpp index 2f1e594685afe..0fe00ce99372d 100644 --- a/fboss/agent/hw/switch_asics/YubaAsic.cpp +++ b/fboss/agent/hw/switch_asics/YubaAsic.cpp @@ -274,6 +274,7 @@ bool YubaAsic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::LINK_LAYER_RETRANSMISSION: case HwAsic::Feature::SLL_HLL_DISCARD_COUNTERS: case HwAsic::Feature::HW_PROTECTION_SWITCHOVER_COUNTERS: + case HwAsic::Feature::TAJO_RX_SERDES_PARAMETERS: case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::MY_MAC: case HwAsic::Feature::ISOLATION_GROUP: diff --git a/fboss/cli/fboss2/commands/show/interface/phy/CmdShowInterfacePhy.cpp b/fboss/cli/fboss2/commands/show/interface/phy/CmdShowInterfacePhy.cpp index cc837b508547a..54bdc1a65dab5 100644 --- a/fboss/cli/fboss2/commands/show/interface/phy/CmdShowInterfacePhy.cpp +++ b/fboss/cli/fboss2/commands/show/interface/phy/CmdShowInterfacePhy.cpp @@ -3,13 +3,110 @@ #include "fboss/cli/fboss2/commands/show/interface/phy/CmdShowInterfacePhy.h" #include "fboss/cli/fboss2/CmdHandler.cpp" -#include #include +#include +#include +#include #include "fboss/cli/fboss2/utils/Table.h" #include "thrift/lib/cpp/util/EnumUtils.h" namespace facebook::fboss { +namespace { + +constexpr auto kNotApplicable = "N/A"; + +// One column of a per-lane table: its header plus one cell per lane. +struct LaneColumn { + std::string header; + std::vector cells; +}; + +std::vector makeLaneColumns( + const std::vector& headers) { + std::vector columns; + columns.reserve(headers.size()); + for (const auto& header : headers) { + columns.push_back(LaneColumn{header, {}}); + } + return columns; +} + +void addLaneCells( + std::vector& columns, + const std::vector& cells) { + CHECK_EQ(columns.size(), cells.size()); + for (size_t i = 0; i < cells.size(); ++i) { + columns[i].cells.push_back(cells[i]); + } +} + +template +std::string optionalStr(const OptionalField& field) { + return field.has_value() ? std::to_string(*field) : kNotApplicable; +} + +// A FIR tap, from the i32 fir* field on platforms that populate it and the +// legacy i16 everywhere else. Only the Cisco SiliconOne path fills fir*, where +// the i16 would wrap above 32767; every other platform takes the fallback. +template +std::string txTapStr(const OptionalField& wide, std::string narrow) { + return wide.has_value() ? std::to_string(*wide) : std::move(narrow); +} + +// Renders a per-lane list, e.g. eye heights, as N/A when nothing was reported +// so that an empty column can be dropped like any other inapplicable one. +std::string joinedStr(const std::vector& values) { + return values.empty() ? kNotApplicable : folly::join(",", values); +} + +bool isNotApplicable(const Table::RowData& cell) { + return std::visit( + [](const auto& data) { + using T = std::decay_t; + if constexpr (std::is_same_v) { + return data == kNotApplicable; + } else { + return data.getData() == kNotApplicable; + } + }, + cell); +} + +// Prints one row per lane, dropping every column that is N/A on all lanes. +// Those parameters are not applicable to the ASIC being queried, and listing +// them only pushes the columns that do have values off the screen. +void printLaneTable( + std::ostream& out, + const std::string& title, + const std::vector& lanes, + const std::vector& columns) { + std::vector applicable; + std::vector header{title, "Lane"}; + for (const auto& column : columns) { + if (std::any_of( + column.cells.begin(), column.cells.end(), [](const auto& cell) { + return !isNotApplicable(cell); + })) { + applicable.push_back(&column); + header.emplace_back(column.header); + } + } + + Table table; + table.setHeader(header); + for (size_t row = 0; row < lanes.size(); ++row) { + std::vector cells{"", std::to_string(lanes[row])}; + for (const auto* column : applicable) { + cells.emplace_back(column->cells[row]); + } + table.addRow(cells); + } + out << table; +} + +} // namespace + CmdShowInterfacePhy::RetType CmdShowInterfacePhy::queryClient( const HostInfo& hostInfo, const utils::PortList& queriedIfs, @@ -63,9 +160,13 @@ void CmdShowInterfacePhy::printPhyInfo( {phyType + " Data Collected", utils::getPrettyElapsedTime(*timeCollected) + " ago"}); out << table; + // phyState/phyStats.system() are thrift field accessors; clang-tidy's + // bugprone-unsafe-functions matches the name against ::system(). + // NOLINTNEXTLINE(bugprone-unsafe-functions) if (auto systemState = phyState.system()) { - printSideStateAndStat( - out, *systemState, phyStats.system().ensure(), prefix + "System "); + // NOLINTNEXTLINE(bugprone-unsafe-functions) + auto& systemStats = phyStats.system().ensure(); + printSideStateAndStat(out, *systemState, systemStats, prefix + "System "); } printSideStateAndStat( out, *phyState.line(), *phyStats.line(), prefix + "Line "); @@ -237,11 +338,8 @@ void CmdShowInterfacePhy::printPmdLaneRxInfo( phy::PhySideStats& sideStats, const std::set& pmdLanes, const std::string& prefix) { - Table pmdRxTable; - pmdRxTable.setHeader( - {prefix + "RX PMD", - "Lane", - "RX Signal Detect Live", + auto columns = makeLaneColumns( + {"RX Signal Detect Live", "RX Signal Detect Changed", "RX CDR Lock Live", "RX CDR Lock Changed", @@ -249,35 +347,14 @@ void CmdShowInterfacePhy::printPmdLaneRxInfo( "Eye Widths", "Rx PPM", "RX SNR"}); + + std::vector lanes; for (auto pmdLane : pmdLanes) { + lanes.push_back(pmdLane); auto laneState = (*sideState.pmd()->lanes())[pmdLane]; auto laneStat = (*sideStats.pmd()->lanes())[pmdLane]; - std::string sigDetLive = "N/A"; - std::string cdrLockLive = "N/A"; - std::string sigDetChanged = "N/A"; - std::string cdrLockChanged = "N/A"; - std::string rxPPM = "N/A"; - std::string rxSNR = "N/A"; std::vector eyeHeights = {}; std::vector eyeWidths = {}; - if (auto rxSigDetLive = laneState.signalDetectLive()) { - sigDetLive = std::to_string(*rxSigDetLive); - } - if (auto rxSigDetChanged = laneStat.signalDetectChangedCount()) { - sigDetChanged = std::to_string(*rxSigDetChanged); - } - if (auto rxCdrLockLive = laneState.cdrLockLive()) { - cdrLockLive = std::to_string(*rxCdrLockLive); - } - if (auto rxCdrLockChanged = laneStat.cdrLockChangedCount()) { - cdrLockChanged = std::to_string(*rxCdrLockChanged); - } - if (auto rxFreqPPM = laneState.rxFrequencyPPM()) { - rxPPM = std::to_string(*rxFreqPPM); - } - if (auto rxLaneSNR = laneStat.snr()) { - rxSNR = std::to_string(*rxLaneSNR); - } if (auto eyes = laneStat.eyes()) { for (const auto& eye : *eyes) { if (auto eyeW = eye.width()) { @@ -288,19 +365,20 @@ void CmdShowInterfacePhy::printPmdLaneRxInfo( } } } - pmdRxTable.addRow( - {"", - std::to_string(pmdLane), - makeColorCellForLiveFlag(sigDetLive), - sigDetChanged, - makeColorCellForLiveFlag(cdrLockLive), - cdrLockChanged, - folly::join(",", eyeHeights), - folly::join(",", eyeWidths), - rxPPM, - rxSNR}); + + addLaneCells( + columns, + {makeColorCellForLiveFlag(optionalStr(laneState.signalDetectLive())), + optionalStr(laneStat.signalDetectChangedCount()), + makeColorCellForLiveFlag(optionalStr(laneState.cdrLockLive())), + optionalStr(laneStat.cdrLockChangedCount()), + joinedStr(eyeHeights), + joinedStr(eyeWidths), + optionalStr(laneState.rxFrequencyPPM()), + optionalStr(laneStat.snr())}); } - out << pmdRxTable; + + printLaneTable(out, prefix + "RX PMD", lanes, columns); } void CmdShowInterfacePhy::printPmdLaneTxInfo( @@ -308,60 +386,50 @@ void CmdShowInterfacePhy::printPmdLaneTxInfo( phy::PhySideState& sideState, const std::set& pmdLanes, const std::string& prefix) { - Table pmdTxTable; - pmdTxTable.setHeader( - {prefix + "TX PMD", - "Lane", - "Pre3", + auto columns = makeLaneColumns( + {"Pre3", "Pre2", "Pre1", "Main", "Post1", "Post2", "Post3", - "Precoding"}); + "Precoding", + "DriverSwing", + "DigGain", + "DiffEncoderEn", + "LdoBypass"}); + + std::vector lanes; for (auto pmdLane : pmdLanes) { + lanes.push_back(pmdLane); auto laneState = (*sideState.pmd()->lanes())[pmdLane]; auto txSettings = *laneState.txSettings(); - std::string pre3 = "N/A"; - auto txPre3 = txSettings.pre3(); - if (txPre3.has_value()) { - pre3 = std::to_string(*txPre3); - } - std::string pre2 = std::to_string(*txSettings.pre2()); - std::string pre = std::to_string(*txSettings.pre()); - std::string main = std::to_string(*txSettings.main()); - std::string post = std::to_string(*txSettings.post()); - std::string post2 = std::to_string(*txSettings.post2()); - std::string post3 = std::to_string(*txSettings.post3()); - std::string precoding = "N/A"; - if (auto txPrecoding = txSettings.precoding()) { - precoding = std::to_string(*txPrecoding); - } - pmdTxTable.addRow( - {"", - std::to_string(pmdLane), - pre3, - pre2, - pre, - main, - post, - post2, - post3, - precoding}); + addLaneCells( + columns, + {txTapStr(txSettings.firPre3(), optionalStr(txSettings.pre3())), + txTapStr(txSettings.firPre2(), std::to_string(*txSettings.pre2())), + txTapStr(txSettings.firPre1(), std::to_string(*txSettings.pre())), + txTapStr(txSettings.firMain(), std::to_string(*txSettings.main())), + txTapStr(txSettings.firPost1(), std::to_string(*txSettings.post())), + txTapStr(txSettings.firPost2(), std::to_string(*txSettings.post2())), + txTapStr(txSettings.firPost3(), std::to_string(*txSettings.post3())), + optionalStr(txSettings.precoding()), + optionalStr(txSettings.driverSwing()), + optionalStr(txSettings.digGain()), + optionalStr(txSettings.diffEncoderEn()), + optionalStr(txSettings.ldoBypass())}); } - out << pmdTxTable; + + printLaneTable(out, prefix + "TX PMD", lanes, columns); } void CmdShowInterfacePhy::printSerdesParametersInfo( std::ostream& out, phy::PmdState& pmdState, const std::string& prefix) { - Table serdesTable; - serdesTable.setHeader( - {prefix + "Serdes Parameters", - "Lane", - "RVga", + auto columns = makeLaneColumns( + {"RVga", "Dco", "TpChn0", "TpChn1", @@ -380,107 +448,83 @@ void CmdShowInterfacePhy::printSerdesParametersInfo( "RxEqP1", "RxEqP2", "RxReach", - "RxPrecoding"}); + "RxPrecoding", + "RxCtleCode", + "RxDspMode", + "RxAfeTrim", + "RxDiffEncoderEn", + "RxInstgBoost1Start", + "RxInstgBoost1Step", + "RxInstgBoost1Stop", + "RxInstgBoost2OrHrStart", + "RxInstgBoost2OrHrStep", + "RxInstgBoost2OrHrStop", + "RxInstgC1Start1p7", + "RxInstgC1Step1p7", + "RxInstgC1Stop1p7", + "RxInstgDfeStart1p7", + "RxInstgDfeStep1p7", + "RxInstgDfeStop1p7", + "RxInstgEnableScan", + "RxInstgScanUseSrSettings", + "RxFfeLengthBitmap", + "RxFfeLmsDynamicGatingEn"}); + std::vector lanes; for (const auto& [laneId, laneState] : *pmdState.lanes()) { + lanes.push_back(laneId); auto serdesParams = laneState.serdesParameters(); - std::string rvga = "N/A"; - std::string dco = "N/A"; - std::string tpChn0 = "N/A"; - std::string tpChn1 = "N/A"; - std::string tpChn2 = "N/A"; - std::string rxPf = "N/A"; - std::string rxPfLfq = "N/A"; - std::string rxPfHfq = "N/A"; - std::string rxFltM = "N/A"; - std::string rxFltS = "N/A"; - std::string rxTap1 = "N/A"; - std::string rxTap2 = "N/A"; - std::string rxEq3 = "N/A"; - std::string rxEq2 = "N/A"; - std::string rxEq1 = "N/A"; - std::string rxEqM = "N/A"; - std::string rxEqP1 = "N/A"; - std::string rxEqP2 = "N/A"; - std::string rxReach = "N/A"; - std::string rxPrecoding = "N/A"; - - if (auto rvgaVal = serdesParams->rvga()) { - rvga = std::to_string(*rvgaVal); - } - if (auto dcoVal = serdesParams->dco()) { - dco = std::to_string(*dcoVal); - } - if (auto tpChn0Val = serdesParams->tpChn0()) { - tpChn0 = std::to_string(*tpChn0Val); - } - if (auto tpChn1Val = serdesParams->tpChn1()) { - tpChn1 = std::to_string(*tpChn1Val); - } - if (auto tpChn2Val = serdesParams->tpChn2()) { - tpChn2 = std::to_string(*tpChn2Val); - } - if (auto rxPfVal = serdesParams->rxPf()) { - rxPf = std::to_string(*rxPfVal); - } - if (auto rxPfLfqVal = serdesParams->rxPfLfq()) { - rxPfLfq = std::to_string(*rxPfLfqVal); - } - if (auto rxPfHfqVal = serdesParams->rxPfHfq()) { - rxPfHfq = std::to_string(*rxPfHfqVal); - } - if (auto rxFltMVal = serdesParams->rxFltM()) { - rxFltM = std::to_string(*rxFltMVal); - } - if (auto rxFltSVal = serdesParams->rxFltS()) { - rxFltS = std::to_string(*rxFltSVal); - } - if (auto rxTap1Val = serdesParams->rxTap1()) { - rxTap1 = std::to_string(*rxTap1Val); - } - if (auto rxTap2Val = serdesParams->rxTap2()) { - rxTap2 = std::to_string(*rxTap2Val); - } - if (auto rxEq3Val = serdesParams->rxEq3()) { - rxEq3 = std::to_string(*rxEq3Val); - } - if (auto rxEq2Val = serdesParams->rxEq2()) { - rxEq2 = std::to_string(*rxEq2Val); - } - if (auto rxEq1Val = serdesParams->rxEq1()) { - rxEq1 = std::to_string(*rxEq1Val); - } - if (auto rxEqMVal = serdesParams->rxEqM()) { - rxEqM = std::to_string(*rxEqMVal); - } - if (auto rxEqP1Val = serdesParams->rxEqP1()) { - rxEqP1 = std::to_string(*rxEqP1Val); - } - if (auto rxEqP2Val = serdesParams->rxEqP2()) { - rxEqP2 = std::to_string(*rxEqP2Val); - } + std::string rxReach = kNotApplicable; if (auto rxReachVal = serdesParams->rxReach()) { rxReach = apache::thrift::util::enumNameSafe(*rxReachVal); } - if (auto rxPrecodingVal = serdesParams->rxPrecoding()) { - rxPrecoding = std::to_string(*rxPrecodingVal); - } - serdesTable.addRow({"", std::to_string(laneId), - rvga, dco, - tpChn0, tpChn1, - tpChn2, rxPf, - rxPfLfq, rxPfHfq, - rxFltM, rxFltS, - rxTap1, rxTap2, - rxEq3, rxEq2, - rxEq1, rxEqM, - rxEqP1, rxEqP2, - rxReach, rxPrecoding}); + addLaneCells( + columns, + {optionalStr(serdesParams->rvga()), + optionalStr(serdesParams->dco()), + optionalStr(serdesParams->tpChn0()), + optionalStr(serdesParams->tpChn1()), + optionalStr(serdesParams->tpChn2()), + optionalStr(serdesParams->rxPf()), + optionalStr(serdesParams->rxPfLfq()), + optionalStr(serdesParams->rxPfHfq()), + optionalStr(serdesParams->rxFltM()), + optionalStr(serdesParams->rxFltS()), + optionalStr(serdesParams->rxTap1()), + optionalStr(serdesParams->rxTap2()), + optionalStr(serdesParams->rxEq3()), + optionalStr(serdesParams->rxEq2()), + optionalStr(serdesParams->rxEq1()), + optionalStr(serdesParams->rxEqM()), + optionalStr(serdesParams->rxEqP1()), + optionalStr(serdesParams->rxEqP2()), + rxReach, + optionalStr(serdesParams->rxPrecoding()), + optionalStr(serdesParams->rxCtleCode()), + optionalStr(serdesParams->rxDspMode()), + optionalStr(serdesParams->rxAfeTrim()), + optionalStr(serdesParams->rxDiffEncoderEn()), + optionalStr(serdesParams->rxInstgBoost1Start()), + optionalStr(serdesParams->rxInstgBoost1Step()), + optionalStr(serdesParams->rxInstgBoost1Stop()), + optionalStr(serdesParams->rxInstgBoost2OrHrStart()), + optionalStr(serdesParams->rxInstgBoost2OrHrStep()), + optionalStr(serdesParams->rxInstgBoost2OrHrStop()), + optionalStr(serdesParams->rxInstgC1Start1p7()), + optionalStr(serdesParams->rxInstgC1Step1p7()), + optionalStr(serdesParams->rxInstgC1Stop1p7()), + optionalStr(serdesParams->rxInstgDfeStart1p7()), + optionalStr(serdesParams->rxInstgDfeStep1p7()), + optionalStr(serdesParams->rxInstgDfeStop1p7()), + optionalStr(serdesParams->rxInstgEnableScan()), + optionalStr(serdesParams->rxInstgScanUseSrSettings()), + optionalStr(serdesParams->rxFfeLengthBitmap()), + optionalStr(serdesParams->rxFfeLmsDynamicGatingEn())}); } - out << serdesTable; + printLaneTable(out, prefix + "Serdes Parameters", lanes, columns); } Table::StyledCell CmdShowInterfacePhy::makeColorCellForLiveFlag( diff --git a/fboss/cli/fboss2/test/CmdShowInterfacePhyTest.cpp b/fboss/cli/fboss2/test/CmdShowInterfacePhyTest.cpp index 4775f0a73bc4b..9841a99ff4aeb 100644 --- a/fboss/cli/fboss2/test/CmdShowInterfacePhyTest.cpp +++ b/fboss/cli/fboss2/test/CmdShowInterfacePhyTest.cpp @@ -1,6 +1,5 @@ // (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. -#include #include #include @@ -56,6 +55,25 @@ TableCells parseTable(const std::string& output) { return rows; } +phy::LaneState makeRxLaneState(int16_t lane, bool signalDetectLive) { + phy::LaneState laneState; + laneState.lane() = lane; + laneState.signalDetectLive() = signalDetectLive; + laneState.cdrLockLive() = true; + return laneState; +} + +phy::LaneStats makeRxLaneStats( + int16_t lane, + std::optional signalDetectChangedCount) { + phy::LaneStats laneStats; + laneStats.lane() = lane; + if (signalDetectChangedCount.has_value()) { + laneStats.signalDetectChangedCount() = *signalDetectChangedCount; + } + return laneStats; +} + phy::LaneState makeTxLaneState(int16_t lane, std::optional precoding) { phy::TxSettings txSettings; txSettings.pre3() = 1; @@ -68,6 +86,9 @@ phy::LaneState makeTxLaneState(int16_t lane, std::optional precoding) { if (precoding.has_value()) { txSettings.precoding() = *precoding; } + // Cisco-native TX fields: set a representative subset to values and leave the + // rest unset so both the value and N/A rendering paths are exercised. + txSettings.driverSwing() = 20; phy::LaneState laneState; laneState.lane() = lane; @@ -87,6 +108,10 @@ phy::LaneState makeSerdesLaneState( if (rxPrecoding.has_value()) { serdesParams.rxPrecoding() = *rxPrecoding; } + // Cisco-native RX fields: set a representative subset to values and leave the + // rest unset so both the value and N/A rendering paths are exercised. + serdesParams.rxCtleCode() = 30; + serdesParams.rxInstgBoost1Start() = 40; phy::LaneState laneState; laneState.lane() = lane; @@ -98,6 +123,32 @@ phy::LaneState makeSerdesLaneState( class CmdShowInterfacePhyTestFixture : public CmdHandlerTestBase {}; +TEST_F(CmdShowInterfacePhyTestFixture, printPmdLaneRxInfoOmitsUnreportedStats) { + phy::PhySideState sideState; + sideState.pmd()->lanes() = { + {0, makeRxLaneState(0, true)}, {1, makeRxLaneState(1, false)}}; + phy::PhySideStats sideStats; + sideStats.pmd()->lanes() = { + {0, makeRxLaneStats(0, 3)}, {1, makeRxLaneStats(1, std::nullopt)}}; + + std::stringstream ss; + CmdShowInterfacePhy().printPmdLaneRxInfo( + ss, sideState, sideStats, {0, 1}, "Line "); + + // Nothing reports eyes, PPM, SNR or a CDR lock change here, so those columns + // are left out; signal detect changed stays because one lane reports it. + const TableCells expected = { + {"Line RX PMD", + "Lane", + "RX Signal Detect Live", + "RX Signal Detect Changed", + "RX CDR Lock Live"}, + {"", "0", "True", "3", "True"}, + {"", "1", "False", "N/A", "True"}, + }; + EXPECT_EQ(parseTable(ss.str()), expected); +} + TEST_F(CmdShowInterfacePhyTestFixture, printPmdLaneTxInfoShowsPrecoding) { phy::PhySideState sideState; sideState.pmd()->lanes() = { @@ -106,6 +157,9 @@ TEST_F(CmdShowInterfacePhyTestFixture, printPmdLaneTxInfoShowsPrecoding) { std::stringstream ss; CmdShowInterfacePhy().printPmdLaneTxInfo(ss, sideState, {0, 1}, "Line "); + // Precoding is printed because one lane has it; the TX fields that no lane + // populates (DigGain, DiffEncoderEn, LdoBypass) are dropped entirely. No lane + // sets a fir* tap, as on a non-Tajo platform, so the i16 fields are printed. const TableCells expected = { {"Line TX PMD", "Lane", @@ -116,9 +170,45 @@ TEST_F(CmdShowInterfacePhyTestFixture, printPmdLaneTxInfoShowsPrecoding) { "Post1", "Post2", "Post3", - "Precoding"}, - {"", "0", "1", "2", "3", "4", "5", "6", "7", "1"}, - {"", "1", "1", "2", "3", "4", "5", "6", "7", "N/A"}, + "Precoding", + "DriverSwing"}, + {"", "0", "1", "2", "3", "4", "5", "6", "7", "1", "20"}, + {"", "1", "1", "2", "3", "4", "5", "6", "7", "N/A", "20"}, + }; + EXPECT_EQ(parseTable(ss.str()), expected); +} + +TEST_F( + CmdShowInterfacePhyTestFixture, + printPmdLaneTxInfoPrefersUntruncatedFirTaps) { + phy::TxSettings txSettings; + // A Cisco SiliconOne main tap of 52428 as it lands in both fields: wrapped in + // the i16, intact in the i32 read-back of the same register. + txSettings.main() = -13108; + txSettings.firMain() = 52428; + txSettings.ldoBypass() = 1; + + phy::LaneState laneState; + laneState.lane() = 0; + laneState.txSettings() = txSettings; + + phy::PhySideState sideState; + sideState.pmd()->lanes() = {{0, laneState}}; + + std::stringstream ss; + CmdShowInterfacePhy().printPmdLaneTxInfo(ss, sideState, {0}, "Line "); + + const TableCells expected = { + {"Line TX PMD", + "Lane", + "Pre2", + "Pre1", + "Main", + "Post1", + "Post2", + "Post3", + "LdoBypass"}, + {"", "0", "0", "0", "52428", "0", "0", "0", "1"}, }; EXPECT_EQ(parseTable(ss.str()), expected); } @@ -134,38 +224,36 @@ TEST_F( std::stringstream ss; CmdShowInterfacePhy().printSerdesParametersInfo(ss, pmdState, "Line "); - // Only the two new trailing columns are populated here; every other serdes - // parameter is left unset and renders as N/A. + // Only the four columns that at least one lane populates are printed; the + // Broadcom-only parameters (RVga, Dco, ...) are not applicable here and are + // left out rather than printed as a wall of N/A. const TableCells expected = { {"Line Serdes Parameters", "Lane", - "RVga", - "Dco", - "TpChn0", - "TpChn1", - "TpChn2", - "RxPf", - "RxPfLfq", - "RxPfHfq", - "RxFltM", - "RxFltS", - "RxTap1", - "RxTap2", - "RxEq3", - "RxEq2", - "RxEq1", - "RxEqM", - "RxEqP1", - "RxEqP2", "RxReach", - "RxPrecoding"}, - {"", "0", "N/A", "N/A", "N/A", "N/A", "N/A", - "N/A", "N/A", "N/A", "N/A", "N/A", "N/A", "N/A", - "N/A", "N/A", "N/A", "N/A", "N/A", "N/A", "RX_EXTENDED_REACH", - "1"}, - {"", "1", "N/A", "N/A", "N/A", "N/A", "N/A", "N/A", - "N/A", "N/A", "N/A", "N/A", "N/A", "N/A", "N/A", "N/A", - "N/A", "N/A", "N/A", "N/A", "N/A", "N/A"}, + "RxPrecoding", + "RxCtleCode", + "RxInstgBoost1Start"}, + {"", "0", "RX_EXTENDED_REACH", "1", "30", "40"}, + {"", "1", "N/A", "N/A", "30", "40"}, + }; + EXPECT_EQ(parseTable(ss.str()), expected); +} + +TEST_F(CmdShowInterfacePhyTestFixture, printSerdesParametersInfoOmitsAllNa) { + phy::PmdState pmdState; + phy::LaneState laneState; + laneState.lane() = 0; + laneState.serdesParameters()->lane() = 0; + pmdState.lanes() = {{0, laneState}}; + + std::stringstream ss; + CmdShowInterfacePhy().printSerdesParametersInfo(ss, pmdState, "Line "); + + // Nothing is populated, so only the lane identity columns remain. + const TableCells expected = { + {"Line Serdes Parameters", "Lane"}, + {"", "0"}, }; EXPECT_EQ(parseTable(ss.str()), expected); } diff --git a/fboss/lib/phy/phy.thrift b/fboss/lib/phy/phy.thrift index 7e3781407998d..869528ffc6498 100644 --- a/fboss/lib/phy/phy.thrift +++ b/fboss/lib/phy/phy.thrift @@ -584,6 +584,29 @@ struct SerdesParameters { 19: optional i32 rxPfHfq; 20: optional RxReach rxReach; 21: optional i32 rxPrecoding; + // Cisco SiliconOne (Tajo/G202X) native RX serdes parameters, read back from + // the SAI_PORT_SERDES_ATTR_EXT_RX_* attributes. These have no Broadcom + // equivalent and are left unset on non-Tajo platforms. + 22: optional i32 rxCtleCode; + 23: optional i32 rxDspMode; + 24: optional i32 rxAfeTrim; + 25: optional i32 rxDiffEncoderEn; + 26: optional i32 rxInstgBoost1Start; + 27: optional i32 rxInstgBoost1Step; + 28: optional i32 rxInstgBoost1Stop; + 29: optional i32 rxInstgBoost2OrHrStart; + 30: optional i32 rxInstgBoost2OrHrStep; + 31: optional i32 rxInstgBoost2OrHrStop; + 32: optional i32 rxInstgC1Start1p7; + 33: optional i32 rxInstgC1Step1p7; + 34: optional i32 rxInstgC1Stop1p7; + 35: optional i32 rxInstgDfeStart1p7; + 36: optional i32 rxInstgDfeStep1p7; + 37: optional i32 rxInstgDfeStop1p7; + 38: optional i32 rxInstgEnableScan; + 39: optional i32 rxInstgScanUseSrSettings; + 40: optional i32 rxFfeLengthBitmap; + 41: optional i32 rxFfeLmsDynamicGatingEn; } struct LaneState {