From fe4480665e2228d556f7ae252089b421239f869f Mon Sep 17 00:00:00 2001 From: Longyin Huang Date: Mon, 14 Jul 2025 15:50:36 -0700 Subject: [PATCH 1/3] Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION --- orchagent/port.h | 1 + orchagent/port/portcnt.h | 5 ++++ orchagent/port/porthlpr.cpp | 29 +++++++++++++++++++- orchagent/port/porthlpr.h | 4 +++ orchagent/port/portschema.h | 1 + orchagent/portsorch.cpp | 45 +++++++++++++++++++++++++------ orchagent/portsorch.h | 3 ++- tests/mock_tests/portsorch_ut.cpp | 7 ++++- 8 files changed, 84 insertions(+), 11 deletions(-) diff --git a/orchagent/port.h b/orchagent/port.h index d53cf3d2ab..cc90c41072 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -243,6 +243,7 @@ class Port /* pre-emphasis */ std::map> m_preemphasis; + std::string m_custom_serdes_attrs; /* Force initial parameter configuration flags */ bool m_an_cfg = false; // Auto-negotiation (AN) diff --git a/orchagent/port/portcnt.h b/orchagent/port/portcnt.h index 94c8217fa3..91af64fed6 100644 --- a/orchagent/port/portcnt.h +++ b/orchagent/port/portcnt.h @@ -190,6 +190,11 @@ class PortConfig final bool is_set = false; } unreliable_los; // Port unreliable_los + struct { + std::string value; + bool is_set = false; + } custom_collection; // Port serdes custom_collection + } serdes; // Port serdes struct { diff --git a/orchagent/port/porthlpr.cpp b/orchagent/port/porthlpr.cpp index 9da05ee249..99ca78389f 100644 --- a/orchagent/port/porthlpr.cpp +++ b/orchagent/port/porthlpr.cpp @@ -20,7 +20,6 @@ using namespace swss; // types -------------------------------------------------------------------------------------------------------------- -typedef decltype(PortConfig::serdes) PortSerdes_t; typedef decltype(PortConfig::link_event_damping_config) PortDampingConfig_t; // constants ---------------------------------------------------------------------------------------------------------- @@ -697,6 +696,27 @@ bool PortHelper::parsePortLinkTraining(PortConfig &port, const std::string &fiel return true; } +// custom_collection specialization +bool PortHelper::parsePortSerdes( + decltype(PortSerdes_t::custom_collection)& serdes, + const std::string& field, + const std::string& value) const +{ + SWSS_LOG_ENTER(); + + if (value.empty()) + { + SWSS_LOG_ERROR("Failed to parse field(%s): empty string is prohibited", + field.c_str()); + return false; + } + + // directly assign the raw string + serdes.value = value; + serdes.is_set = true; + return true; +} + template bool PortHelper::parsePortSerdes(T &serdes, const std::string &field, const std::string &value) const { @@ -1182,6 +1202,13 @@ bool PortHelper::parsePortConfig(PortConfig &port) const return false; } } + else if (field == PORT_CUSTOM_SERDES_ATTRS) + { + if (!this->parsePortSerdes(port.serdes.custom_collection, field, value)) + { + return false; + } + } else if (field == PORT_ROLE) { if (!this->parsePortRole(port, field, value)) diff --git a/orchagent/port/porthlpr.h b/orchagent/port/porthlpr.h index 34853a3661..85359c5d99 100644 --- a/orchagent/port/porthlpr.h +++ b/orchagent/port/porthlpr.h @@ -7,6 +7,9 @@ #include "portcnt.h" +typedef decltype(PortConfig::serdes) PortSerdes_t; +typedef decltype(PortSerdes_t::custom_collection) CustomSerdes_t; + class PortHelper final { public: @@ -36,6 +39,7 @@ class PortHelper final private: std::string getFieldValueStr(const PortConfig &port, const std::string &field) const; + bool parsePortSerdes(CustomSerdes_t& serdes, const std::string& field, const std::string& value) const; template bool parsePortSerdes(T &serdes, const std::string &field, const std::string &value) const; diff --git a/orchagent/port/portschema.h b/orchagent/port/portschema.h index 63df8bfdc1..b2667baa25 100644 --- a/orchagent/port/portschema.h +++ b/orchagent/port/portschema.h @@ -89,6 +89,7 @@ #define PORT_OBNLEV "obnlev" #define PORT_REGN_BFM1P "regn_bfm1p" #define PORT_REGN_BFM1N "regn_bfm1n" +#define PORT_CUSTOM_SERDES_ATTRS "custom_serdes_attrs" #define PORT_ROLE "role" #define PORT_ADMIN_STATUS "admin_status" #define PORT_DESCRIPTION "description" diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 811f904d13..ff271d62e7 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -4280,7 +4280,11 @@ void PortsOrch::doPortTask(Consumer &consumer) else { PortSerdesAttrMap_t serdes_attr; + string custom_serdes_attrs_str; getPortSerdesAttr(serdes_attr, pCfg); + if (pCfg.serdes.custom_collection.is_set) { + custom_serdes_attrs_str = pCfg.serdes.custom_collection.value; + } // Saved configured admin status bool admin_status = p.m_admin_state_up; @@ -4391,9 +4395,14 @@ void PortsOrch::doPortTask(Consumer &consumer) updatePortStatePoll(p, PORT_STATE_POLL_LT, pCfg.link_training.value); // Restore pre-emphasis when LT is transitioned from ON to OFF - if (!p.m_link_training && serdes_attr.empty()) + if (!p.m_link_training && (serdes_attr.empty() || custom_serdes_attrs_str.empty())) { - serdes_attr = p.m_preemphasis; + if (serdes_attr.empty()) { + serdes_attr = p.m_preemphasis; + } + if (custom_serdes_attrs_str.empty()) { + custom_serdes_attrs_str = p.m_custom_serdes_attrs; + } } SWSS_LOG_NOTICE( @@ -4904,12 +4913,17 @@ void PortsOrch::doPortTask(Consumer &consumer) } } - if (!serdes_attr.empty()) + if (!serdes_attr.empty() or !custom_serdes_attrs_str.empty()) { if (p.m_link_training) { SWSS_LOG_NOTICE("Save port %s preemphasis for LT", p.m_alias.c_str()); - p.m_preemphasis = serdes_attr; + if (!serdes_attr.empty()) { + p.m_preemphasis = serdes_attr; + } + if (!custom_serdes_attrs_str.empty()) { + p.m_custom_serdes_attrs = custom_serdes_attrs_str; + } m_portList[p.m_alias] = p; } else @@ -4928,10 +4942,15 @@ void PortsOrch::doPortTask(Consumer &consumer) m_portList[p.m_alias] = p; } - if (setPortSerdesAttribute(p.m_port_id, gSwitchId, serdes_attr)) + if (setPortSerdesAttribute(p.m_port_id, gSwitchId, serdes_attr, custom_serdes_attrs_str)) { SWSS_LOG_NOTICE("Set port %s SI settings is successful", p.m_alias.c_str()); - p.m_preemphasis = serdes_attr; + if (!serdes_attr.empty()) { + p.m_preemphasis = serdes_attr; + } + if (!custom_serdes_attrs_str.empty()) { + p.m_custom_serdes_attrs = custom_serdes_attrs_str; + } m_portList[p.m_alias] = p; } else @@ -9009,7 +9028,8 @@ bool PortsOrch::removeAclTableGroup(const Port &p) } bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id, sai_object_id_t switch_id, - map> &serdes_attr) + map> &serdes_attr, + const std::string& custom_serdes_attrs_str) { SWSS_LOG_ENTER(); @@ -9060,8 +9080,17 @@ bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id, sai_object_id_t port_serdes_attr.value.u32list.list = it->second.data(); attr_list.emplace_back(port_serdes_attr); } + + if (!custom_serdes_attrs_str.empty()) + { + port_serdes_attr.id = SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION; + port_serdes_attr.value.json.json.count = static_cast(custom_serdes_attrs_str.size()); + port_serdes_attr.value.json.json.list = reinterpret_cast(const_cast(custom_serdes_attrs_str.data())); + attr_list.emplace_back(port_serdes_attr); + } + status = sai_port_api->create_port_serdes(&port_serdes_id, switch_id, - static_cast(serdes_attr.size()+1), + static_cast(attr_list.size()), attr_list.data()); if (status != SAI_STATUS_SUCCESS) diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 8dace4661a..a9e7dcd15b 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -517,7 +517,8 @@ class PortsOrch : public Orch, public Subject void getPortSerdesVal(const std::string& s, std::vector &lane_values, int base = 16); bool setPortSerdesAttribute(sai_object_id_t port_id, sai_object_id_t switch_id, - std::map> &serdes_attr); + std::map> &serdes_attr, + const std::string &custom_serdes_attrs_str = ""); void removePortSerdesAttribute(sai_object_id_t port_id); diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index 9ff15a71f3..31d88c0fb9 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -1088,7 +1088,8 @@ namespace portsorch_test { "obplev", "0x69,0x6b,0x6a,0x6c" }, { "obnlev", "0x5f,0x61,0x60,0x62" }, { "regn_bfm1p", "0x1e,0x20,0x1f,0x21" }, - { "regn_bfm1n", "0xaa,0xac,0xab,0xad" } + { "regn_bfm1n", "0xaa,0xac,0xab,0xad" }, + { "custom_serdes_attrs", "{'attributes':[]}" } } }}; @@ -1174,6 +1175,10 @@ namespace portsorch_test // Verify unreliablelos ASSERT_EQ(p.m_unreliable_los, false); + // Verify custom_serdes_attrs + std::string custom_serdes_attrs = "{'attributes':[]}"; + ASSERT_EQ(p.m_custom_serdes_attrs, custom_serdes_attrs); + // Dump pending tasks std::vector taskList; gPortsOrch->dumpPendingTasks(taskList); From 181eacbdd96b68bab2c009f4ccde9eb4d84a0162 Mon Sep 17 00:00:00 2001 From: Longyin Huang Date: Wed, 10 Sep 2025 14:29:14 -0700 Subject: [PATCH 2/3] Use boost::variant to support both vector and string values for serdes attrs --- orchagent/port.h | 28 +++++++- orchagent/port/porthlpr.cpp | 36 +++++----- orchagent/port/porthlpr.h | 12 ++-- orchagent/portsorch.cpp | 105 ++++++++++++------------------ orchagent/portsorch.h | 4 +- tests/mock_tests/portsorch_ut.cpp | 57 +++++++++------- 6 files changed, 128 insertions(+), 114 deletions(-) diff --git a/orchagent/port.h b/orchagent/port.h index cc90c41072..dd36ec0008 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -14,6 +14,7 @@ extern "C" { #include #include #include +#include #include #include @@ -33,6 +34,28 @@ extern "C" { #define VNID_NONE 0xFFFFFFFF +// SerdesValue using boost::variant to support both vector and string values +using SerdesValue = boost::variant, std::string>; + +// Visitor class for processing SerdesValue in SAI attribute setting +class SerdesValueVisitor : public boost::static_visitor { +public: + explicit SerdesValueVisitor(sai_attribute_t& attr) : attr_(attr) {} + + void operator()(const std::vector& values) const { + attr_.value.u32list.count = static_cast(values.size()); + attr_.value.u32list.list = const_cast(values.data()); + } + + void operator()(const std::string& str_value) const { + attr_.value.json.json.count = static_cast(str_value.size()); + attr_.value.json.json.list = reinterpret_cast(const_cast(str_value.data())); + } + +private: + sai_attribute_t& attr_; +}; + namespace swss { struct VlanMemberEntry @@ -241,9 +264,8 @@ class Port /* Port oper error status to event map*/ std::unordered_map m_portOperErrorToEvent; - /* pre-emphasis */ - std::map> m_preemphasis; - std::string m_custom_serdes_attrs; + /* serdes attributes */ + std::map m_serdes_attrs; /* Force initial parameter configuration flags */ bool m_an_cfg = false; // Auto-negotiation (AN) diff --git a/orchagent/port/porthlpr.cpp b/orchagent/port/porthlpr.cpp index 2581bb3cd2..15ccd3585d 100644 --- a/orchagent/port/porthlpr.cpp +++ b/orchagent/port/porthlpr.cpp @@ -20,6 +20,7 @@ using namespace swss; // types -------------------------------------------------------------------------------------------------------------- +typedef decltype(PortConfig::serdes) PortSerdes_t; typedef decltype(PortConfig::link_event_damping_config) PortDampingConfig_t; // constants ---------------------------------------------------------------------------------------------------------- @@ -696,38 +697,36 @@ bool PortHelper::parsePortLinkTraining(PortConfig &port, const std::string &fiel return true; } -// custom_collection specialization -bool PortHelper::parsePortSerdes( - decltype(PortSerdes_t::custom_collection)& serdes, - const std::string& field, - const std::string& value) const +template +bool PortHelper::parsePortSerdes(T &serdes, const std::string &field, const std::string &value) const { SWSS_LOG_ENTER(); if (value.empty()) { - SWSS_LOG_ERROR("Failed to parse field(%s): empty string is prohibited", - field.c_str()); + SWSS_LOG_ERROR("Failed to parse field(%s): empty string is prohibited", field.c_str()); return false; } - // directly assign the raw string - serdes.value = value; + // Use SFINAE with enable_if for extensible type handling for serdes.value + return parseSerdesValueImpl(serdes, field, value); +} + +// Helper function for JSON string-based serdes (custom_collection) +template +typename std::enable_if::value, bool>::type +PortHelper::parseSerdesValueImpl(T &serdes, const std::string &field, const std::string &value) const +{ + serdes.value = value; serdes.is_set = true; return true; } +// Helper function for vector-based serdes (most serdes attributes) template -bool PortHelper::parsePortSerdes(T &serdes, const std::string &field, const std::string &value) const +typename std::enable_if>::value, bool>::type +PortHelper::parseSerdesValueImpl(T &serdes, const std::string &field, const std::string &value) const { - SWSS_LOG_ENTER(); - - if (value.empty()) - { - SWSS_LOG_ERROR("Failed to parse field(%s): empty string is prohibited", field.c_str()); - return false; - } - const auto &serdesList = tokenize(value, ','); try @@ -765,6 +764,7 @@ template bool PortHelper::parsePortSerdes(decltype(PortSerdes_t::obplev) &serdes template bool PortHelper::parsePortSerdes(decltype(PortSerdes_t::obnlev) &serdes, const std::string &field, const std::string &value) const; template bool PortHelper::parsePortSerdes(decltype(PortSerdes_t::regn_bfm1p) &serdes, const std::string &field, const std::string &value) const; template bool PortHelper::parsePortSerdes(decltype(PortSerdes_t::regn_bfm1n) &serdes, const std::string &field, const std::string &value) const; +template bool PortHelper::parsePortSerdes(decltype(PortSerdes_t::custom_collection) &serdes, const std::string &field, const std::string &value) const; diff --git a/orchagent/port/porthlpr.h b/orchagent/port/porthlpr.h index 85359c5d99..153c784be7 100644 --- a/orchagent/port/porthlpr.h +++ b/orchagent/port/porthlpr.h @@ -7,9 +7,6 @@ #include "portcnt.h" -typedef decltype(PortConfig::serdes) PortSerdes_t; -typedef decltype(PortSerdes_t::custom_collection) CustomSerdes_t; - class PortHelper final { public: @@ -39,10 +36,17 @@ class PortHelper final private: std::string getFieldValueStr(const PortConfig &port, const std::string &field) const; - bool parsePortSerdes(CustomSerdes_t& serdes, const std::string& field, const std::string& value) const; template bool parsePortSerdes(T &serdes, const std::string &field, const std::string &value) const; + template + typename std::enable_if::value, bool>::type + parseSerdesValueImpl(T &serdes, const std::string &field, const std::string &value) const; + + template + typename std::enable_if>::value, bool>::type + parseSerdesValueImpl(T &serdes, const std::string &field, const std::string &value) const; + bool parsePortLinkEventDampingAlgorithm(PortConfig &port, const std::string &field, const std::string &value) const; template bool parsePortLinkEventDampingConfig(T &damping_config_attr, const std::string &field, const std::string &value) const; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index e4db5c6556..647b7c5dd9 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -91,7 +91,7 @@ struct PortAttrValue }; typedef PortAttrValue PortAttrValue_t; -typedef std::map> PortSerdesAttrMap_t; +typedef std::map PortSerdesAttrMap_t; // constants ---------------------------------------------------------------------------------------------------------- @@ -370,90 +370,93 @@ static void getPortSerdesAttr(PortSerdesAttrMap_t &map, const PortConfig &port) { if (port.serdes.preemphasis.is_set) { - map[SAI_PORT_SERDES_ATTR_PREEMPHASIS] = port.serdes.preemphasis.value; + map[SAI_PORT_SERDES_ATTR_PREEMPHASIS] = SerdesValue(port.serdes.preemphasis.value); } if (port.serdes.idriver.is_set) { - map[SAI_PORT_SERDES_ATTR_IDRIVER] = port.serdes.idriver.value; + map[SAI_PORT_SERDES_ATTR_IDRIVER] = SerdesValue(port.serdes.idriver.value); } if (port.serdes.ipredriver.is_set) { - map[SAI_PORT_SERDES_ATTR_IPREDRIVER] = port.serdes.ipredriver.value; + map[SAI_PORT_SERDES_ATTR_IPREDRIVER] = SerdesValue(port.serdes.ipredriver.value); } if (port.serdes.pre1.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE1] = port.serdes.pre1.value; + map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE1] = SerdesValue(port.serdes.pre1.value); } if (port.serdes.pre2.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE2] = port.serdes.pre2.value; + map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE2] = SerdesValue(port.serdes.pre2.value); } if (port.serdes.pre3.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE3] = port.serdes.pre3.value; + map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE3] = SerdesValue(port.serdes.pre3.value); } if (port.serdes.main.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_FIR_MAIN] = port.serdes.main.value; + map[SAI_PORT_SERDES_ATTR_TX_FIR_MAIN] = SerdesValue(port.serdes.main.value); } if (port.serdes.post1.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_FIR_POST1] = port.serdes.post1.value; + map[SAI_PORT_SERDES_ATTR_TX_FIR_POST1] = SerdesValue(port.serdes.post1.value); } if (port.serdes.post2.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_FIR_POST2] = port.serdes.post2.value; + map[SAI_PORT_SERDES_ATTR_TX_FIR_POST2] = SerdesValue(port.serdes.post2.value); } if (port.serdes.post3.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_FIR_POST3] = port.serdes.post3.value; + map[SAI_PORT_SERDES_ATTR_TX_FIR_POST3] = SerdesValue(port.serdes.post3.value); } if (port.serdes.attn.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_FIR_ATTN] = port.serdes.attn.value; + map[SAI_PORT_SERDES_ATTR_TX_FIR_ATTN] = SerdesValue(port.serdes.attn.value); } if (port.serdes.ob_m2lp.is_set) { - - map[SAI_PORT_SERDES_ATTR_TX_PAM4_RATIO] = port.serdes.ob_m2lp.value; + map[SAI_PORT_SERDES_ATTR_TX_PAM4_RATIO] = SerdesValue(port.serdes.ob_m2lp.value); } if (port.serdes.ob_alev_out.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_OUT_COMMON_MODE] = port.serdes.ob_alev_out.value; + map[SAI_PORT_SERDES_ATTR_TX_OUT_COMMON_MODE] = SerdesValue(port.serdes.ob_alev_out.value); } if (port.serdes.obplev.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_PMOS_COMMON_MODE] = port.serdes.obplev.value; + map[SAI_PORT_SERDES_ATTR_TX_PMOS_COMMON_MODE] = SerdesValue(port.serdes.obplev.value); } if (port.serdes.obnlev.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_NMOS_COMMON_MODE] = port.serdes.obnlev.value; + map[SAI_PORT_SERDES_ATTR_TX_NMOS_COMMON_MODE] = SerdesValue(port.serdes.obnlev.value); } if (port.serdes.regn_bfm1p.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_PMOS_VLTG_REG] = port.serdes.regn_bfm1p.value; + map[SAI_PORT_SERDES_ATTR_TX_PMOS_VLTG_REG] = SerdesValue(port.serdes.regn_bfm1p.value); } if (port.serdes.regn_bfm1n.is_set) { - map[SAI_PORT_SERDES_ATTR_TX_NMOS_VLTG_REG] = port.serdes.regn_bfm1n.value; + map[SAI_PORT_SERDES_ATTR_TX_NMOS_VLTG_REG] = SerdesValue(port.serdes.regn_bfm1n.value); } + if (port.serdes.custom_collection.is_set) + { + map[SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION] = SerdesValue(port.serdes.custom_collection.value); + } } @@ -4284,11 +4287,7 @@ void PortsOrch::doPortTask(Consumer &consumer) else { PortSerdesAttrMap_t serdes_attr; - string custom_serdes_attrs_str; getPortSerdesAttr(serdes_attr, pCfg); - if (pCfg.serdes.custom_collection.is_set) { - custom_serdes_attrs_str = pCfg.serdes.custom_collection.value; - } // Saved configured admin status bool admin_status = p.m_admin_state_up; @@ -4398,15 +4397,10 @@ void PortsOrch::doPortTask(Consumer &consumer) m_portList[p.m_alias] = p; updatePortStatePoll(p, PORT_STATE_POLL_LT, pCfg.link_training.value); - // Restore pre-emphasis when LT is transitioned from ON to OFF - if (!p.m_link_training && (serdes_attr.empty() || custom_serdes_attrs_str.empty())) + // Restore serdes attributes when LT is transitioned from ON to OFF + if (!p.m_link_training && serdes_attr.empty()) { - if (serdes_attr.empty()) { - serdes_attr = p.m_preemphasis; - } - if (custom_serdes_attrs_str.empty()) { - custom_serdes_attrs_str = p.m_custom_serdes_attrs; - } + serdes_attr = p.m_serdes_attrs; } SWSS_LOG_NOTICE( @@ -4917,17 +4911,12 @@ void PortsOrch::doPortTask(Consumer &consumer) } } - if (!serdes_attr.empty() or !custom_serdes_attrs_str.empty()) + if (!serdes_attr.empty()) { if (p.m_link_training) { - SWSS_LOG_NOTICE("Save port %s preemphasis for LT", p.m_alias.c_str()); - if (!serdes_attr.empty()) { - p.m_preemphasis = serdes_attr; - } - if (!custom_serdes_attrs_str.empty()) { - p.m_custom_serdes_attrs = custom_serdes_attrs_str; - } + SWSS_LOG_NOTICE("Save port %s serdes attributes for LT", p.m_alias.c_str()); + p.m_serdes_attrs = serdes_attr; m_portList[p.m_alias] = p; } else @@ -4946,15 +4935,10 @@ void PortsOrch::doPortTask(Consumer &consumer) m_portList[p.m_alias] = p; } - if (setPortSerdesAttribute(p.m_port_id, gSwitchId, serdes_attr, custom_serdes_attrs_str)) + if (setPortSerdesAttribute(p.m_port_id, gSwitchId, serdes_attr)) { SWSS_LOG_NOTICE("Set port %s SI settings is successful", p.m_alias.c_str()); - if (!serdes_attr.empty()) { - p.m_preemphasis = serdes_attr; - } - if (!custom_serdes_attrs_str.empty()) { - p.m_custom_serdes_attrs = custom_serdes_attrs_str; - } + p.m_serdes_attrs = serdes_attr; m_portList[p.m_alias] = p; } else @@ -9032,8 +9016,7 @@ bool PortsOrch::removeAclTableGroup(const Port &p) } bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id, sai_object_id_t switch_id, - map> &serdes_attr, - const std::string& custom_serdes_attrs_str) + map &serdes_attr) { SWSS_LOG_ENTER(); @@ -9080,19 +9063,13 @@ bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id, sai_object_id_t for (auto it = serdes_attr.begin(); it != serdes_attr.end(); it++) { port_serdes_attr.id = it->first; - port_serdes_attr.value.u32list.count = (uint32_t)it->second.size(); - port_serdes_attr.value.u32list.list = it->second.data(); - attr_list.emplace_back(port_serdes_attr); - } - if (!custom_serdes_attrs_str.empty()) - { - port_serdes_attr.id = SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION; - port_serdes_attr.value.json.json.count = static_cast(custom_serdes_attrs_str.size()); - port_serdes_attr.value.json.json.list = reinterpret_cast(const_cast(custom_serdes_attrs_str.data())); + // Use boost::variant visitor to handle both vector and string types + boost::apply_visitor(SerdesValueVisitor(port_serdes_attr), it->second); + attr_list.emplace_back(port_serdes_attr); } - + assert(serdes_attr.size() + 1 == attr_list.size()); status = sai_port_api->create_port_serdes(&port_serdes_id, switch_id, static_cast(attr_list.size()), attr_list.data()); @@ -9494,8 +9471,8 @@ bool PortsOrch::initGearboxPort(Port &port) m_gbcounterTable->set("", fields); /* Set serdes tx taps on system and line side */ - map> serdes_attr; - typedef pair> serdes_attr_pair; + map serdes_attr; + typedef pair serdes_attr_pair; vector attr_val; for (auto pair: tx_fir_strings_system_side) { if (m_gearboxInterfaceMap[port.m_index].tx_firs.find(pair.first) != m_gearboxInterfaceMap[port.m_index].tx_firs.end() ) { @@ -9508,11 +9485,11 @@ bool PortsOrch::initGearboxPort(Port &port) { if (setPortSerdesAttribute(systemPort, phyOid, serdes_attr)) { - SWSS_LOG_NOTICE("Set port %s system side preemphasis is success", port.m_alias.c_str()); + SWSS_LOG_NOTICE("Set port %s system side serdes attributes is success", port.m_alias.c_str()); } else { - SWSS_LOG_ERROR("Failed to set port %s system side pre-emphasis", port.m_alias.c_str()); + SWSS_LOG_ERROR("Failed to set port %s system side serdes attributes", port.m_alias.c_str()); return false; } } @@ -9528,11 +9505,11 @@ bool PortsOrch::initGearboxPort(Port &port) { if (setPortSerdesAttribute(linePort, phyOid, serdes_attr)) { - SWSS_LOG_NOTICE("Set port %s line side preemphasis is success", port.m_alias.c_str()); + SWSS_LOG_NOTICE("Set port %s line side serdes attributes is success", port.m_alias.c_str()); } else { - SWSS_LOG_ERROR("Failed to set port %s line side pre-emphasis", port.m_alias.c_str()); + SWSS_LOG_ERROR("Failed to set port %s line side serdes attributes", port.m_alias.c_str()); return false; } } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 92b681aa84..f7d1cd5bd9 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -520,9 +520,7 @@ class PortsOrch : public Orch, public Subject void getPortSerdesVal(const std::string& s, std::vector &lane_values, int base = 16); bool setPortSerdesAttribute(sai_object_id_t port_id, sai_object_id_t switch_id, - std::map> &serdes_attr, - const std::string &custom_serdes_attrs_str = ""); - + std::map &serdes_attr); void removePortSerdesAttribute(sai_object_id_t port_id); diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index b4ab31378d..18dbac193c 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -20,6 +20,19 @@ #include +// Add operator<< for vector to support boost::variant printing in tests for SerdesValue +namespace std { + inline ostream& operator<<(ostream& os, const vector& vec) { + os << "["; + for (size_t i = 0; i < vec.size(); ++i) { + if (i > 0) os << ", "; + os << vec[i]; // Decimal format + } + os << "]"; + return os; + } +} + extern redisReply *mockReply; extern sai_redis_communication_mode_t gRedisCommunicationMode; using ::testing::_; @@ -1106,79 +1119,79 @@ namespace portsorch_test // Verify preemphasis std::vector preemphasis = { 0xcad0, 0xc6e0, 0xc6e0, 0xd2b0 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_PREEMPHASIS), preemphasis); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_PREEMPHASIS), SerdesValue(preemphasis)); // Verify idriver std::vector idriver = { 0x5, 0x3, 0x4, 0x1 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_IDRIVER), idriver); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_IDRIVER), SerdesValue(idriver)); // Verify ipredriver std::vector ipredriver = { 0x1, 0x4, 0x3, 0x5 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_IPREDRIVER), ipredriver); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_IPREDRIVER), SerdesValue(ipredriver)); // Verify pre1 std::vector pre1 = { 0xfff0, 0xfff2, 0xfff1, 0xfff3 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_FIR_PRE1), pre1); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_FIR_PRE1), SerdesValue(pre1)); // Verify pre2 std::vector pre2 = { 0xfff0, 0xfff2, 0xfff1, 0xfff3 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_FIR_PRE2), pre2); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_FIR_PRE2), SerdesValue(pre2)); // Verify pre3 std::vector pre3 = { 0xfff0, 0xfff2, 0xfff1, 0xfff3 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_FIR_PRE3), pre3); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_FIR_PRE3), SerdesValue(pre3)); // Verify main std::vector main = { 0x90, 0x92, 0x91, 0x93 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_FIR_MAIN), main); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_FIR_MAIN), SerdesValue(main)); // Verify post1 std::vector post1 = { 0x10, 0x12, 0x11, 0x13 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_FIR_POST1), post1); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_FIR_POST1), SerdesValue(post1)); // Verify post2 std::vector post2 = { 0x10, 0x12, 0x11, 0x13 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_FIR_POST2), post2); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_FIR_POST2), SerdesValue(post2)); // Verify post3 std::vector post3 = { 0x10, 0x12, 0x11, 0x13 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_FIR_POST3), post3); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_FIR_POST3), SerdesValue(post3)); // Verify attn std::vector attn = { 0x80, 0x82, 0x81, 0x83 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_FIR_ATTN), attn); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_FIR_ATTN), SerdesValue(attn)); // Verify ob_m2lp std::vector ob_m2lp = { 0x4, 0x6, 0x5, 0x7 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_PAM4_RATIO), ob_m2lp); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_PAM4_RATIO), SerdesValue(ob_m2lp)); // Verify ob_alev_out std::vector ob_alev_out = { 0xf, 0x11, 0x10, 0x12 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_OUT_COMMON_MODE), ob_alev_out); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_OUT_COMMON_MODE), SerdesValue(ob_alev_out)); // Verify obplev std::vector obplev = { 0x69, 0x6b, 0x6a, 0x6c }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_PMOS_COMMON_MODE), obplev); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_PMOS_COMMON_MODE), SerdesValue(obplev)); // Verify obnlev std::vector obnlev = { 0x5f, 0x61, 0x60, 0x62 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_NMOS_COMMON_MODE), obnlev); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_NMOS_COMMON_MODE), SerdesValue(obnlev)); // Verify regn_bfm1p std::vector regn_bfm1p = { 0x1e, 0x20, 0x1f, 0x21 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_PMOS_VLTG_REG), regn_bfm1p); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_PMOS_VLTG_REG), SerdesValue(regn_bfm1p)); // Verify regn_bfm1n std::vector regn_bfm1n = { 0xaa, 0xac, 0xab, 0xad }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_TX_NMOS_VLTG_REG), regn_bfm1n); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_NMOS_VLTG_REG), SerdesValue(regn_bfm1n)); + + // Verify custom_serdes_attrs + std::string custom_serdes_attrs = "{'attributes':[{'attr_xyz':{'value':[1,2,3,4]}}]}"; + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION), SerdesValue(custom_serdes_attrs)); // Verify unreliablelos ASSERT_EQ(p.m_unreliable_los, false); - // Verify custom_serdes_attrs - std::string custom_serdes_attrs = "{'attributes':[]}"; - ASSERT_EQ(p.m_custom_serdes_attrs, custom_serdes_attrs); - // Dump pending tasks std::vector taskList; gPortsOrch->dumpPendingTasks(taskList); @@ -1261,7 +1274,7 @@ namespace portsorch_test // Verify idriver std::vector idriver = { 0x6, 0x6, 0x6, 0x6 }; - ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_IDRIVER), idriver); + ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_IDRIVER), SerdesValue(idriver)); // Verify admin-disable then admin-enable ASSERT_EQ(_sai_set_admin_state_down_count, ++current_sai_api_call_count); From 9c45758702e3a41c2cc9c6a187a4773521bcdd30 Mon Sep 17 00:00:00 2001 From: Longyin Huang Date: Wed, 10 Sep 2025 18:25:52 -0700 Subject: [PATCH 3/3] Fix testcase --- tests/mock_tests/portsorch_ut.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index 18dbac193c..939930925a 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -1080,6 +1080,7 @@ namespace portsorch_test // Port count: 32 Data + 1 CPU ASSERT_EQ(gPortsOrch->getAllPorts().size(), ports.size() + 1); + std::string custom_serdes_attrs = "{'attributes':[{'attr_xyz':{'value':[1,2,3,4]}}]}"; // Generate port serdes config std::deque kfvList = {{ "Ethernet0", @@ -1102,7 +1103,7 @@ namespace portsorch_test { "obnlev", "0x5f,0x61,0x60,0x62" }, { "regn_bfm1p", "0x1e,0x20,0x1f,0x21" }, { "regn_bfm1n", "0xaa,0xac,0xab,0xad" }, - { "custom_serdes_attrs", "{'attributes':[]}" } + { "custom_serdes_attrs", custom_serdes_attrs } } }}; @@ -1186,7 +1187,6 @@ namespace portsorch_test ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_TX_NMOS_VLTG_REG), SerdesValue(regn_bfm1n)); // Verify custom_serdes_attrs - std::string custom_serdes_attrs = "{'attributes':[{'attr_xyz':{'value':[1,2,3,4]}}]}"; ASSERT_EQ(p.m_serdes_attrs.at(SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION), SerdesValue(custom_serdes_attrs)); // Verify unreliablelos