Skip to content

Fix flakiness in pfcwd/test_pfcwd_cli.py #17411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
StormLiangMS merged 2 commits into from
Mar 20, 2025
Merged

Fix flakiness in pfcwd/test_pfcwd_cli.py #17411

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83aa5d3
Fix flakiness in pfcwd/test_pfcwd_cli.py
vivekverma-arista Mar 7, 2025
3e4d9b9
Fix pre-commit errors
vivekverma-arista Mar 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions tests/pfcwd/test_pfcwd_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from tests.common.helpers.pfcwd_helper import send_background_traffic, verify_pfc_storm_in_expected_state, parser_show_pfcwd_stat # noqa E501
from tests.common.utilities import wait_until
from tests.common.cisco_data import is_cisco_device
from tests.common import config_reload

pytestmark = [
pytest.mark.topology("t0", "t1")
Expand Down Expand Up @@ -298,6 +299,43 @@ def send_rx_ingress(self, action, verify):

class TestPfcwdFunc(SetupPfcwdFunc):
""" Test PFC function and supporting methods """
def __shutdown_lag_members(self, duthost, selected_port):
Copy link
Copy Markdown
Contributor

@lipxu lipxu Mar 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this issue. But still not understand the situation clearly.
Do you mean, if the selected interface which was one of lag, and only this interface has the Storm, the drop count would be mismatch?

Copy link
Copy Markdown
Contributor Author

@vivekverma-arista vivekverma-arista Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes drop count will mismatch because some of the traffic won't even make it to the port being stormed, it will successfully egress through other LAG members.

Copy link
Copy Markdown
Contributor

@lipxu lipxu Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your information, I checked one of the failure logs, the drop count from "show interfaces counters" and "show pfcwd stats" seems mismatch. but sure whether it is related to this issue.
Could you please share the test result? Will this fix make the case 100% pass? thanks

show interfaces counters:
      IFACE    STATE    RX_OK     RX_BPS    RX_UTIL    RX_ERR    RX_DRP    RX_OVR    TX_OK      TX_BPS    TX_UTIL    TX_ERR    TX_DRP    TX_OVR
-----------  -------  -------  ---------  ---------  --------  --------  --------  -------  ----------  ---------  --------  --------  --------
 Ethernet88        U       11  14.38 MB/s      0.12%         0         0         0       19    1.24 B/s      0.00%         0     **1,014**         0


show pfcwd stats
       QUEUE    STATUS    STORM DETECTED/RESTORED    TX OK/DROP    RX OK/DROP    TX LAST OK/DROP    RX LAST OK/DROP
------------  --------  -------------------------  ------------  ------------  -----------------  -----------------
Ethernet88:4   stormed                        1/0         0/507           0/0              0/**507**                0/0

Copy link
Copy Markdown
Contributor Author

@vivekverma-arista vivekverma-arista Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated to the issue fixed by the PR. I will share some counters data that will explain the kind of failure this PR fixes.


if self.ports[selected_port]['test_port_type']!='portchannel':
return

config_facts = duthost.config_facts(host=duthost.hostname, source="persistent")['ansible_facts']
portChannels = config_facts['PORTCHANNEL_MEMBER']
portChannel = None
portChannelMembers = None
for intf in portChannels:
if selected_port in portChannels[intf]:
portChannel = intf
portChannelMembers = portChannels[intf]
break

cmd_data = f'.PORTCHANNEL.{portChannel}.min_links = "1"'

for port in portChannelMembers:
if port == selected_port:
continue
cmd_data += f' | .PORT.{port}.admin_status="down"'

cmd = f"""jq '{cmd_data}' /etc/sonic/config_db.json > /tmp/config_db.json"""

duthost.command("cp /etc/sonic/config_db.json /tmp/config_db_backup.json", _uses_shell=True)
duthost.command(cmd, _uses_shell=True)
duthost.command("sudo cp /tmp/config_db.json /etc/sonic/config_db.json", _uses_shell=True)
config_reload(duthost, config_source='config_db', safe_reload=True, check_intf_up_ports=True, wait_for_bgp=True)

def __restore_original_config(self, duthost, selected_port):

if self.ports[selected_port]['test_port_type']!='portchannel':
return

duthost.command("sudo mv /tmp/config_db_backup.json /etc/sonic/config.json", _uses_shell=True)
config_reload(duthost, config_source='config_db', safe_reload=True, check_intf_up_ports=True, wait_for_bgp=True)

def storm_detect_path(self, dut, port, action):
"""
Storm detection action and associated verifications
Expand Down Expand Up @@ -476,6 +514,9 @@ def test_pfcwd_show_stat(self, request, setup_pfc_test, setup_dut_test_params, e

# for idx, port in enumerate(self.ports):
port = list(self.ports.keys())[0]

self.__shutdown_lag_members(duthost, port)

logger.info("--- Testing various Pfcwd actions on {} ---".format(port))
self.setup_test_params(port, setup_info['vlan'], init=True)
self.traffic_inst = SendVerifyTraffic(
Expand Down Expand Up @@ -506,3 +547,4 @@ def test_pfcwd_show_stat(self, request, setup_pfc_test, setup_dut_test_params, e
self.storm_hndle.stop_storm()
logger.info("--- Stop PFC WD ---")
self.dut.command("pfcwd stop")
self.__restore_original_config(duthost, port)
Loading