[QOS - Test PFC Pause] Multi VLAN Support by matthew-soulsby · Pull Request #16725 · sonic-net/sonic-mgmt

matthew-soulsby · 2025-01-30T02:02:20Z

Description of PR

Summary:

Type of change

Back port request

Approach

What is the motivation for this PR?

As new testbeds are being created with more VLANs, the assumption that a device should only contain 1 VLAN no longer holds. As such, certain helpers/parts of the framework need to be modified to allow for multiple VLANs to be present and tested.

How did you do it?

Added a get_all_vlans function to extract all vlans from the duthost object
The functions which get information related to vlans (get_active_vlan_members and get_vlan_subnet) now take a vlan configuration dictionary (which itself is taken from the get_all_vlans function)
pfc_test_setup now returns a list of VLAN configurations, instead of just one
The run_test wrapper now runs the relevant test for each VLAN supplied
Also removed gen_testbed_t0, which is not used anywhere (validated with grep)

How did you verify/test it?

Before this change, running the qos/test_pfc_pause.py on a testbed with multiple VLANs produced the following output:

@pytest.fixture(scope="module", autouse=True)
    def pfc_test_setup(duthosts, rand_one_dut_hostname, tbinfo, ptfhost):
        """
        Generate configurations for the tests
   
        Args:
            duthosts(AnsibleHost) : multi dut instance
            rand_one_dut_hostname(string) : one of the dut instances from the multi dut
   
        Yields:
            setup(dict): DUT interfaces, PTF interfaces, PTF IP addresses, and PTF MAC addresses
        """
   
        """ Get all the active physical interfaces enslaved to the Vlan """
        """ These interfaces are actually server-faced interfaces at T0 """
        duthost = duthosts[rand_one_dut_hostname]
>       vlan_members, vlan_id = get_active_vlan_members(duthost)
E       TypeError: cannot unpack non-iterable NoneType object

With these changes, it is now successfully running:

202405 Evidence:

202411 Evidence:

Any platform specific information?

Any platform with only one VLAN - as each of the new lists/dicts will only contain one VLAN, iterating through them will cause the same behaviour as before.

Supported testbed topology if it's a new test case?

N/A

Documentation

mssonicbld · 2025-01-30T02:02:24Z

/azp run

azure-pipelines · 2025-01-30T02:02:35Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-30T02:11:05Z

/azp run

azure-pipelines · 2025-01-30T02:11:17Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-30T02:21:47Z

/azp run

azure-pipelines · 2025-01-30T02:21:59Z

Azure Pipelines successfully started running 1 pipeline(s).

ZhaohuiS · 2025-02-05T06:17:32Z

@matthew-soulsby This PR almost refactors the qos_pause related scripts, could you please also verify if this change will not fail the test result on single vlan testbed?

matthew-soulsby · 2025-02-05T06:24:46Z

@matthew-soulsby This PR almost refactors the qos_pause related scripts, could you please also verify if this change will not fail the test result on single vlan testbed?

Sure, sounds good, I'll run on a single VLAN testbed and add a comment with the results.

matthew-soulsby · 2025-02-06T00:19:28Z

@ZhaohuiS Running on a single VLAN testbed results in expected pass:

How does that look?

ZhaohuiS

LGTM

tests/qos/qos_helpers.py

tests/qos/test_pfc_pause.py

lipxu

Approved with comments.

StormLiangMS · 2025-02-18T03:12:57Z

Hi @matthew-soulsby could you update test results with sonic-mgmt 202405 and 202411 branches before cherry pick?

StormLiangMS

LGTM

matthew-soulsby · 2025-02-18T07:57:31Z

Hi @matthew-soulsby could you update test results with sonic-mgmt 202405 and 202411 branches before cherry pick?

Hi @StormLiangMS, just updated the results section with the requested results - how does it look now?

@pytest

What is the motivation for this PR? As new testbeds are being created with more VLANs, the assumption that a device should only contain 1 VLAN no longer holds. As such, certain helpers/parts of the framework need to be modified to allow for multiple VLANs to be present and tested. How did you do it? Added a get_all_vlans function to extract all vlans from the duthost object The functions which get information related to vlans (get_active_vlan_members and get_vlan_subnet) now take a vlan configuration dictionary (which itself is taken from the get_all_vlans function) pfc_test_setup now returns a list of VLAN configurations, instead of just one The run_test wrapper now runs the relevant test for each VLAN supplied Also removed gen_testbed_t0, which is not used anywhere (validated with grep) How did you verify/test it? Before this change, running the qos/test_pfc_pause.py on a testbed with multiple VLANs produced the following output: @pytest.fixture(scope="module", autouse=True) def pfc_test_setup(duthosts, rand_one_dut_hostname, tbinfo, ptfhost): """ Generate configurations for the tests Args: duthosts(AnsibleHost) : multi dut instance rand_one_dut_hostname(string) : one of the dut instances from the multi dut Yields: setup(dict): DUT interfaces, PTF interfaces, PTF IP addresses, and PTF MAC addresses """ """ Get all the active physical interfaces enslaved to the Vlan """ """ These interfaces are actually server-faced interfaces at T0 """ duthost = duthosts[rand_one_dut_hostname] > vlan_members, vlan_id = get_active_vlan_members(duthost) E TypeError: cannot unpack non-iterable NoneType object With these changes, it is now successfully running: image Any platform specific information? Any platform with only one VLAN - as each of the new lists/dicts will only contain one VLAN, iterating through them will cause the same behaviour as before.

mssonicbld · 2025-02-19T08:08:05Z

Cherry-pick PR to 202411: #17021

@pytest

What is the motivation for this PR? As new testbeds are being created with more VLANs, the assumption that a device should only contain 1 VLAN no longer holds. As such, certain helpers/parts of the framework need to be modified to allow for multiple VLANs to be present and tested. How did you do it? Added a get_all_vlans function to extract all vlans from the duthost object The functions which get information related to vlans (get_active_vlan_members and get_vlan_subnet) now take a vlan configuration dictionary (which itself is taken from the get_all_vlans function) pfc_test_setup now returns a list of VLAN configurations, instead of just one The run_test wrapper now runs the relevant test for each VLAN supplied Also removed gen_testbed_t0, which is not used anywhere (validated with grep) How did you verify/test it? Before this change, running the qos/test_pfc_pause.py on a testbed with multiple VLANs produced the following output: @pytest.fixture(scope="module", autouse=True) def pfc_test_setup(duthosts, rand_one_dut_hostname, tbinfo, ptfhost): """ Generate configurations for the tests Args: duthosts(AnsibleHost) : multi dut instance rand_one_dut_hostname(string) : one of the dut instances from the multi dut Yields: setup(dict): DUT interfaces, PTF interfaces, PTF IP addresses, and PTF MAC addresses """ """ Get all the active physical interfaces enslaved to the Vlan """ """ These interfaces are actually server-faced interfaces at T0 """ duthost = duthosts[rand_one_dut_hostname] > vlan_members, vlan_id = get_active_vlan_members(duthost) E TypeError: cannot unpack non-iterable NoneType object With these changes, it is now successfully running: image Any platform specific information? Any platform with only one VLAN - as each of the new lists/dicts will only contain one VLAN, iterating through them will cause the same behaviour as before.

mssonicbld · 2025-02-19T08:08:08Z

Cherry-pick PR to 202405: #17022

@pytest

What is the motivation for this PR? As new testbeds are being created with more VLANs, the assumption that a device should only contain 1 VLAN no longer holds. As such, certain helpers/parts of the framework need to be modified to allow for multiple VLANs to be present and tested. How did you do it? Added a get_all_vlans function to extract all vlans from the duthost object The functions which get information related to vlans (get_active_vlan_members and get_vlan_subnet) now take a vlan configuration dictionary (which itself is taken from the get_all_vlans function) pfc_test_setup now returns a list of VLAN configurations, instead of just one The run_test wrapper now runs the relevant test for each VLAN supplied Also removed gen_testbed_t0, which is not used anywhere (validated with grep) How did you verify/test it? Before this change, running the qos/test_pfc_pause.py on a testbed with multiple VLANs produced the following output: @pytest.fixture(scope="module", autouse=True) def pfc_test_setup(duthosts, rand_one_dut_hostname, tbinfo, ptfhost): """ Generate configurations for the tests Args: duthosts(AnsibleHost) : multi dut instance rand_one_dut_hostname(string) : one of the dut instances from the multi dut Yields: setup(dict): DUT interfaces, PTF interfaces, PTF IP addresses, and PTF MAC addresses """ """ Get all the active physical interfaces enslaved to the Vlan """ """ These interfaces are actually server-faced interfaces at T0 """ duthost = duthosts[rand_one_dut_hostname] > vlan_members, vlan_id = get_active_vlan_members(duthost) E TypeError: cannot unpack non-iterable NoneType object With these changes, it is now successfully running: image Any platform specific information? Any platform with only one VLAN - as each of the new lists/dicts will only contain one VLAN, iterating through them will cause the same behaviour as before.

