Skip to content

[action] [PR:18117] Add type check for ptf_ports to adapt physical port interface and portchannel interface#19464

Merged
mssonicbld merged 1 commit intosonic-net:202505from
mssonicbld:cherry/202505/18117
Jul 8, 2025
Merged

[action] [PR:18117] Add type check for ptf_ports to adapt physical port interface and portchannel interface#19464
mssonicbld merged 1 commit intosonic-net:202505from
mssonicbld:cherry/202505/18117

Conversation

@mssonicbld
Copy link
Copy Markdown
Collaborator

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 Author

Original PR: #18117

@mssonicbld
Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld mssonicbld merged commit 3093f5f into sonic-net:202505 Jul 8, 2025
12 checks passed
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.

2 participants