From 80dfc978d6973d042e9be61235a12c1758c95275 Mon Sep 17 00:00:00 2001 From: gechiang <62408185+gechiang@users.noreply.github.com> Date: Tue, 1 Jun 2021 13:46:30 -0700 Subject: [PATCH] [sonic-swss:TPID CONFIG]Added Orchagent TPID config support for Port/LAG (#1747) * [sonic-swss:TPID CONFIG]Added Orchagent TPID config support for Port/LAG Signed-off-by: Gen-Hwa Chiang * Changed per review comment to move TPID capability query under switchorch class --- cfgmgr/portmgr.cpp | 27 ++++++++- cfgmgr/portmgr.h | 1 + cfgmgr/teammgr.cpp | 26 +++++++++ cfgmgr/teammgr.h | 1 + orchagent/port.h | 7 +++ orchagent/portsorch.cpp | 113 ++++++++++++++++++++++++++++++++++++++ orchagent/portsorch.h | 2 + orchagent/switchorch.cpp | 54 ++++++++++++++++++ orchagent/switchorch.h | 4 ++ tests/test_port.py | 33 +++++++++++ tests/test_portchannel.py | 37 +++++++++++++ 11 files changed, 304 insertions(+), 1 deletion(-) diff --git a/cfgmgr/portmgr.cpp b/cfgmgr/portmgr.cpp index d37a50f78a3..8fc43ca47f2 100644 --- a/cfgmgr/portmgr.cpp +++ b/cfgmgr/portmgr.cpp @@ -38,6 +38,21 @@ bool PortMgr::setPortMtu(const string &alias, const string &mtu) return true; } +bool PortMgr::setPortTpid(const string &alias, const string &tpid) +{ + stringstream cmd; + string res; + + // Set the port TPID in application database to update port TPID + vector fvs; + FieldValueTuple fv("tpid", tpid); + fvs.push_back(fv); + m_appPortTable.set(alias, fvs); + + return true; +} + + bool PortMgr::setPortAdminStatus(const string &alias, const bool up) { stringstream cmd; @@ -102,7 +117,7 @@ void PortMgr::doTask(Consumer &consumer) continue; } - string admin_status, mtu, learn_mode; + string admin_status, mtu, learn_mode, tpid; bool configured = (m_portList.find(alias) != m_portList.end()); @@ -131,6 +146,10 @@ void PortMgr::doTask(Consumer &consumer) { learn_mode = fvValue(i); } + else if (fvField(i) == "tpid") + { + tpid = fvValue(i); + } } if (!mtu.empty()) @@ -150,6 +169,12 @@ void PortMgr::doTask(Consumer &consumer) setPortLearnMode(alias, learn_mode); SWSS_LOG_NOTICE("Configure %s MAC learn mode to %s", alias.c_str(), learn_mode.c_str()); } + + if (!tpid.empty()) + { + setPortTpid(alias, tpid); + SWSS_LOG_NOTICE("Configure %s TPID to %s", alias.c_str(), tpid.c_str()); + } } else if (op == DEL_COMMAND) { diff --git a/cfgmgr/portmgr.h b/cfgmgr/portmgr.h index a9724365a7f..809cd1c0048 100644 --- a/cfgmgr/portmgr.h +++ b/cfgmgr/portmgr.h @@ -30,6 +30,7 @@ class PortMgr : public Orch void doTask(Consumer &consumer); bool setPortMtu(const std::string &alias, const std::string &mtu); + bool setPortTpid(const std::string &alias, const std::string &tpid); bool setPortAdminStatus(const std::string &alias, const bool up); bool setPortLearnMode(const std::string &alias, const std::string &learn_mode); bool isPortStateOk(const std::string &alias); diff --git a/cfgmgr/teammgr.cpp b/cfgmgr/teammgr.cpp index db1df6884c8..a0f7c41698f 100644 --- a/cfgmgr/teammgr.cpp +++ b/cfgmgr/teammgr.cpp @@ -145,6 +145,7 @@ void TeamMgr::doLagTask(Consumer &consumer) string admin_status = DEFAULT_ADMIN_STATUS_STR; string mtu = DEFAULT_MTU_STR; string learn_mode; + string tpid; for (auto i : kfvFieldsValues(t)) { @@ -178,6 +179,11 @@ void TeamMgr::doLagTask(Consumer &consumer) SWSS_LOG_INFO("Get learn_mode %s", learn_mode.c_str()); } + else if (fvField(i) == "tpid") + { + tpid = fvValue(i); + SWSS_LOG_INFO("Get TPID %s", tpid.c_str()); + } } if (m_lagList.find(alias) == m_lagList.end()) @@ -198,6 +204,11 @@ void TeamMgr::doLagTask(Consumer &consumer) setLagLearnMode(alias, learn_mode); SWSS_LOG_NOTICE("Configure %s MAC learn mode to %s", alias.c_str(), learn_mode.c_str()); } + if (!tpid.empty()) + { + setLagTpid(alias, tpid); + SWSS_LOG_NOTICE("Configure %s TPID to %s", alias.c_str(), tpid.c_str()); + } } else if (op == DEL_COMMAND) { @@ -395,6 +406,21 @@ bool TeamMgr::setLagMtu(const string &alias, const string &mtu) return true; } +bool TeamMgr::setLagTpid(const string &alias, const string &tpid) +{ + SWSS_LOG_ENTER(); + + vector fvs; + FieldValueTuple fv("tpid", tpid); + fvs.push_back(fv); + m_appLagTable.set(alias, fvs); + + SWSS_LOG_NOTICE("Set port channel %s TPID to %s", alias.c_str(), tpid.c_str()); + + return true; +} + + bool TeamMgr::setLagLearnMode(const string &alias, const string &learn_mode) { // Set the port MAC learn mode in application database diff --git a/cfgmgr/teammgr.h b/cfgmgr/teammgr.h index 3941d1d5a01..4d2f20bc9f3 100644 --- a/cfgmgr/teammgr.h +++ b/cfgmgr/teammgr.h @@ -49,6 +49,7 @@ class TeamMgr : public Orch bool setLagAdminStatus(const std::string &alias, const std::string &admin_status); bool setLagMtu(const std::string &alias, const std::string &mtu); bool setLagLearnMode(const std::string &alias, const std::string &learn_mode); + bool setLagTpid(const std::string &alias, const std::string &tpid); bool isPortEnslaved(const std::string &); diff --git a/orchagent/port.h b/orchagent/port.h index ca9977075f4..28dc5a731a6 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -20,6 +20,12 @@ extern "C" { */ #define DEFAULT_MTU 1492 +/* + * Default TPID is 8100 + * User can configure other values such as 9100, 9200, or 88A8 + */ +#define DEFAULT_TPID 0x8100 + #define VNID_NONE 0xFFFFFFFF namespace swss { @@ -128,6 +134,7 @@ class Port std::vector m_priority_group_ids; sai_port_priority_flow_control_mode_t m_pfc_asym = SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_COMBINED; uint8_t m_pfc_bitmask = 0; + uint16_t m_tpid = DEFAULT_TPID; uint32_t m_nat_zone_id = 0; uint32_t m_vnid = VNID_NONE; uint32_t m_fdb_count = 0; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 667e9d976be..97bac5e65c5 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -968,6 +968,36 @@ bool PortsOrch::setPortMtu(sai_object_id_t id, sai_uint32_t mtu) return true; } + +bool PortsOrch::setPortTpid(sai_object_id_t id, sai_uint16_t tpid) +{ + SWSS_LOG_ENTER(); + sai_status_t status = SAI_STATUS_SUCCESS; + sai_attribute_t attr; + + attr.id = SAI_PORT_ATTR_TPID; + + attr.value.u16 = (uint16_t)tpid; + + status = sai_port_api->set_port_attribute(id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set TPID 0x%x to port pid:%" PRIx64 ", rv:%d", + attr.value.u16, id, status); + task_process_status handle_status = handleSaiSetStatus(SAI_API_PORT, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + else + { + SWSS_LOG_NOTICE("Set TPID 0x%x to port pid:%" PRIx64, attr.value.u16, id); + } + return true; +} + + bool PortsOrch::setPortFec(Port &port, sai_port_fec_mode_t mode) { SWSS_LOG_ENTER(); @@ -2451,6 +2481,8 @@ void PortsOrch::doPortTask(Consumer &consumer) vector adv_speeds; sai_port_interface_type_t interface_type; vector adv_interface_types; + string tpid_string; + uint16_t tpid = 0; for (auto i : kfvFieldsValues(t)) { @@ -2482,6 +2514,15 @@ void PortsOrch::doPortTask(Consumer &consumer) { mtu = (uint32_t)stoul(fvValue(i)); } + /* Set port TPID */ + if (fvField(i) == "tpid") + { + tpid_string = fvValue(i); + // Need to get rid of the leading 0x + tpid_string.erase(0,2); + tpid = (uint16_t)stoi(tpid_string, 0, 16); + SWSS_LOG_DEBUG("Handling TPID to 0x%x, string value:%s", tpid, tpid_string.c_str()); + } /* Set port speed */ else if (fvField(i) == "speed") { @@ -2914,6 +2955,22 @@ void PortsOrch::doPortTask(Consumer &consumer) } } + if (tpid != 0 && tpid != p.m_tpid) + { + SWSS_LOG_DEBUG("Set port %s TPID to 0x%x", alias.c_str(), tpid); + if (setPortTpid(p.m_port_id, tpid)) + { + p.m_tpid = tpid; + m_portList[alias] = p; + } + else + { + SWSS_LOG_ERROR("Failed to set port %s TPID to 0x%x", alias.c_str(), tpid); + it++; + continue; + } + } + if (!fec_mode.empty()) { if (fec_mode_map.find(fec_mode) != fec_mode_map.end()) @@ -3366,6 +3423,8 @@ void PortsOrch::doLagTask(Consumer &consumer) string operation_status; uint32_t lag_id = 0; int32_t switch_id = -1; + string tpid_string; + uint16_t tpid = 0; for (auto i : kfvFieldsValues(t)) { @@ -3395,6 +3454,14 @@ void PortsOrch::doLagTask(Consumer &consumer) { switch_id = stoi(fvValue(i)); } + else if (fvField(i) == "tpid") + { + tpid_string = fvValue(i); + // Need to get rid of the leading 0x + tpid_string.erase(0,2); + tpid = (uint16_t)stoi(tpid_string, 0, 16); + SWSS_LOG_DEBUG("reading TPID string:%s to uint16: 0x%x", tpid_string.c_str(), tpid); + } } if (table_name == CHASSIS_APP_LAG_TABLE_NAME) @@ -3451,6 +3518,23 @@ void PortsOrch::doLagTask(Consumer &consumer) updateChildPortsMtu(l, mtu); } + if (tpid != 0) + { + if (tpid != l.m_tpid) + { + if(!setLagTpid(l.m_lag_id, tpid)) + { + SWSS_LOG_ERROR("Failed to set LAG %s TPID 0x%x", alias.c_str(), tpid); + } + else + { + SWSS_LOG_DEBUG("Set LAG %s TPID to 0x%x", alias.c_str(), tpid); + l.m_tpid = tpid; + m_portList[alias] = l; + } + } + } + if (!learn_mode.empty() && (l.m_learn_mode != learn_mode)) { if (l.m_bridge_port_id != SAI_NULL_OBJECT_ID) @@ -4656,6 +4740,35 @@ bool PortsOrch::removeLagMember(Port &lag, Port &port) return true; } +bool PortsOrch::setLagTpid(sai_object_id_t id, sai_uint16_t tpid) +{ + SWSS_LOG_ENTER(); + sai_status_t status = SAI_STATUS_SUCCESS; + sai_attribute_t attr; + + attr.id = SAI_LAG_ATTR_TPID; + + attr.value.u16 = (uint16_t)tpid; + + status = sai_lag_api->set_lag_attribute(id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set TPID 0x%x to LAG pid:%" PRIx64 ", rv:%d", + attr.value.u16, id, status); + task_process_status handle_status = handleSaiSetStatus(SAI_API_LAG, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + else + { + SWSS_LOG_NOTICE("Set TPID 0x%x to LAG pid:%" PRIx64 , attr.value.u16, id); + } + return true; +} + + bool PortsOrch::setCollectionOnLagMember(Port &lagMember, bool enableCollection) { /* Port must be LAG member */ diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 9560cedcd50..6a60de60efc 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -247,6 +247,7 @@ class PortsOrch : public Orch, public Subject bool addLag(string lag, uint32_t spa_id, int32_t switch_id); bool removeLag(Port lag); + bool setLagTpid(sai_object_id_t id, sai_uint16_t tpid); bool addLagMember(Port &lag, Port &port, bool enableForwarding); bool removeLagMember(Port &lag, Port &port); bool setCollectionOnLagMember(Port &lagMember, bool enableCollection); @@ -260,6 +261,7 @@ class PortsOrch : public Orch, public Subject bool setPortAdminStatus(Port &port, bool up); bool getPortAdminStatus(sai_object_id_t id, bool& up); bool setPortMtu(sai_object_id_t id, sai_uint32_t mtu); + bool setPortTpid(sai_object_id_t id, sai_uint16_t tpid); bool setPortPvid (Port &port, sai_uint32_t pvid); bool getPortPvid(Port &port, sai_uint32_t &pvid); bool setPortFec(Port &port, sai_port_fec_mode_t mode); diff --git a/orchagent/switchorch.cpp b/orchagent/switchorch.cpp index cc4e103fee7..77c8336123e 100644 --- a/orchagent/switchorch.cpp +++ b/orchagent/switchorch.cpp @@ -46,6 +46,7 @@ SwitchOrch::SwitchOrch(DBConnector *db, vector& connectors, Tabl Orch::addExecutor(restartCheckNotifier); initSensorsTable(); + querySwitchTpidCapability(); auto executorT = new ExecutableTimer(m_sensorsPollerTimer, this, "ASIC_SENSORS_POLL_TIMER"); Orch::addExecutor(executorT); } @@ -486,3 +487,56 @@ void SwitchOrch::set_switch_capability(const std::vector& value { m_switchTable.set("switch", values); } + +void SwitchOrch::querySwitchTpidCapability() +{ + SWSS_LOG_ENTER(); + // Check if SAI is capable of handling TPID config and store result in StateDB switch capability table + { + vector fvVector; + sai_status_t status = SAI_STATUS_SUCCESS; + sai_attr_capability_t capability; + + // Check if SAI is capable of handling TPID for Port + status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_PORT, SAI_PORT_ATTR_TPID, &capability); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("Could not query port TPID capability %d", status); + // Since pre-req of TPID support requires querry capability failed, it means TPID not supported + fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_TPID_CAPABLE, "false"); + } + else + { + if (capability.set_implemented) + { + fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_TPID_CAPABLE, "true"); + } + else + { + fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_TPID_CAPABLE, "false"); + } + SWSS_LOG_NOTICE("port TPID capability %d", capability.set_implemented); + } + // Check if SAI is capable of handling TPID for LAG + status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_LAG, SAI_LAG_ATTR_TPID, &capability); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("Could not query LAG TPID capability %d", status); + // Since pre-req of TPID support requires querry capability failed, it means TPID not supported + fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_LAG_TPID_CAPABLE, "false"); + } + else + { + if (capability.set_implemented) + { + fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_LAG_TPID_CAPABLE, "true"); + } + else + { + fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_LAG_TPID_CAPABLE, "false"); + } + SWSS_LOG_NOTICE("LAG TPID capability %d", capability.set_implemented); + } + set_switch_capability(fvVector); + } +} diff --git a/orchagent/switchorch.h b/orchagent/switchorch.h index 5248ba63e46..3da3a07f94a 100644 --- a/orchagent/switchorch.h +++ b/orchagent/switchorch.h @@ -7,6 +7,9 @@ #define ASIC_SENSORS_POLLER_STATUS "ASIC_SENSORS_POLLER_STATUS" #define ASIC_SENSORS_POLLER_INTERVAL "ASIC_SENSORS_POLLER_INTERVAL" +#define SWITCH_CAPABILITY_TABLE_PORT_TPID_CAPABLE "PORT_TPID_CAPABLE" +#define SWITCH_CAPABILITY_TABLE_LAG_TPID_CAPABLE "LAG_TPID_CAPABLE" + struct WarmRestartCheck { bool checkRestartReadyState; @@ -31,6 +34,7 @@ class SwitchOrch : public Orch void doCfgSensorsTableTask(Consumer &consumer); void doAppSwitchTableTask(Consumer &consumer); void initSensorsTable(); + void querySwitchTpidCapability(); swss::NotificationConsumer* m_restartCheckNotificationConsumer; void doTask(swss::NotificationConsumer& consumer); diff --git a/tests/test_port.py b/tests/test_port.py index d5edc74adcf..4766c87debe 100644 --- a/tests/test_port.py +++ b/tests/test_port.py @@ -6,6 +6,39 @@ class TestPort(object): + def test_PortTpid(self, dvs, testlog): + pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) + adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) + cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) + + # set TPID to port + cdb_port_tbl = swsscommon.Table(cdb, "PORT") + fvs = swsscommon.FieldValuePairs([("tpid", "0x9200")]) + cdb_port_tbl.set("Ethernet8", fvs) + time.sleep(1) + + # check application database + pdb_port_tbl = swsscommon.Table(pdb, "PORT_TABLE") + (status, fvs) = pdb_port_tbl.get("Ethernet8") + assert status == True + for fv in fvs: + if fv[0] == "tpid": + tpid = fv[1] + assert tpid == "0x9200" + + # Check ASIC DB + atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + # get TPID and validate it to be 0x9200 (37376) + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet8"]) + assert status == True + asic_tpid = "0" + + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_TPID": + asic_tpid = fv[1] + + assert asic_tpid == "37376" + def test_PortMtu(self, dvs, testlog): pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) diff --git a/tests/test_portchannel.py b/tests/test_portchannel.py index d03e8611230..7c520706c95 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -329,6 +329,43 @@ def test_Portchannel_oper_down(self, dvs, testlog): dvs.servers[0].runcmd("ip link set up dev eth0") time.sleep(1) + def test_Portchannel_tpid(self, dvs, testlog): + adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) + cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) + pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) + + # Create PortChannel + tbl = swsscommon.Table(cdb, "PORTCHANNEL") + fvs = swsscommon.FieldValuePairs([("admin_status", "up"),("mtu", "9100"),("tpid", "0x9200")]) + + tbl.set("PortChannel002", fvs) + time.sleep(1) + + # set oper_status for PortChannels + ps = swsscommon.ProducerStateTable(pdb, "LAG_TABLE") + fvs = swsscommon.FieldValuePairs([("admin_status", "up"),("mtu", "9100"),("tpid", "0x9200"),("oper_status", "up")]) + ps.set("PortChannel002", fvs) + time.sleep(1) + + # Check ASIC DB + # get TPID and validate it to be 0x9200 (37376) + atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_LAG") + lag = atbl.getKeys()[0] + (status, fvs) = atbl.get(lag) + assert status == True + asic_tpid = "0" + + for fv in fvs: + if fv[0] == "SAI_LAG_ATTR_TPID": + asic_tpid = fv[1] + + assert asic_tpid == "37376" + + # remove port channel + tbl = swsscommon.Table(cdb, "PORTCHANNEL") + tbl._del("PortChannel0002") + time.sleep(1) + # Add Dummy always-pass test at end as workaroud # for issue when Flaky fail on final test it invokes module tear-down before retrying