Skip to content

Commit 99d251f

Browse files
ayurkiv-nvdaayurkiv-nvda
authored
Enable PFCWD only on ports where PFC is enabled (mudsut4ke#1508)
- What I did Prevent errors in the log when PFCWD starts on ports that PFC was not previously enabled. Example of such errors in the log: Nov 13 17:27:22.878468 arc-switch1038 ERR swss#orchagent: :- createEntry: Failed to start PFC Watchdog on port Ethernet100 Nov 13 17:27:23.878620 arc-switch1038 NOTICE swss#orchagent: :- registerInWdDb: No lossless TC found on port Ethernet100 Add unit test to cover the new flow. - How I did it Before enabling PFCWD on a port, verify if PFC was previously enabled. If not, reply with an error to the user and as a warning in the log. In the case where the command to enable PFCWD is done for all ports, the configuration will be applied for only those that has PCF enabled and for the rest the configuration will be skipped. In this case the output of the requested command will be only with a list of skipped ports. - How to verify it 'pfcwd start EthernetX 600' where EthernetX is a port that has not configured PFC (usually admin state 'down' by default)
1 parent eb7945f commit 99d251f

6 files changed

Lines changed: 169 additions & 22 deletions

File tree

‎pfcwd/main.py‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111
from tabulate import tabulate
1212
from utilities_common import multi_asic as multi_asic_util
1313
from utilities_common import constants
14+
from sonic_py_common import logger
15+
16+
SYSLOG_IDENTIFIER = "config"
17+
18+
log = logger.Logger(SYSLOG_IDENTIFIER)
1419

1520
# mock the redis for unit test purposes #
1621
try:
@@ -53,7 +58,7 @@
5358
CONFIG_HEADER = ('PORT',) + list(zip(*CONFIG_DESCRIPTION))[0]
5459

5560
CONFIG_DB_PFC_WD_TABLE_NAME = 'PFC_WD'
56-
61+
PORT_QOS_MAP = "PORT_QOS_MAP"
5762

5863
# Main entrypoint
5964
@click.group()
@@ -242,6 +247,20 @@ def start(self, action, restoration_time, ports, detection_time):
242247
exit()
243248
self.start_cmd(action, restoration_time, ports, detection_time)
244249

250+
251+
def verify_pfc_enable_status_per_port(self, port, pfcwd_info):
252+
pfc_status = self.config_db.get_entry(PORT_QOS_MAP, port).get('pfc_enable')
253+
if pfc_status is None:
254+
log.log_warning("SKIPPED: PFC is not enabled on port: {}".format(port), also_print_to_console=True)
255+
return
256+
257+
self.config_db.mod_entry(
258+
CONFIG_DB_PFC_WD_TABLE_NAME, port, None
259+
)
260+
self.config_db.mod_entry(
261+
CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
262+
)
263+
245264
@multi_asic_util.run_on_multi_asic
246265
def start_cmd(self, action, restoration_time, ports, detection_time):
247266
if os.geteuid() != 0:
@@ -272,21 +291,11 @@ def start_cmd(self, action, restoration_time, ports, detection_time):
272291
for port in ports:
273292
if port == "all":
274293
for p in all_ports:
275-
self.config_db.mod_entry(
276-
CONFIG_DB_PFC_WD_TABLE_NAME, p, None
277-
)
278-
self.config_db.mod_entry(
279-
CONFIG_DB_PFC_WD_TABLE_NAME, p, pfcwd_info
280-
)
294+
self.verify_pfc_enable_status_per_port(p, pfcwd_info)
281295
else:
282296
if port not in all_ports:
283297
continue
284-
self.config_db.mod_entry(
285-
CONFIG_DB_PFC_WD_TABLE_NAME, port, None
286-
)
287-
self.config_db.mod_entry(
288-
CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
289-
)
298+
self.verify_pfc_enable_status_per_port(port, pfcwd_info)
290299

291300
@multi_asic_util.run_on_multi_asic
292301
def interval(self, poll_interval):
@@ -375,9 +384,7 @@ def start_default(self):
375384
}
376385

377386
for port in active_ports:
378-
self.config_db.set_entry(
379-
CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
380-
)
387+
self.verify_pfc_enable_status_per_port(port, pfcwd_info)
381388

382389
pfcwd_info = {}
383390
pfcwd_info['POLL_INTERVAL'] = DEFAULT_POLL_INTERVAL * multiply

