Skip to content

Fix dhcpv6 counter test failure#6953

Merged
jcaiMR merged 2 commits intosonic-net:masterfrom
jcaiMR:dev/fix_test_dhcpv6_counter
Dec 17, 2022
Merged

Fix dhcpv6 counter test failure#6953
jcaiMR merged 2 commits intosonic-net:masterfrom
jcaiMR:dev/fix_test_dhcpv6_counter

Conversation

@jcaiMR
Copy link
Copy Markdown
Contributor

@jcaiMR jcaiMR commented Dec 3, 2022

Description of PR

Summary:
Fixes # (issue)

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?

Fix test dhcpv6 counter failure
Dual tor issue:

  1. Solicit counter not increase, random select dut will select standby port dut
  2. Advertise counter not increase, downlink mac is not right, should always dut_mac

Single Tor issue:
Decline/Release/Information request/unknow counter not increase
test case send 14 kinds of packets almost at the same time.
As it's multicast packets, each packet get duplicated on fanout switch (maybe due to some topo setting), on one of local DUT,
observed 23 times duplicated packets (around 270 pakcets) send in 22 ms.
By default raw socket recv buffer size is 32k (270x128B = 34.5K), so some kinds of packets (always the packets at the last sequence like Release, Decline, Information request ...) get drooped and cause test case failure.

image

image

How did you do it?

  1. toggle_all_simulator_ports_to_upper_tor and always use upper tor to avoid mux status inconsistent issue
  2. use dut_mac for downlink interface mac (always same as port channel mac)
  3. Add 1 second delay during testing for each kind of packets, in case any multicast packet duplicate issue happened

How did you verify/test it?

Test on single tor and dual tor DUT.

Any platform specific information?

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

Documentation

@kellyyeh kellyyeh self-requested a review December 7, 2022 17:43
kellyyeh
kellyyeh previously approved these changes Dec 7, 2022
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Dec 7, 2022

This pull request fixes 1 alert when merging 4769461797b76f5d4141a1aa782de02851ac0e36 into c05753c - view on LGTM.com

fixed alerts:

  • 1 for Unused import

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Dec 8, 2022

This pull request fixes 1 alert when merging 90c632145d841b5f9ab15f7b5b6b03c14e363eba into fec265e - view on LGTM.com

fixed alerts:

  • 1 for Unused import

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

StormLiangMS
StormLiangMS previously approved these changes Dec 12, 2022
Copy link
Copy Markdown
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with a minor suggestion.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that 300 seconds delay too long? maybe 60 seconds do the job? @jcaiMR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, 60 should enough here, will apply in diff.

Copy link
Copy Markdown
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wangxin
Copy link
Copy Markdown
Collaborator

wangxin commented Dec 19, 2022

@jcaiMR Cherry-picking this PR to 202012 branch has conflicts that are hard to resolve. Can you create separate PR to 202012 branch to have this change?

wangxin pushed a commit that referenced this pull request Dec 19, 2022
* fix test dhcpv6 counter issue, fix dual-tor issue

* add missing import for ptf copy
jcaiMR added a commit that referenced this pull request Jan 18, 2023
double commit #6953, fix dhcpv6 counter test issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants