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

Support shared headroom pool on top of dynamic buffer calculation #1581

Merged
merged 5 commits into from
Feb 7, 2021

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Jan 6, 2021

What I did
Support shared headroom pool on top of dynamic buffer calculation.

Signed-off-by: Stephen Sun [email protected]

Why I did it

Support shared headroom pool on top of dynamic buffer calculation:

  • Feature is enabled/disabled on-the-fly by configuring over-subscribe-ration and shared headroom pool size.
    If both are configured, the shared headroom pool size will take effect.
    When turn on/off the feature, all the lossless profiles and buffer pool size will be recalculated.
  • Support calculating shared headroom pool while ingress lossless pool is statically configured.

How I verified it
Run vs test and regression test.
Details if related

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

@stephenxs
Copy link
Collaborator Author

retest this, please

@stephenxs stephenxs marked this pull request as ready for review January 10, 2021 03:39
@stephenxs stephenxs force-pushed the shared-headroom-pool branch from 9499458 to 2341dc7 Compare January 15, 2021 06:53
@stephenxs stephenxs requested a review from neethajohn January 15, 2021 08:55
@stephenxs
Copy link
Collaborator Author

Hi @neethajohn ,
As discussed, this PR only contains the changes for the shared headroom pool. All the other changes have been moved to #1601.
Thanks.

@stephenxs
Copy link
Collaborator Author

retest this, please

@stephenxs
Copy link
Collaborator Author

retest this please

 - Feature is enabled/disabled on-the-fly by configuring over-subscribe-ration and shared headroom pool size.
   If both are configured, the shared headroom pool size will take effect.
   When turn on/off the feature, all the lossless profiles and buffer pool size will be recalculated.
 - Support calculating shared headroom pool while ingress lossless pool is statically configured.

Signed-off-by: Stephen Sun <[email protected]>
- Check accumulative headroom before toggling SHP state
  To disable SHP results in size of PG increasing.
  Hence needs to check whether accumulative headroom exceed limit
- Split the function doUpdateStaticProfileTask into two functions
  Originally it was called for static profile only and consisted of two parts:
  - One is for dynamic th updated. It will go over all the buffer profiles
    dynamically generated according to the dynamic th and update them
  - The other is for size updated. It will go over each port referencing
    the profile and check whether the accumulative headroom exceeds limit
  Now that it is also called by shared headroom pool, we split it into
  two functions to make it more clear

Signed-off-by: Stephen Sun <[email protected]>
@stephenxs stephenxs force-pushed the shared-headroom-pool branch from 2341dc7 to b600102 Compare January 25, 2021 06:17
@liat-grozovik
Copy link
Collaborator

@neethajohn can you please review?

cfgmgr/buffer_headroom_mellanox.lua Outdated Show resolved Hide resolved
cfgmgr/buffer_headroom_mellanox.lua Outdated Show resolved Hide resolved
tests/test_buffer_dynamic.py Show resolved Hide resolved
tests/test_buffer_dynamic.py Show resolved Hide resolved
cfgmgr/buffermgrdyn.cpp Outdated Show resolved Hide resolved
cfgmgr/buffermgrdyn.cpp Show resolved Hide resolved
cfgmgr/buffermgrdyn.cpp Outdated Show resolved Hide resolved
cfgmgr/buffermgrdyn.cpp Outdated Show resolved Hide resolved
cfgmgr/buffermgrdyn.cpp Outdated Show resolved Hide resolved
cfgmgr/buffermgrdyn.cpp Outdated Show resolved Hide resolved
- Adjust code, add comments
- Check the buffer profile in each step of the shared headroom pool update

Signed-off-by: Stephen Sun <[email protected]>
@stephenxs stephenxs requested a review from neethajohn February 1, 2021 22:48
@neethajohn neethajohn merged commit 5fa2329 into sonic-net:master Feb 7, 2021
@stephenxs stephenxs deleted the shared-headroom-pool branch February 7, 2021 23:33
@liat-grozovik
Copy link
Collaborator

@daall can you please add a label for 202012?

yxieca pushed a commit that referenced this pull request Feb 23, 2021
)

* Support shared headroom pool on top of dynamic buffer calculation

 - Feature is enabled/disabled on-the-fly by configuring over-subscribe-ration and shared headroom pool size.
   If both are configured, the shared headroom pool size will take effect.
   When turn on/off the feature, all the lossless profiles and buffer pool size will be recalculated.
 - Support calculating shared headroom pool while ingress lossless pool is statically configured.
- Check accumulative headroom before toggling SHP state
  To disable SHP results in size of PG increasing.
  Hence needs to check whether accumulative headroom exceed limit
- Split the function doUpdateStaticProfileTask into two functions
  Originally it was called for static profile only and consisted of two parts:
  - One is for dynamic th updated. It will go over all the buffer profiles
    dynamically generated according to the dynamic th and update them
  - The other is for size updated. It will go over each port referencing
    the profile and check whether the accumulative headroom exceeds limit
  Now that it is also called by shared headroom pool, we split it into
  two functions to make it more clear

Signed-off-by: Stephen Sun <[email protected]>

How I verified it
Run vs test and regression test.
DavidZagury pushed a commit to DavidZagury/sonic-swss that referenced this pull request Mar 4, 2021
…nic-net#1581)

* Support shared headroom pool on top of dynamic buffer calculation

 - Feature is enabled/disabled on-the-fly by configuring over-subscribe-ration and shared headroom pool size.
   If both are configured, the shared headroom pool size will take effect.
   When turn on/off the feature, all the lossless profiles and buffer pool size will be recalculated.
 - Support calculating shared headroom pool while ingress lossless pool is statically configured.
- Check accumulative headroom before toggling SHP state
  To disable SHP results in size of PG increasing.
  Hence needs to check whether accumulative headroom exceed limit
- Split the function doUpdateStaticProfileTask into two functions
  Originally it was called for static profile only and consisted of two parts:
  - One is for dynamic th updated. It will go over all the buffer profiles
    dynamically generated according to the dynamic th and update them
  - The other is for size updated. It will go over each port referencing
    the profile and check whether the accumulative headroom exceeds limit
  Now that it is also called by shared headroom pool, we split it into
  two functions to make it more clear

Signed-off-by: Stephen Sun <[email protected]>

How I verified it
Run vs test and regression test.
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…nic-net#1581)

* Support shared headroom pool on top of dynamic buffer calculation

 - Feature is enabled/disabled on-the-fly by configuring over-subscribe-ration and shared headroom pool size.
   If both are configured, the shared headroom pool size will take effect.
   When turn on/off the feature, all the lossless profiles and buffer pool size will be recalculated.
 - Support calculating shared headroom pool while ingress lossless pool is statically configured.
- Check accumulative headroom before toggling SHP state
  To disable SHP results in size of PG increasing.
  Hence needs to check whether accumulative headroom exceed limit
- Split the function doUpdateStaticProfileTask into two functions
  Originally it was called for static profile only and consisted of two parts:
  - One is for dynamic th updated. It will go over all the buffer profiles
    dynamically generated according to the dynamic th and update them
  - The other is for size updated. It will go over each port referencing
    the profile and check whether the accumulative headroom exceeds limit
  Now that it is also called by shared headroom pool, we split it into
  two functions to make it more clear

Signed-off-by: Stephen Sun <[email protected]>

How I verified it
Run vs test and regression test.
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