Skip to content

Commit 2c83b68

Browse files
qiluo-msftlguohan
qiluo-msft
authored and
lguohan
committed
Warm reboot: bring up ports for whole system warm reboot (sonic-net#653)
* Remove isWarmStart check Signed-off-by: Qi Luo <[email protected]> * No bring down in warm start Signed-off-by: Qi Luo <[email protected]> * Protect doPortTask to apply repeated SET once Signed-off-by: Qi Luo <[email protected]> * Refine setPortMtu Signed-off-by: Qi Luo <[email protected]> * (comment) Signed-off-by: Qi Luo <[email protected]> * Fix warm reboot: port state ok, port admin up, port autoneg ignore Signed-off-by: Qi Luo <[email protected]>
1 parent d049d65 commit 2c83b68

File tree

4 files changed

+183
-65
lines changed

4 files changed

+183
-65
lines changed

‎orchagent/portsorch.cpp

+37-13
Original file line numberDiff line numberDiff line change
@@ -595,16 +595,21 @@ bool PortsOrch::setPortPfcAsym(Port &port, string pfc_asym)
595595
return false;
596596
}
597597

598-
try
599-
{
600-
port.m_pfc_asym = pfc_asym_map.at(pfc_asym);
601-
}
602-
catch (...)
598+
auto found = pfc_asym_map.find(pfc_asym);
599+
if (found == pfc_asym_map.end())
603600
{
604601
SWSS_LOG_ERROR("Incorrect asymmetric PFC mode: %s", pfc_asym.c_str());
605602
return false;
606603
}
607604

605+
auto new_pfc_asym = found->second;
606+
if (port.m_pfc_asym == new_pfc_asym)
607+
{
608+
SWSS_LOG_NOTICE("Already set asymmetric PFC mode: %s", pfc_asym.c_str());
609+
return true;
610+
}
611+
612+
port.m_pfc_asym = new_pfc_asym;
608613
m_portList[port.m_alias] = port;
609614

610615
attr.id = SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL_MODE;
@@ -1373,6 +1378,13 @@ void PortsOrch::doPortTask(Consumer &consumer)
13731378

13741379
if (alias == "PortConfigDone")
13751380
{
1381+
if (m_portConfigDone)
1382+
{
1383+
// Already done, ignore this task
1384+
it = consumer.m_toSync.erase(it);
1385+
continue;
1386+
}
1387+
13761388
m_portConfigDone = true;
13771389

13781390
for (auto i : kfvFieldsValues(t))
@@ -1585,11 +1597,20 @@ void PortsOrch::doPortTask(Consumer &consumer)
15851597
p.m_autoneg = an;
15861598
m_portList[alias] = p;
15871599

1588-
/* Once AN is changed, need to reset the port speed or
1589-
port adv speed accordingly */
1590-
if (speed == 0 && p.m_speed != 0)
1600+
// Once AN is changed
1601+
// - no speed specified: need to reapply the port speed or port adv speed accordingly
1602+
// - speed specified: need to apply the port speed or port adv speed by the specified one
1603+
// Note: one special case is
1604+
// - speed specified as existing m_speed: need to apply even they are the same
1605+
auto old_speed = p.m_speed;
1606+
p.m_speed = 0;
1607+
auto new_speed = speed ? speed : old_speed;
1608+
if (new_speed)
15911609
{
1592-
speed = p.m_speed;
1610+
// Modify the task in place
1611+
kfvFieldsValues(t).emplace_back("speed", to_string(new_speed));
1612+
// Fallthrough to process `speed'
1613+
speed = new_speed;
15931614
}
15941615
}
15951616
else
@@ -1608,9 +1629,8 @@ void PortsOrch::doPortTask(Consumer &consumer)
16081629
* 3. Set port admin status to DOWN before changing the speed
16091630
* 4. Set port speed
16101631
*/
1611-
if (speed != 0)
1632+
if (speed != 0 && speed != p.m_speed)
16121633
{
1613-
p.m_speed = speed;
16141634
m_portList[alias] = p;
16151635

16161636
if (p.m_autoneg)
@@ -1656,17 +1676,22 @@ void PortsOrch::doPortTask(Consumer &consumer)
16561676
else
16571677
{
16581678
SWSS_LOG_ERROR("Failed to set port admin status DOWN to set speed");
1679+
it++;
1680+
continue;
16591681
}
16601682
}
16611683
}
16621684
else
16631685
{
16641686
SWSS_LOG_ERROR("Failed to get current speed for port %s", alias.c_str());
1687+
it++;
1688+
continue;
16651689
}
16661690
}
1691+
m_portList[alias].m_speed = speed;
16671692
}
16681693

