Skip to content

Commit

Permalink
Enable PFCWD only on ports where PFC is enabled (sonic-net#1508)
Browse files Browse the repository at this point in the history 
- 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)
  • Loading branch information
@ayurkiv-nvda
ayurkiv-nvda authored Mar 24, 2021
1 parent eb7945f commit 99d251f
Show file tree
Hide file tree
Showing 6 changed files with 169 additions and 22 deletions.
39 changes: 23 additions & 16 deletions pfcwd/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
from tabulate import tabulate
from utilities_common import multi_asic as multi_asic_util
from utilities_common import constants
from sonic_py_common import logger

SYSLOG_IDENTIFIER = "config"

log = logger.Logger(SYSLOG_IDENTIFIER)

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

CONFIG_DB_PFC_WD_TABLE_NAME = 'PFC_WD'

PORT_QOS_MAP = "PORT_QOS_MAP"

# Main entrypoint
@click.group()
Expand Down Expand Up @@ -242,6 +247,20 @@ def start(self, action, restoration_time, ports, detection_time):
exit()
self.start_cmd(action, restoration_time, ports, detection_time)


def verify_pfc_enable_status_per_port(self, port, pfcwd_info):
pfc_status = self.config_db.get_entry(PORT_QOS_MAP, port).get('pfc_enable')
if pfc_status is None:
log.log_warning("SKIPPED: PFC is not enabled on port: {}".format(port), also_print_to_console=True)
return

self.config_db.mod_entry(
CONFIG_DB_PFC_WD_TABLE_NAME, port, None
)
self.config_db.mod_entry(
CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
)

@multi_asic_util.run_on_multi_asic
def start_cmd(self, action, restoration_time, ports, detection_time):
if os.geteuid() != 0:
Expand Down Expand Up @@ -272,21 +291,11 @@ def start_cmd(self, action, restoration_time, ports, detection_time):
for port in ports:
if port == "all":
for p in all_ports:
self.config_db.mod_entry(
CONFIG_DB_PFC_WD_TABLE_NAME, p, None
)
self.config_db.mod_entry(
CONFIG_DB_PFC_WD_TABLE_NAME, p, pfcwd_info
)
self.verify_pfc_enable_status_per_port(p, pfcwd_info)
else:
if port not in all_ports:
continue
self.config_db.mod_entry(
CONFIG_DB_PFC_WD_TABLE_NAME, port, None
)
self.config_db.mod_entry(
CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
)
self.verify_pfc_enable_status_per_port(port, pfcwd_info)

@multi_asic_util.run_on_multi_asic
def interval(self, poll_interval):
Expand Down Expand Up @@ -375,9 +384,7 @@ def start_default(self):
}

for port in active_ports:
self.config_db.set_entry(
CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
)
self.verify_pfc_enable_status_per_port(port, pfcwd_info)

