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

[Mellanox] PFC watchdog long term solution to reduce false alarm #3036

Merged
merged 2 commits into from
May 9, 2024

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Feb 2, 2024

What I did

Adjust PFC watchdog detection algorithm to reduce false alarms.

In the old PFC watchdog detection algorithm, the PFC watchdog can be triggered if either of the following conditions is satisfied in a detection interval:

  1. There are packets accumulated in the queue && there is no packet sent out of the queue && there are PFC frames received in the queue
  2. There is no packet accumulated in the queue and there are PFC frames received and blocking more than 80% of the detection interval.

The new PFC watchdog detection algorithm merges two conditions into one:
The PFC watchdog is triggered only if:

  • There are packets accumulated in the queue && there is no packet sent out of the queue && there are PFC frames received and blocking more than 99% of the detection interval.

Signed-off-by: Stephen Sun stephens@nvidia.com

Why I did it

There are some rare scenarios in which the PFC watchdog can be mistriggered

  1. No PFC frames were received until the last moment of the polling period
  2. In accurate polling interval due to unexpected delay introduced after counter-polling in the previous polling interval and before counter-polling in the next polling interval
  3. In accurate polling interval due to unexpected delay introduced after counter-polling in a polling interval and before the lua script invoking in the same polling interval

Scenarios 1 and 2 are addressed in this PR.

How I verified it

Run PFC watchdog regression test with background traffic.

Details if related

@bingwang-ms
Copy link
Contributor

Can you please help me understand why we set the criteria to more than 99% of the detection interval is blocked? If the condition packets - packets_last == 0 is true, does that mean the queue is already fully paused?

@stephenxs
Copy link
Collaborator Author

Can you please help me understand why we set the criteria to more than 99% of the detection interval is blocked? If the condition packets - packets_last == 0 is true, does that mean the queue is already fully paused?

Hi @bingwang-ms
Yes
By setting a strict criteria, we can avoid false alarm as much as possible.
packets - packets_last == 0 means no packets were sent during the last interval but it can be caused by multiple reasons:

  • no packets to be sent, which can be ruled out by buffer occupancy > 0
  • egress scheduling algorithm, which is ruled out by 99% time was blocked by PFC

@bingwang-ms
Copy link
Contributor

@neethajohn Can you please help review this PR? Thanks

@liat-grozovik
Copy link
Collaborator

@neethajohn kindly reminder. this is supposed to get into 202405 and we wish to close it with prio.

@neethajohn
Copy link
Contributor

neethajohn commented May 8, 2024

Changes look good to me. @stephenxs , will the current sonic-mgmt pfcwd functionality tests need enhancement with the new logic ?

@stephenxs
Copy link
Collaborator Author

Changes look good to me. @stephenxs , will the current sonic-mgmt pfcwd functionality tests need enhancement with the new logic ?

@neethajohn yes sonic-net/sonic-mgmt#12733

Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@prsunny prsunny merged commit f62eed2 into sonic-net:master May 9, 2024
17 checks passed
@stephenxs stephenxs deleted the pfcwd-long-term branch May 9, 2024 21:43
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.

None yet

5 participants