diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 06f5c83346e4..abe82b64b757 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -595,16 +595,21 @@ bool PortsOrch::setPortPfcAsym(Port &port, string pfc_asym) return false; } - try - { - port.m_pfc_asym = pfc_asym_map.at(pfc_asym); - } - catch (...) + auto found = pfc_asym_map.find(pfc_asym); + if (found == pfc_asym_map.end()) { SWSS_LOG_ERROR("Incorrect asymmetric PFC mode: %s", pfc_asym.c_str()); return false; } + auto new_pfc_asym = found->second; + if (port.m_pfc_asym == new_pfc_asym) + { + SWSS_LOG_NOTICE("Already set asymmetric PFC mode: %s", pfc_asym.c_str()); + return true; + } + + port.m_pfc_asym = new_pfc_asym; m_portList[port.m_alias] = port; attr.id = SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL_MODE; @@ -1373,6 +1378,13 @@ void PortsOrch::doPortTask(Consumer &consumer) if (alias == "PortConfigDone") { + if (m_portConfigDone) + { + // Already done, ignore this task + it = consumer.m_toSync.erase(it); + continue; + } + m_portConfigDone = true; for (auto i : kfvFieldsValues(t)) @@ -1585,11 +1597,20 @@ void PortsOrch::doPortTask(Consumer &consumer) p.m_autoneg = an; m_portList[alias] = p; - /* Once AN is changed, need to reset the port speed or - port adv speed accordingly */ - if (speed == 0 && p.m_speed != 0) + // Once AN is changed + // - no speed specified: need to reapply the port speed or port adv speed accordingly + // - speed specified: need to apply the port speed or port adv speed by the specified one + // Note: one special case is + // - speed specified as existing m_speed: need to apply even they are the same + auto old_speed = p.m_speed; + p.m_speed = 0; + auto new_speed = speed ? speed : old_speed; + if (new_speed) { - speed = p.m_speed; + // Modify the task in place + kfvFieldsValues(t).emplace_back("speed", to_string(new_speed)); + // Fallthrough to process `speed' + speed = new_speed; } } else @@ -1608,9 +1629,8 @@ void PortsOrch::doPortTask(Consumer &consumer) * 3. Set port admin status to DOWN before changing the speed * 4. Set port speed */ - if (speed != 0) + if (speed != 0 && speed != p.m_speed) { - p.m_speed = speed; m_portList[alias] = p; if (p.m_autoneg) @@ -1656,17 +1676,22 @@ void PortsOrch::doPortTask(Consumer &consumer) else { SWSS_LOG_ERROR("Failed to set port admin status DOWN to set speed"); + it++; + continue; } } } else { SWSS_LOG_ERROR("Failed to get current speed for port %s", alias.c_str()); + it++; + continue; } } + m_portList[alias].m_speed = speed; } - if (mtu != 0) + if (mtu != 0 && mtu != p.m_mtu) { if (setPortMtu(p.m_port_id, mtu)) { @@ -1700,7 +1725,6 @@ void PortsOrch::doPortTask(Consumer &consumer) } } - if (!fec_mode.empty()) { if (fec_mode_map.find(fec_mode) != fec_mode_map.end()) diff --git a/portsyncd/portsyncd.cpp b/portsyncd/portsyncd.cpp index 726fde30fedc..100073b4c890 100644 --- a/portsyncd/portsyncd.cpp +++ b/portsyncd/portsyncd.cpp @@ -40,8 +40,8 @@ void usage() cout << " use configDB data if not specified" << endl; } -void handlePortConfigFile(ProducerStateTable &p, string file); -void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb); +void handlePortConfigFile(ProducerStateTable &p, string file, bool warm); +void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm); void handleVlanIntfFile(string file); void handlePortConfig(ProducerStateTable &p, map &port_cfg_map); void checkPortInitDone(DBConnector *appl_db); @@ -77,13 +77,7 @@ int main(int argc, char **argv) WarmStart::initialize("portsyncd"); WarmStart::checkWarmStart("portsyncd"); - if (WarmStart::isWarmStart()) - { - /* clear the init port config buffer */ - cout << "portsyncd warm start" << endl; - deque vkco; - portCfg.pops(vkco); - } + const bool warm = WarmStart::isWarmStart(); LinkSync sync(&appl_db, &state_db); NetDispatcher::getInstance().registerMessageHandler(RTM_NEWLINK, &sync); @@ -95,17 +89,14 @@ int main(int argc, char **argv) Select s; netlink.registerGroup(RTNLGRP_LINK); + netlink.dumpRequest(RTM_GETLINK); cout << "Listen to link messages..." << endl; - /* For portsyncd warm start, don't process init port config again */ - if (!WarmStart::isWarmStart()) + if (!port_config_file.empty()) { - if (!port_config_file.empty()) - { - handlePortConfigFile(p, port_config_file); - } else { - handlePortConfigFromConfigDB(p, cfgDb); - } + handlePortConfigFile(p, port_config_file, warm); + } else { + handlePortConfigFromConfigDB(p, cfgDb, warm); } s.addSelectable(&netlink); @@ -137,6 +128,7 @@ int main(int argc, char **argv) FieldValueTuple finish_notice("lanes", "0"); vector attrs = { finish_notice }; p.set("PortInitDone", attrs); + SWSS_LOG_NOTICE("PortInitDone"); g_init = true; } @@ -183,7 +175,7 @@ static void notifyPortConfigDone(ProducerStateTable &p) p.set("PortConfigDone", attrs); } -void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb) +void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm) { cout << "Get port configuration from ConfigDB..." << endl; @@ -200,13 +192,19 @@ void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb) FieldValueTuple attr(v.first, v.second); attrs.push_back(attr); } - p.set(k, attrs); + if (!warm) + { + p.set(k, attrs); + } g_portSet.insert(k); } - notifyPortConfigDone(p); + if (!warm) + { + notifyPortConfigDone(p); + } } -void handlePortConfigFile(ProducerStateTable &p, string file) +void handlePortConfigFile(ProducerStateTable &p, string file, bool warm) { cout << "Read port configuration file..." << endl; @@ -246,49 +244,55 @@ void handlePortConfigFile(ProducerStateTable &p, string file) iss >> entry[column]; } - /* If port has no alias, then use its name as alias */ - string alias; - if ((entry.find("alias") != entry.end()) && (entry["alias"] != "")) + if (!warm) { - alias = entry["alias"]; - } - else - { - alias = entry["name"]; - } + /* If port has no alias, then use its name as alias */ + string alias; + if ((entry.find("alias") != entry.end()) && (entry["alias"] != "")) + { + alias = entry["alias"]; + } + else + { + alias = entry["name"]; + } - FieldValueTuple lanes_attr("lanes", entry["lanes"]); - FieldValueTuple alias_attr("alias", alias); + FieldValueTuple lanes_attr("lanes", entry["lanes"]); + FieldValueTuple alias_attr("alias", alias); - vector attrs; - attrs.push_back(lanes_attr); - attrs.push_back(alias_attr); + vector attrs; + attrs.push_back(lanes_attr); + attrs.push_back(alias_attr); - if ((entry.find("speed") != entry.end()) && (entry["speed"] != "")) - { - FieldValueTuple speed_attr("speed", entry["speed"]); - attrs.push_back(speed_attr); - } + if ((entry.find("speed") != entry.end()) && (entry["speed"] != "")) + { + FieldValueTuple speed_attr("speed", entry["speed"]); + attrs.push_back(speed_attr); + } - if ((entry.find("autoneg") != entry.end()) && (entry["autoneg"] != "")) - { - FieldValueTuple autoneg_attr("autoneg", entry["autoneg"]); - attrs.push_back(autoneg_attr); - } + if ((entry.find("autoneg") != entry.end()) && (entry["autoneg"] != "")) + { + FieldValueTuple autoneg_attr("autoneg", entry["autoneg"]); + attrs.push_back(autoneg_attr); + } - if ((entry.find("fec") != entry.end()) && (entry["fec"] != "")) - { - FieldValueTuple fec_attr("fec", entry["fec"]); - attrs.push_back(fec_attr); - } + if ((entry.find("fec") != entry.end()) && (entry["fec"] != "")) + { + FieldValueTuple fec_attr("fec", entry["fec"]); + attrs.push_back(fec_attr); + } - p.set(entry["name"], attrs); + p.set(entry["name"], attrs); + } g_portSet.insert(entry["name"]); } infile.close(); - notifyPortConfigDone(p); + if (!warm) + { + notifyPortConfigDone(p); + } } void handlePortConfig(ProducerStateTable &p, map &port_cfg_map) diff --git a/tests/test_port_an.py b/tests/test_port_an_cold.py similarity index 100% rename from tests/test_port_an.py rename to tests/test_port_an_cold.py diff --git a/tests/test_port_an_warm.py b/tests/test_port_an_warm.py new file mode 100644 index 000000000000..4a768d03a82b --- /dev/null +++ b/tests/test_port_an_warm.py @@ -0,0 +1,90 @@ +from swsscommon import swsscommon +import time +import os + +def test_PortAutoNeg_warm(dvs, testlog): + + db = swsscommon.DBConnector(0, dvs.redis_sock, 0) + cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) + sdb = swsscommon.DBConnector(6, dvs.redis_sock, 0) + + tbl = swsscommon.ProducerStateTable(db, "PORT_TABLE") + ctbl = swsscommon.Table(cdb, "PORT") + stbl = swsscommon.Table(sdb, "PORT_TABLE") + + # set autoneg = false and speed = 1000 + fvs = swsscommon.FieldValuePairs([("autoneg","1"), ("speed", "1000")]) + tbl.set("Ethernet0", fvs) + + time.sleep(1) + + adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) + + atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + assert status == True + + assert "SAI_PORT_ATTR_AUTO_NEG_MODE" in [fv[0] for fv in fvs] + assert "SAI_PORT_ATTR_ADVERTISED_SPEED" in [fv[0] for fv in fvs] + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_AUTO_NEG_MODE": + assert fv[1] == "true" + elif fv[0] == "SAI_PORT_ATTR_ADVERTISED_SPEED": + assert fv[1] == "1:1000" + + # set speed = 100 + fvs = swsscommon.FieldValuePairs([("speed", "100")]) + + tbl.set("Ethernet0", fvs) + + time.sleep(1) + + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + assert status == True + + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_AUTO_NEG_MODE": + assert fv[1] == "true" + elif fv[0] == "SAI_PORT_ATTR_ADVERTISED_SPEED": + assert fv[1] == "1:100" + + # set admin up + cfvs = swsscommon.FieldValuePairs([("admin_status", "up")]) + ctbl.set("Ethernet0", cfvs) + + # enable warm restart + (exitcode, result) = dvs.runcmd("config warm_restart enable swss") + assert exitcode == 0 + + # freeze orchagent for warm restart + (exitcode, result) = dvs.runcmd("/usr/bin/orchagent_restart_check") + assert result == "RESTARTCHECK succeeded\n" + time.sleep(2) + + try: + # restart orchagent + # clean port state + dvs.stop_swss() + ports = stbl.getKeys() + for port in ports: + stbl._del(port) + dvs.start_swss() + time.sleep(2) + + # check ASIC DB after warm restart + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + assert status == True + + assert "SAI_PORT_ATTR_AUTO_NEG_MODE" in [fv[0] for fv in fvs] + assert "SAI_PORT_ATTR_ADVERTISED_SPEED" in [fv[0] for fv in fvs] + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_AUTO_NEG_MODE": + assert fv[1] == "true" + elif fv[0] == "SAI_PORT_ATTR_ADVERTISED_SPEED": + assert fv[1] == "1:100" + + finally: + # disable warm restart + dvs.runcmd("config warm_restart disable swss") + # slow down crm polling + dvs.runcmd("crm config polling interval 10000")