@pytest

What is the motivation for this PR? As new testbeds are being created with more VLANs, the assumption that a device should only contain 1 VLAN no longer holds. As such, certain helpers/parts of the framework need to be modified to allow for multiple VLANs to be present and tested. How did you do it? Added a get_all_vlans function to extract all vlans from the duthost object The functions which get information related to vlans (get_active_vlan_members and get_vlan_subnet) now take a vlan configuration dictionary (which itself is taken from the get_all_vlans function) pfc_test_setup now returns a list of VLAN configurations, instead of just one The run_test wrapper now runs the relevant test for each VLAN supplied Also removed gen_testbed_t0, which is not used anywhere (validated with grep) How did you verify/test it? Before this change, running the qos/test_pfc_pause.py on a testbed with multiple VLANs produced the following output: @pytest.fixture(scope="module", autouse=True) def pfc_test_setup(duthosts, rand_one_dut_hostname, tbinfo, ptfhost): """ Generate configurations for the tests Args: duthosts(AnsibleHost) : multi dut instance rand_one_dut_hostname(string) : one of the dut instances from the multi dut Yields: setup(dict): DUT interfaces, PTF interfaces, PTF IP addresses, and PTF MAC addresses """ """ Get all the active physical interfaces enslaved to the Vlan """ """ These interfaces are actually server-faced interfaces at T0 """ duthost = duthosts[rand_one_dut_hostname] > vlan_members, vlan_id = get_active_vlan_members(duthost) E TypeError: cannot unpack non-iterable NoneType object With these changes, it is now successfully running: image Any platform specific information? Any platform with only one VLAN - as each of the new lists/dicts will only contain one VLAN, iterating through them will cause the same behaviour as before.

@pytest

What is the motivation for this PR? As new testbeds are being created with more VLANs, the assumption that a device should only contain 1 VLAN no longer holds. As such, certain helpers/parts of the framework need to be modified to allow for multiple VLANs to be present and tested. How did you do it? Added a get_all_vlans function to extract all vlans from the duthost object The functions which get information related to vlans (get_active_vlan_members and get_vlan_subnet) now take a vlan configuration dictionary (which itself is taken from the get_all_vlans function) pfc_test_setup now returns a list of VLAN configurations, instead of just one The run_test wrapper now runs the relevant test for each VLAN supplied Also removed gen_testbed_t0, which is not used anywhere (validated with grep) How did you verify/test it? Before this change, running the qos/test_pfc_pause.py on a testbed with multiple VLANs produced the following output: @pytest.fixture(scope="module", autouse=True) def pfc_test_setup(duthosts, rand_one_dut_hostname, tbinfo, ptfhost): """ Generate configurations for the tests Args: duthosts(AnsibleHost) : multi dut instance rand_one_dut_hostname(string) : one of the dut instances from the multi dut Yields: setup(dict): DUT interfaces, PTF interfaces, PTF IP addresses, and PTF MAC addresses """ """ Get all the active physical interfaces enslaved to the Vlan """ """ These interfaces are actually server-faced interfaces at T0 """ duthost = duthosts[rand_one_dut_hostname] > vlan_members, vlan_id = get_active_vlan_members(duthost) E TypeError: cannot unpack non-iterable NoneType object With these changes, it is now successfully running: image Any platform specific information? Any platform with only one VLAN - as each of the new lists/dicts will only contain one VLAN, iterating through them will cause the same behaviour as before.

@pytest

What is the motivation for this PR? As new testbeds are being created with more VLANs, the assumption that a device should only contain 1 VLAN no longer holds. As such, certain helpers/parts of the framework need to be modified to allow for multiple VLANs to be present and tested. How did you do it? Added a get_all_vlans function to extract all vlans from the duthost object The functions which get information related to vlans (get_active_vlan_members and get_vlan_subnet) now take a vlan configuration dictionary (which itself is taken from the get_all_vlans function) pfc_test_setup now returns a list of VLAN configurations, instead of just one The run_test wrapper now runs the relevant test for each VLAN supplied Also removed gen_testbed_t0, which is not used anywhere (validated with grep) How did you verify/test it? Before this change, running the qos/test_pfc_pause.py on a testbed with multiple VLANs produced the following output: @pytest.fixture(scope="module", autouse=True) def pfc_test_setup(duthosts, rand_one_dut_hostname, tbinfo, ptfhost): """ Generate configurations for the tests Args: duthosts(AnsibleHost) : multi dut instance rand_one_dut_hostname(string) : one of the dut instances from the multi dut Yields: setup(dict): DUT interfaces, PTF interfaces, PTF IP addresses, and PTF MAC addresses """ """ Get all the active physical interfaces enslaved to the Vlan """ """ These interfaces are actually server-faced interfaces at T0 """ duthost = duthosts[rand_one_dut_hostname] > vlan_members, vlan_id = get_active_vlan_members(duthost) E TypeError: cannot unpack non-iterable NoneType object With these changes, it is now successfully running: image Any platform specific information? Any platform with only one VLAN - as each of the new lists/dicts will only contain one VLAN, iterating through them will cause the same behaviour as before.

matthew-soulsby added 3 commits January 30, 2025 12:38

Add support for multiple vlans

2027e37

Fix dictionary iteration

d920eb8

Fix small issue

d6247cb

Fix pre-commit issues

ae1c3fe

Fix pre-commit

18c110e

ZhaohuiS approved these changes Feb 6, 2025

View reviewed changes

ZhaohuiS requested a review from lipxu February 6, 2025 01:22

ZhaohuiS added Request for 202405 branch Request for 202411 branch labels Feb 11, 2025

lipxu reviewed Feb 12, 2025

View reviewed changes

tests/qos/qos_helpers.py Show resolved Hide resolved

tests/qos/test_pfc_pause.py Show resolved Hide resolved

tests/qos/test_pfc_pause.py Show resolved Hide resolved

lipxu approved these changes Feb 13, 2025

View reviewed changes

StormLiangMS approved these changes Feb 18, 2025

View reviewed changes

StormLiangMS merged commit e8cacfb into sonic-net:master Feb 18, 2025
2 checks passed

ZhaohuiS added Approved for 202405 branch Approved for 202411 branch labels Feb 19, 2025

mssonicbld added the Created PR to 202411 branch label Feb 19, 2025

mssonicbld added the Created PR to 202405 branch label Feb 19, 2025

This was referenced Feb 19, 2025

[action] [PR:16725] [QOS - Test PFC Pause] Multi VLAN Support #17021

Merged

[action] [PR:16725] [QOS - Test PFC Pause] Multi VLAN Support #17022

Merged

mssonicbld added Included in 202405 branch and removed Created PR to 202405 branch labels Feb 20, 2025

matthew-soulsby deleted the mattsoulsby/pfc-multi-vlan branch February 21, 2025 00:03

mssonicbld added Included in 202411 branch and removed Created PR to 202411 branch labels Feb 21, 2025

Conversation

matthew-soulsby commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

mssonicbld commented Jan 30, 2025

Uh oh!

azure-pipelines bot commented Jan 30, 2025

Uh oh!

mssonicbld commented Jan 30, 2025

Uh oh!

azure-pipelines bot commented Jan 30, 2025

Uh oh!

mssonicbld commented Jan 30, 2025

Uh oh!

azure-pipelines bot commented Jan 30, 2025

Uh oh!

ZhaohuiS commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthew-soulsby commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthew-soulsby commented Feb 6, 2025

Uh oh!

ZhaohuiS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lipxu left a comment

Choose a reason for hiding this comment

Uh oh!

StormLiangMS commented Feb 18, 2025

Uh oh!

StormLiangMS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matthew-soulsby commented Feb 18, 2025

Uh oh!

mssonicbld commented Feb 19, 2025

Uh oh!

mssonicbld commented Feb 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

matthew-soulsby commented Jan 30, 2025 •

edited

Loading

ZhaohuiS commented Feb 5, 2025 •

edited

Loading

matthew-soulsby commented Feb 5, 2025 •

edited

Loading