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

[bufferpool] Support for buffer pool type both #1735

Merged
merged 2 commits into from Jun 15, 2021

Conversation

vganesan-nokia
Copy link
Contributor

@vganesan-nokia vganesan-nokia commented Apr 30, 2021

What I did

Changes done to allow orchagent accepting "both" type for buffer pool
configuration.

Why I did it

Currently sonic supports only 'ingress' and 'egress' types of buffer pools.
Since some devices like DNX based switches only support buffer pool
type 'both', we are unable to create buffer pools for these devices.
This patch fixes this problem by enhancing buffer pool configuration to
accept "both" type.

How I verified it

In a switch that uses DNX arch based chips (like VOQ chassis using line cards with Jerico family of switches), the following configuration is applied

"BUFFER_POOL": {
        "lossless_pool": {
            "size": "12766208",
            "type": "both",
            "mode": "dynamic"
        }
 }

when the switch became operational, it was observed, there are no error logs indicating invalid parameter or failure of buffer pool creation.

Details if needed

Currently sonic supports only 'ingress' and 'egress' types of buffer pools.
Since some devices like DNX based switches only support buffer pool
type 'both', we are unable to create buffer pools for these devices.
This patch fixes this problem by enhancing buffer pool configuration to
accept "both" type. The changes include

 - Changes done to allow orchagent accepting "both" type for buffer pool
configuration.
 - For DOT1P QOS map, the map list was not initialized correctly. So
"color" field in the key was sending garbage. Because of this, orchagent
serialization function is unable to translate and serialize the attribute
value properly and returns warning. During subsequent operation with comes
with a different garbage, this was treated as a different "color" and
syncd/SAI returns error saying "modification not implemented". This is
fixed by initializing the dotp1 map list attribute.

Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>
@anshuv-mfst
Copy link

@ysmanman, @abdosi - could you please review, thanks.

@anshuv-mfst
Copy link

@neethajohn , @lguohan - please take a look, thanks.

orchagent/qosorch.cpp Outdated Show resolved Hide resolved
As response to code review comments, undid the changes done
to fix the dot1p map list initialization fix since this is not
related to the title objective of this PR sonic-net#1735
(sonic-net#1735)

This fix moved to a different PR sonic-net#1746
(sonic-net#1746)

Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>
@anshuv-mfst
Copy link

@neethajohn - can you please help with merge, thanks!

@neethajohn neethajohn merged commit 1d19185 into sonic-net:master Jun 15, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
This PR is fix for the issue sonic-net/sonic-buildimage#7497

Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

What I did

Changes done to allow orchagent accepting "both" type for buffer pool
configuration.

Why I did it

Currently sonic supports only 'ingress' and 'egress' types of buffer pools.
Since some devices like DNX based switches only support buffer pool
type 'both', we are unable to create buffer pools for these devices.
This patch fixes this problem by enhancing buffer pool configuration to
accept "both" type.

How I verified it

In a switch that uses DNX arch based chips (like VOQ chassis using line cards with Jerico family of switches), the following configuration is applied

"BUFFER_POOL": {
        "lossless_pool": {
            "size": "12766208",
            "type": "both",
            "mode": "dynamic"
        }
 }
when the switch became operational, it was observed, there are no error logs indicating invalid parameter or failure of buffer pool creation.
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