1669-
if (mtu != 0)
1694+
if (mtu != 0 && mtu != p.m_mtu)
16701695
{
16711696
if (setPortMtu(p.m_port_id, mtu))
16721697
{
@@ -1700,7 +1725,6 @@ void PortsOrch::doPortTask(Consumer &consumer)
17001725
}
17011726
}
17021727

1703-
17041728
if (!fec_mode.empty())
17051729
{
17061730
if (fec_mode_map.find(fec_mode) != fec_mode_map.end())

‎portsyncd/portsyncd.cpp

+56-52
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ void usage()
4040
cout << " use configDB data if not specified" << endl;
4141
}
4242

43-
void handlePortConfigFile(ProducerStateTable &p, string file);
44-
void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb);
43+
void handlePortConfigFile(ProducerStateTable &p, string file, bool warm);
44+
void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm);
4545
void handleVlanIntfFile(string file);
4646
void handlePortConfig(ProducerStateTable &p, map<string, KeyOpFieldsValuesTuple> &port_cfg_map);
4747
void checkPortInitDone(DBConnector *appl_db);
@@ -77,13 +77,7 @@ int main(int argc, char **argv)
7777

7878
WarmStart::initialize("portsyncd");
7979
WarmStart::checkWarmStart("portsyncd");
80-
if (WarmStart::isWarmStart())
81-
{
82-
/* clear the init port config buffer */
83-
cout << "portsyncd warm start" << endl;
84-
deque<KeyOpFieldsValuesTuple> vkco;
85-
portCfg.pops(vkco);
86-
}
80+
const bool warm = WarmStart::isWarmStart();
8781

8882
LinkSync sync(&appl_db, &state_db);
8983
NetDispatcher::getInstance().registerMessageHandler(RTM_NEWLINK, &sync);
@@ -95,17 +89,14 @@ int main(int argc, char **argv)
9589
Select s;
9690

9791
netlink.registerGroup(RTNLGRP_LINK);
92+
netlink.dumpRequest(RTM_GETLINK);
9893
cout << "Listen to link messages..." << endl;
9994

100-
/* For portsyncd warm start, don't process init port config again */
101-
if (!WarmStart::isWarmStart())
95+
if (!port_config_file.empty())
10296
{
103-
if (!port_config_file.empty())
104-
{
105-
handlePortConfigFile(p, port_config_file);
106-
} else {
107-
handlePortConfigFromConfigDB(p, cfgDb);
108-
}
97+
handlePortConfigFile(p, port_config_file, warm);
98+
} else {
99+
handlePortConfigFromConfigDB(p, cfgDb, warm);
109100
}
110101

111102
s.addSelectable(&netlink);
@@ -137,6 +128,7 @@ int main(int argc, char **argv)
137128
FieldValueTuple finish_notice("lanes", "0");
138129
vector<FieldValueTuple> attrs = { finish_notice };
139130
p.set("PortInitDone", attrs);
131+
SWSS_LOG_NOTICE("PortInitDone");
140132

141133
g_init = true;
142134
}
@@ -183,7 +175,7 @@ static void notifyPortConfigDone(ProducerStateTable &p)
183175
p.set("PortConfigDone", attrs);
184176
}
185177

186-
void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb)
178+
void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm)
187179
{
188180
cout << "Get port configuration from ConfigDB..." << endl;
189181

@@ -200,13 +192,19 @@ void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb)
200192
FieldValueTuple attr(v.first, v.second);
201193
attrs.push_back(attr);
202194
}
203-
p.set(k, attrs);
195+
if (!warm)
196+
{
197+
p.set(k, attrs);
198+
}
204199
g_portSet.insert(k);
205200
}
206-
notifyPortConfigDone(p);
201+
if (!warm)
202+
{
203+
notifyPortConfigDone(p);
204+
}
207205
}
208206

209-
void handlePortConfigFile(ProducerStateTable &p, string file)
207+
void handlePortConfigFile(ProducerStateTable &p, string file, bool warm)
210208
{
211209
cout << "Read port configuration file..." << endl;
212210

@@ -246,49 +244,55 @@ void handlePortConfigFile(ProducerStateTable &p, string file)
246244
iss >> entry[column];
247245
}
248246

249-
/* If port has no alias, then use its name as alias */
250-
string alias;
251-
if ((entry.find("alias") != entry.end()) && (entry["alias"] != ""))
247+
if (!warm)
252248
{
253-
alias = entry["alias"];
254-
}
255-
else
256-
{
257-
alias = entry["name"];
258-
}
249+
/* If port has no alias, then use its name as alias */
250+
string alias;
251+
if ((entry.find("alias") != entry.end()) && (entry["alias"] != ""))
252+
{
253+
alias = entry["alias"];
254+
}
255+
else
256+
{
257+
alias = entry["name"];
258+
}
259259

260-
FieldValueTuple lanes_attr("lanes", entry["lanes"]);
261-
FieldValueTuple alias_attr("alias", alias);
260+
FieldValueTuple lanes_attr("lanes", entry["lanes"]);
261+
FieldValueTuple alias_attr("alias", alias);
262262

263-
vector<FieldValueTuple> attrs;
264-
attrs.push_back(lanes_attr);
265-
attrs.push_back(alias_attr);
263+
vector<FieldValueTuple> attrs;
264+
attrs.push_back(lanes_attr);
265+
attrs.push_back(alias_attr);
266266

267-
if ((entry.find("speed") != entry.end()) && (entry["speed"] != ""))
268-
{
269-
FieldValueTuple speed_attr("speed", entry["speed"]);
270-
attrs.push_back(speed_attr);
271-
}
267+
if ((entry.find("speed") != entry.end()) && (entry["speed"] != ""))
268+
{
269+
FieldValueTuple speed_attr("speed", entry["speed"]);
270+
attrs.push_back(speed_attr);
271+
}
272272

273-
if ((entry.find("autoneg") != entry.end()) && (entry["autoneg"] != ""))
274-
{
275-
FieldValueTuple autoneg_attr("autoneg", entry["autoneg"]);
276-
attrs.push_back(autoneg_attr);
277-
}
273+
if ((entry.find("autoneg") != entry.end()) && (entry["autoneg"] != ""))
274+
{
275+
FieldValueTuple autoneg_attr("autoneg", entry["autoneg"]);
276+
attrs.push_back(autoneg_attr);
277+
}
278278

279-
if ((entry.find("fec") != entry.end()) && (entry["fec"] != ""))
280-
{
281-
FieldValueTuple fec_attr("fec", entry["fec"]);
282-
attrs.push_back(fec_attr);
283-
}
279+
if ((entry.find("fec") != entry.end()) && (entry["fec"] != ""))
280+
{
281+
FieldValueTuple fec_attr("fec", entry["fec"]);
282+
attrs.push_back(fec_attr);
283+
}
284284

285-
p.set(entry["name"], attrs);
285+
p.set(entry["name"], attrs);
286+
}
286287

287288
g_portSet.insert(entry["name"]);
288289
}
289290

290291
infile.close();
291-
notifyPortConfigDone(p);
292+
if (!warm)
293+
{
294+
notifyPortConfigDone(p);
295+
}
292296
}
293297

294298
void handlePortConfig(ProducerStateTable &p, map<string, KeyOpFieldsValuesTuple> &port_cfg_map)

‎tests/test_port_an.py renamed to ‎tests/test_port_an_cold.py

File renamed without changes.

‎tests/test_port_an_warm.py

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
from swsscommon import swsscommon
2+
import time
3+
import os
4+
5+
def test_PortAutoNeg_warm(dvs, testlog):
6+
7+
db = swsscommon.DBConnector(0, dvs.redis_sock, 0)
8+
cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0)
9+
sdb = swsscommon.DBConnector(6, dvs.redis_sock, 0)
10+
11+
tbl = swsscommon.ProducerStateTable(db, "PORT_TABLE")
12+
ctbl = swsscommon.Table(cdb, "PORT")
13+
stbl = swsscommon.Table(sdb, "PORT_TABLE")
14+
15+
# set autoneg = false and speed = 1000
16+
fvs = swsscommon.FieldValuePairs([("autoneg","1"), ("speed", "1000")])
17+
tbl.set("Ethernet0", fvs)
18+
19+
time.sleep(1)
20+
21+
adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)
22+
23+
atbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT")
24+
(status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"])
25+
assert status == True
26+
27+
assert "SAI_PORT_ATTR_AUTO_NEG_MODE" in [fv[0] for fv in fvs]
28+
assert "SAI_PORT_ATTR_ADVERTISED_SPEED" in [fv[0] for fv in fvs]
29+
for fv in fvs:
30+
if fv[0] == "SAI_PORT_ATTR_AUTO_NEG_MODE":
31+
assert fv[1] == "true"
32+
elif fv[0] == "SAI_PORT_ATTR_ADVERTISED_SPEED":
33+
assert fv[1] == "1:1000"
34+
35+
# set speed = 100
36+
fvs = swsscommon.FieldValuePairs([("speed", "100")])
37+
38+
tbl.set("Ethernet0", fvs)
39+
40+
time.sleep(1)
41+
42+
(status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"])
43+
assert status == True
44+
45+
for fv in fvs:
46+
if fv[0] == "SAI_PORT_ATTR_AUTO_NEG_MODE":
47+
assert fv[1] == "true"
48+
elif fv[0] == "SAI_PORT_ATTR_ADVERTISED_SPEED":
49+
assert fv[1] == "1:100"
50+
51+
# set admin up
52+
cfvs = swsscommon.FieldValuePairs([("admin_status", "up")])
53+
ctbl.set("Ethernet0", cfvs)
54+
55+
# enable warm restart
56+
(exitcode, result) = dvs.runcmd("config warm_restart enable swss")
57+
assert exitcode == 0
58+
59+
# freeze orchagent for warm restart
60+
(exitcode, result) = dvs.runcmd("/usr/bin/orchagent_restart_check")
61+
assert result == "RESTARTCHECK succeeded\n"
62+
time.sleep(2)
63+
64+
try:
65+
# restart orchagent
66+
# clean port state
67+
dvs.stop_swss()
68+
ports = stbl.getKeys()
69+
for port in ports:
70+
stbl._del(port)
71+
dvs.start_swss()
72+
time.sleep(2)
73+
74+
# check ASIC DB after warm restart
75+
(status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"])
76+
assert status == True
77+
78+
assert "SAI_PORT_ATTR_AUTO_NEG_MODE" in [fv[0] for fv in fvs]
79+
assert "SAI_PORT_ATTR_ADVERTISED_SPEED" in [fv[0] for fv in fvs]
80+
for fv in fvs:
81+
if fv[0] == "SAI_PORT_ATTR_AUTO_NEG_MODE":
82+
assert fv[1] == "true"
83+
elif fv[0] == "SAI_PORT_ATTR_ADVERTISED_SPEED":
84+
assert fv[1] == "1:100"
85+
86+
finally:
87+
# disable warm restart
88+
dvs.runcmd("config warm_restart disable swss")
89+
# slow down crm polling
90+
dvs.runcmd("crm config polling interval 10000")

0 commit comments

Comments
 (0)