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

[202012] Fix issue: sometimes PFC WD unable to create zero buffer pool #2185

Merged
merged 1 commit into from
Apr 2, 2022

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Mar 16, 2022

This is to backport PR #2164 to 202012 branch.

What I did

Fix issue: sometimes PFC WD is unable to create zero buffer pool.
On some platforms, an ingress/egress zero buffer profile will be applied on the PG and queue which are under PFC storm. The zero buffer profile is created based on zero buffer pool. However, sometimes it fails to create zero buffer pool due to too many buffer pools existing in the system.
Sometimes, there is a zero buffer pool existing on the system for reclaiming buffer. In that case, we can leverage it to create zero buffer profile for PFC WD.

Why I did it

Fix the issue via sharing the zero buffer pool between PFC WD and buffer orchagent

How I verified it

Manually test
Run PFC WD test and PFC WD warm reboot test
Run unit test

Details if related
The detailed flow is like this:
PFC Storm detected:

  1. If there is a zero pool in PFC WD's cache, just create the zero buffer profile based on it
  2. Otherwise, fetching the zero pool from buffer orchagent
    • If got one, create the zero buffer profile based on it
    • Otherwise,
      • create a zero buffer pool
      • notify the zero buffer pool about the buffer orch
    • In both cases, PFC WD should notify buffer orch to increase the reference number of the zero buffer pool.

Buffer orchagent:

  • When creating the zero buffer pool,
    • check whether there is one. if yes, skip the SAI API create_buffer_pool
    • increase the reference number.
  • Before removing the zero buffer pool, decrease and check the reference number. if it is zero (after decreased), skip SAI API destroy_buffer_pool.
  • When PFC WD decrease reference number: remove the zero buffer pool if the reference number becomes zero

Notes
We do not leverage the object_reference_map infrastructure to track the dependency because:

  • it assumes the dependency will eventually be removed if an object is removed. that's NOT true in this scenario because the PFC storm can last for a relatively long time and even cross warm reboot.
  • the interfaces differ.

What I did

Why I did it

How I verified it

Details if related

**What I did**

Fix issue: sometimes PFC WD is unable to create zero buffer pool.
On some platforms, an ingress/egress zero buffer profile will be applied on the PG and queue which are under PFC storm. The zero buffer profile is created based on zero buffer pool. However, sometimes it fails to create zero buffer pool due to too many buffer pools existing in the system.
Sometimes, there is a zero buffer pool existing on the system for reclaiming buffer. In that case, we can leverage it to create zero buffer profile for PFC WD.

**Why I did it**

Fix the issue via sharing the zero buffer pool between PFC WD and buffer orchagent

**How I verified it**

Manually test
Run PFC WD test and PFC WD warm reboot test
Run unit test

**Details if related**
***The detailed flow is like this:***
PFC Storm detected:
1. If there is a zero pool in PFC WD's cache, just create the zero buffer profile based on it
2. Otherwise, fetching the zero pool from buffer orchagent
   - If got one, create the zero buffer profile based on it
   - Otherwise,
     - create a zero buffer pool
     - notify the zero buffer pool about the buffer orch
   - In both cases, PFC WD should notify buffer orch to increase the reference number of the zero buffer pool.

Buffer orchagent:
- When creating the zero buffer pool,
  - check whether there is one. if yes, skip the SAI API create_buffer_pool
  - increase the reference number.
- Before removing the zero buffer pool, decrease and check the reference number. if it is zero (after decreased), skip SAI API destroy_buffer_pool.
- When PFC WD decrease reference number: remove the zero buffer pool if the reference number becomes zero

***Notes***
We do not leverage the `object_reference_map` infrastructure to track the dependency because:
 - it assumes the dependency will eventually be removed if an object is removed. that's NOT true in this scenario because the PFC storm can last for a relatively long time and even cross warm reboot.
 - the interfaces differ.
@stephenxs stephenxs requested a review from neethajohn March 16, 2022 23:42
@stephenxs stephenxs marked this pull request as draft March 16, 2022 23:43
@stephenxs
Copy link
Collaborator Author

This is to backport PR #2164 to 202012 branch.

@liat-grozovik
Copy link
Collaborator

@neethajohn LGTM. Can you please signoff?

@liat-grozovik
Copy link
Collaborator

@neethajohn kindly reminder to review and signoff

@liat-grozovik liat-grozovik merged commit 13ccaba into sonic-net:202012 Apr 2, 2022
@stephenxs stephenxs deleted the fix-pfcwd-202012 branch April 2, 2022 07:32
@svsivm
Copy link

svsivm commented Jun 7, 2022

Hi, While I understand the motivation behind the buffer pool changes, the commit message does not mention anything about why the zero buffer profile attribute values have been changed. For example, the SAI_BUFFER_PROFILE_ATTR_THRESHOLD_MODE attribute value was set to SAI_BUFFER_PROFILE_THRESHOLD_MODE_DYNAMIC with the threshold SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH being programmed as '-8' for zero buffer profile. Why has this been changed now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants