Added tests to verify MAX MTU packet handling by prsunny · Pull Request #280 · sonic-net/sonic-mgmt

prsunny · 2017-09-21T00:05:49Z

Test sends a jumbo ICMP echo frame with MAX MTU size and expects the reply back. It also sends a jumbo frame to a route destination for verifying the forwarding functionality.

For the device configured with IP-MTU=9100, PHY-MTU=9122,

ICMP frame, the packet-len is 9114 (Subtracting 8 bytes of ICMP header from 9122)
IP frame, the packet-len is 9122 (This includes the Layer 2 Ethernet header + FCS)

Note: For the test case to run, the PTF option must be set for socket buffer size to 16384

msftclas · 2017-09-21T00:06:03Z

@prsunny,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

lguohan · 2017-09-21T06:20:05Z

ansible/roles/test/files/ptftests/mtu_test.py

+
+    def check_icmp_mtu(self):
+        '''
+        @summary: Check ICMP/Ping works for MAX MTU.


more verbose description.

Check ICMP/Ping DUT works for MAX MTU

Sure. Will modify the description

lguohan · 2017-09-21T06:23:05Z

ansible/roles/test/files/ptftests/mtu_test.py

+        src_port = 0
+        send_packet(self, src_port, pkt)
+        logging.info("Sending packet from port " + str(src_port) + " to " + ip_dst)
+        dst_port_list = [0,1]


why are there two ports in the list? the icmp reply packet should arrive from the src_port as well. It should not be received at other ports.

In t1-lag topology, 0 and 1 are part of PortChannel0. The reply packet can be received on either port. In t1 topology, the ICMP reply will be received on 0

lguohan · 2017-09-21T06:26:04Z

ansible/roles/test/files/ptftests/mtu_test.py

+        masked_exp_pkt.set_do_not_care_scapy(scapy.Ether, "dst")
+        masked_exp_pkt.set_do_not_care_scapy(scapy.IP, "id")
+        masked_exp_pkt.set_do_not_care_scapy(scapy.IP, "chksum")
+        masked_exp_pkt.set_do_not_care_scapy(scapy.ICMP, "chksum")


maybe we should match the ttl as well, it is not guaranteed that the icmp reply also use ttl=64. You do not want that factor to fail your test.

Yes. Will add "ttl" to _do_not_care field in the reply packet

lguohan · 2017-09-21T06:28:12Z

ansible/roles/test/files/ptftests/mtu_test.py

+
+        pktlen = (self.DEFAULT_PACKET_LEN - self.ICMP_HDR_LEN)
+
+        pkt = simple_icmp_packet(pktlen=pktlen,


should we set the DF bit in the packet to make sure we are not fragment the packet anywhere and we were not really testing the MTU sized packet?

I don't see a param in the API to set the DF bit. Will discuss with you!

lguohan · 2017-09-21T06:32:42Z

ansible/roles/test/tasks/mtu.yml

+  minigraph_facts: host={{ inventory_hostname }}
+  connection: local
+
+- debug : msg="Start MTU Test"


can we first get the mtu Ethernet0 on the DUT use the mtu as a parameter to send the packet. Then, it makes our test more generic?

You can get the mtu via ansible_facts, check below.

https://stackoverflow.com/questions/18839509/where-can-i-get-a-list-of-ansible-pre-defined-variables

This shall be addressed in the next phase commit

…epare_test_port fixture (sonic-net#280)  ### Description of PR  Summary: Fixes sonic-net#18291 ### Type of change  - [x] 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? Fix bug, and let code run by design. #### How did you do it? Adjust the check condition for if there is portchannel. #### How did you verify/test it? Verified by running local test on testbed with no portchannel, and was erroring out before the fix. ![image](https://github.com/user-attachments/assets/8a81f43a-8066-4768-852b-1cffa0ebf7f1) #### Any platform specific information? #### Supported testbed topology if it's a new test case? ### Documentation

) ``` 81505c8 (az_upstream/202412) Merge branch '202411' of https://github.com/sonic-net/sonic-mgmt into 202412 30b7d73 (pub_upstream/202411) [202411 ] Updating the BGP, Reboot and LACP convergence cases to latest Snappi Api Model instead of Snappi_convergence module [ PR 18044 ] (sonic-net#18391) e146100 [action] [PR:18292] Fix Skip tests if no portchannels are found in prepare_test_port fixture (sonic-net#280) d8b9193 [action] [PR:17985] [tests_mark_conditions.yaml]: Skip GNMI/ZMQ test for isolated topology (sonic-net#281) ef951e6 [action] [PR:17943] Add default routes to t1-isolated-d128 (sonic-net#282) 2d3aec5 [action] [PR:18341] [route/test_route_perf] Support reading l3_alpm_template from config.bcm for TH5 platforms (sonic-net#284) 6390c7d [202412][PR: 18350] disable default pfcwd for all lossy DUT (sonic-net#271) b621644 [Cherry-pick] modifying Ignore message for 'Failed to get port by bridge port ID' (sonic-net#263) bc9a2d2 modifying Ignore message for 'Failed to get port by bridge port ID' ```

…nic-net#10938) Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com> This PR adds the following commits are added in sonic-platform-common ac25515 (HEAD -> 202012, origin/202012) [Credo][Ycable] changes for synchronizing executing Telemetry API's when mux toggle is inprogress (sonic-net#280) fb304c1 [sonic_ssd] Nokia-7215: "show platform ssdhealth" not showing health percent (sonic-net#279) 8bfe9c0 [Credo][Ycable] improve logging for Server Powered off/Faulty cables (sonic-net#272) The following commits are added in sonic-platform-daemons 6695c55 (HEAD -> 202012, origin/202012) [ycabled] fix the posting for mux_cable_static_info per downlink when ycabled is spawned; synchronizing executing Telemetry API (sonic-net#257) 6a315ae (HEAD -> 202012, origin/202012) [ycabled] Fix some syntax warnings in ycabled (sonic-net#263)

…onic-net#11684) The follwing commit is added in sonic-utilities b034f0c (HEAD -> 202012, origin/202012) [config][muxcable] add support to enable/disable ycable telemetry (sonic-net#2… (sonic-net#2304) The follwing commit is added in sonic-platform-daemons 978667c (HEAD -> 202012, origin/202012) [ycabled] add capability to enable/disable telemetry (sonic-net#279) (sonic-net#280 Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com> Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

#### Why I did it Update sonic-snmpagent submodule to include below commit: Revert "[202012]: snmp vlan support per RFC1213 and added the missing support for RFC2863 (sonic-net#279)" (sonic-net#280)

linkmgrd: * 3c2b546 2022-05-31 | Add default route support to `active-active` state machine (sonic-net#78) (github/202205, master, 202205) [Jing Zhang] * 6fa892e 2022-05-27 | Degrade `LinkProberStateMachineBase` virtual function logging level (sonic-net#80) [Longxiang Lyu] * 7b695ca 2022-05-27 | Fix mux wait timer and peer mux wait timer (sonic-net#81) [Longxiang Lyu] platform-daemons: * 0d90023 2022-05-31 | grpc client implementation for active-active dualtor (sonic-net#248) (github/master, github/202205, master, 202205) [vdahiya12] * 6b8bf69 2022-05-27 | [ycabled] Fix some syntax warnings in ycabled (sonic-net#263) [vdahiya12] * 2bcf936 2022-05-24 | [ycabled] fix the posting for mux_cable_static_info per downlink when ycabled is spawned; synchronizing executing Telemetry API (sonic-net#257) [vdahiya12] * ce217c0 2022-04-25 | Include changes from xcvr_api in transceiver_info table (sonic-net#253) [qinchuanares] * e0f8a35 2022-04-22 | Fix checkReplyType failed issue via recreating xcvr_table_helper on forking subprocess (sonic-net#255) [Stephen Sun] platform-common: * f575a40 2022-05-24 | [Credo][Ycable] changes for synchronizing executing Telemetry API's when mux toggle is inprogress (sonic-net#280) (github/202205, master, 202205) [vdahiya12] * b043372 2022-05-11 | [sonic_ssd] Nokia-7215: "show platform ssdhealth" not showing health percent (sonic-net#279) [bill-nokia] * d62d3d6 2022-05-04 | [CMIS]Fix low-power to high power mode transition (sonic-net#268) [Prince George] * f918125 2022-05-02 | [syseeprom] Enable display of vendor extension TLV content (sonic-net#270) [dflynn-Nokia] * 4e08440 2022-04-14 | [Credo][Ycable] improve logging for Server Powered off/Faulty cables (sonic-net#272) [vdahiya12] Signed-off-by: Ying Xie <ying.xie@microsoft.com>

Added tests to verify MAX MTU packet handling

f3b34ef

msftclas added the cla-not-required label Sep 21, 2017

prsunny mentioned this pull request Sep 21, 2017

Add test case to check Max MTU #279

Closed

lguohan self-requested a review September 21, 2017 00:39

lguohan reviewed Sep 21, 2017

View reviewed changes

lguohan requested changes Sep 21, 2017

View reviewed changes

Addressed first set of review comments

593a1f5

lguohan approved these changes Sep 21, 2017

View reviewed changes

prsunny merged commit 1c5f6cc into sonic-net:master Sep 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added tests to verify MAX MTU packet handling#280

Added tests to verify MAX MTU packet handling#280
prsunny merged 2 commits intosonic-net:masterfrom
prsunny:master

prsunny commented Sep 21, 2017

Uh oh!

msftclas commented Sep 21, 2017

Uh oh!

lguohan Sep 21, 2017

Uh oh!

prsunny Sep 21, 2017

Uh oh!

lguohan Sep 21, 2017

Uh oh!

prsunny Sep 21, 2017

Uh oh!

lguohan Sep 21, 2017

Uh oh!

prsunny Sep 21, 2017

Uh oh!

lguohan Sep 21, 2017

Uh oh!

prsunny Sep 21, 2017

Uh oh!

lguohan Sep 21, 2017

Uh oh!

prsunny Sep 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		pktlen = (self.DEFAULT_PACKET_LEN - self.ICMP_HDR_LEN)

		pkt = simple_icmp_packet(pktlen=pktlen,

Conversation

prsunny commented Sep 21, 2017

Uh oh!

msftclas commented Sep 21, 2017

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants