[Nexthop][m4340nhp] Build VOQ system ports from the TM slot the Qumran4D SAI assigned

fboss/agent/hw/sai/api/PortApi.h

@@ -15,11 +15,12 @@
 #include "fboss/agent/hw/sai/api/SaiVersion.h"
 #include "fboss/agent/hw/sai/api/Types.h"
 
-#include <folly/logging/xlog.h>
-
+#include <algorithm>
+#include <iterator>
 #include <limits>
 #include <optional>
 #include <tuple>
+#include <vector>
 
 extern "C" {
 #include <sai.h>
@@ -287,6 +288,16 @@ struct SaiPortTraits {
     };
     using SystemPortId =
         SaiExtensionAttribute<sai_uint16_t, AttributeSystemPortId>;
+    struct AttributeAttachedCoreIndex {
+      std::optional<sai_attr_id_t> operator()();
+    };
+    using AttachedCoreIndex =
+        SaiExtensionAttribute<sai_uint32_t, AttributeAttachedCoreIndex>;
+    struct AttributeAttachedCorePortIndex {
+      std::optional<sai_attr_id_t> operator()();
+    };
+    using AttachedCorePortIndex =
+        SaiExtensionAttribute<sai_uint32_t, AttributeAttachedCorePortIndex>;
     using PtpMode = SaiAttribute<
         EnumType,
         SAI_PORT_ATTR_PTP_MODE,
@@ -790,15 +801,14 @@ struct SaiPortTraits {
    *   - Enhancing AdapterHostKey as well as CreateAttribute means that the
    *     AdatperHostKey remains computable from AdaterKey, CreateAttributes: a
    *     requirement for our design.
-   *   - However, this also means that attempt to create port will pass Type (a
-   *     Read Only attribute today) and that call will fail.
-   *   - This is not a problem at the moment since passing Type is a
-   *     requirement only on DNX and ports are always pre-created on this
-   *     platform and loaded by SaiStore during init and never created.
-   *   - In future, if we support breakout ports on DNX, we would need to call
-   *     create port which would fail: but as mentioned above, we plan to
-   *     enhance the SAI spec and work with SAI vendors to support passing port
-   *     type during creation.
+   *   - It also means create_port would receive Type, a read-only attribute,
+   *     and some adapters reject it only after creating the port, leaving a
+   *     half-created port behind. Ports created at runtime (breakout) hit
+   *     this, so PortApi::_create leaves a LOGICAL Type out of the attribute
+   *     list it passes to create_port; the other types (fabric, recycle,
+   *     eventor, management, hyper) are still passed, and Type still keys the
+   *     SaiStore.
+   *   - Long term the fix is a SAI spec change allowing Type on create.
    */
 
   using AdapterHostKey = std::tuple<
@@ -1094,6 +1104,8 @@ SAI_ATTRIBUTE_NAME(Port, IsolationGroup)
 SAI_ATTRIBUTE_NAME(Port, IngressMacSecAcl)
 SAI_ATTRIBUTE_NAME(Port, EgressMacSecAcl)
 SAI_ATTRIBUTE_NAME(Port, SystemPortId)
+SAI_ATTRIBUTE_NAME(Port, AttachedCoreIndex)
+SAI_ATTRIBUTE_NAME(Port, AttachedCorePortIndex)
 SAI_ATTRIBUTE_NAME(Port, PtpMode)
 SAI_ATTRIBUTE_NAME(Port, PortEyeValues)
 SAI_ATTRIBUTE_NAME(Port, PriorityFlowControlMode)
@@ -1910,7 +1922,24 @@ class PortApi : public SaiApi<PortApi> {
       sai_object_id_t switch_id,
       size_t count,
       sai_attribute_t* attr_list) const {
-    return api_->create_port(rawSaiId(id), switch_id, count, attr_list);
+    // SAI_PORT_ATTR_TYPE is read-only in the SAI spec. It is part of the DNX
+    // CreateAttributes so that SaiStore can key port types apart; the logical
+    // (front-panel) type is not sent to the adapter. An adapter that rejects it
+    // only after creating the port (the Broadcom DNX SAI) would otherwise leave
+    // a half-created port behind that no retry can replace. Other types
+    // (fabric, recycle, eventor, management, hyper port) are still passed.
+    std::vector<sai_attribute_t> attrs;
+    attrs.reserve(count);
+    std::copy_if(
+        attr_list,
+        attr_list + count,
+        std::back_inserter(attrs),
+        [](const sai_attribute_t& attr) {
+          return attr.id != SAI_PORT_ATTR_TYPE ||
+              attr.value.s32 != SAI_PORT_TYPE_LOGICAL;
+        });
+    return api_->create_port(
+        rawSaiId(id), switch_id, attrs.size(), attrs.data());
   }
   sai_status_t _remove(PortSaiId key) const {
     return api_->remove_port(key);

fboss/agent/hw/sai/api/bcm/PortApi.cpp

@@ -268,6 +268,22 @@ std::optional<sai_attr_id_t>
 SaiPortTraits::Attributes::AttributeSystemPortId::operator()() {
   return std::nullopt;
 }
+std::optional<sai_attr_id_t>
+SaiPortTraits::Attributes::AttributeAttachedCoreIndex::operator()() {
+#if defined(BRCM_SAI_SDK_DNX)
+  return SAI_PORT_ATTR_ATTACHED_CORE_INDEX;
+#else
+  return std::nullopt;
+#endif
+}
+std::optional<sai_attr_id_t>
+SaiPortTraits::Attributes::AttributeAttachedCorePortIndex::operator()() {
+#if defined(BRCM_SAI_SDK_DNX)
+  return SAI_PORT_ATTR_ATTACHED_CORE_PORT_INDEX;
+#else
+  return std::nullopt;
+#endif
+}
 std::optional<sai_attr_id_t> SaiPortSerdesTraits::Attributes::
     AttributeRxAfeAdaptiveEnableWrapper::operator()() {
   return std::nullopt;

fboss/agent/hw/sai/api/chenab/PortApi.cpp

@@ -29,6 +29,14 @@ std::optional<sai_attr_id_t>
 SaiPortTraits::Attributes::AttributeSystemPortId::operator()() {
   return std::nullopt;
 }
+std::optional<sai_attr_id_t>
+SaiPortTraits::Attributes::AttributeAttachedCoreIndex::operator()() {
+  return std::nullopt;
+}
+std::optional<sai_attr_id_t>
+SaiPortTraits::Attributes::AttributeAttachedCorePortIndex::operator()() {
+  return std::nullopt;
+}
 std::optional<sai_attr_id_t> SaiPortSerdesTraits::Attributes::
     AttributeRxAfeAdaptiveEnableWrapper::operator()() {
   return std::nullopt;

fboss/agent/hw/sai/api/fake/FakeSaiExtensions.cpp

@@ -130,6 +130,14 @@ SaiPortTraits::Attributes::AttributeSystemPortId::operator()() {
   return SAI_PORT_ATTR_EXT_FAKE_SYSTEM_PORT_ID;
 }
 std::optional<sai_attr_id_t>
+SaiPortTraits::Attributes::AttributeAttachedCoreIndex::operator()() {
+  return std::nullopt;
+}
+std::optional<sai_attr_id_t>
+SaiPortTraits::Attributes::AttributeAttachedCorePortIndex::operator()() {
+  return std::nullopt;
+}
+std::optional<sai_attr_id_t>
 SaiPortTraits::Attributes::AttributeArsLinkState::operator()() {
   return std::nullopt;
 }

fboss/agent/hw/sai/api/tajo/PortApi.cpp

@@ -44,6 +44,14 @@ std::optional<sai_attr_id_t>
 SaiPortTraits::Attributes::AttributeSystemPortId::operator()() {
   return SAI_PORT_ATTR_EXT_SYSTEM_PORT_ID;
 }
+std::optional<sai_attr_id_t>
+SaiPortTraits::Attributes::AttributeAttachedCoreIndex::operator()() {
+  return std::nullopt;
+}
+std::optional<sai_attr_id_t>
+SaiPortTraits::Attributes::AttributeAttachedCorePortIndex::operator()() {
+  return std::nullopt;
+}
 std::optional<sai_attr_id_t> SaiPortSerdesTraits::Attributes::
     AttributeRxAfeAdaptiveEnableWrapper::operator()() {
   return SAI_PORT_SERDES_ATTR_EXT_RX_AFE_ADAPTIVE_ENABLE;

fboss/agent/hw/sai/switch/SaiSystemPortManager.cpp

@@ -18,6 +18,7 @@
 #include "fboss/agent/VoqUtils.h"
 #include "fboss/agent/hw/HwSysPortFb303Stats.h"
 #include "fboss/agent/hw/gen-cpp2/hardware_stats_constants.h"
+#include "fboss/agent/hw/sai/api/SaiApiTable.h"
 #include "fboss/agent/hw/sai/store/SaiStore.h"
 #include "fboss/agent/hw/sai/switch/ConcurrentIndices.h"
 #include "fboss/agent/hw/sai/switch/SaiManagerTable.h"
@@ -78,16 +79,72 @@ SaiSystemPortManager::SaiSystemPortManager(
 
 SaiSystemPortManager::~SaiSystemPortManager() {}
 
+std::optional<std::pair<uint32_t, uint32_t>>
+SaiSystemPortManager::coreAndPortIndexFromPort(
+    const std::shared_ptr<SystemPort>& swSystemPort) const {
+  const auto* asic = platform_->getAsic();
+  if (!asic->isSupported(HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT) ||
+      swSystemPort->getPortType() != cfg::PortType::INTERFACE_PORT ||
+      swSystemPort->getRemoteSystemPortType().has_value() ||
+      !asic->getSwitchId().has_value() ||
+      static_cast<uint64_t>(swSystemPort->getSwitchId()) !=
+          *asic->getSwitchId()) {
+    return std::nullopt;
+  }
+  // Inverse of getSystemPortID(): a local interface port's system port id is
+  // the port id plus the switch's local or global system port offset, minus
+  // the start of the switch's port id range.
+  auto offset = swSystemPort->getScope() == cfg::Scope::GLOBAL
+      ? asic->getGlobalSystemPortOffset()
+      : asic->getLocalSystemPortOffset();
+  const auto& switchIdToSwitchInfo = *platform_->getConfig()
+                                          ->thrift.sw()
+                                          ->switchSettings()
+                                          ->switchIdToSwitchInfo();
+  auto switchInfo = switchIdToSwitchInfo.find(*asic->getSwitchId());
+  if (!offset.has_value() || switchInfo == switchIdToSwitchInfo.end()) {
+    return std::nullopt;
+  }
+  PortID portId(
+      static_cast<int>(
+          static_cast<int64_t>(swSystemPort->getID()) - *offset +
+          *switchInfo->second.portIdRange()->minimum()));
+  const auto* portHandle = managerTable_->portManager().getPortHandle(portId);
+  if (!portHandle) {
+    XLOG(DBG2) << "System port " << swSystemPort->getID() << ": port " << portId
+               << " not created yet, using the platform mapping";
+    return std::nullopt;
+  }
+  auto& portApi = SaiApiTable::getInstance()->portApi();
+  auto saiPortId = portHandle->port->adapterKey();
+  auto coreIndex = portApi.getAttribute(
+      saiPortId, SaiPortTraits::Attributes::AttachedCoreIndex{});
+  auto corePortIndex = portApi.getAttribute(
+      saiPortId, SaiPortTraits::Attributes::AttachedCorePortIndex{});
+  if (coreIndex != swSystemPort->getCoreIndex() ||
+      corePortIndex != swSystemPort->getCorePortIndex()) {
+    XLOG(DBG2) << "System port " << swSystemPort->getID() << ": port " << portId
+               << " is on core " << coreIndex << " port index " << corePortIndex
+               << ", platform mapping says core "
+               << swSystemPort->getCoreIndex() << " port index "
+               << swSystemPort->getCorePortIndex();
+  }
+  return std::make_pair(coreIndex, corePortIndex);
+}
+
 SaiSystemPortTraits::CreateAttributes
 SaiSystemPortManager::attributesFromSwSystemPort(
     const std::shared_ptr<SystemPort>& swSystemPort) const {
+  auto coreIndex = static_cast<uint32_t>(swSystemPort->getCoreIndex());
+  auto corePortIndex = static_cast<uint32_t>(swSystemPort->getCorePortIndex());
+  if (auto fromPort = coreAndPortIndexFromPort(swSystemPort)) {
+    std::tie(coreIndex, corePortIndex) = *fromPort;
+  }
   sai_system_port_config_t config{
       .port_id = static_cast<uint32_t>(swSystemPort->getID()),
       .attached_switch_id = static_cast<uint32_t>(swSystemPort->getSwitchId()),
-      .attached_core_index =
-          static_cast<uint32_t>(swSystemPort->getCoreIndex()),
-      .attached_core_port_index =
-          static_cast<uint32_t>(swSystemPort->getCorePortIndex()),
+      .attached_core_index = coreIndex,
+      .attached_core_port_index = corePortIndex,
       .speed = static_cast<uint32_t>(swSystemPort->getSpeedMbps()),
       .num_voq = static_cast<uint32_t>(swSystemPort->getNumVoqs()),
   };

fboss/agent/hw/sai/switch/SaiSystemPortManager.h

@@ -123,6 +123,8 @@ class SaiSystemPortManager {
   void loadQueues(
       SaiSystemPortHandle& sysPortHandle,
       const std::shared_ptr<SystemPort>& swSystemPort);
+  std::optional<std::pair<uint32_t, uint32_t>> coreAndPortIndexFromPort(
+      const std::shared_ptr<SystemPort>& swSystemPort) const;
   SaiSystemPortTraits::CreateAttributes attributesFromSwSystemPort(
       const std::shared_ptr<SystemPort>& swSystemPort) const;
   SaiQueueHandle* FOLLY_NULLABLE

fboss/agent/hw/sai/switch/npu/SaiPortManager.cpp

@@ -1634,17 +1634,30 @@ void SaiPortManager::programSerdes(
      * Serdes objects are no longer created beforehand. Therefore, agent only
      * needs to create a new serdes object without knowing the serdes Id or
      * reloading the object to sai store.
+     *
+     * On the Broadcom DNX SAI that holds only where the ASIC does not claim
+     * PORT_SERDES_CREATED_WITH_PORT: the Qumran4D SAI attaches a default
+     * serdes object to every port it creates and answers a second
+     * create_port_serdes with SAI_STATUS_ITEM_ALREADY_EXISTS, so a port
+     * created at runtime (breakout) must adopt it, as on the other SAIs.
      */
-#if !defined(BRCM_SAI_SDK_XGS_AND_DNX)
-    // serdes is not yet programmed or reloaded from adapter
-    std::optional<SaiPortTraits::Attributes::SerdesId> serdesAttr{};
-    auto serdesId = SaiApiTable::getInstance()->portApi().getAttribute(
-        saiPort->adapterKey(), serdesAttr);
-    if (serdesId.has_value() && serdesId.value() != SAI_NULL_OBJECT_ID) {
-      // but default serdes exists in the adapter.
-      serdes =
-          store.reloadObject(static_cast<PortSerdesSaiId>(serdesId.value()));
+#if !defined(BRCM_SAI_SDK_XGS_AND_DNX) || defined(BRCM_SAI_SDK_DNX)
+#if defined(BRCM_SAI_SDK_DNX)
+    if (platform_->getAsic()->isSupported(
+            HwAsic::Feature::PORT_SERDES_CREATED_WITH_PORT)) {
+#endif
+      // serdes is not yet programmed or reloaded from adapter
+      std::optional<SaiPortTraits::Attributes::SerdesId> serdesAttr{};
+      auto serdesId = SaiApiTable::getInstance()->portApi().getAttribute(
+          saiPort->adapterKey(), serdesAttr);
+      if (serdesId.has_value() && serdesId.value() != SAI_NULL_OBJECT_ID) {
+        // but default serdes exists in the adapter.
+        serdes =
+            store.reloadObject(static_cast<PortSerdesSaiId>(serdesId.value()));
+      }
+#if defined(BRCM_SAI_SDK_DNX)
     }
+#endif
 #endif
   } else {
     // ensure warm boot handles are reclaimed removed

fboss/agent/hw/switch_asics/ChenabAsic.cpp

@@ -272,6 +272,8 @@ bool ChenabAsic::isSupportedNonFabric(Feature feature) const {
     case HwAsic::Feature::PBR_ACL_BYTE_COUNTER:
     case HwAsic::Feature::LAG_TTL_DECREMENT_DISABLE:
     case HwAsic::Feature::PORT_LATCH_STATUS:
+    case HwAsic::Feature::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
       return false;
   }
   return false;

fboss/agent/hw/switch_asics/EbroAsic.cpp

@@ -283,6 +283,8 @@ bool EbroAsic::isSupportedNonFabric(Feature feature) const {
     case HwAsic::Feature::MY_MAC:
     case HwAsic::Feature::ISOLATION_GROUP:
     case HwAsic::Feature::CSIG:
+    case HwAsic::Feature::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
     case HwAsic::Feature::SAI_ACL_MPLS_LABEL0_TTL:
     case HwAsic::Feature::SAI_MPLS_NEXTHOP_OUTSEG_TYPE:
     case HwAsic::Feature::ACL_ENTRY_OUT_PORT_QUALIFIER:

fboss/agent/hw/switch_asics/G202xAsic.cpp

@@ -262,6 +262,8 @@ bool G202xAsic::isSupportedNonFabric(Feature feature) const {
     case HwAsic::Feature::MY_MAC:
     case HwAsic::Feature::ISOLATION_GROUP:
     case HwAsic::Feature::CSIG:
+    case HwAsic::Feature::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
     case HwAsic::Feature::SWITCH_WIDE_LINK_UP_DEBOUNCE:
     case HwAsic::Feature::ACL_ENTRY_OUT_PORT_QUALIFIER:
     case HwAsic::Feature::XPHY_SERDES_TX_FIR_POST3:

fboss/agent/hw/switch_asics/HwAsic.h

@@ -649,6 +649,13 @@ class HwAsic {
     CSIG,
     // TTL control on the aggregate's shared forwarding service.
     LAG_TTL_DECREMENT_DISABLE,
+    // The adapter attaches a default serdes object to every port it creates
+    // and rejects a second create_port_serdes; the agent adopts that object
+    PORT_SERDES_CREATED_WITH_PORT,
+    // The adapter picks a runtime-created port's TM slot itself, so a system
+    // port names the core and core port index read back from the port, not
+    // the platform mapping's prediction
+    SYSTEM_PORT_CORE_INDEX_FROM_PORT,
   };
 
   enum class AsicMode {

fboss/agent/hw/switch_asics/Jericho2Asic.cpp

@@ -271,6 +271,8 @@ bool Jericho2Asic::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::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
       return false;
   }
   return false;

fboss/agent/hw/switch_asics/Jericho3Asic.cpp

@@ -307,6 +307,8 @@ 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::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
       return false;
   }
   return false;

fboss/agent/hw/switch_asics/Jericho4Asic.cpp

@@ -307,6 +307,8 @@ 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::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
       return false;
   }
   return false;

fboss/agent/hw/switch_asics/P200Asic.cpp

@@ -284,6 +284,8 @@ bool P200Asic::isSupportedNonFabric(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::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
       return false;
     case HwAsic::Feature::SAI_ACL_MPLS_LABEL0_TTL:
       // Leaba does not implement the MPLS label0 TTL ACL qualifier yet.

fboss/agent/hw/switch_asics/Qumran4DAsic.cpp

@@ -80,6 +80,8 @@ bool Qumran4DAsic::isSupported(Feature feature) const {
     case HwAsic::Feature::ACL_METADATA_QUALIFER:
     case HwAsic::Feature::EVENTOR_PORT_FOR_SFLOW:
     case HwAsic::Feature::SFLOWv6:
+    case HwAsic::Feature::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
     case HwAsic::Feature::ZERO_SDK_WRITE_WARMBOOT:
     case HwAsic::Feature::SWITCH_REACHABILITY_CHANGE_NOTIFY:
     case HwAsic::Feature::ACL_ENTRY_ETHER_TYPE:

fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp

@@ -111,6 +111,8 @@ bool Tomahawk3Asic::isSupported(Feature feature) const {
       return true;
     case HwAsic::Feature::ISOLATION_GROUP:
     case HwAsic::Feature::CSIG:
+    case HwAsic::Feature::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
     case HwAsic::Feature::QCM:
     case HwAsic::Feature::SMAC_EQUALS_DMAC_CHECK_ENABLED:
     case HwAsic::Feature::PORT_TTL_DECREMENT_DISABLE:

fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp

@@ -280,6 +280,8 @@ bool Tomahawk4Asic::isSupported(Feature feature) const {
     case HwAsic::Feature::CSIG:
     case HwAsic::Feature::LAG_TTL_DECREMENT_DISABLE:
     case HwAsic::Feature::PORT_LATCH_STATUS:
+    case HwAsic::Feature::PORT_SERDES_CREATED_WITH_PORT:
+    case HwAsic::Feature::SYSTEM_PORT_CORE_INDEX_FROM_PORT:
       return false;
   }
   return false;