Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[static route] Extend config reload wait time for static route tests #3749

Merged
merged 1 commit into from
Jul 9, 2021

Conversation

shi-su
Copy link
Contributor

@shi-su shi-su commented Jul 7, 2021

Description of PR

Summary: Extend config reload wait time for static route tests
Fixes sonic-net/sonic-buildimage#7968

Type of change

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

Back port request

  • 201911

Approach

What is the motivation for this PR?

The arp_update script updates unresolved neighbors every 300 seconds. As the default config reload wait time is 120 seconds, the arp_update script may not finish the neighbor resolution before the config reload wait time and cause the static route test to fail. This PR aims to fix the timing issue for static route tests.

The test case test_static_route_ecmp_ipv6 may be affected by sonic-net/sonic-buildimage#4930, so the test case may still fail in some scenarios

Note:

How did you do it?

Extend config reload wait time for static route tests to 350 seconds.

How did you verify/test it?

Confirm that test_static_route_ecmp test case could pass with neighbors not resolved right after config reload.

Any platform specific information?

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

Documentation

@shi-su shi-su requested a review from a team as a code owner July 7, 2021 22:47
@shi-su shi-su requested a review from prsunny July 7, 2021 22:48
@kevinskwang
Copy link
Collaborator

I think there is still a issue need to be addressed. If you check the accuracy ARP/NDP that the DUT learned, you can see all three IP neighbors will have the same MAC address. So it will always send to the port which has this MAC, the other two ports can't even receive any packets. It should not the ECMP's purpose.

@shi-su
Copy link
Contributor Author

shi-su commented Jul 8, 2021

I think there is still a issue need to be addressed. If you check the accuracy ARP/NDP that the DUT learned, you can see all three IP neighbors will have the same MAC address. So it will always send to the port which has this MAC, the other two ports can't even receive any packets. It should not the ECMP's purpose.

Thanks for this comment. Will look into this behavior and circle back.

@prsunny prsunny merged commit 74e37bb into sonic-net:master Jul 9, 2021
@shi-su
Copy link
Contributor Author

shi-su commented Jul 14, 2021

I think there is still a issue need to be addressed. If you check the accuracy ARP/NDP that the DUT learned, you can see all three IP neighbors will have the same MAC address. So it will always send to the port which has this MAC, the other two ports can't even receive any packets. It should not the ECMP's purpose.

The changes in this PR #3791 should address the issue for APR learned in ipv4 tests. For ipv6 tests, the neighbors seem to be right.

vmittal-msft pushed a commit to vmittal-msft/sonic-mgmt that referenced this pull request Sep 28, 2021
*The arp_update script updates unresolved neighbors every 300 seconds. As the default config reload wait time is 120 seconds, the arp_update script may not finish the neighbor resolution before the config reload wait time and cause the static route test to fail. This PR aims to fix the timing issue for static route tests.
*Fix: Extend config reload wait time for static route tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[static route] No arp entries for nexthops in the static route after config reload
4 participants