[gearbox] Support setting tx taps on gearbox ports (#2158)

What I did
This change adds support for setting tx tap or tuning values on gearbox ports. It uses the SAI attributes such as SAI_PORT_SERDES_ATTR_TX_FIR_PRE1 to communicate with SAI-based gearbox drivers. For the values, they are provided in the format like "system_tx_fir_pre2": [1,1] for an interface from gearbox_config.json.

Why I did it

How I verified it
We verified that values provided in #10084 are set to the chip with this change.

Added test to tests/test_gearbox.py. The added test will not pass until the following two changes (which should be merged first) are merged:
Support SAI_PORT_ATTR_PORT_SERDES_ID on vs gearbox: sonic-net/sonic-sairedis#1082
Add gearbox taps to vs gearbox_config.json: #11480

gearsyncd/gearboxparser.cpp

@@ -15,6 +15,7 @@
  */
 
 #include "gearboxparser.h"
+#include "gearboxutils.h"
 #include "phyparser.h"
 #include <vector>
 
@@ -42,7 +43,7 @@ bool GearboxParser::parse()
  return false;
  }
 
- json phys, phy, interfaces, interface, val, lanes;
+ json phys, phy, interfaces, interface, val, lanes, txFir;
 
  std::vector<swss::FieldValueTuple> attrs;
 
@@ -285,6 +286,27 @@ bool GearboxParser::parse()
  SWSS_LOG_ERROR("missing 'line_lanes' field in 'interfaces' item %d in gearbox configuration", iter);
  return false;
  }
+
+ for (std::string txFirKey: swss::tx_fir_strings)
+ {
+ if (interface.find(txFirKey) != interface.end())
+ {
+ txFir = interface[txFirKey]; // vec
+ std::string txFirValuesStr("");
+ for (uint32_t iter2 = 0; iter2 < txFir.size(); iter2++)
+ {
+ val = txFir[iter2];
+ if (txFirValuesStr.length() > 0)
+ {
+ txFirValuesStr += ",";
+ }
+ txFirValuesStr += std::to_string(val.get<int>());
+ }
+ attr = std::make_pair(txFirKey, txFirValuesStr);
+ attrs.push_back(attr);
+ }
+ }
+
  std::string key;
  key = "interface:" + std::to_string(index);
  if (getWriteToDb() == true) 

lib/gearboxutils.cpp

@@ -266,6 +266,11 @@ std::map<int, gearbox_interface_t> GearboxUtils::loadInterfaceMap(Table *gearbox
  }
  }
  }
+ else if (tx_fir_strings.find(val.first) != tx_fir_strings.end())
+ {
+ SWSS_LOG_DEBUG("Parsed key:%s, val:%s", val.first.c_str(), val.second.c_str());
+ interface.tx_firs[val.first] = val.second;
+ }
  }
  gearboxInterfaceMap[interface.index] = interface;
  }

lib/gearboxutils.h

@@ -30,6 +30,24 @@
 
 namespace swss {
 
+static const std::set<std::string> tx_fir_strings =
+{
+ "system_tx_fir_pre1",
+ "system_tx_fir_pre2",
+ "system_tx_fir_pre3",
+ "system_tx_fir_post1",
+ "system_tx_fir_post2",
+ "system_tx_fir_post3",
+ "system_tx_fir_main",
+ "line_tx_fir_pre1",
+ "line_tx_fir_pre2",
+ "line_tx_fir_pre3",
+ "line_tx_fir_post1",
+ "line_tx_fir_post2",
+ "line_tx_fir_post3",
+ "line_tx_fir_main"
+};
+
 typedef struct
 {
  int phy_id;
@@ -54,6 +72,7 @@ typedef struct
  int phy_id;
  std::set<int> line_lanes;
  std::set<int> system_lanes;
+ std::map<std::string, std::string> tx_firs;
 } gearbox_interface_t;
 
 typedef struct

orchagent/p4orch/tests/fake_portorch.cpp

@@ -644,7 +644,7 @@ void PortsOrch::updateDbPortOperSpeed(Port &port, sai_uint32_t speed)
 {
 }
 
-void PortsOrch::getPortSerdesVal(const std::string &s, std::vector<uint32_t> &lane_values)
+void PortsOrch::getPortSerdesVal(const std::string &s, std::vector<uint32_t> &lane_values, int base)
 {
 }
 

orchagent/portsorch.cpp

@@ -3761,7 +3761,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
  p.m_preemphasis = serdes_attr;
  m_portList[alias] = p;
  }
- else if (setPortSerdesAttribute(p.m_port_id, serdes_attr))
+ else if (setPortSerdesAttribute(p.m_port_id, gSwitchId, serdes_attr))
  {
  SWSS_LOG_NOTICE("Set port %s preemphasis is success", alias.c_str());
  p.m_preemphasis = serdes_attr;
@@ -6923,7 +6923,7 @@ bool PortsOrch::removeAclTableGroup(const Port &p)
  return true;
 }
 
-bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id,
+bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id, sai_object_id_t switch_id,
  map<sai_port_serdes_attr_t, vector<uint32_t>> &serdes_attr)
 {
  SWSS_LOG_ENTER();
@@ -6975,7 +6975,7 @@ bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id,
  port_serdes_attr.value.u32list.list = it->second.data();
  attr_list.emplace_back(port_serdes_attr);
  }
- status = sai_port_api->create_port_serdes(&port_serdes_id, gSwitchId,
+ status = sai_port_api->create_port_serdes(&port_serdes_id, switch_id,
  static_cast<uint32_t>(serdes_attr.size()+1),
  attr_list.data());
 
@@ -7026,7 +7026,8 @@ void PortsOrch::removePortSerdesAttribute(sai_object_id_t port_id)
 }
 
 void PortsOrch::getPortSerdesVal(const std::string& val_str,
- std::vector<uint32_t> &lane_values)
+ std::vector<uint32_t> &lane_values,
+ int base)
 {
  SWSS_LOG_ENTER();
 
@@ -7036,7 +7037,7 @@ void PortsOrch::getPortSerdesVal(const std::string& val_str,
 
  while (std::getline(iss, lane_str, ','))
  {
- lane_val = (uint32_t)std::stoul(lane_str, NULL, 16);
+ lane_val = (uint32_t)std::stoul(lane_str, NULL, base);
  lane_values.push_back(lane_val);
  }
 }
@@ -7412,6 +7413,50 @@ bool PortsOrch::initGearboxPort(Port &port)
 
  fields[0] = FieldValueTuple(port.m_alias + "_line", sai_serialize_object_id(linePort));
  m_gbcounterTable->set("", fields);
+
+ /* Set serdes tx taps on system and line side */
+ map<sai_port_serdes_attr_t, vector<uint32_t>> serdes_attr;
+ typedef pair<sai_port_serdes_attr_t, vector<uint32_t>> serdes_attr_pair;
+ vector<uint32_t> 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() ) {
+ attr_val.clear();
+ getPortSerdesVal(m_gearboxInterfaceMap[port.m_index].tx_firs[pair.first], attr_val, 10);
+ serdes_attr.insert(serdes_attr_pair(pair.second, attr_val));
+ }
+ }
+ if (serdes_attr.size() != 0)
+ {
+ if (setPortSerdesAttribute(systemPort, phyOid, serdes_attr))
+ {
+ SWSS_LOG_NOTICE("Set port %s system side preemphasis 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());
+ return false;
+ }
+ }
+ serdes_attr.clear();
+ for (auto pair: tx_fir_strings_line_side) {
+ if (m_gearboxInterfaceMap[port.m_index].tx_firs.find(pair.first) != m_gearboxInterfaceMap[port.m_index].tx_firs.end() ) {
+ attr_val.clear();
+ getPortSerdesVal(m_gearboxInterfaceMap[port.m_index].tx_firs[pair.first], attr_val, 10);
+ serdes_attr.insert(serdes_attr_pair(pair.second, attr_val));
+ }
+ }
+ if (serdes_attr.size() != 0)
+ {
+ if (setPortSerdesAttribute(linePort, phyOid, serdes_attr))
+ {
+ SWSS_LOG_NOTICE("Set port %s line side preemphasis 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());
+ return false;
+ }
+ }
  }
  }
 

orchagent/portsorch.h

@@ -49,6 +49,28 @@ static const unordered_map<string, sai_port_oper_status_t> string_oper_status =
  { "not present", SAI_PORT_OPER_STATUS_NOT_PRESENT }
 };
 
+static const std::map<std::string, sai_port_serdes_attr_t> tx_fir_strings_system_side =
+{
+ {"system_tx_fir_pre1", SAI_PORT_SERDES_ATTR_TX_FIR_PRE1},
+ {"system_tx_fir_pre2", SAI_PORT_SERDES_ATTR_TX_FIR_PRE2},
+ {"system_tx_fir_pre3", SAI_PORT_SERDES_ATTR_TX_FIR_PRE3},
+ {"system_tx_fir_post1", SAI_PORT_SERDES_ATTR_TX_FIR_POST1},
+ {"system_tx_fir_post2", SAI_PORT_SERDES_ATTR_TX_FIR_POST2},
+ {"system_tx_fir_post3", SAI_PORT_SERDES_ATTR_TX_FIR_POST3},
+ {"system_tx_fir_main", SAI_PORT_SERDES_ATTR_TX_FIR_MAIN}
+};
+
+static const std::map<std::string, sai_port_serdes_attr_t> tx_fir_strings_line_side =
+{
+ {"line_tx_fir_pre1", SAI_PORT_SERDES_ATTR_TX_FIR_PRE1},
+ {"line_tx_fir_pre2", SAI_PORT_SERDES_ATTR_TX_FIR_PRE2},
+ {"line_tx_fir_pre3", SAI_PORT_SERDES_ATTR_TX_FIR_PRE3},
+ {"line_tx_fir_post1", SAI_PORT_SERDES_ATTR_TX_FIR_POST1},
+ {"line_tx_fir_post2", SAI_PORT_SERDES_ATTR_TX_FIR_POST2},
+ {"line_tx_fir_post3", SAI_PORT_SERDES_ATTR_TX_FIR_POST3},
+ {"line_tx_fir_main", SAI_PORT_SERDES_ATTR_TX_FIR_MAIN}
+};
+
 struct PortUpdate
 {
  Port port;
@@ -389,12 +411,12 @@ class PortsOrch : public Orch, public Subject
  void refreshPortStateAutoNeg(const Port &port);
  void refreshPortStateLinkTraining(const Port &port);
 
- void getPortSerdesVal(const std::string& s, std::vector<uint32_t> &lane_values);
+ void getPortSerdesVal(const std::string& s, std::vector<uint32_t> &lane_values, int base = 16);
  bool getPortAdvSpeedsVal(const std::string &s, std::vector<uint32_t> &speed_values);
  bool getPortInterfaceTypeVal(const std::string &s, sai_port_interface_type_t &interface_type);
  bool getPortAdvInterfaceTypesVal(const std::string &s, std::vector<uint32_t> &type_values);
 
- bool setPortSerdesAttribute(sai_object_id_t port_id,
+ bool setPortSerdesAttribute(sai_object_id_t port_id, sai_object_id_t switch_id,
  std::map<sai_port_serdes_attr_t, std::vector<uint32_t>> &serdes_attr);
 
 

tests/test_gearbox.py

@@ -70,6 +70,7 @@ def __init__(self, db_id: int, connector: str, gearbox: Gearbox):
  DVSDatabase.__init__(self, db_id, connector)
  self.gearbox = gearbox
  self.ports = {}
+ self.port_oid_to_intf_idx = {}
  self._wait_for_gb_asic_db_to_initialize()
 
  for connector in self.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT_CONNECTOR"):
@@ -88,9 +89,31 @@ def __init__(self, db_id: int, connector: str, gearbox: Gearbox):
  if intf["system_lanes"] == system_lanes:
  assert intf["line_lanes"] == line_lanes
  self.ports[intf["index"]] = (system_port_oid, line_port_oid)
+ self.port_oid_to_intf_idx[system_port_oid] = (i, True)
+ self.port_oid_to_intf_idx[line_port_oid] = (i, False)
 
  assert len(self.ports) == len(self.gearbox.interfaces)
 
+ for serdes in self.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT_SERDES"):
+ fvs = self.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_PORT_SERDES", serdes)
+ port_oid = fvs.get("SAI_PORT_SERDES_ATTR_PORT_ID")
+ intf_idx, is_system = self.port_oid_to_intf_idx[port_oid]
+ intf = self.gearbox.interfaces[ intf_idx ]
+ appl_db_key_prefix = 'system_' if is_system else 'line_'
+ for asic_db_key, appl_db_key_suffix in [
+ ("SAI_PORT_SERDES_ATTR_TX_FIR_MAIN", "tx_fir_main"),
+ ("SAI_PORT_SERDES_ATTR_TX_FIR_PRE1", "tx_fir_pre1"),
+ ("SAI_PORT_SERDES_ATTR_TX_FIR_PRE2", "tx_fir_pre2"),
+ ("SAI_PORT_SERDES_ATTR_TX_FIR_PRE3", "tx_fir_pre3"),
+ ("SAI_PORT_SERDES_ATTR_TX_FIR_POST1", "tx_fir_post1"),
+ ("SAI_PORT_SERDES_ATTR_TX_FIR_POST2", "tx_fir_post2"),
+ ("SAI_PORT_SERDES_ATTR_TX_FIR_POST3", "tx_fir_post3"),
+ ]:
+ if asic_db_key not in fvs:
+ continue
+ asic_db_value = fvs.get(asic_db_key).split(":")[-1]
+ assert intf[appl_db_key_prefix + appl_db_key_suffix] == asic_db_value
+
  def _wait_for_gb_asic_db_to_initialize(self) -> None:
  """Wait up to 30 seconds for the default fields to appear in ASIC DB."""
  def _verify_db_contents():