pfcwd_info = {}
pfcwd_info['POLL_INTERVAL'] = DEFAULT_POLL_INTERVAL * multiply
Expand Down
21 changes: 21 additions & 0 deletions tests/mock_tables/asic0/config_db.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,27 @@
"BIG_RED_SWITCH": "enable",
"POLL_INTERVAL": "199"
},
"PORT_QOS_MAP|Ethernet0": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet4": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet8": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet-BP0": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet-BP4": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet-BP256": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet-BP260": {
"pfc_enable": "3,4"
},
"CRM|Config": {
"acl_table_threshold_type": "percentage",
"nexthop_group_threshold_type": "percentage",
Expand Down
20 changes: 20 additions & 0 deletions tests/mock_tables/asic1/config_db.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,26 @@
"BIG_RED_SWITCH": "enable",
"POLL_INTERVAL": "199"
},
"PORT_QOS_MAP|Ethernet0": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet4": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet8": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet-BP0": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet-BP4": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet-BP256": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet-BP260": {
},
"CRM|Config": {
"acl_table_threshold_type": "percentage",
"nexthop_group_threshold_type": "percentage",
Expand Down
9 changes: 9 additions & 0 deletions tests/mock_tables/config_db.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,10 @@
"peer_switch": "sonic-switch",
"type": "ToRRouter"
},
"DEVICE_NEIGHBOR|Ethernet0": {
"name": "Servers",
"port": "eth0"
},
"DEVICE_NEIGHBOR|Ethernet4": {
"name": "Servers0",
"port": "eth0"
Expand Down Expand Up @@ -1450,6 +1454,11 @@
"PORT_QOS_MAP|Ethernet0": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet4": {
"pfc_enable": "3,4"
},
"PORT_QOS_MAP|Ethernet8": {
},
"DEFAULT_LOSSLESS_BUFFER_PARAMETER|AZURE": {
"default_dynamic_th": "0",
"over_subscribe_ratio": "2"
Expand Down
24 changes: 18 additions & 6 deletions tests/pfcwd_input/pfcwd_test_vectors.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
--------- -------- ---------------- ------------------
Ethernet0 forward 302 301
Ethernet4 forward 302 301
Ethernet8 forward 302 301
Ethernet8 drop 600 600
"""

pfcwd_show_start_action_alert_output = """\
Expand All @@ -31,7 +31,7 @@
--------- -------- ---------------- ------------------
Ethernet0 alert 502 501
Ethernet4 alert 502 501
Ethernet8 alert 502 501
Ethernet8 drop 600 600
"""

pfcwd_show_start_action_drop_output = """\
Expand All @@ -40,7 +40,16 @@
--------- -------- ---------------- ------------------
Ethernet0 drop 602 601
Ethernet4 drop 602 601
Ethernet8 drop 602 601
Ethernet8 drop 600 600
"""

pfcwd_show_start_default = """\
Changed polling interval to 200ms
PORT ACTION DETECTION TIME RESTORATION TIME
--------- -------- ---------------- ------------------
Ethernet0 drop 200 200
Ethernet4 drop 200 200
Ethernet8 drop 600 600
"""

pfcwd_show_start_config_output_fail = """\
Expand Down Expand Up @@ -94,6 +103,9 @@
------- -------- ------------------------- ------------ ------------ ----------------- -----------------
"""

pfc_is_not_enabled = "SKIPPED: PFC is not enabled on port: Ethernet8\n"
pfc_is_not_enabled_masic = "SKIPPED: PFC is not enabled on port: Ethernet-BP260\n"

testData = {
'pfcwd_show_config' : [ {'cmd' : ['show', 'config'],
'args': [],
Expand Down Expand Up @@ -290,7 +302,7 @@
Ethernet-BP0 drop 302 301
Ethernet-BP4 drop 302 301
Ethernet-BP256 drop 302 301
Ethernet-BP260 drop 302 301
Ethernet-BP260 drop 200 200
"""

show_pfc_config_start_action_alert_masic = """\
Expand All @@ -305,7 +317,7 @@
Ethernet-BP0 alert 402 401
Ethernet-BP4 alert 402 401
Ethernet-BP256 alert 402 401
Ethernet-BP260 alert 402 401
Ethernet-BP260 drop 200 200
"""

show_pfc_config_start_action_forward_masic = """\
Expand All @@ -320,7 +332,7 @@
Ethernet-BP0 forward 702 701
Ethernet-BP4 forward 702 701
Ethernet-BP256 forward 702 701
Ethernet-BP260 forward 702 701
Ethernet-BP260 drop 200 200
"""

show_pfc_config_start_fail = """\
Expand Down
78 changes: 78 additions & 0 deletions tests/pfcwd_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ def test_pfcwd_start_actions(self, mock_os):
print(result.output)
assert result.output == pfcwd_show_config_output

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

result = runner.invoke(
pfcwd.cli.commands["start_default"],
[],
obj=db
)

assert result.exit_code == 0

result = runner.invoke(
pfcwd.cli.commands["show"].commands["config"],
obj=db
)

print(result.output)
assert result.exit_code == 0
assert result.output == pfcwd_show_start_default


@patch('pfcwd.main.os')
def test_pfcwd_pfc_not_enabled(self, mock_os):
import pfcwd.main as pfcwd
runner = CliRunner()
db = Db()

# get initial config
result = runner.invoke(
pfcwd.cli.commands["show"].commands["config"],
obj=db
)
print(result.output)
assert result.output == pfcwd_show_config_output

mock_os.geteuid.return_value = 0

result = runner.invoke(
pfcwd.cli.commands["start"],
[
"--action", "drop", "--restoration-time", "601",
"Ethernet8", "602"
],
obj=db
)
print(result.output)
assert result.exit_code == 0
assert pfc_is_not_enabled == result.output


def test_pfcwd_start_ports_invalid(self):
# pfcwd start --action drop --restoration-time 200 Ethernet0 200
import pfcwd.main as pfcwd
Expand Down Expand Up @@ -322,6 +370,7 @@ def test_pfcwd_start_actions_masic(self, mock_os):
print(result.output)
assert result.output == show_pfc_config_all

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

@patch('pfcwd.main.os')
def test_pfcwd_pfc_not_enabled_masic(self, mock_os):
import pfcwd.main as pfcwd
runner = CliRunner()
db = Db()

mock_os.geteuid.return_value = 0
result = runner.invoke(
pfcwd.cli.commands["start"],
[
"--action", "drop", "--restoration-time", "601",
"Ethernet-BP260", "602"
],
obj=db
)

assert result.exit_code == 0
assert pfc_is_not_enabled_masic == result.output

result = runner.invoke(
pfcwd.cli.commands["show"].commands["config"],
obj=db
)

print(result.output)
assert result.exit_code == 0
# same as original config
assert result.output == show_pfc_config_all

@classmethod
def teardown_class(cls):
print("TEARDOWN")
Expand Down

0 comments on commit 99d251f

Please sign in to comment.