‎tests/mock_tables/asic0/config_db.json‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,27 @@
108108
"BIG_RED_SWITCH": "enable",
109109
"POLL_INTERVAL": "199"
110110
},
111+
"PORT_QOS_MAP|Ethernet0": {
112+
"pfc_enable": "3,4"
113+
},
114+
"PORT_QOS_MAP|Ethernet4": {
115+
"pfc_enable": "3,4"
116+
},
117+
"PORT_QOS_MAP|Ethernet8": {
118+
"pfc_enable": "3,4"
119+
},
120+
"PORT_QOS_MAP|Ethernet-BP0": {
121+
"pfc_enable": "3,4"
122+
},
123+
"PORT_QOS_MAP|Ethernet-BP4": {
124+
"pfc_enable": "3,4"
125+
},
126+
"PORT_QOS_MAP|Ethernet-BP256": {
127+
"pfc_enable": "3,4"
128+
},
129+
"PORT_QOS_MAP|Ethernet-BP260": {
130+
"pfc_enable": "3,4"
131+
},
111132
"CRM|Config": {
112133
"acl_table_threshold_type": "percentage",
113134
"nexthop_group_threshold_type": "percentage",

‎tests/mock_tables/asic1/config_db.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,26 @@
7777
"BIG_RED_SWITCH": "enable",
7878
"POLL_INTERVAL": "199"
7979
},
80+
"PORT_QOS_MAP|Ethernet0": {
81+
"pfc_enable": "3,4"
82+
},
83+
"PORT_QOS_MAP|Ethernet4": {
84+
"pfc_enable": "3,4"
85+
},
86+
"PORT_QOS_MAP|Ethernet8": {
87+
"pfc_enable": "3,4"
88+
},
89+
"PORT_QOS_MAP|Ethernet-BP0": {
90+
"pfc_enable": "3,4"
91+
},
92+
"PORT_QOS_MAP|Ethernet-BP4": {
93+
"pfc_enable": "3,4"
94+
},
95+
"PORT_QOS_MAP|Ethernet-BP256": {
96+
"pfc_enable": "3,4"
97+
},
98+
"PORT_QOS_MAP|Ethernet-BP260": {
99+
},
80100
"CRM|Config": {
81101
"acl_table_threshold_type": "percentage",
82102
"nexthop_group_threshold_type": "percentage",

‎tests/mock_tables/config_db.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,10 @@
683683
"peer_switch": "sonic-switch",
684684
"type": "ToRRouter"
685685
},
686+
"DEVICE_NEIGHBOR|Ethernet0": {
687+
"name": "Servers",
688+
"port": "eth0"
689+
},
686690
"DEVICE_NEIGHBOR|Ethernet4": {
687691
"name": "Servers0",
688692
"port": "eth0"
@@ -1450,6 +1454,11 @@
14501454
"PORT_QOS_MAP|Ethernet0": {
14511455
"pfc_enable": "3,4"
14521456
},
1457+
"PORT_QOS_MAP|Ethernet4": {
1458+
"pfc_enable": "3,4"
1459+
},
1460+
"PORT_QOS_MAP|Ethernet8": {
1461+
},
14531462
"DEFAULT_LOSSLESS_BUFFER_PARAMETER|AZURE": {
14541463
"default_dynamic_th": "0",
14551464
"over_subscribe_ratio": "2"

‎tests/pfcwd_input/pfcwd_test_vectors.py‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
--------- -------- ---------------- ------------------
2323
Ethernet0 forward 302 301
2424
Ethernet4 forward 302 301
25-
Ethernet8 forward 302 301
25+
Ethernet8 drop 600 600
2626
"""
2727

2828
pfcwd_show_start_action_alert_output = """\
@@ -31,7 +31,7 @@
3131
--------- -------- ---------------- ------------------
3232
Ethernet0 alert 502 501
3333
Ethernet4 alert 502 501
34-
Ethernet8 alert 502 501
34+
Ethernet8 drop 600 600
3535
"""
3636

3737
pfcwd_show_start_action_drop_output = """\
@@ -40,7 +40,16 @@
4040
--------- -------- ---------------- ------------------
4141
Ethernet0 drop 602 601
4242
Ethernet4 drop 602 601
43-
Ethernet8 drop 602 601
43+
Ethernet8 drop 600 600
44+
"""
45+
46+
pfcwd_show_start_default = """\
47+
Changed polling interval to 200ms
48+
PORT ACTION DETECTION TIME RESTORATION TIME
49+
--------- -------- ---------------- ------------------
50+
Ethernet0 drop 200 200
51+
Ethernet4 drop 200 200
52+
Ethernet8 drop 600 600
4453
"""
4554

4655
pfcwd_show_start_config_output_fail = """\
@@ -94,6 +103,9 @@
94103
------- -------- ------------------------- ------------ ------------ ----------------- -----------------
95104
"""
96105

106+
pfc_is_not_enabled = "SKIPPED: PFC is not enabled on port: Ethernet8\n"
107+
pfc_is_not_enabled_masic = "SKIPPED: PFC is not enabled on port: Ethernet-BP260\n"
108+
97109
testData = {
98110
'pfcwd_show_config' : [ {'cmd' : ['show', 'config'],
99111
'args': [],
@@ -290,7 +302,7 @@
290302
Ethernet-BP0 drop 302 301
291303
Ethernet-BP4 drop 302 301
292304
Ethernet-BP256 drop 302 301
293-
Ethernet-BP260 drop 302 301
305+
Ethernet-BP260 drop 200 200
294306
"""
295307

296308
show_pfc_config_start_action_alert_masic = """\
@@ -305,7 +317,7 @@
305317
Ethernet-BP0 alert 402 401
306318
Ethernet-BP4 alert 402 401
307319
Ethernet-BP256 alert 402 401
308-
Ethernet-BP260 alert 402 401
320+
Ethernet-BP260 drop 200 200
309321
"""
310322

311323
show_pfc_config_start_action_forward_masic = """\
@@ -320,7 +332,7 @@
320332
Ethernet-BP0 forward 702 701
321333
Ethernet-BP4 forward 702 701
322334
Ethernet-BP256 forward 702 701
323-
Ethernet-BP260 forward 702 701
335+
Ethernet-BP260 drop 200 200
324336
"""
325337

326338
show_pfc_config_start_fail = """\

‎tests/pfcwd_test.py‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ def test_pfcwd_start_actions(self, mock_os):
131131
print(result.output)
132132
assert result.output == pfcwd_show_config_output
133133

134+
# always skip Ethernet8 because 'pfc_enable' not configured for this port
134135
mock_os.geteuid.return_value = 0
135136
result = runner.invoke(
136137
pfcwd.cli.commands["start"],
@@ -192,6 +193,53 @@ def test_pfcwd_start_actions(self, mock_os):
192193
assert result.exit_code == 0
193194
assert result.output == pfcwd_show_start_action_drop_output
194195

196+
result = runner.invoke(
197+
pfcwd.cli.commands["start_default"],
198+
[],
199+
obj=db
200+
)
201+
202+
assert result.exit_code == 0
203+
204+
result = runner.invoke(
205+
pfcwd.cli.commands["show"].commands["config"],
206+
obj=db
207+
)
208+
209+
print(result.output)
210+
assert result.exit_code == 0
211+
assert result.output == pfcwd_show_start_default
212+
213+
214+
@patch('pfcwd.main.os')
215+
def test_pfcwd_pfc_not_enabled(self, mock_os):
216+
import pfcwd.main as pfcwd
217+
runner = CliRunner()
218+
db = Db()
219+
220+
# get initial config
221+
result = runner.invoke(
222+
pfcwd.cli.commands["show"].commands["config"],
223+
obj=db
224+
)
225+
print(result.output)
226+
assert result.output == pfcwd_show_config_output
227+
228+
mock_os.geteuid.return_value = 0
229+
230+
result = runner.invoke(
231+
pfcwd.cli.commands["start"],
232+
[
233+
"--action", "drop", "--restoration-time", "601",
234+
"Ethernet8", "602"
235+
],
236+
obj=db
237+
)
238+
print(result.output)
239+
assert result.exit_code == 0
240+
assert pfc_is_not_enabled == result.output
241+
242+
195243
def test_pfcwd_start_ports_invalid(self):
196244
# pfcwd start --action drop --restoration-time 200 Ethernet0 200
197245
import pfcwd.main as pfcwd
@@ -322,6 +370,7 @@ def test_pfcwd_start_actions_masic(self, mock_os):
322370
print(result.output)
323371
assert result.output == show_pfc_config_all
324372

373+
# always skip Ethernet-BP260 because 'pfc_enable' not configured for this port
325374
mock_os.geteuid.return_value = 0
326375
result = runner.invoke(
327376
pfcwd.cli.commands["start"],
@@ -411,6 +460,35 @@ def test_pfcwd_start_ports_masic_invalid(self):
411460
# same as original config
412461
assert result.output == show_pfc_config_all
413462

463+
@patch('pfcwd.main.os')
464+
def test_pfcwd_pfc_not_enabled_masic(self, mock_os):
465+
import pfcwd.main as pfcwd
466+
runner = CliRunner()
467+
db = Db()
468+
469+
mock_os.geteuid.return_value = 0
470+
result = runner.invoke(
471+
pfcwd.cli.commands["start"],
472+
[
473+
"--action", "drop", "--restoration-time", "601",
474+
"Ethernet-BP260", "602"
475+
],
476+
obj=db
477+
)
478+
479+
assert result.exit_code == 0
480+
assert pfc_is_not_enabled_masic == result.output
481+
482+
result = runner.invoke(
483+
pfcwd.cli.commands["show"].commands["config"],
484+
obj=db
485+
)
486+
487+
print(result.output)
488+
assert result.exit_code == 0
489+
# same as original config
490+
assert result.output == show_pfc_config_all
491+
414492
@classmethod
415493
def teardown_class(cls):
416494
print("TEARDOWN")

0 commit comments

Comments
 (0)