Update `icmp_responder` to only respond only linkmgrd requests by zjswhhh · Pull Request #9089 · sonic-net/sonic-mgmt

zjswhhh · 2023-07-21T22:50:12Z

Description of PR

Summary:
Fixes # (issue)

To make sure icmp_responder only replies ICMP requests sent by linkmgrd, So

it won't reply packets from other tests.
to be consistent with production behavior

sign-off: Jing Zhang zhangjing@microsoft.com

Type of change

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

Back port request

201911
202012
202205

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Run test_normal_op case.

- generated xml file: /var/src/sonic-mgmt-int/tests/logs/dualtor_io/test_normal_op.py::test_normal_op_upstream.xml -
---------------------------- live log sessionfinish ----------------------------
21:39:05 __init__.pytest_terminal_summary         L0064 INFO   | Can not get Allure report URL. Please check logs
=========================== short test summary info ============================
SKIPPED [1] /var/src/sonic-mgmt-int/tests/common/dualtor/dual_tor_common.py:54: Skip as no mux ports of 'active-active' cable type
==================== 1 passed, 1 skipped in 643.57 seconds =====================

Any platform specific information?

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

Documentation

lolyu

Thanks for the fix

…-net#9089) Summary: Fixes # (issue) To make sure `icmp_responder` only replies ICMP requests sent by linkmgrd, So 1. it won't reply packets from other tests. 2. to be consistent with production behavior sign-off: Jing Zhang zhangjing@microsoft.com

mssonicbld · 2023-08-04T07:55:10Z

Cherry-pick PR to 202205: #9293

Summary: Fixes # (issue) To make sure `icmp_responder` only replies ICMP requests sent by linkmgrd, So 1. it won't reply packets from other tests. 2. to be consistent with production behavior sign-off: Jing Zhang zhangjing@microsoft.com

…unt for sent and rcvd packets (#9599) Summary: Currently dual tor warmboot fails w/ control plane not ready for warmboot. This is because the test expects 2 responses for every ping packet sent to Loopback. This expectation is not correct after PR #9089 is merged. Update the testcase to now only expect 1 icmp response for every icmp packet sent to Loopback0. Updated the test to remove the earlier expectation of removing the packets that were sent by icmp_responder. This essentially reverts part of older PR: #6968 How did you verify/test it? Tested on a physical testbed and this test issue is not seen. The test is now able to see the responses of ping packets

…unt for sent and rcvd packets (sonic-net#9599) Summary: Currently dual tor warmboot fails w/ control plane not ready for warmboot. This is because the test expects 2 responses for every ping packet sent to Loopback. This expectation is not correct after PR sonic-net#9089 is merged. Update the testcase to now only expect 1 icmp response for every icmp packet sent to Loopback0. Updated the test to remove the earlier expectation of removing the packets that were sent by icmp_responder. This essentially reverts part of older PR: sonic-net#6968 How did you verify/test it? Tested on a physical testbed and this test issue is not seen. The test is now able to see the responses of ping packets

…unt for sent and rcvd packets (#9599) Summary: Currently dual tor warmboot fails w/ control plane not ready for warmboot. This is because the test expects 2 responses for every ping packet sent to Loopback. This expectation is not correct after PR #9089 is merged. Update the testcase to now only expect 1 icmp response for every icmp packet sent to Loopback0. Updated the test to remove the earlier expectation of removing the packets that were sent by icmp_responder. This essentially reverts part of older PR: #6968 How did you verify/test it? Tested on a physical testbed and this test issue is not seen. The test is now able to see the responses of ping packets

…unt for sent and rcvd packets (sonic-net#9599) Summary: Currently dual tor warmboot fails w/ control plane not ready for warmboot. This is because the test expects 2 responses for every ping packet sent to Loopback. This expectation is not correct after PR sonic-net#9089 is merged. Update the testcase to now only expect 1 icmp response for every icmp packet sent to Loopback0. Updated the test to remove the earlier expectation of removing the packets that were sent by icmp_responder. This essentially reverts part of older PR: sonic-net#6968 How did you verify/test it? Tested on a physical testbed and this test issue is not seen. The test is now able to see the responses of ping packets

…unt for sent and rcvd packets (#9599) (#10068) Raising PR for cherry-pick conflict: 225e257 [advanced-reboot][dualtor] Update Loopback0 ping test: expect same count for sent and rcvd packets (#9599) Summary: Currently dual tor warmboot fails w/ control plane not ready for warmboot. This is because the test expects 2 responses for every ping packet sent to Loopback. This expectation is not correct after PR #9089 is merged. Update the testcase to now only expect 1 icmp response for every icmp packet sent to Loopback0. Updated the test to remove the earlier expectation of removing the packets that were sent by icmp_responder. This essentially reverts part of older PR: #6968 How did you verify/test it? Tested on a physical testbed and this test issue is not seen. The test is now able to see the responses of ping packets Co-authored-by: Vaibhav Hemant Dixit <vaibhav.dixit@microsoft.com>

…-net#9089) Summary: Fixes # (issue) To make sure `icmp_responder` only replies ICMP requests sent by linkmgrd, So 1. it won't reply packets from other tests. 2. to be consistent with production behavior sign-off: Jing Zhang zhangjing@microsoft.com

…unt for sent and rcvd packets (sonic-net#9599) Summary: Currently dual tor warmboot fails w/ control plane not ready for warmboot. This is because the test expects 2 responses for every ping packet sent to Loopback. This expectation is not correct after PR sonic-net#9089 is merged. Update the testcase to now only expect 1 icmp response for every icmp packet sent to Loopback0. Updated the test to remove the earlier expectation of removing the packets that were sent by icmp_responder. This essentially reverts part of older PR: sonic-net#6968 How did you verify/test it? Tested on a physical testbed and this test issue is not seen. The test is now able to see the responses of ping packets

src host ip

01a8358

zjswhhh requested review from bingwang-ms and lolyu July 21, 2023 23:13

zjswhhh marked this pull request as ready for review July 24, 2023 21:40

zjswhhh requested a review from yxieca July 24, 2023 21:40

zjswhhh added the Request for 202205 branch label Jul 24, 2023

lolyu approved these changes Jul 25, 2023

View reviewed changes

zjswhhh merged commit 44ee0ca into sonic-net:master Jul 25, 2023

wangxin added the Approved for 202205 branch label Jul 27, 2023

wangxin added Approved for 202205 branch and removed Approved for 202205 branch labels Aug 4, 2023

mssonicbld added the Created PR to 202205 branch label Aug 4, 2023

mssonicbld mentioned this pull request Aug 4, 2023

[action] [PR:9089] Update icmp_responder to only respond only linkmgrd requests #9293

Merged

6 tasks

mssonicbld added Included in 202205 branch and removed Created PR to 202205 branch labels Aug 4, 2023

vaibhavhd mentioned this pull request Aug 22, 2023

[advanced-reboot][dualtor] Update Loopback0 ping test: expect same count for sent and rcvd packets #9599

Merged

6 tasks

mssonicbld mentioned this pull request Aug 29, 2023

[action] [PR:9599] [advanced-reboot][dualtor] Update Loopback0 ping test: expect same count for sent and rcvd packets #9727

Merged

6 tasks

zjswhhh deleted the icmp_reponder_public_master branch July 31, 2024 22:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `icmp_responder` to only respond only linkmgrd requests#9089

Update `icmp_responder` to only respond only linkmgrd requests#9089
zjswhhh merged 1 commit intosonic-net:masterfrom
zjswhhh:icmp_reponder_public_master

zjswhhh commented Jul 21, 2023 •

edited by lolyu

Loading

Uh oh!

lolyu left a comment

Uh oh!

mssonicbld commented Aug 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

zjswhhh commented Jul 21, 2023 • edited by lolyu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

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

Uh oh!

lolyu left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Aug 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zjswhhh commented Jul 21, 2023 •

edited by lolyu

Loading