From 7c2768ce2bdbdcfed7104d937de1c1e069717c4c Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Mon, 17 May 2021 17:37:10 +0000 Subject: [PATCH 1/8] [sonic-swss:TPID CONFIG]Added Orchagent TPID config support for Port/LAG Signed-off-by: Gen-Hwa Chiang --- cfgmgr/portmgr.cpp | 27 ++++++++- cfgmgr/portmgr.h | 1 + cfgmgr/teammgr.cpp | 27 +++++++++ cfgmgr/teammgr.h | 1 + orchagent/orchdaemon.cpp | 55 +++++++++++++++++++ orchagent/orchdaemon.h | 3 + orchagent/port.h | 7 +++ orchagent/portsorch.cpp | 113 ++++++++++++++++++++++++++++++++++++++ orchagent/portsorch.h | 2 + tests/test_port.py | 45 +++++++++++++++ tests/test_portchannel.py | 47 ++++++++++++++++ 11 files changed, 327 insertions(+), 1 deletion(-) diff --git a/cfgmgr/portmgr.cpp b/cfgmgr/portmgr.cpp index d37a50f78a..8fc43ca47f 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 a9724365a7..809cd1c004 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 db1df6884c..382d373597 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,22 @@ 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 3941d1d5a0..4d2f20bc9f 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/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 4f01e291a6..478215246a 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -10,6 +10,11 @@ #include "sairedis.h" #include "chassisorch.h" +extern "C" { +#include "sai.h" +} +#include "sai_serialize.h" + using namespace std; using namespace swss; @@ -275,6 +280,56 @@ bool OrchDaemon::init() gMacsecOrch = new MACsecOrch(m_applDb, m_stateDb, macsec_app_tables, gPortsOrch); + Table m_switchTable(stateDbSwitchTable.first, stateDbSwitchTable.second); + // 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); + } + m_switchTable.set("switch", fvVector); + } + /* * The order of the orch list is important for state restore of warm start and * the queued processing in m_toSync map after gPortsOrch->allPortsReady() is set. diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 1829414265..1257acc481 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -37,6 +37,9 @@ using namespace swss; +#define SWITCH_CAPABILITY_TABLE_PORT_TPID_CAPABLE "PORT_TPID_CAPABLE" +#define SWITCH_CAPABILITY_TABLE_LAG_TPID_CAPABLE "LAG_TPID_CAPABLE" + class OrchDaemon { public: diff --git a/orchagent/port.h b/orchagent/port.h index f6abce87b0..7495c380f1 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 335d1d8d35..2d340c5750 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -917,6 +917,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(); @@ -2353,6 +2383,8 @@ void PortsOrch::doPortTask(Consumer &consumer) string learn_mode; int an = -1; int index = -1; + string tpid_string; + uint16_t tpid = 0; for (auto i : kfvFieldsValues(t)) { @@ -2384,6 +2416,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") { @@ -2699,6 +2740,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()) @@ -3151,6 +3208,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)) { @@ -3180,6 +3239,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) @@ -3236,6 +3303,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) @@ -4441,6 +4525,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 a2d669c263..f4e86f7ddb 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -246,6 +246,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); @@ -259,6 +260,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/tests/test_port.py b/tests/test_port.py index d5edc74adc..c9e0aaddd0 100644 --- a/tests/test_port.py +++ b/tests/test_port.py @@ -6,6 +6,51 @@ 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) + + # check application database for default TPID before set operation + pdb_port_tbl = swsscommon.Table(pdb, "PORT_TABLE") + (status, fvs) = pdb_port_tbl.get("Ethernet8") + assert status == True + tpid = "0x0000" + for fv in fvs: + if fv[0] == "tpid": + tpid = fv[1] + + # Old Application DB do not have tpid field yet. After TPID feature is complete + # We can revisit here and assert it is 0x8100 only + assert tpid == "0x8100" or tpid == "0x0000" + + # 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 + (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 d03e861123..05dd7a803b 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -151,6 +151,53 @@ def test_Portchannel_lacpkey(self, dvs, testlog): tbl._del(portchannel[0]) 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 port channel + tbl = swsscommon.Table(cdb, "PORTCHANNEL") + fvs = swsscommon.FieldValuePairs([("admin_status", "up"), + ("mtu", "9100")]) + tbl.set("PortChannel0002", fvs) + time.sleep(1) + + fvs = swsscommon.FieldValuePairs([("tpid", "0x9200")]) + tbl.set("PortChannel0002", fvs) + time.sleep(1) + + # check application database for the correct TPID + pdb_port_tbl = swsscommon.Table(pdb, "LAG_TABLE") + (status, fvs) = pdb_port_tbl.get("PortChannel0002") + assert status == True + tpid = "0" + for fv in fvs: + if fv[0] == "tpid": + tpid = fv[1] + + assert tpid == "0x9200" + + # 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) + + def test_Portchannel_oper_down(self, dvs, testlog): self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) From 3522e816466ca4c4229d63fc416871edd7fe9ada Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Wed, 19 May 2021 01:53:50 +0000 Subject: [PATCH 2/8] Changed per review comment to move TPID capability query under switchorch class --- cfgmgr/teammgr.cpp | 3 +-- orchagent/orchdaemon.cpp | 55 ---------------------------------------- orchagent/orchdaemon.h | 3 --- orchagent/switchorch.cpp | 54 +++++++++++++++++++++++++++++++++++++++ orchagent/switchorch.h | 4 +++ 5 files changed, 59 insertions(+), 60 deletions(-) diff --git a/cfgmgr/teammgr.cpp b/cfgmgr/teammgr.cpp index 382d373597..a0f7c41698 100644 --- a/cfgmgr/teammgr.cpp +++ b/cfgmgr/teammgr.cpp @@ -415,8 +415,7 @@ bool TeamMgr::setLagTpid(const string &alias, const string &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()); + SWSS_LOG_NOTICE("Set port channel %s TPID to %s", alias.c_str(), tpid.c_str()); return true; } diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 478215246a..4f01e291a6 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -10,11 +10,6 @@ #include "sairedis.h" #include "chassisorch.h" -extern "C" { -#include "sai.h" -} -#include "sai_serialize.h" - using namespace std; using namespace swss; @@ -280,56 +275,6 @@ bool OrchDaemon::init() gMacsecOrch = new MACsecOrch(m_applDb, m_stateDb, macsec_app_tables, gPortsOrch); - Table m_switchTable(stateDbSwitchTable.first, stateDbSwitchTable.second); - // 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); - } - m_switchTable.set("switch", fvVector); - } - /* * The order of the orch list is important for state restore of warm start and * the queued processing in m_toSync map after gPortsOrch->allPortsReady() is set. diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 1257acc481..1829414265 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -37,9 +37,6 @@ using namespace swss; -#define SWITCH_CAPABILITY_TABLE_PORT_TPID_CAPABLE "PORT_TPID_CAPABLE" -#define SWITCH_CAPABILITY_TABLE_LAG_TPID_CAPABLE "LAG_TPID_CAPABLE" - class OrchDaemon { public: diff --git a/orchagent/switchorch.cpp b/orchagent/switchorch.cpp index 8b105384cd..bd1b1a54fe 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); } @@ -485,3 +486,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 5248ba63e4..3da3a07f94 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); From 9bc5391d3cbcc3996cff22080d81948d3a814993 Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Wed, 26 May 2021 18:37:41 +0000 Subject: [PATCH 3/8] Removed unnecessary application DB check before the TPID test begins --- tests/test_port.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/tests/test_port.py b/tests/test_port.py index c9e0aaddd0..4766c87deb 100644 --- a/tests/test_port.py +++ b/tests/test_port.py @@ -11,19 +11,6 @@ def test_PortTpid(self, dvs, testlog): adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) - # check application database for default TPID before set operation - pdb_port_tbl = swsscommon.Table(pdb, "PORT_TABLE") - (status, fvs) = pdb_port_tbl.get("Ethernet8") - assert status == True - tpid = "0x0000" - for fv in fvs: - if fv[0] == "tpid": - tpid = fv[1] - - # Old Application DB do not have tpid field yet. After TPID feature is complete - # We can revisit here and assert it is 0x8100 only - assert tpid == "0x8100" or tpid == "0x0000" - # set TPID to port cdb_port_tbl = swsscommon.Table(cdb, "PORT") fvs = swsscommon.FieldValuePairs([("tpid", "0x9200")]) @@ -31,6 +18,7 @@ def test_PortTpid(self, dvs, testlog): 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: From 88619b07c43ad57ec76f21ef1910c4bd1aae3cc0 Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Sat, 29 May 2021 07:34:22 +0000 Subject: [PATCH 4/8] Fix test case issue --- tests/test_port.py | 9 --------- tests/test_portchannel.py | 20 ++------------------ 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/tests/test_port.py b/tests/test_port.py index 4766c87deb..28e720fd0f 100644 --- a/tests/test_port.py +++ b/tests/test_port.py @@ -17,15 +17,6 @@ def test_PortTpid(self, dvs, testlog): 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) diff --git a/tests/test_portchannel.py b/tests/test_portchannel.py index 05dd7a803b..a2bce114b7 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -156,28 +156,12 @@ def test_Portchannel_tpid(self, dvs, testlog): cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) - # create port channel + # create port channel with tpid 0x9200 tbl = swsscommon.Table(cdb, "PORTCHANNEL") - fvs = swsscommon.FieldValuePairs([("admin_status", "up"), - ("mtu", "9100")]) - tbl.set("PortChannel0002", fvs) - time.sleep(1) - - fvs = swsscommon.FieldValuePairs([("tpid", "0x9200")]) + fvs = swsscommon.FieldValuePairs([("admin_status", "up"), ("mtu", "9100"), ("tpid", "0x9200"), ("oper_status", "up")]) tbl.set("PortChannel0002", fvs) time.sleep(1) - # check application database for the correct TPID - pdb_port_tbl = swsscommon.Table(pdb, "LAG_TABLE") - (status, fvs) = pdb_port_tbl.get("PortChannel0002") - assert status == True - tpid = "0" - for fv in fvs: - if fv[0] == "tpid": - tpid = fv[1] - - assert tpid == "0x9200" - # Check ASIC DB # get TPID and validate it to be 0x9200 (37376) atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_LAG") From a4855a779d2bcc9c9273f6641532e0031da9b25d Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Sat, 29 May 2021 07:48:11 +0000 Subject: [PATCH 5/8] Fix test case issue --- tests/test_port.py | 9 +++++++++ tests/test_portchannel.py | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/tests/test_port.py b/tests/test_port.py index 28e720fd0f..4766c87deb 100644 --- a/tests/test_port.py +++ b/tests/test_port.py @@ -17,6 +17,15 @@ def test_PortTpid(self, dvs, testlog): 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) diff --git a/tests/test_portchannel.py b/tests/test_portchannel.py index a2bce114b7..e38ff22954 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -162,6 +162,17 @@ def test_Portchannel_tpid(self, dvs, testlog): tbl.set("PortChannel0002", fvs) time.sleep(1) + # check application database for the correct TPID + pdb_port_tbl = swsscommon.Table(pdb, "LAG_TABLE") + (status, fvs) = pdb_port_tbl.get("PortChannel0002") + assert status == True + tpid = "0" + for fv in fvs: + if fv[0] == "tpid": + tpid = fv[1] + + assert tpid == "0x9200" + # Check ASIC DB # get TPID and validate it to be 0x9200 (37376) atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_LAG") From 88eea45bfaecb72c91969585c625de5b030caf65 Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Mon, 31 May 2021 01:01:20 +0000 Subject: [PATCH 6/8] remove unnecessary check --- tests/test_portchannel.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/test_portchannel.py b/tests/test_portchannel.py index e38ff22954..68b68d49eb 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -158,21 +158,11 @@ def test_Portchannel_tpid(self, dvs, testlog): # create port channel with tpid 0x9200 tbl = swsscommon.Table(cdb, "PORTCHANNEL") - fvs = swsscommon.FieldValuePairs([("admin_status", "up"), ("mtu", "9100"), ("tpid", "0x9200"), ("oper_status", "up")]) + fvs = swsscommon.FieldValuePairs([("admin_status", "up"),("mtu", "9100"),("tpid", "0x9200")]) + tbl.set("PortChannel0002", fvs) time.sleep(1) - # check application database for the correct TPID - pdb_port_tbl = swsscommon.Table(pdb, "LAG_TABLE") - (status, fvs) = pdb_port_tbl.get("PortChannel0002") - assert status == True - tpid = "0" - for fv in fvs: - if fv[0] == "tpid": - tpid = fv[1] - - assert tpid == "0x9200" - # Check ASIC DB # get TPID and validate it to be 0x9200 (37376) atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_LAG") From 090a2b703dde3a2a2640df10acf1cdf2ccc87e56 Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Tue, 1 Jun 2021 00:47:01 +0000 Subject: [PATCH 7/8] Re-enable Portchannel TPID testing after merged latest from master in attempt to address other non-TPID related test failures --- tests/test_portchannel.py | 68 +++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/tests/test_portchannel.py b/tests/test_portchannel.py index 68b68d49eb..e6a89f15d7 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -151,38 +151,6 @@ def test_Portchannel_lacpkey(self, dvs, testlog): tbl._del(portchannel[0]) 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 port channel with tpid 0x9200 - tbl = swsscommon.Table(cdb, "PORTCHANNEL") - fvs = swsscommon.FieldValuePairs([("admin_status", "up"),("mtu", "9100"),("tpid", "0x9200")]) - - tbl.set("PortChannel0002", 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) - - def test_Portchannel_oper_down(self, dvs, testlog): self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) @@ -361,6 +329,42 @@ 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) + + # 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 From 67b0f871ae4fb450f2ceda6d32dbdbeeae7aa818 Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Tue, 1 Jun 2021 05:57:41 +0000 Subject: [PATCH 8/8] Fix missing Producer DB that accidentally deleted on previous merge --- tests/test_portchannel.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_portchannel.py b/tests/test_portchannel.py index e6a89f15d7..7c520706c9 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -332,6 +332,7 @@ def test_Portchannel_oper_down(self, dvs, testlog): 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")