Skip to content

[action] [PR:15772] Cisco-8000 test pfcwd function background traffic revision#15841

Merged
mssonicbld merged 1 commit intosonic-net:202405from
mssonicbld:cherry/202405/15772
Dec 11, 2024
Merged

[action] [PR:15772] Cisco-8000 test pfcwd function background traffic revision#15841
mssonicbld merged 1 commit intosonic-net:202405from
mssonicbld:cherry/202405/15772

Conversation

@mssonicbld
Copy link
Copy Markdown
Collaborator

Description of PR

Summary:

  • Revise Cisco-8000's current method of sending background traffic from an extra "verify_tx_egress" call to use the pre-existing background traffic support. The verify tx egress operation could fail if some packets leakout due to slow pfc_gen.py frames from fanout.
  • Remove the test failure catcher, as it simply rethrows an exception that is more difficult to debug. Using a simple "try-finally" pattern allows the "finally" clause to always execute even with an exception, but also continues to throw the exception without catching it, resulting in the required test failure in that case.

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Related PRs

#14711 is not in 202405, but is requested for the branch. Without that PR in 202405, the double commit of this PR will have a either have a merge conflict or possibly fail if it happens to not conflict. Possible to get that PR in 202405?

Approach

What is the motivation for this PR?

Fix flaky test on Cisco-8000.
Clarify exceptions that do occur for all vendors.

How did you do it?

How did you verify/test it?

Verified pass 10 times on Cisco-8122 T0 on 202405 branch. But master/202405 aren't in sync yet due to missing 14711 PR.

Any platform specific information?

The verify_tx_egress change is cisco-only.
The exception catcher removal should improve exception readability for all vendors.

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

Documentation

@mssonicbld
Copy link
Copy Markdown
Collaborator Author

Original PR: #15772

@rbpittman
Copy link
Copy Markdown
Contributor

Do not merge, this will cause a test failure in 202405 for cisco without #14711.

@auspham
Copy link
Copy Markdown
Contributor

auspham commented Dec 9, 2024

Hi @rbpittman please confirm if this is ready to merge

@rbpittman
Copy link
Copy Markdown
Contributor

@auspham This is not ready to merge.
There's a subtle "conflict" that doesn't show up as a code conflict, merging without the PR you just messaged on will cause a variable-not-defined exception for Cisco.

@auspham auspham mentioned this pull request Dec 9, 2024
8 tasks
@auspham
Copy link
Copy Markdown
Contributor

auspham commented Dec 9, 2024

Hi @rbpittman i have merged #14711 for you. I'll retrigger this CI,

Meanwhile could you please help confirming if it should be okay to merge now.

@yejianquan
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld mssonicbld merged commit f3dddbc into sonic-net:202405 Dec 11, 2024
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.

4 participants