diff --git a/fboss/agent/hw/sai/api/PortApi.h b/fboss/agent/hw/sai/api/PortApi.h index e16f315dcb016..02161594facd0 100644 --- a/fboss/agent/hw/sai/api/PortApi.h +++ b/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 - +#include +#include #include #include #include +#include extern "C" { #include @@ -790,15 +791,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< @@ -1910,7 +1910,24 @@ class PortApi : public SaiApi { 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 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); diff --git a/fboss/agent/hw/sai/switch/npu/SaiPortManager.cpp b/fboss/agent/hw/sai/switch/npu/SaiPortManager.cpp index 6d5d1e544ef23..49a87f56aaabd 100644 --- a/fboss/agent/hw/sai/switch/npu/SaiPortManager.cpp +++ b/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 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(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 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(serdesId.value())); + } +#if defined(BRCM_SAI_SDK_DNX) } +#endif #endif } else { // ensure warm boot handles are reclaimed removed diff --git a/fboss/agent/hw/switch_asics/ChenabAsic.cpp b/fboss/agent/hw/switch_asics/ChenabAsic.cpp index 827cc31ae30f4..644a3b01b3b2b 100644 --- a/fboss/agent/hw/switch_asics/ChenabAsic.cpp +++ b/fboss/agent/hw/switch_asics/ChenabAsic.cpp @@ -272,6 +272,7 @@ 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: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/EbroAsic.cpp b/fboss/agent/hw/switch_asics/EbroAsic.cpp index ad7befea1f181..c23490daf2b93 100644 --- a/fboss/agent/hw/switch_asics/EbroAsic.cpp +++ b/fboss/agent/hw/switch_asics/EbroAsic.cpp @@ -283,6 +283,7 @@ 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::SAI_ACL_MPLS_LABEL0_TTL: case HwAsic::Feature::SAI_MPLS_NEXTHOP_OUTSEG_TYPE: case HwAsic::Feature::ACL_ENTRY_OUT_PORT_QUALIFIER: diff --git a/fboss/agent/hw/switch_asics/G202xAsic.cpp b/fboss/agent/hw/switch_asics/G202xAsic.cpp index 5b73cb19ebaa4..6452f9c7db3e3 100644 --- a/fboss/agent/hw/switch_asics/G202xAsic.cpp +++ b/fboss/agent/hw/switch_asics/G202xAsic.cpp @@ -262,6 +262,7 @@ 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::SWITCH_WIDE_LINK_UP_DEBOUNCE: case HwAsic::Feature::ACL_ENTRY_OUT_PORT_QUALIFIER: case HwAsic::Feature::XPHY_SERDES_TX_FIR_POST3: diff --git a/fboss/agent/hw/switch_asics/HwAsic.h b/fboss/agent/hw/switch_asics/HwAsic.h index 354f9475495e4..2737d74ff63a0 100644 --- a/fboss/agent/hw/switch_asics/HwAsic.h +++ b/fboss/agent/hw/switch_asics/HwAsic.h @@ -649,6 +649,9 @@ 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, }; enum class AsicMode { diff --git a/fboss/agent/hw/switch_asics/Jericho2Asic.cpp b/fboss/agent/hw/switch_asics/Jericho2Asic.cpp index 196a49f0b902e..05f07589f53ea 100644 --- a/fboss/agent/hw/switch_asics/Jericho2Asic.cpp +++ b/fboss/agent/hw/switch_asics/Jericho2Asic.cpp @@ -271,6 +271,7 @@ 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: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Jericho3Asic.cpp b/fboss/agent/hw/switch_asics/Jericho3Asic.cpp index 2ec940940f721..fe12c8bae8e84 100644 --- a/fboss/agent/hw/switch_asics/Jericho3Asic.cpp +++ b/fboss/agent/hw/switch_asics/Jericho3Asic.cpp @@ -307,6 +307,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::PORT_SERDES_CREATED_WITH_PORT: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Jericho4Asic.cpp b/fboss/agent/hw/switch_asics/Jericho4Asic.cpp index e2644b2ea7a3f..723408b9556a8 100644 --- a/fboss/agent/hw/switch_asics/Jericho4Asic.cpp +++ b/fboss/agent/hw/switch_asics/Jericho4Asic.cpp @@ -307,6 +307,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::PORT_SERDES_CREATED_WITH_PORT: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/P200Asic.cpp b/fboss/agent/hw/switch_asics/P200Asic.cpp index 8ff2987cde1d4..087c38bf676e0 100644 --- a/fboss/agent/hw/switch_asics/P200Asic.cpp +++ b/fboss/agent/hw/switch_asics/P200Asic.cpp @@ -284,6 +284,7 @@ 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: return false; case HwAsic::Feature::SAI_ACL_MPLS_LABEL0_TTL: // Leaba does not implement the MPLS label0 TTL ACL qualifier yet. diff --git a/fboss/agent/hw/switch_asics/Qumran4DAsic.cpp b/fboss/agent/hw/switch_asics/Qumran4DAsic.cpp index 95e11be53a854..449311ded2b5b 100644 --- a/fboss/agent/hw/switch_asics/Qumran4DAsic.cpp +++ b/fboss/agent/hw/switch_asics/Qumran4DAsic.cpp @@ -80,6 +80,7 @@ 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::ZERO_SDK_WRITE_WARMBOOT: case HwAsic::Feature::SWITCH_REACHABILITY_CHANGE_NOTIFY: case HwAsic::Feature::ACL_ENTRY_ETHER_TYPE: diff --git a/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp index 84bffba1f590c..1114e8a251b23 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp @@ -111,6 +111,7 @@ 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::QCM: case HwAsic::Feature::SMAC_EQUALS_DMAC_CHECK_ENABLED: case HwAsic::Feature::PORT_TTL_DECREMENT_DISABLE: diff --git a/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp index 32b9999460f75..5e7152d4bd3bc 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp @@ -280,6 +280,7 @@ 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: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp index 1ca339685914c..d1a6b66f2a754 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp @@ -263,6 +263,7 @@ bool Tomahawk5Asic::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: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp index e0224edc72500..57ffc5990129a 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk6Asic.cpp @@ -270,6 +270,7 @@ bool Tomahawk6Asic::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: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/TomahawkAsic.cpp b/fboss/agent/hw/switch_asics/TomahawkAsic.cpp index b2c4e9bcc2f5d..403f4b23e4297 100644 --- a/fboss/agent/hw/switch_asics/TomahawkAsic.cpp +++ b/fboss/agent/hw/switch_asics/TomahawkAsic.cpp @@ -261,6 +261,7 @@ bool TomahawkAsic::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: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/TomahawkUltra1Asic.cpp b/fboss/agent/hw/switch_asics/TomahawkUltra1Asic.cpp index bfbc1af05b14e..36e42b749acff 100644 --- a/fboss/agent/hw/switch_asics/TomahawkUltra1Asic.cpp +++ b/fboss/agent/hw/switch_asics/TomahawkUltra1Asic.cpp @@ -266,6 +266,7 @@ bool TomahawkUltra1Asic::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: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/YubaAsic.cpp b/fboss/agent/hw/switch_asics/YubaAsic.cpp index 788a6dbad8997..39efbdd03fe18 100644 --- a/fboss/agent/hw/switch_asics/YubaAsic.cpp +++ b/fboss/agent/hw/switch_asics/YubaAsic.cpp @@ -276,6 +276,7 @@ bool YubaAsic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::RX_PACKET_TYPE: case HwAsic::Feature::MY_MAC: case HwAsic::Feature::ISOLATION_GROUP: + case HwAsic::Feature::PORT_SERDES_CREATED_WITH_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: