From f258b9373032abce115a60cbd540bee6c36e16ee Mon Sep 17 00:00:00 2001 From: byu343 Date: Wed, 7 Dec 2022 17:52:56 -0800 Subject: [PATCH] [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 sonic-net/sonic-buildimage#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: sonic-net/sonic-buildimage#11480 Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID fixed compile issues Updated code for the flow where VRF VNI mapping is processed first followed by VLAN VNI mapping --- gearsyncd/gearboxparser.cpp | 24 ++++++- lib/gearboxutils.cpp | 5 ++ lib/gearboxutils.h | 19 ++++++ orchagent/p4orch/tests/fake_portorch.cpp | 2 +- orchagent/portsorch.cpp | 55 ++++++++++++++-- orchagent/portsorch.h | 26 +++++++- orchagent/vrforch.h | 13 ++++ orchagent/vxlanorch.cpp | 81 +++++++++++++++++++++++- orchagent/vxlanorch.h | 8 +++ tests/test_gearbox.py | 23 +++++++ 10 files changed, 244 insertions(+), 12 deletions(-) diff --git a/gearsyncd/gearboxparser.cpp b/gearsyncd/gearboxparser.cpp index dfd68be2ec3..879624fd252 100644 --- a/gearsyncd/gearboxparser.cpp +++ b/gearsyncd/gearboxparser.cpp @@ -15,6 +15,7 @@ */ #include "gearboxparser.h" +#include "gearboxutils.h" #include "phyparser.h" #include @@ -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 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()); + } + attr = std::make_pair(txFirKey, txFirValuesStr); + attrs.push_back(attr); + } + } + std::string key; key = "interface:" + std::to_string(index); if (getWriteToDb() == true) diff --git a/lib/gearboxutils.cpp b/lib/gearboxutils.cpp index f9b32286213..bc35ed34568 100644 --- a/lib/gearboxutils.cpp +++ b/lib/gearboxutils.cpp @@ -266,6 +266,11 @@ std::map 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; } diff --git a/lib/gearboxutils.h b/lib/gearboxutils.h index 28ab48761e6..a239aa3a10f 100644 --- a/lib/gearboxutils.h +++ b/lib/gearboxutils.h @@ -30,6 +30,24 @@ namespace swss { +static const std::set 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 line_lanes; std::set system_lanes; + std::map tx_firs; } gearbox_interface_t; typedef struct diff --git a/orchagent/p4orch/tests/fake_portorch.cpp b/orchagent/p4orch/tests/fake_portorch.cpp index d2b7651a882..6913d01e1aa 100644 --- a/orchagent/p4orch/tests/fake_portorch.cpp +++ b/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 &lane_values) +void PortsOrch::getPortSerdesVal(const std::string &s, std::vector &lane_values, int base) { } diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index eaeba5ae82a..3170df044c6 100755 --- a/orchagent/portsorch.cpp +++ b/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> &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(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 &lane_values) + std::vector &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> 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() ) { + 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; + } + } } } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index cec49a7bda5..da396b3d589 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -49,6 +49,28 @@ static const unordered_map string_oper_status = { "not present", SAI_PORT_OPER_STATUS_NOT_PRESENT } }; +static const std::map 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 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 &lane_values); + void getPortSerdesVal(const std::string& s, std::vector &lane_values, int base = 16); bool getPortAdvSpeedsVal(const std::string &s, std::vector &speed_values); bool getPortInterfaceTypeVal(const std::string &s, sai_port_interface_type_t &interface_type); bool getPortAdvInterfaceTypesVal(const std::string &s, std::vector &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> &serdes_attr); diff --git a/orchagent/vrforch.h b/orchagent/vrforch.h index 195015fa083..07e0df55ec2 100644 --- a/orchagent/vrforch.h +++ b/orchagent/vrforch.h @@ -155,6 +155,19 @@ class VRFOrch : public Orch2 return (-1); } } + + bool isL3VniVlan(const uint32_t vni) const + { + if (l3vni_table_.find(vni) != std::end(l3vni_table_)) + { + return l3vni_table_.at(vni).l3_vni; + } + else + { + return false; + } + } + int updateL3VniVlan(uint32_t vni, uint16_t vlan_id); private: virtual bool addOperation(const Request& request); diff --git a/orchagent/vxlanorch.cpp b/orchagent/vxlanorch.cpp index 1995675fdd1..31ac9cc2730 100644 --- a/orchagent/vxlanorch.cpp +++ b/orchagent/vxlanorch.cpp @@ -1903,6 +1903,7 @@ bool VxlanTunnelMapOrch::addOperation(const Request& request) sai_vlan_id_t vlan_id = (sai_vlan_id_t)request.getAttrVlan("vlan"); Port tempPort; + bool isL3Vni = false; const auto full_tunnel_map_entry_name = request.getFullKey(); SWSS_LOG_INFO("Full name = %s",full_tunnel_map_entry_name.c_str()); @@ -1974,11 +1975,21 @@ bool VxlanTunnelMapOrch::addOperation(const Request& request) tunnel_obj->vlan_vrf_vni_count++; SWSS_LOG_INFO("vni count increased to %d",tunnel_obj->vlan_vrf_vni_count); + VRFOrch* vrf_orch = gDirectory.get(); + isL3Vni = vrf_orch->isL3VniVlan(vni_id); + try { - auto tunnel_map_entry_id = create_tunnel_map_entry(MAP_T::VNI_TO_VLAN_ID, - tunnel_map_id, vni_id, vlan_id); - vxlan_tunnel_map_table_[full_tunnel_map_entry_name].map_entry_id = tunnel_map_entry_id; + if (isL3Vni == false) + { + auto tunnel_map_entry_id = create_tunnel_map_entry(MAP_T::VNI_TO_VLAN_ID, + tunnel_map_id, vni_id, vlan_id); + vxlan_tunnel_map_table_[full_tunnel_map_entry_name].map_entry_id = tunnel_map_entry_id; + } + else + { + vxlan_tunnel_map_table_[full_tunnel_map_entry_name].map_entry_id = SAI_NULL_OBJECT_ID; + } vxlan_tunnel_map_table_[full_tunnel_map_entry_name].vlan_id = vlan_id; vxlan_tunnel_map_table_[full_tunnel_map_entry_name].vni_id = vni_id; } @@ -2124,9 +2135,13 @@ bool VxlanTunnelMapOrch::delOperation(const Request& request) bool VxlanVrfMapOrch::addOperation(const Request& request) { SWSS_LOG_ENTER(); + std::string vniVlanMapName; + uint32_t vlan_id = 0; + sai_object_id_t tnl_map_entry_id = SAI_NULL_OBJECT_ID; auto tunnel_name = request.getKeyString(0); VxlanTunnelOrch* tunnel_orch = gDirectory.get(); + VxlanTunnelMapOrch* vxlan_tun_map_orch = gDirectory.get(); if (!tunnel_orch->isTunnelExists(tunnel_name)) { SWSS_LOG_WARN("Vxlan tunnel '%s' doesn't exist", tunnel_name.c_str()); @@ -2188,6 +2203,15 @@ bool VxlanVrfMapOrch::addOperation(const Request& request) vrf_map_entry_t entry; try { + entry.isL2Vni = vxlan_tun_map_orch->isVniVlanMapExists(vni_id, vniVlanMapName, &tnl_map_entry_id, &vlan_id); + if (entry.isL2Vni) + { + entry.vniVlanMapName = vniVlanMapName; + entry.vlan_id = vlan_id; + entry.vni_id = vni_id; + remove_tunnel_map_entry(tnl_map_entry_id); + SWSS_LOG_DEBUG("remove_tunnel_map_entry name %s, vlan %d, vni %d\n", entry.vniVlanMapName.c_str(), entry.vlan_id, entry.vni_id); + } /* * Create encap and decap mapper */ @@ -2219,6 +2243,8 @@ bool VxlanVrfMapOrch::delOperation(const Request& request) SWSS_LOG_ENTER(); VRFOrch* vrf_orch = gDirectory.get(); + VxlanTunnelOrch* tunnel_orch = gDirectory.get(); + VxlanTunnelMapOrch* vxlan_tun_map_orch = gDirectory.get(); const auto full_map_entry_name = request.getFullKey(); if (!isVrfMapExists(full_map_entry_name)) @@ -2241,6 +2267,9 @@ bool VxlanVrfMapOrch::delOperation(const Request& request) return false; } SWSS_LOG_NOTICE("VxlanVrfMapOrch VRF VNI mapping '%s' remove vrf %s", full_map_entry_name.c_str(), vrf_name.c_str()); + auto tunnel_name = request.getKeyString(0); + auto tunnel_obj = tunnel_orch->getVxlanTunnel(tunnel_name); + vrf_map_entry_t entry; try { @@ -2256,6 +2285,20 @@ bool VxlanVrfMapOrch::delOperation(const Request& request) vrf_orch->decreaseVrfRefCount(vrf_name); remove_tunnel_map_entry(entry.decap_id); vrf_orch->decreaseVrfRefCount(vrf_name); + + if(entry.isL2Vni) + { + const auto tunnel_map_id = tunnel_obj->getDecapMapId(TUNNEL_MAP_T_VLAN); + SWSS_LOG_NOTICE("Adding tunnel map entry. Tunnel: %s %s",tunnel_name.c_str(),entry.vniVlanMapName.c_str()); + + SWSS_LOG_DEBUG("create_tunnel_map_entry vni %d, vlan %d\n", entry.vni_id, entry.vlan_id); + auto tunnel_map_entry_id = create_tunnel_map_entry(MAP_T::VNI_TO_VLAN_ID, + tunnel_map_id, entry.vni_id, (uint16_t)entry.vlan_id); + SWSS_LOG_DEBUG("updateTnlMapId name %s\n", entry.vniVlanMapName.c_str()); + + vxlan_tun_map_orch->updateTnlMapId(entry.vniVlanMapName, tunnel_map_entry_id); + } + vxlan_vrf_table_.erase(full_map_entry_name); vxlan_vrf_tunnel_.erase(vrf_name); } @@ -2599,3 +2642,35 @@ bool EvpnNvoOrch::delOperation(const Request& request) return true; } + +bool VxlanTunnelMapOrch::isVniVlanMapExists(uint32_t vni_id, std::string& vniVlanMapName, sai_object_id_t *tnl_map_entry_id, uint32_t *vlan_id) +{ + SWSS_LOG_ENTER(); + bool map_entry_exists = false; + std::map::iterator it; + for(it = vxlan_tunnel_map_table_.begin(); it != vxlan_tunnel_map_table_.end(); it++) + { + auto full_tunnel_map_entry_name = it->first; + tunnel_map_entry_t tunnel_map_entry = it->second; + + if (vni_id == tunnel_map_entry.vni_id) + { + vniVlanMapName = full_tunnel_map_entry_name; + *tnl_map_entry_id = tunnel_map_entry.map_entry_id; + *vlan_id = tunnel_map_entry.vlan_id; + map_entry_exists = true; + SWSS_LOG_NOTICE("vniVlanMapName %s, vlan %d\n", vniVlanMapName.c_str(), *vlan_id); + break; + } + } + + return map_entry_exists; +} + +void VxlanTunnelMapOrch::updateTnlMapId(std::string vniVlanMapName, sai_object_id_t tunnel_map_id) +{ + SWSS_LOG_ENTER(); + SWSS_LOG_NOTICE("name %s\n", vniVlanMapName.c_str()); + vxlan_tunnel_map_table_[vniVlanMapName].map_entry_id = tunnel_map_id; +} + diff --git a/orchagent/vxlanorch.h b/orchagent/vxlanorch.h index 9529a86ce78..695f7441e05 100644 --- a/orchagent/vxlanorch.h +++ b/orchagent/vxlanorch.h @@ -410,6 +410,10 @@ class VxlanTunnelMapOrch : public Orch2 { return vxlan_tunnel_map_table_.find(name) != std::end(vxlan_tunnel_map_table_); } + + bool isVniVlanMapExists(uint32_t vni_id, std::string& vniVlanMapName, sai_object_id_t *tnl_map_entry_id, uint32_t *vlan_id); + + void updateTnlMapId(std::string vniVlanMapName, sai_object_id_t tunnel_map_id); private: virtual bool addOperation(const Request& request); virtual bool delOperation(const Request& request); @@ -436,6 +440,10 @@ class VxlanVrfRequest : public Request struct vrf_map_entry_t { sai_object_id_t encap_id; sai_object_id_t decap_id; + bool isL2Vni; + std::string vniVlanMapName; + uint32_t vlan_id; + uint32_t vni_id; }; typedef std::map VxlanVrfTable; diff --git a/tests/test_gearbox.py b/tests/test_gearbox.py index 7d5b568661b..6707213990b 100644 --- a/tests/test_gearbox.py +++ b/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():