[Sub-ports] Added new sub-ports test cases #2669

OleksandrKozodoi · 2020-12-15T13:24:41Z

Signed-off-by: Oleksandr Kozodoi [email protected]

Description of PR

Summary:

Added new sub-ports tests:

Updated Test Plan
Added support of LAG port
Added support of t1 topology
Changed pre-configuration of test cases
Updated teardown of test cases

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Approach

What is the motivation for this PR?

Coverage of sub-ports feature by test cases to improve the quality of SONiC.

How did you do it?

Added new test cases to PyTest.
Implementation is based on the Sub-ports design spec

How did you verify/test it?

py.test --testbed=testbed-t0 --inventory=../ansible/lab --testbed_file=../ansible/testbed.csv --host-pattern=testbed-t0 -- module-path=../ansible/library sub_port_interfaces

Any platform specific information?

SONiC Software Version: SONiC.master.130-dirty-20210221.030317
Distribution: Debian 10.8
Kernel: 4.19.0-12-2-amd64
Build commit: ce3b2cbf
Build date: Sun Feb 21 10:23:58 UTC 2021
Platform: x86_64-accton_wedge100bf_32x-r0
HwSKU: montara
ASIC: barefoot

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

T0, T1

Documentation

Signed-off-by: Oleksandr Kozodoi <[email protected]>

OleksandrKozodoi · 2020-12-17T09:11:24Z

retest vsimage please

OleksandrKozodoi · 2020-12-17T10:06:02Z

@yxieca @wangxin @lguohan Please review

tests/sub_port_interfaces/sub_ports_helpers.py

tests/sub_port_interfaces/test_sub_port_interfaces.py

OleksandrKozodoi · 2021-01-11T19:05:41Z

@wangxin I am going to add new changes after the merging of #1573

Signed-off-by: Oleksandr Kozodoi <[email protected]>

OleksandrKozodoi · 2021-02-23T14:58:18Z

@yxieca @wangxin @lguohan I've added new changes. Please review.

SavchukRomanLv · 2021-03-01T07:29:13Z

@wangxin @yxieca @lguohan can you take a look? Thank you in advance.

Signed-off-by: Oleksandr Kozodoi <[email protected]>

tests/sub_port_interfaces/conftest.py

tests/sub_port_interfaces/test_sub_port_interfaces.py

wangxin · 2021-03-02T08:50:48Z

tests/sub_port_interfaces/test_sub_port_interfaces.py

+        sub_ports_new[sub_ports.keys()[0]] = sub_ports[sub_ports.keys()[0]]
+        sub_ports_new[sub_ports.keys()[-1]] = sub_ports[sub_ports.keys()[-1]]
+
+        rand_sub_ports = sub_ports.keys()[random.randint(1, len(sub_ports)-1)]


Shall the length of sub_ports (256 in this case) be verified here?

The length of sub_ports is controlled in the block of the define_sub_ports_configuration fixture. The test case uses range of sub-ports from 1 to 256 for port and range of sub-ports from 1 to 96 for lag-port.

wangxin · 2021-03-02T08:52:46Z

tests/sub_port_interfaces/conftest.py

+        # For example: 'PortChannel1.96'
+        if request.param == 'port_in_lag':
+            vlan_ranges_dut = range(1, 97, 24)
+            vlan_ranges_ptf = range(1, 97, 24)


Can you add comments elaborating why 256 sub-ports configuration is generated here?

Linux has the limitation of 15 characters on an interface name and name of LAG port should have prefix 'PortChannel' and suffix '<0-9999>' on SONiC. So max length of sub-port suffix can be 4 characters (such as, PortChannelX.XX, 15 characters). If we use just port, suffix can be 7 charecters (such as, EthernetXXX.XXX, 15 characters).

For test case test_max_numbers_of_sub_ports, is it suppose to create totally 256 number of sub-ports, or just verify that sub-port with suffix like PortChannelX.XX can be created?

I picked define_sub_ports_configuration and tested it from my side on t0. For test case with max_numbers in name, the dicts generated by this fixture are like below:

{ "sub_ports": { "Ethernet4.129": { "ip": "172.16.0.9/30", "neighbor_port": "eth1.129", "neighbor_ip": "172.16.0.10/30" }, "Ethernet8.129": { "ip": "172.16.4.9/30", "neighbor_port": "eth2.129", "neighbor_ip": "172.16.4.10/30" }, "Ethernet8.65": { "ip": "172.16.4.5/30", "neighbor_port": "eth2.65", "neighbor_ip": "172.16.4.6/30" }, "Ethernet4.1": { "ip": "172.16.0.1/30", "neighbor_port": "eth1.1", "neighbor_ip": "172.16.0.2/30" }, "Ethernet4.65": { "ip": "172.16.0.5/30", "neighbor_port": "eth1.65", "neighbor_ip": "172.16.0.6/30" }, "Ethernet4.193": { "ip": "172.16.0.13/30", "nei\r\nghbor_port": "eth1.193", "neighbor_ip": "172.16.0.14/30" }, "Ethernet8.1": { "ip": "172.16.4.1/30", "neighbor_port": "eth2.1", "neighbor_ip": "172.16.4.2/30" }, "Ethernet8.193": { "ip": "172.16.4.13/30", "neighbor_port": "eth2.193", "neighbor_ip": "172.16.4.14/30" } }, "dut_ports": { "1": "Ethernet4", "2": "Ethernet8" }, "ptf_ports": [ "eth1", "eth2" ] }

and

{ "sub_ports": { "PortChannel1.1": { "ip": "172.16.0.1/30", "neighbor_port": "bond1.1", "neighbor_ip": "172.16.0.2/30" }, "PortChannel2.73": { "ip": "172.16.4.\r\n13/30", "neighbor_port": "bond2.73", "neighbor_ip": "172.16.4.14/30" }, "PortChannel2.49": { "ip": "172.16.4.9/30", "neighbor_port": "bond2.49", "neighbor_ip": "172.16.4.10/30" }, "PortChannel1.25": { "ip": "172.\r\n16.0.5/30", "neighbor_port": "bond1.25", "neighbor_ip": "172.16.0.6/30" }, "PortChannel2.25": { "ip": "172.16.4.5/30", "neighbor_port": "bond2.25", "neighbor_ip": "172.16.4.6/30" }, "PortChannel1.49": { "ip": "172.16.0.9/30", "neighbor_port": "bond1.49", "neighbor_ip": "172.16.0.10/30" }, "PortChannel1.73": { "ip": "172.16.0.13/30", "neighbor_port": "bond1.73", "neighbor_ip": "172.16.0.14/30" }, "PortChannel2.1": { "ip": "172.16.4.1/30", "neighbor_port": "bond2.1", "neighbor_ip": "172.16.4.2/30" } }, "dut_ports": { "1": "PortChannel1", "2": "PortChannel2" }, "ptf_ports": { "bond1": "eth1", "bond2": "eth2" } }

The total number of sub-ports is far less than 256. Is this intended?

Test case test_max_numbers_of_sub_ports must create 256 sub-ports for port and 96 sub-ports for lag-port. Thanks for your help in investigating. I am working on this issue.

This issue is fixed by changing steps of range in block (64 to 1), but the test needs too much time (~70 minutes) to run in this case. This behavior due to running of CLI commands for test setup and test teardown on the PTF and DUT. Does it make sense to change the steps of the range for that test case?

Can you try the shell_cmds module? By using this module, you can put all the CLI commands to be executed in a list. Then use this shell_cmds module to run the commands one by one on remote host. It is more efficient than calling the shell module for each CLI command. Can you try it to see how much time is required then?

I've tried the shell_cmds module. By using this module, the test case needs 40 minutes to run.

Do we need to change the sub-ports range in this test case?

I think 40 minutes is still too long. What about this idea:

Add an option for specifying how many sub-ports to be covered. Value range of this option should be 1-256.

Default value of the option can be much lesser to limit the test execution time.
Then we can run this test case by specifying value 256 for the option to have a full coverage when necessary. During daily regression, we can just cover the default range.

I've added --max_numbers_of_sub_ports option for test_max_numbers_of_sub_ports case. Example of the command line how to run this tests:
py.test --testbed=testbed-t0 --inventory=../ansible/lab --testbed_file=../ansible/testbed.csv --host-pattern=testbed-t0 -- module-path=../ansible/library sub_port_interfaces --max_numbers_of_sub_ports=100

Signed-off-by: Oleksandr Kozodoi <[email protected]>

azure-pipelines · 2021-03-11T10:56:17Z

Commenter does not have sufficient privileges for PR 2669 in repo Azure/sonic-mgmt

OleksandrKozodoi · 2021-03-11T17:26:32Z

@wangxin Could you please review?

wangxin · 2021-03-12T02:45:55Z

/azp run

azure-pipelines · 2021-03-12T02:46:06Z

Azure Pipelines successfully started running 1 pipeline(s).

[Sub-ports] Added new sub-ports test cases

44ed7ec

Signed-off-by: Oleksandr Kozodoi <[email protected]>

yxieca requested a review from a team December 23, 2020 00:00

wangxin reviewed Jan 5, 2021

View reviewed changes

tests/sub_port_interfaces/sub_ports_helpers.py Outdated Show resolved Hide resolved

tests/sub_port_interfaces/test_sub_port_interfaces.py Show resolved Hide resolved

OleksandrKozodoi added 2 commits January 26, 2021 04:52

[Sub-ports] Updated new sub-ports test cases

6b09c33

Signed-off-by: Oleksandr Kozodoi <[email protected]>

Added support of LAG ports and t1 topology

d27cd74

Signed-off-by: Oleksandr Kozodoi <[email protected]>

OleksandrKozodoi requested a review from wangxin February 23, 2021 14:58

Changed approach to set up vlan for test case

fecbcd1

Signed-off-by: Oleksandr Kozodoi <[email protected]>

wangxin reviewed Mar 2, 2021

View reviewed changes

Removed ptfadapter fixture from mtu test case

48d084b

Signed-off-by: Oleksandr Kozodoi <[email protected]>

OleksandrKozodoi requested a review from wangxin March 9, 2021 16:03

Added max_numbers_of_sub_ports options

ace44d1

Signed-off-by: Oleksandr Kozodoi <[email protected]>

wangxin approved these changes Mar 12, 2021

View reviewed changes

wangxin merged commit 38ad99a into sonic-net:master Mar 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Sub-ports] Added new sub-ports test cases #2669

[Sub-ports] Added new sub-ports test cases #2669

OleksandrKozodoi commented Dec 15, 2020 •

edited

Loading

OleksandrKozodoi commented Dec 17, 2020

OleksandrKozodoi commented Dec 17, 2020

OleksandrKozodoi commented Jan 11, 2021

OleksandrKozodoi commented Feb 23, 2021

SavchukRomanLv commented Mar 1, 2021

wangxin Mar 2, 2021

OleksandrKozodoi Mar 2, 2021

wangxin Mar 2, 2021

OleksandrKozodoi Mar 2, 2021

wangxin Mar 3, 2021

OleksandrKozodoi Mar 3, 2021

OleksandrKozodoi Mar 3, 2021

wangxin Mar 4, 2021

OleksandrKozodoi Mar 4, 2021

OleksandrKozodoi Mar 9, 2021

wangxin Mar 10, 2021

OleksandrKozodoi Mar 10, 2021

azure-pipelines bot commented Mar 11, 2021

OleksandrKozodoi commented Mar 11, 2021

wangxin commented Mar 12, 2021

azure-pipelines bot commented Mar 12, 2021

[Sub-ports] Added new sub-ports test cases #2669

[Sub-ports] Added new sub-ports test cases #2669

Conversation

OleksandrKozodoi commented Dec 15, 2020 • edited Loading

Description of PR

Type of change

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

OleksandrKozodoi commented Dec 17, 2020

OleksandrKozodoi commented Dec 17, 2020

OleksandrKozodoi commented Jan 11, 2021

OleksandrKozodoi commented Feb 23, 2021

SavchukRomanLv commented Mar 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azure-pipelines bot commented Mar 11, 2021

OleksandrKozodoi commented Mar 11, 2021

wangxin commented Mar 12, 2021

azure-pipelines bot commented Mar 12, 2021

OleksandrKozodoi commented Dec 15, 2020 •

edited

Loading