[ixia/Keysight] Adding first Tgen-based rdma test cases. by abhijit-dhar · Pull Request #2246 · sonic-net/sonic-mgmt

abhijit-dhar · 2020-09-19T10:00:16Z

Adding Tgen-Api based Pfc lossy test case.
Files added

~/sonic-mgmt/tests/pfc/test_pfc_pause_loss.py
~/sonic-mgmt/tests/pfc/files/configs/init.py
~/sonic-mgmt/tests/pfc/files/configs/pfc.py
Files changed
~/sonic-mgmt/tests/common/ixia/ixia_fixtures.py
~/sonic-mgmt/tests/common/ixia/ixia_helpers.py

This directory should contain the Ixia ECN and RDMA test cases. The lib folder contains * fixtures required to run ixia test cases * helper function required to write ixia test cases

As per review comments given against the 1819 we have to keep the ixia libraries under sonic-mgmt/tests/common/ixia/

This file should be in tests/common/ixia folder. So deleting it from tests/ixia/lib/ folder

This file should be under the folder tests/common/ixia Hence deleting it form tests/ixia/lib

This file should be under tests/common/ixia So deleting it from tests/ixia/lib

There were many review comment against this file. Some of the comments I have implemented. But some of the comments may be implemented in future. Like "(Wei Bai) Can you move all the low level IXIA API functions to ixia_helpers.py?" or "I have a high level comment. can we design some IXIA wrapper functions (e.g., configure ports, create a PFC traffic item) and only call these high level wrapper functions in py.test scripts without exposing too many IXIA low level details? (Wei Bai)" This comment may be addressed in future PRs once we come up with the high level wrapper APIs.

Added a new fixture ixia_api_server_session

This class is added to efficiently manage ixia fanout devices

Making ixia directory a package, to access ixia_fixtures.py and ixia_helpers.py

This class is a place holder right now. Ixia fanout devices (chassis) do not require this abstraction layer. Ixia PTF (ixia API server) takes care of all the operation that we can do on the ixia fanout devices (chassis) and hence we execute them via API server only. But it is still nice to have all abstractions in the same location. So future users knows where to find them.

Added a new class that should make chassis port management easier

There was an typo

Deletes accidentally

…xia_helpers.py

lgtm-com · 2020-10-13T05:43:25Z

This pull request introduces 1 alert when merging eb0044c into 857cfb2 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'

lgtm-com · 2020-10-14T07:54:40Z

This pull request introduces 1 alert when merging e568baa into ad43809 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'

lgtm-com · 2020-10-14T13:42:57Z

This pull request introduces 1 alert when merging 7662ed3 into ad43809 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'

tests/common/ixia/ixia_fixtures.py

baiwei0427 · 2020-10-16T16:25:40Z

tests/pfc/files/configs/pfc.py

+from abstract_open_traffic_generator.flow import Ethernet as EthernetHeader
+from abstract_open_traffic_generator.port import Options as PortOptions
+
+def calculate_priority_vector(v) :


This function should be moved to another file

Add comments for this function

Relocation of this function to another generic/common file will be tracked by a separate PR. Let it be here now.

This function is updated with appropriate comment section.

baiwei0427 · 2020-10-16T16:27:33Z

tests/pfc/files/configs/pfc.py

+           print(i)
+    return "%x"%(s)
+
+def lossless_iteration_list (lst) :


Add comments for this function

This function is updated with appropriate comment section.

baiwei0427 · 2020-10-19T16:34:31Z

tests/pfc/files/configs/pfc.py

