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

[orchagent]fix SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH value type mismatch bug #495

Merged
merged 1 commit into from
May 10, 2018

Conversation

yangbashuang
Copy link
Contributor

What I did
fix SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH value type mismatch bug
Why I did it
according to SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH defination in saibuffer.h:
/**
* @brief Dynamic threshold for the shared usage
*
* The threshold is set to the 2^n of available buffer of the pool.
*
* @type sai_int8_t
* @flags MANDATORY_ON_CREATE | CREATE_AND_SET
* @condition SAI_BUFFER_PROFILE_ATTR_THRESHOLD_MODE == SAI_BUFFER_PROFILE_THRESHOLD_MODE_DYNAMIC
*/
SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH,

the value type is a sai_int8_t, support negative value

on centec goldengate chip platform, we support range [-7, 3],
but currnetly in swss, uint32 type value is used, have to fix it
How I verified it

  1. build success
  2. startup and init success
  3. set a negative value to SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH

Details if related

@msftclas
Copy link

msftclas commented May 7, 2018

CLA assistant check
All CLA requirements met.

@yangbashuang yangbashuang changed the title [SWSS]fix SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH value type mismatch bug [orchagent]fix SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH value type mismatch bug May 8, 2018
@lguohan lguohan requested a review from yxieca May 9, 2018 05:36
Copy link
Contributor

@yxieca yxieca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great code clean up change. Thanks for noticing it and making the change.

However, this change in execution doesn't have functional impact. We've used negative dynamic threshold on other platforms without issue.

If you hit issue on your platform, this change shouldn't fix it. What issue did you hit?

@yangbashuang
Copy link
Contributor Author

@yxieca not hit any issue related, i found this issue through code review

@lguohan lguohan merged commit 78c86ca into sonic-net:master May 10, 2018
@yangbashuang
Copy link
Contributor Author

@lguohan howto merge this fix to branch 201803?

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

Successfully merging this pull request may close these issues.

4 participants