[QoS] Support dynamic headroom calculation for Barefoot platforms #11104
Conversation
Signed-off-by: Michał Mysłek <michal.myslek@intel.com>
| @@ -14,12 +14,17 @@ | |||
| {%- macro generate_buffer_pool_and_profiles() %} | |||
| "BUFFER_POOL": { | |||
| "ingress_lossless_pool": { | |||
| {%- if dynamic_mode is not defined %} | |||
There was a problem hiding this comment.
Hi
From this PR I see the buffer pool sizes (ingress_lossless_pool_size, ingress_lossy_pool_size, and egress_lossy_pool) are dynamically calculated and the shared headroom pool size (ingress_lossless_pool.xoff) is a fixed number.
But in PR sonic-net/sonic-swss#2317 and sonic-net/sonic-swss#2323, I see the buffer pool sizes are fixed and the shared headroom pool size is dynamically calculated.
These result in both the buffer pool sizes and the shared headroom pool size are static. Is this a mistake or by intention?
If you are going to have the buffer pool sizes static and the shared headroom pool size dynamic, I would like to suggest
- always defining
sizes in all pools regardless of whetherdynamic_modeis defined - defining
xoffiningress_lossless_poolonly ifdynamic_modeis defined
There was a problem hiding this comment.
@stephenxs, thank you for the great suggestions! We decided to push the basic version of the feature so far. But of course it should be ready for further improvements. I like your proposals. I would try to follow your second approach with defining XOFF when dynamic mode is enabled, but I believe it would be worth to verify the first one too. The work will be continued by other engineers on our side.
Signed-off-by: Michał Mysłek michal.myslek@intel.com
What I did
Adding the dynamic headroom calculation support for Barefoot platforms.
Why I did it
Enabling dynamic mode for barefoot case.
How I verified it
The community tests are adjusted and pass.
Details if related