+
+
+@pytest.fixture
+def one_hundred_gbe(conn_graph_facts,


The name of 'one_hundred_gbe' is not so straightforward. How about changing the name to 'l1_config' and extending this function to support any link speeds.

The name 'one_hundred_gbe' has been replaced by the name 'l1_config' everywhere in the code.

baiwei0427 · 2020-10-16T16:44:47Z

tests/pfc/files/configs/pfc.py

+        ######################################################################
+        tx_ipv4 = Ipv4(name='Tx Ipv4',
+                       address=Pattern(tx_port_ip),
+                       prefix=Pattern('24'),


We cannot assume the prefix length is 24. You should be able to get this from vlan_subnet

This change request is implemented. I have extracted from 'vlan_subnet'

baiwei0427 · 2020-10-19T16:10:34Z

tests/pfc/files/configs/pfc.py

+
+
+@pytest.fixture
+def global_pause(conn_graph_facts,


Why we need this?

This file is shared by two test cases 1) test_pfc_pause_lossy.py 2) test_global_pause.py. So fixture used by both the test cases exists here. This particular function is used by test_global_pause.py. So this should be here.

baiwei0427 · 2020-10-19T16:20:24Z

tests/pfc/test_pfc_pause_lossy.py

+        api.set_state(State(FlowTransmitState(state='stop')))
+
+        # Get statistics
+        stat_captions =['Test Data', 'Background Data']


we need to define 'Test Data' and 'Background Data' in tests/pfc/files/configs/pfc.py

I have defined 'Test Data' and 'Background Data' in pfc/test_pfc_pause_lossy.py and made them available in pfc.py as parameter

baiwei0427 · 2020-10-19T16:24:13Z

tests/pfc/test_pfc_pause_lossy.py

+       all lossless priorities.
+    6. From Rx port send pause frames on all lossless priorities. Then
+       start 'Test Data Traffic' and 'Background Data Traffic'.
+    7. Verify the following: 


The verification is wrong. The right description is 'When all the traffic items stop, the Rx port receives all the packets of test traffic and background traffic. And their throughput are not impacted by the PFC pause storm'.

baiwei0427 · 2020-10-19T16:24:39Z

tests/pfc/test_pfc_pause_lossy.py

+    """
+    duthost.shell('sudo pfcwd stop')
+
+    for base_config in lossy_configs:


Add comments

baiwei0427 · 2020-10-19T16:38:39Z

tests/pfc/files/configs/pfc.py

+
+        config.flows.append(pause_flow)
+
+    return one_hundred_gbe


It is a little bit weird to return an input argument (though I understand your point)

baiwei0427 · 2020-10-19T16:44:20Z

Remove the serialization

yxieca · 2020-10-20T23:24:20Z

tests/pfc/files/configs/pfc.py

+
+    for config in one_hundred_gbe :
+
+        delay = start_delay * 1000000000.0


I would rename start_delay to start_delay_secs and name delay as delay_nsec. Also create a global variable NSECS_IN_SEC for 1000000000.0, or better, create a function sec_to_nsec() for the conversion.

I have changed the name of variable/fixture and wrote a function as given in the suggestion.

yxieca · 2020-10-20T23:25:16Z

tests/common/ixia/ixia_helpers.py

+        location = str("%s;%s;%s"%(intf['ip'],intf['card_id'],intf['port_id']))
+    except :
+        pytest_assert(False,
+            "Interface must have the keys 'ip', 'card_id' and 'port_id'")


Please use pytest.fail(msg) instead.

I have replaced this pytest_assert(False, ...) with pytest.fail(msg)

yxieca · 2020-10-20T23:27:00Z

tests/pfc/test_pfc_pause_lossy.py

+
+                if ((row['frames_rx'] == 0) or (row['frames_tx'] != row['frames_rx'])):
+                     pytest_assert(False, 
+                         "Not all %s reached Rx End" %(rows[caption_index]))


Either use if (cond): pytest.fail(msg) or use pytest_assert(! cond, msg). Please don't use pytest_assert(False, msg).

I have replaced this pytest_assert(False, ...) with pytest.fail(msg)

yxieca · 2020-10-20T23:27:25Z

tests/pfc/test_pfc_pause_lossy.py

+                if ((tolerance_ratio < TOLERANCE_THRESHOLD) or
+                    (tolerance_ratio > 1)) :
+                    pytest_assert(False,
+                        "expected % of packets not received at the RX port")


Please don't use pytest_assert(False, msg) (see comments above)

I have replaced this pytest_assert(False, ...) with pytest.fail(msg)

yxieca · 2020-10-20T23:29:20Z

tests/pfc/files/configs/pfc.py

+    for i in range(8)  :
+        if v[i] != '0' :
+           s += 2**(7 - i)
+           print(i)


please remove this debug statement. If needed, use log instead.

I have removed the print statement.

abhijit-dhar · 2020-09-27T06:11:07Z

tests/pfc/files/configs/pfc.py

+# Imports for Tgen and IxNetwork abstract class
+###############################################################################
+
+from abstract_open_traffic_generator.port import Port


These links are already shared by Subhajit. We have refactored the code a bit after the new implementation. Our baseline is bstract-open-traffic-generator==0.0.50 and ixnetwork-open-traffic-generator==0.0.15 . Please install them using pip.
sudo pip install abstract-open-traffic-generator==0.0.50
sudo pip install ixnetwork-open-traffic-generator==0.0.15

abhijit-dhar · 2020-09-27T06:14:41Z

tests/pfc/files/configs/pfc.py

+from abstract_open_traffic_generator.flow import Ethernet as EthernetHeader
+from abstract_open_traffic_generator.port import Options as PortOptions
+
+@pytest.fixture


Implemented. Please check the file "test_pfc_pause_lossy.py"

abhijit-dhar · 2020-09-27T06:15:15Z

tests/pfc/files/configs/pfc.py

+    return start_delay
+
+
+@pytest.fixture


implemented. Please check the file "test_pfc_pause_lossy.py"

abhijit-dhar · 2020-09-27T06:17:56Z

tests/pfc/files/configs/pfc.py

+    return traffic_duration  
+
+
+@pytest.fixture(scope='session')


This requires a discussion. I want place it in ~/sonic-mgmt/tests/common/ixia/ixia_fixtures.py. Should it be ok?

abhijit-dhar · 2020-09-27T06:23:33Z

tests/pfc/files/configs/pfc.py

+
+        flow_ctl = FlowControl(choice=pfc)
+
+        l1_oneHundredGbe = OneHundredGbe(link_training=True,


If L1 config is different the you have to define a new fixture for that.

abhijit-dhar · 2020-09-27T06:23:53Z

tests/pfc/test_pfc_pause_lossy.py

+                (rows[caption_index] == 'Background Data')):
+                if rows[tx_frame_index] != rows[rx_frame_index] :
+                    pytest_assert(False,
+                        "Not all %s reached Rx End" %(rows[caption_index]))


Implemented

lgtm-com · 2020-10-27T13:57:03Z

This pull request introduces 3 alerts when merging 997e27b into 553e9ff - view on LGTM.com

new alerts:

2 for Variable defined multiple times
1 for Except block handles 'BaseException'

lgtm-com · 2020-10-29T07:45:34Z

This pull request introduces 3 alerts when merging 089b53a into 4413d9d - view on LGTM.com

new alerts:

2 for Variable defined multiple times
1 for Except block handles 'BaseException'

lgtm-com · 2020-10-29T15:16:35Z

This pull request introduces 3 alerts when merging 7628179 into 4413d9d - view on LGTM.com

new alerts:

2 for Variable defined multiple times
1 for Except block handles 'BaseException'

lgtm-com · 2020-10-30T05:37:23Z

This pull request introduces 1 alert when merging c958741 into 43915ed - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'

lgtm-com · 2020-10-30T18:10:35Z

This pull request introduces 7 alerts when merging 512aa4b into 6b27e48 - view on LGTM.com

new alerts:

6 for Unused import
1 for Except block handles 'BaseException'

lgtm-com · 2020-11-03T08:37:50Z

This pull request introduces 2 alerts when merging bec37fe into 1306417 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'
1 for Syntax error

This PR is to advance sonic-swss submodule for 202012 branch. Changes include eef993f [202012][cherry-pick]Update orchagent to support new field pfcwd_sw_enable (sonic-net#2302) ab675b3 [202012][cherry-pick] Support tunnel traffic QoS remapping (sonic-net#2246) Signed-off-by: bingwang <[email protected]>

… submodule head (sonic-net#12494) utilities: * 02eb899e 2022-07-12 | [config/load_mgmt_config] Support load IPv6 mgmt IP (sonic-net#2206) (sonic-net#2246) (sonic-net#2256) (HEAD -> 201811, github/201811) [Jing Kan] platform-daemon: * fc288cc 2022-05-05 | Mem leak caused by Xcvrd in Send-Q of REDIS-DB socket connection (sonic-net#260) (HEAD -> 201811, github/201811) [Prince George] platform-common: * edb062b 2022-02-09 | [sonic_sfp] Interpret sff 'int' element =0 as valid value (sonic-net#261) (HEAD -> 201811, github/201811) [Prince George] kernel: * 9d2d1a1 2022-02-11 | [201811] Increase log_buf_len size to 1M (HEAD -> 201811, github/201811) [Sujin Kang] * b34a213 2022-02-10 | [201811] Increase log_buf_len size to 1M [Samuel Angebault] * c4684cb 2022-02-11 | [201811] Apply kernel patches to fix emmc unreliability [Sujin Kang] * df68771 2022-02-03 | [201811] Apply kernel patches to fix emmc unreliability [Samuel Angebault] * f21cb06 2021-03-26 | [ci]: remove 201811 suffix (sonic-net#204) [lguohan] * 5439b2a 2021-02-08 | [dni_dps460] Add attributes to retrieve PMBus status command codes (sonic-net#194) [Arun Saravanan Balachandran] Signed-off-by: Ying Xie <[email protected]> Signed-off-by: Ying Xie <[email protected]>

Why I did it 62b7b56 2022-07-13 | Remove disabled and not loaded services before calling reset-failed and restart services (sonic-net#2266) [Zain Budhwani] 09b4678 2022-07-05 | [config/load_mgmt_config] Support load IPv6 mgmt IP (sonic-net#2206) (sonic-net#2246) [Jing Kan] How I did it Pulled latest commit from 201911 sonic-utilities branch and created PR How to verify it Look at build-image

abhijit-dhar and others added 30 commits June 25, 2020 21:38

[ixia/Keysight] sample test cases and fixtures

06cd811

This directory should contain the Ixia ECN and RDMA test cases. The lib folder contains * fixtures required to run ixia test cases * helper function required to write ixia test cases

[ixia/Keysight] adding ixia libraries

b6a0963

As per review comments given against the 1819 we have to keep the ixia libraries under sonic-mgmt/tests/common/ixia/

[ixia/Keysight] This file should be in common/ixia

2a8ad2b

This file should be in tests/common/ixia folder. So deleting it from tests/ixia/lib/ folder

[ixia/Keysight] lib/ixia_fixtures.py

694ae89

This file should be under the folder tests/common/ixia Hence deleting it form tests/ixia/lib

[ixia/Keysight] deleting ixia_helpers.py

b5316e8

This file should be under tests/common/ixia So deleting it from tests/ixia/lib

Update test_ixia_traffic_restpy.py

61ff293

[ixia/Keysight] Changed as per review comments

721b6b1

Added a new fixture ixia_api_server_session

[ixia/Keysight] added new IxiaFanoutManager class

4833513

This class is added to efficiently manage ixia fanout devices

[ixia/Keysight] Adding __init__().py

b0f1bf6

Making ixia directory a package, to access ixia_fixtures.py and ixia_helpers.py

[ixia/Keysight] corrected typo added a new class

cbf0235

Added a new class that should make chassis port management easier

[ixia/Keysight] Deleted from the review

84f3201

There was an typo

[ixia/Keysight] adding ixia_fixtures

f0098d1

Deletes accidentally

[ixia/Keysight] updated ixia_fixtures.py

b91dc5e

modifying and rdma test case according to ixia proposal

b288123

modifying and rdma test case according to ixia proposal

318c287

removing the low level api from test_ixia_traffic_restpy.py

c33e7c8

changed test scripts and lib functions

b789490

added google style comments and changed function names

0678c57

Added the google doc string for the methods in ixia_fixtures.py and i…

a0182c8

…xia_helpers.py

Implementing PR1819 review comments

cfe3cbc

Removing test_ixia_traffic_restpy.py adding test_ixia_traffic.py

ed09027

Changing some comment string

b54f209

Added stop_protocols function in ixia_helpers.py

c4c5310

Removed a sleep and added a debugging statement

c5ca281

[ixia/Keysight] updating the review comments PR1819

9ab2d65

adding a simple poc

aba759a

Changed ixia/test_ixia_poc.py a bit

23a687f

Implementing the comments of PR1819

bc27595

removing testbedinfo from pfc.py

eb0044c

Updating pfc.py

e568baa

refactored pfc.py

7662ed3

baiwei0427 requested changes Oct 19, 2020

View reviewed changes

yxieca reviewed Oct 20, 2020

View reviewed changes

abhijit-dhar commented Oct 27, 2020

View reviewed changes

Implementing review comments

997e27b

updated pfc.py according latest refactoring of tgen API

089b53a

refactoring the pfc lossy test according to the comments received

7628179

added several refactoring

c958741

Structured the test cases accrding to new norm

512aa4b

yxieca force-pushed the master branch from 885053e to 8f9cb56 Compare October 31, 2020 02:57

Done some refactoring in pfc.py

bec37fe


		for config in one_hundred_gbe :

		delay = start_delay * 1000000000.0


		flow_ctl = FlowControl(choice=pfc)

		l1_oneHundredGbe = OneHundredGbe(link_training=True,



		@pytest.fixture
		def one_hundred_gbe(conn_graph_facts,



		@pytest.fixture
		def global_pause(conn_graph_facts,

Conversation

abhijit-dhar commented Sep 19, 2020

Uh oh!

lgtm-com bot commented Oct 13, 2020

Uh oh!

lgtm-com bot commented Oct 14, 2020

Uh oh!

lgtm-com bot commented Oct 14, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhijit-dhar Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baiwei0427 commented Oct 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Oct 27, 2020

Uh oh!

abhijit-dhar Oct 27, 2020 •

edited

Loading