[Ixia/Keysight] Adding first RDMA test cases#1979
[Ixia/Keysight] Adding first RDMA test cases#1979neethajohn merged 57 commits intosonic-net:masterfrom abhijit-dhar:master
Conversation
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
upcading my master with the remote master
merging master with Rdma1
|
This pull request introduces 1 alert when merging e23970c into f785cd0 - view on LGTM.com new alerts:
|
| "The topology should have at least two interfaces") | ||
|
|
||
| # Test pausing each lossless priority individually. | ||
| session = ixia_api_server_session |
There was a problem hiding this comment.
Can you also test pausing all the lossless priorities simultaneously?
|
|
||
| if 'Test' in flow_stat['Traffic Item']: | ||
| if paused: | ||
| pytest_assert(tx_frames > 0 and rx_frames == 0, |
There was a problem hiding this comment.
We can also check if tx_frames * frame_size is smaller than switch buffer size (optional)
There was a problem hiding this comment.
This seem to be complected. Since it is optional I want to address it as a future enhancement. Please consider it.
There was a problem hiding this comment.
May be with ECN test this is relevant
| device_conn = conn_graph_facts['device_conn'] | ||
| for intf in fanout_devices.get_ports(): | ||
| peer_port = intf['peer_port'] | ||
| intf['speed'] = int(device_conn[peer_port]['speed']) * 100 |
There was a problem hiding this comment.
why multiply by 100 here?
There was a problem hiding this comment.
Not required - removed it.
| start_traffic(session) | ||
|
|
||
| # Wait for test and background traffic to finish. | ||
| time.sleep(exp_dur + 1.5) |
There was a problem hiding this comment.
Here, the extra 1.5 seconds is because of the start delay is 1 second (line 94 and 105). We should declare a variable called 'start_delay' and avoid hardcode.
tests/common/ixia/ixia_helpers.py
Outdated
| traffic_config.FrameSize.FixedSize = pkt_size | ||
|
|
||
| if pkt_count is not None and duration is not None: | ||
| logger.info('You can only specify either pkt_count or duration') |
There was a problem hiding this comment.
Replaced: logger.info --> logger.error
tests/common/ixia/ixia_helpers.py
Outdated
| FrameCount=pkt_count) | ||
| elif duration is not None: | ||
| if type(duration) != int or duration <= 0: | ||
| logger.info('Invalid duration value {} (positive integer only)'. |
There was a problem hiding this comment.
Replaced: logger.info --> logger.error
tests/common/ixia/ixia_helpers.py
Outdated
| if pause_prio_list is not None: | ||
| for prio in pause_prio_list: | ||
| if prio < 0 or prio > 7: | ||
| logger.info('Invalid pause priorities {}'. |
There was a problem hiding this comment.
Replaced: logger.info --> logger.error
tests/common/ixia/ixia_helpers.py
Outdated
| traffic_config.FrameSize.FixedSize = 64 | ||
|
|
||
| if pkt_count is not None and duration is not None: | ||
| logger.info('You can only specify either pkt_count or duration') |
There was a problem hiding this comment.
Replaced: logger.info --> logger.error
tests/common/ixia/ixia_helpers.py
Outdated
|
|
||
| elif duration is not None: | ||
| if type(duration) != int or duration <= 0: | ||
| logger.info('Invalid duration value {} (positive integer only)'. |
There was a problem hiding this comment.
Replaced: logger.info --> logger.error
tests/common/ixia/ixia_helpers.py
Outdated
| for i in range(8): | ||
| pause_duration = pfc_stack_obj.\ | ||
| find(DisplayName='PFC Queue {}'.format(i)) | ||
|
|
There was a problem hiding this comment.
Removed the new line.
| find(DisplayName='PFC Queue {}'.format(i)) | ||
|
|
||
| pause_duration.ValueType = 'singleValue' | ||
|
|
There was a problem hiding this comment.
Removed the new line
| for i in range(8): | ||
| pause_duration = pfc_stack_obj.\ | ||
| find(DisplayName='PFC Queue {}'.format(i)) | ||
|
|
There was a problem hiding this comment.
Removed the new line.
tests/common/ixia/qos_fixtures.py
Outdated
|
|
||
| # Read the port QOS map. If pfc_enable flag is false then return None. | ||
| port_qos_map = config_facts["PORT_QOS_MAP"] | ||
| lossless_priorities = list() |
There was a problem hiding this comment.
Removed the statement "lossless_priorities = list()"
| START_DELAY = 1.5 | ||
|
|
||
| def run_pfc_exp(session, dut, tx_port, rx_port, port_bw, test_prio_list, | ||
| test_dscp_list, bg_dscp_list, exp_dur, start_delay=START_DELAY, |
There was a problem hiding this comment.
Arguments should be aligned.
There was a problem hiding this comment.
This change is implemented.
| pkt_per_sec (int): Packets per second. | ||
| pkt_count (int): Packet count. | ||
| duration (int): Traffic duration in second (positive integer only!). | ||
| start_delay (int): Start delay in second. |
There was a problem hiding this comment.
This is float. Change is implemented.
| if start_delay > 0: | ||
| traffic_config.TransmissionControl.update( | ||
| StartDelayUnits='nanoseconds', | ||
| StartDelay=start_delay*(10**6)) |
There was a problem hiding this comment.
I think start_delay can be a float. But we need to ensure that start_delay * 1000000 is an int,
There was a problem hiding this comment.
I am not changing this for the following reasons:
- It look quite awkward the people will give a value like start_delay 3.4623187 so the start_delay * 1000000 becomes an floating-point number (3462318.7).
- Even if start delay is some number like this 1.5, the start_delay * 1000000 becomes 1500000.0 the function which handles 1500000.0 should be able to handle 3462318.7
- The function handles "traffic_config.TransmissionControl.update" handles floating point numbers gracefully.
| 2. IXIA sends PFC pause frames from rx_port to pause priorities. | ||
| 3. Background traffic should not be interruped - all background traffic | ||
| will be received at the rx_port. | ||
| 4. No PFC traffic will be received at the rx_port when pause priority |
There was a problem hiding this comment.
PFC pause frames should always be dropped, regardless of their pause priorities.
There was a problem hiding this comment.
Comment added.
| start_delay (float): approximated initial delay to start the traffic. | ||
| test_traffic_pause_expected (bool): Do you expect test traffic to | ||
| be stopped? If yes, this should be true; false otherwise. | ||
| send_pause_frame (bool): True/False depending on whether you wand to |
There was a problem hiding this comment.
Change is implemented: wand -> want
| has the TC value m. | ||
| b. Background data traffic can assume priority m if 'Test Data Traffic' | ||
| has the TC value n. | ||
| c. Neither m or n when 'Test Data Traffic' has both the priorities. |
There was a problem hiding this comment.
This change is implemented.
| has the TC value n. | ||
| c. Neither m or n when 'Test Data Traffic' has both the priorities. | ||
| 6. Start 'Test Data Traffic' and 'Background Data Traffic'. | ||
| 7. From Rx port send pause frames on priorities either m or n. Such that |
There was a problem hiding this comment.
send pause frames on priority Pn
There was a problem hiding this comment.
This comment is implemented.
| b. Background data traffic can assume priority m if 'Test Data Traffic' | ||
| has the TC value n. | ||
| c. Neither m or n when 'Test Data Traffic' has both the priorities. | ||
| 6. Start 'Test Data Traffic' and 'Background Data Traffic'. |
There was a problem hiding this comment.
Test and background traffic should be started after PFC pause storm
There was a problem hiding this comment.
This comment is implemented.
| 6. Start 'Test Data Traffic' and 'Background Data Traffic'. | ||
| 7. From Rx port send pause frames on priorities either m or n. Such that | ||
| priority of 'Test Data Traffic' at Tx end == Pause Priority at Rx end. | ||
| 8. You may repeat the steps 6 and 7 several times. |
There was a problem hiding this comment.
Repeat the steps 6 and 7 to test each lossless priority.
There was a problem hiding this comment.
This comment is implemented
| test_traffic_pause_expected=True) | ||
|
|
||
| clean_configuration(session=session) | ||
|
|
There was a problem hiding this comment.
Can we split this into two functions?
There was a problem hiding this comment.
This comment is implemented. I have splatted the main function into two function as below:
test_pfc_pause_single_lossless_priority
test_pfc_pause_multi_lossless_priorities
|
retest this please |
tests/common/ixia/ixia_helpers.py
Outdated
|
|
||
| # Construct global pause and PFC packets. | ||
| if global_pause: | ||
| set_global_pause_fields(pfc_stack_obj) |
There was a problem hiding this comment.
The function should be set_global_pause_fields
| "PFC packets should be dropped") | ||
| else: | ||
| pytest_assert(tx_frames > 0 and tx_frames == rx_frames, | ||
| "Background traffic should not be impacted") |
There was a problem hiding this comment.
Why this check is required - can you please explain. Not implementing this change now.
To ensure that background traffic is not affected, we need to check if background traffic keeps sending packets at the target rate (e.g., rate_percent = 50). tx_frames > 0 can only indicate that background traffic delivers some packets.
| 4. The flow 'Test Data Traffic' can assume one of the lossless priority | ||
| values Pi. | ||
| 5. The flow 'Background Data Traffic' can assume all the priority values | ||
| which is not in 'Test Data Traffic'. For example if the priority of |
| 7. Start 'Test Data Traffic' and 'Background Data Traffic'. | ||
| 8. You may repeat step 6 and 7 for each lossless priorities. | ||
| 9. Expected result - | ||
| a. No 'Test Data Traffic' will not flow. Since priority of |
There was a problem hiding this comment.
No 'Test Data Traffic' will flow
| fanout_graph_facts): | ||
| """ | ||
| RDMA PFC - Pauses on multiple lossless priorities. | ||
| 1. On SONiC DUT enable PFC any two priorities Pm, Pn. (0 <= n<= 7) also |
There was a problem hiding this comment.
The step 1 seems incorrect to me as this test should be able to cover any number of priorities.
There was a problem hiding this comment.
This statement appears correct to me. Since this is describing actual test step. Where we are configuring two priorities.
There was a problem hiding this comment.
The code has nothing to do with two priorities. It can handle any number of priorities.
| test_dscp_list = [] | ||
| test_priority_list = [prio for prio in lossless_prio_dscp_map] | ||
| for prio in lossless_prio_dscp_map: | ||
| for p in lossless_prio_dscp_map[prio]: |
There was a problem hiding this comment.
You can use test_dscp_list += lossless_prio_dscp_map[prio] to directly merge the DSCP value list
| fanout_graph_facts): | ||
| """ | ||
| RDMA PFC - Pauses on multiple lossless priorities. | ||
| 1. On SONiC DUT enable PFC any two priorities Pm, Pn. (0 <= n<= 7) also |
There was a problem hiding this comment.
This test function should be able to cover more than two priorities. Can you modify the description correspondingly?
There was a problem hiding this comment.
This statement appears correct to me. Since this is describing actual test step. Where we are configuring two priorities.
| "Test traffic should be fully paused") | ||
| else: | ||
| pytest_assert(tx_frames > 0 and tx_frames == rx_frames, | ||
| "Test traffic should not be impacted") |
There was a problem hiding this comment.
Can you also check if tx_frames * PKT_SIZE is close to rate_percent (50/100) * duration * link capacity?
There was a problem hiding this comment.
An equivalent check is implemented.
| pytest_assert(tx_frames > 0 and rx_frames == 0, | ||
| "PFC packets should be dropped") | ||
| else: | ||
| pytest_assert(tx_frames > 0 and tx_frames == rx_frames, |
There was a problem hiding this comment.
Can you also check if tx_frames * PKT_SIZE is close to rate_percent (50/100) * duration * link capacity?
There was a problem hiding this comment.
An equivalent check is implemented.
| 7. When pause frames are started 'Test Data Traffic' will stop; | ||
| and when pause frames are stopped 'Test Data Traffic' will start. | ||
| 8. 'Background Data Traffic' will always flow. | ||
| 9. Repeat the steps 4 to 8 several times. |
There was a problem hiding this comment.
Why we need repeat here?
| 3. On the Ixia Tx port create two flows - a) 'Test Data Traffic' and | ||
| b) 'Background Data traffic'. | ||
| 4. Configure 'Test Data Traffic' such that it contains traffic items | ||
| of two lossless priorities Pm and Pn. |
There was a problem hiding this comment.
two lossless priorities -> all the lossless priorities.
| b) 'Background Data traffic'. | ||
| 4. Configure 'Test Data Traffic' such that it contains traffic items | ||
| of two lossless priorities Pm and Pn. | ||
| 5. Configure 'Background Data Traffic' it contains all other traffic |
There was a problem hiding this comment.
it contains all the lossy priorities. For example, if lossless priorities are 3 and 4, then the ...
| except priorities Pm and Pn. For example if Pm = 3 and Pn = 4 then | ||
| the priorities of the 'Background Data Traffic' should be 0, 1, 2, | ||
| 5, 6 and 7. | ||
| 6. From Rx port send pause frames on priorities both Pm and Pn. Then |
There was a problem hiding this comment.
send pause frames on all the lossless priorities.
tests/pfc/test_pfc_pause_lossless.py
Outdated
| session = ixia_api_server_session | ||
| send_pause_frame = False | ||
| for i in range(len(port_list)): | ||
| vports = configure_ports(session, port_list) |
There was a problem hiding this comment.
Add another loop 'for send_pause_frame in [True, False]':
tests/pfc/test_pfc_pause_lossless.py
Outdated
| 7. When pause frames are started 'Test Data Traffic' will stop; | ||
| and when pause frames are stopped 'Test Data Traffic' will start. | ||
| 8. 'Background Data Traffic' will always flow. | ||
| 9. Repeat the steps 4 to 8 several times. |
There was a problem hiding this comment.
several times -> all the ports
tests/pfc/test_pfc_pause_lossless.py
Outdated
| # Test pausing each lossless priority individually. | ||
| session = ixia_api_server_session | ||
| for prio in lossless_prio_dscp_map: | ||
| send_pause_frame = False |
There was a problem hiding this comment.
Add another loop: for send_pause_frame in [True, False]:
tests/pfc/test_pfc_pause_lossless.py
Outdated
| 'Test Data Traffic' at Tx end == Pause Priority at Rx end. That is, | ||
| send pause frames on priority Pi. | ||
| 7. Start 'Test Data Traffic' and 'Background Data Traffic'. | ||
| 8. You may repeat step 6 and 7 for each lossless priorities. |
tests/pfc/test_pfc_pause_lossless.py
Outdated
| "Test traffic should be fully paused") | ||
| else: | ||
| pytest_assert(tx_frames > 0 and tx_frames == rx_frames, | ||
| "Test traffic should not be impacted") |
There was a problem hiding this comment.
'Test traffic packets should not be dropped'
tests/pfc/test_pfc_pause_lossless.py
Outdated
| DATA_PKT_SIZE = 1024 | ||
| START_DELAY = 1.5 | ||
| RATE_PERCENTAGE = 50 | ||
| TOLERANCE_PARAMETER = .97 |
There was a problem hiding this comment.
TOLERANCE_PARAMETER -> TOLERANCE_THRESHOLD
| destination=topo_receiver, | ||
| pkt_size=DATA_PKT_SIZE, | ||
| duration=exp_dur, | ||
| rate_percent=RATE_PERCENTAGE, |
There was a problem hiding this comment.
RATE_PERCENTAGE -> 100 - RATE_PERCENTAGE
There was a problem hiding this comment.
Implemented.
# Assumption: Line rate percentage of background data traffic
# is equal to Line rate percentage of test data traffic.
pytest_assert(2 * RATE_PERCENTAGE <= 100,
"Value of RATE_PERCENTAGE should not be more than 50!")
tests/pfc/test_pfc_pause_lossless.py
Outdated
| flow_statistics = get_traffic_statistics(session) | ||
| logger.info(flow_statistics) | ||
|
|
||
| exp_tx_byte = (exp_dur * port_bw * 1000000 * (RATE_PERCENTAGE / 100.0)) / 8 |
There was a problem hiding this comment.
Test data traffic and background traffic may have different expected TX bytes.
There was a problem hiding this comment.
And please change 'exp_tx_byte' to 'exp_tx_bytes'
There was a problem hiding this comment.
implemented.
# Assumption: Line rate percentage of background data traffic
# is equal to Line rate percentage of test data traffic.
pytest_assert(2 * RATE_PERCENTAGE <= 100,
"Value of RATE_PERCENTAGE should not be more than 50!")
tests/pfc/test_pfc_pause_lossless.py
Outdated
| flow_statistics = get_traffic_statistics(session) | ||
| logger.info(flow_statistics) | ||
|
|
||
| exp_tx_byte = (exp_dur * port_bw * 1000000 * (RATE_PERCENTAGE / 100.0)) / 8 |
There was a problem hiding this comment.
And please change 'exp_tx_byte' to 'exp_tx_bytes'
| gw_start=gw_addr, | ||
| gw_incr_step='0.0.0.0') | ||
| start_protocols(session) | ||
|
|
There was a problem hiding this comment.
pytest_assert(RATE_PERCENTAGE <= 50,"xxxx")
tests/pfc/test_pfc_pause_lossless.py
Outdated
| (tolerance_ratio > 1)) : | ||
| logger.error("Expected Tx/Rx = %s actual Rx = %s" | ||
| %(exp_tx_byte, exp_tx_byte)) | ||
| %(exp_tx_bytes, exp_tx_bytes)) |
There was a problem hiding this comment.
the second one should be 'rx_bytes'
There was a problem hiding this comment.
Made the changed exp_tx_bytes -> rx_bytes
* e0b115a 2021-10-22 | [copp] add dhcpv6 copp rules (sonic-net#1979) (HEAD -> 201811, github/201811) [Ying Xie] Signed-off-by: Ying Xie <ying.xie@microsoft.com>
* Add DHCPv6 minigraph parsing support Co-authored-by: shlomibitton <60430976+shlomibitton@users.noreply.github.com> Logrotate for wtmp and btmp files to fix size getting too large. (sonic-net#8744) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com> [201811][utilities][swss][snmpagent] advance sub module head snmpagent * 187aa10 2021-09-16 | [201811][RFC1213]: Initialize lag oid map in reinit_data (sonic-net#233) (github/201811) [SuvarnaMeenakshi] swss: * 3503705 2021-09-05 | [201811][Cherry-pick] [acl mirror action] Mirror session ref count fix at acl rule attachment (sonic-net#1898) (HEAD -> 201811, github/201811) [bingwang-ms] utilities: * f3f8667 2021-10-15 | [201811] disk_check.py: Allow remote user access when disk is read-only (sonic-net#1873) (HEAD -> 201811, github/201811) [Renuka Manavalan] * 6b351c9 2021-10-14 | [201811] Remove exec from platform_reboot_plugin call to handle any hang issue. (sonic-net#1880) [Sujin Kang] * d8d0461 2021-07-29 | [minigraph][port_config] Consume port_config.json while reloading minigraph (sonic-net#1726) [Blueve] Signed-off-by: Ying Xie <ying.xie@microsoft.com> [201811] Invoke disk check periodically (sonic-net#8951) * Invoke disk check periodically. (sonic-net#7374) Why I did it Helps with periodic scan of disk for RO state. If found, this script makes transient fix and raise error message. Save DB dump after warm/fast reboot (sonic-net#8913) Back porting the master branch change - sonic-net#8803 Save the redis DB dump after warm reboot. [201811][swss] advance swss submodule head (sonic-net#9049) * e0b115a 2021-10-22 | [copp] add dhcpv6 copp rules (sonic-net#1979) (HEAD -> 201811, github/201811) [Ying Xie] Signed-off-by: Ying Xie <ying.xie@microsoft.com> [swssconfig] load dhcpv6 copp rules by default (sonic-net#9047) Why I did it Need to enable DHCPv6 copp rule How I did it Add a separate DHCPv6 copp rule config file and load it during cold reboot. How to verify it cold reboot, and verify config being loaded and dhcpv6 rules got installed. Signed-off-by: Ying Xie ying.xie@microsoft.com [warmboot finalizer] load dhcpv6 copp rules when missing (sonic-net#9048) Why I did it Need to enable DHCPv6 COPP rules. How I did it Load the separate DHCPv6 COPP rules after warm reboot if the rules are missing. How to verify it Warm reboot from an image doesn't have DHCPv6 COPP rules installed. Warm reboot from an image have DHCPv6 COPP rules already installed. In either case, the script did the right thing and only install the COPP rules if it is missing. Signed-off-by: Ying Xie ying.xie@microsoft.com
Description of PR
Summary:
Fixes # (issue)
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