Skip to content

Add type check for ptf_ports to adapt physical port interface and portchannel interface#18117

Merged
bingwang-ms merged 1 commit intosonic-net:masterfrom
weiguo-nvidia:fix_issue_16529
Jul 7, 2025
Merged

Add type check for ptf_ports to adapt physical port interface and portchannel interface#18117
bingwang-ms merged 1 commit intosonic-net:masterfrom
weiguo-nvidia:fix_issue_16529

Conversation

@weiguo-nvidia
Copy link
Copy Markdown
Contributor

@weiguo-nvidia weiguo-nvidia commented Apr 24, 2025

Description of PR

Summary: Fix github issue #16529, add type check for ptf_ports, handle both list and dict ptf_ports
Fixes #

Issue Description


Background

Test case sub_port_interfaces.test_sub_port_interfaces.TestSubPorts#test_balancing_sub_ports has two fixture parameters: ['port', 'port_in_lag']

  • test_balancing_sub_ports[port]
  • test_balancing_sub_ports[port_in_lag]

Issue

  • Case test_balancing_sub_ports[port] always pass
  • Case test_balancing_sub_ports[port_in_lag] sometimes fail because Failed: Expected packet not available.
    In the test case, the src_port is selected randomly. If the src_port selects eth0 or eth1, the case will fail. The eth0 and eth1 are the ptf tested ports, which should not be selected as src_port
def test_balancing_sub_ports(self, duthost, ptfhost, ptfadapter, apply_balancing_config):
    new_sub_ports = apply_balancing_config['new_sub_ports']
    sub_ports = apply_balancing_config['sub_ports']
    src_ports = apply_balancing_config['src_ports']
    src_port = random.choice(src_ports) <<<<<< HERE

Root cause

The src_ports list is generated by function apply_balancing_config, which is defined intests/sub_port_interfaces/conftest.py.
In the current code, there already has a logic to remove ptf port from the src_ports list

But the ptf_ports type is different for Ethernet interface and portchannel interface, which lead to error

  • For Ethernet interface, the ptf_ports type is list
  • For portchannel interface, the ptf_ports type is dict
mg_facts = duthost.get_extended_minigraph_facts(tbinfo)
all_up_ports = set()
for port in list(mg_facts['minigraph_ports'].keys()):
    all_up_ports.add("eth" + str(mg_facts['minigraph_ptf_indices'][port]))
src_ports = tuple(all_up_ports.difference(ptf_ports)) <<<<<< HERE
  • When the test case is test_balancing_sub_ports[port], the ptf_ports value is ['eth0', 'eth1']
(Pdb) all_up_ports
{'eth45', 'eth37', 'eth24', 'eth13', 'eth52', 'eth41', 'eth43', 'eth4', 'eth21', 'eth20', 'eth9', 'eth32', 'eth17', 'eth25', 'eth33', 'eth14', 'eth44', 'eth5', 'eth40', 'eth12', 'eth49', 'eth8', 'eth29', 'eth36', 'eth48', 'eth53', 'eth42', 'eth1', 'eth0', 'eth15', 'eth28', 'eth16'}

(Pdb) ptf_ports
['eth0', 'eth1']

After src_ports = tuple(all_up_ports.difference(ptf_ports)), the eth0 and eth1 are removed, so the src_ports list is correct

(Pdb) src_ports
('eth45', 'eth37', 'eth24', 'eth13', 'eth52', 'eth41', 'eth43', 'eth4', 'eth21', 'eth20', 'eth9', 'eth32', 'eth17', 'eth25', 'eth33', 'eth14', 'eth44', 'eth5', 'eth40', 'eth12', 'eth49', 'eth8', 'eth29', 'eth36', 'eth48', 'eth53', 'eth42', 'eth15', 'eth28', 'eth16')
  • When the test case is test_balancing_sub_ports[port_in_lag], the ptf_ports value is OrderedDict([('bond0', 'eth0'), ('bond1', 'eth1')])
(Pdb) all_up_ports
{'eth52', 'eth14', 'eth0', 'eth1', 'eth33', 'eth4', 'eth17', 'eth32', 'eth24', 'eth36', 'eth53', 'eth41', 'eth49', 'eth44', 'eth13', 'eth5', 'eth9', 'eth15', 'eth45', 'eth40', 'eth16', 'eth20', 'eth42', 'eth21', 'eth25', 'eth48', 'eth12', 'eth28', 'eth43', 'eth29', 'eth8', 'eth37'}

(Pdb) ptf_ports
OrderedDict([('bond0', 'eth0'), ('bond1', 'eth1')])

After src_ports = tuple(all_up_ports.difference(ptf_ports)), the eth0 and eth1 still exist in src_ports list. When the src_port is randomly selected eth0 or eth1, the case will fail

(Pdb) src_ports
('eth52', 'eth14', 'eth0', 'eth1', 'eth33', 'eth4', 'eth17', 'eth32', 'eth24', 'eth36', 'eth53', 'eth41', 'eth49', 'eth44', 'eth13', 'eth5', 'eth9', 'eth15', 'eth45', 'eth40', 'eth16', 'eth20', 'eth42', 'eth21', 'eth25', 'eth48', 'eth12', 'eth28', 'eth43', 'eth29', 'eth8', 'eth37')

Fix

Add type check for ptf_ports, handle both list and dict ptf_ports

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

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

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@nhe-NV
Copy link
Copy Markdown
Contributor

nhe-NV commented Jun 27, 2025

@bingwang-ms can you help to review it?

@bingwang-ms
Copy link
Copy Markdown
Collaborator

@weiguo-nvidia The change LGTM. Can you update the title of PR?

@weiguo-nvidia weiguo-nvidia changed the title Fix github issue #16529 Add type check for ptf_ports to adapt physical port interface and portchannel interface Jul 4, 2025
@weiguo-nvidia
Copy link
Copy Markdown
Contributor Author

@weiguo-nvidia The change LGTM. Can you update the title of PR?

Hi @bingwang-ms
I have updated the title.

@bingwang-ms bingwang-ms merged commit 4aba39f into sonic-net:master Jul 7, 2025
14 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Jul 8, 2025
@mssonicbld
Copy link
Copy Markdown
Collaborator

Cherry-pick PR to 202505: #19464

mssonicbld pushed a commit that referenced this pull request Jul 8, 2025
nissampa pushed a commit to nissampa/sonic-mgmt_dpu_test that referenced this pull request Aug 7, 2025
ashutosh-agrawal pushed a commit to ashutosh-agrawal/sonic-mgmt that referenced this pull request Aug 14, 2025
vidyac86 pushed a commit to vidyac86/sonic-mgmt that referenced this pull request Oct 23, 2025
opcoder0 pushed a commit to opcoder0/sonic-mgmt that referenced this pull request Dec 8, 2025
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Dec 16, 2025
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Dec 16, 2025
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Dec 21, 2025
venu-nexthop pushed a commit to venu-nexthop/sonic-mgmt that referenced this pull request Jan 13, 2026
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Jan 26, 2026
ytzur1 pushed a commit to ytzur1/sonic-mgmt that referenced this pull request Feb 2, 2026
venu-nexthop pushed a commit to venu-nexthop/sonic-mgmt that referenced this pull request Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants