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

fix the type for SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE #1942

Merged
merged 1 commit into from
Oct 11, 2021
Merged

fix the type for SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE #1942

merged 1 commit into from
Oct 11, 2021

Conversation

alpeshspatel
Copy link
Contributor

@alpeshspatel alpeshspatel commented Oct 5, 2021

Fixing the type for SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE

It is incorrectly set to u32 and is not clearing the higher 32 bits in the union

Sample code:

attr.id = SAI_BUFFER_PROFILE_ATTR_POOL_ID;
attr.value.oid = getPool(ingress); <<<<-- the higher order 32 bits were retrieved for  BUFFER_PROFILE_ATTR_BUFFER_SIZE
attribs.push_back(attr);

attr.id = SAI_BUFFER_PROFILE_ATTR_THRESHOLD_MODE;
attr.value.u32 = SAI_BUFFER_PROFILE_THRESHOLD_MODE_DYNAMIC;
attribs.push_back(attr);

attr.id = SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE;
attr.value.u64 = 0;      <<<<-------------------------------- This was u32 earlier
attribs.push_back(attr);

Signed-off-by: Alpesh S Patel alpesh@cisco.com

What I did

Change the attribute value type from u32 to u64 as in SAI header file - https://github.com/opencomputeproject/SAI/blob/v1.7/inc/saibuffer.h

/**
     * @brief Reserved buffer size in bytes
     *
     * @type sai_uint64_t
     * @flags MANDATORY_ON_CREATE | CREATE_AND_SET
     */
    SAI_BUFFER_PROFILE_ATTR_RESERVED_BUFFER_SIZE,

    /** @ignore - for backward compatibility */
    SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE = SAI_BUFFER_PROFILE_ATTR_RESERVED_BUFFER_SIZE,

Why I did it

since u32 was getting set and the actual data type is u64, at SAI layer, when the value is retrieved as u64, the upper 32 bits are read from the previously written attribute value (pool oid)

How I verified it
Made this change in sonic and verified it on the hardware platform via debug and correct operation

Details if related

Signed-off-by: Alpesh S Patel <alpesh@cisco.com>
@prsunny
Copy link
Collaborator

prsunny commented Oct 7, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@neethajohn
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@alpeshspatel
Copy link
Contributor Author

Hello Prince,
Can you take a look and do the needful.
Thx

@abdosi
Copy link
Contributor

abdosi commented Nov 24, 2021

@qiluo-msft

qiluo-msft pushed a commit that referenced this pull request Nov 24, 2021
Fixing the type for SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE

It is incorrectly set to u32 and is not clearing the higher 32 bits in the union

Sample code:

attr.id = SAI_BUFFER_PROFILE_ATTR_POOL_ID;
attr.value.oid = getPool(ingress); <<<<-- the higher order 32 bits were retrieved for  BUFFER_PROFILE_ATTR_BUFFER_SIZE
attribs.push_back(attr);

attr.id = SAI_BUFFER_PROFILE_ATTR_THRESHOLD_MODE;
attr.value.u32 = SAI_BUFFER_PROFILE_THRESHOLD_MODE_DYNAMIC;
attribs.push_back(attr);

attr.id = SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE;
attr.value.u64 = 0;      <<<<-------------------------------- This was u32 earlier
attribs.push_back(attr);
Signed-off-by: Alpesh S Patel alpesh@cisco.com

What I did

Change the attribute value type from u32 to u64 as in SAI header file - https://github.com/opencomputeproject/SAI/blob/v1.7/inc/saibuffer.h

/**
     * @brief Reserved buffer size in bytes
     *
     * @type sai_uint64_t
     * @flags MANDATORY_ON_CREATE | CREATE_AND_SET
     */
    SAI_BUFFER_PROFILE_ATTR_RESERVED_BUFFER_SIZE,

    /** @ignore - for backward compatibility */
    SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE = SAI_BUFFER_PROFILE_ATTR_RESERVED_BUFFER_SIZE,

Why I did it

since u32 was getting set and the actual data type is u64, at SAI layer, when the value is retrieved as u64, the upper 32 bits are read from the previously written attribute value (pool oid)

How I verified it
Made this change in sonic and verified it on the hardware platform via debug and correct operation
yxieca pushed a commit that referenced this pull request Dec 17, 2021
What I did

Enables support for Rx traffic drop for a port/tc by applying the zero buffer profile
Changed class hierarchy so that default getHwCounters() function is used (since the Rx counter support is enabled)
Fixes a pfc-wd off by 1 detection in case of PFC-WD detection without traffic
Why I did it

enabling a missing functionality
leveraging sonic code as our sdk now implements the missing counter
fixes a bug
How I verified it
on a cisco-8000 router:

detected pfc-wd detection and restore happens within the (detection/restore-time + 1 poll interval) duration and that the changeset passes MSFT tests
Verified that when PFC-WD is detected, both Rx and Tx traffic for a port/tc is dropped and no forwarding happens
Details if related

this patch will need to be double committed to the 202012 branch along with #1942 , #1748 and #1962

Signed-off-by: Alpesh S Patel <alpesh@cisco.com>
qiluo-msft pushed a commit that referenced this pull request Dec 20, 2021
What I did

Enables support for Rx traffic drop for a port/tc by applying the zero buffer profile
Changed class hierarchy so that default getHwCounters() function is used (since the Rx counter support is enabled)
Fixes a pfc-wd off by 1 detection in case of PFC-WD detection without traffic
Why I did it

enabling a missing functionality
leveraging sonic code as our sdk now implements the missing counter
fixes a bug
How I verified it
on a cisco-8000 router:

detected pfc-wd detection and restore happens within the (detection/restore-time + 1 poll interval) duration and that the changeset passes MSFT tests
Verified that when PFC-WD is detected, both Rx and Tx traffic for a port/tc is dropped and no forwarding happens
Details if related

this patch will need to be double committed to the 202012 branch along with #1942 , #1748 and #1962

Signed-off-by: Alpesh S Patel <alpesh@cisco.com>
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
What I did
Add show command for BFD sessions.

How I did it
Add show command for BFD session and unit tests.

How to verify it
Verify the show command locally and unit test passes.
preetham-singh pushed a commit to preetham-singh/sonic-swss that referenced this pull request Aug 6, 2022
…00 (sonic-net#2041)

What I did

Enables support for Rx traffic drop for a port/tc by applying the zero buffer profile
Changed class hierarchy so that default getHwCounters() function is used (since the Rx counter support is enabled)
Fixes a pfc-wd off by 1 detection in case of PFC-WD detection without traffic
Why I did it

enabling a missing functionality
leveraging sonic code as our sdk now implements the missing counter
fixes a bug
How I verified it
on a cisco-8000 router:

detected pfc-wd detection and restore happens within the (detection/restore-time + 1 poll interval) duration and that the changeset passes MSFT tests
Verified that when PFC-WD is detected, both Rx and Tx traffic for a port/tc is dropped and no forwarding happens
Details if related

this patch will need to be double committed to the 202012 branch along with sonic-net#1942 , sonic-net#1748 and sonic-net#1962

Signed-off-by: Alpesh S Patel <alpesh@cisco.com>
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.

6 participants