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

[yang]YANG model for policer table #9948

Merged
merged 4 commits into from
Feb 14, 2022

Conversation

dgsudharsan
Copy link
Collaborator

@dgsudharsan dgsudharsan commented Feb 9, 2022

Why I did it

Added yang model for policer table
Fixes #9742 and #9743

How I did it

Creating yang model for policer

How to verify it

Added UT to verify the yang model

The configuration schema for policer is added in the pull request sonic-net/sonic-swss#2144

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
@dgsudharsan dgsudharsan added the Request for 202111 Branch For PRs being requested for 202111 branch label Feb 9, 2022
@@ -0,0 +1,61 @@
{
"POLICER_TABLE": {
"desc": "Configure policer with all fields."
Copy link
Collaborator

@qiluo-msft qiluo-msft Feb 9, 2022

Choose a reason for hiding this comment

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

"desc"

Could you reformat this json file? #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@qiluo-msft qiluo-msft requested a review from ganglyu February 9, 2022 21:33
description
"Committed information rate for the dual-rate token
bucket policer. This value represents the rate at which
tokens are added to the primary bucket.";
Copy link
Contributor

@ganglyu ganglyu Feb 10, 2022

Choose a reason for hiding this comment

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

what's the unit? bytes per second or packets per second?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It depends on the meter type defined. The meter type can be bytes or packets. This field by itself has no units. Please refer to https://github.com/opencomputeproject/SAI/blob/16c11ee7062e630f1eaf4bb3e44d31ab58eafa5b/inc/saipolicer.h#L123

description
"Committed burst size for the dual-rate token bucket
policer. This value represents the depth of the token
bucket.";
Copy link
Contributor

@ganglyu ganglyu Feb 10, 2022

Choose a reason for hiding this comment

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

what's the unit? bytes or packets?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It depends on the meter type defined. The meter type can be bytes or packets. This field by itself has no units. Please refer to https://github.com/opencomputeproject/SAI/blob/16c11ee7062e630f1eaf4bb3e44d31ab58eafa5b/inc/saipolicer.h#L123

must "((current()/../cir) and (current()/../cir > 0))" {
error-message "cbs can't be configured without cir.";
}
must "(current() >= current()/../cir)" {
Copy link
Contributor

@ganglyu ganglyu Feb 10, 2022

Choose a reason for hiding this comment

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

Please refer to https://datatracker.ietf.org/doc/html/rfc4115.
The CBS and EBS are measured in bytes and must configure to be greater than the expected maximum length of the incoming PDU. The CIR and EIR are both measured in bits/s.
Maybe we need to convert unit for comparing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please refer to my earlier comments regarding the units.

@dgsudharsan
Copy link
Collaborator Author

@ganglyu Please refer to copp yang table. I have reused the same definitions used there. Basically copp internally uses policer and both should be aligned in yang definitions and so I defined as in https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-copp.yang

@dgsudharsan dgsudharsan added the YANG YANG model related changes label Feb 10, 2022
@ganglyu
Copy link
Contributor

ganglyu commented Feb 10, 2022

@ganglyu Please refer to copp yang table. I have reused the same definitions used there. Basically copp internally uses policer and both should be aligned in yang definitions and so I defined as in https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-copp.yang

Can you update the descriptions for CIR, PIR, CBS and PBS to add unit?

@dgsudharsan
Copy link
Collaborator Author

@ganglyu Please refer to copp yang table. I have reused the same definitions used there. Basically copp internally uses policer and both should be aligned in yang definitions and so I defined as in https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-copp.yang

Can you update the descriptions for CIR, PIR, CBS and PBS to add unit?

Done

@liat-grozovik
Copy link
Collaborator

@lguohan could you please signoff?

@qiluo-msft qiluo-msft merged commit 0b59f0b into sonic-net:master Feb 14, 2022
judyjoseph pushed a commit that referenced this pull request Feb 22, 2022
#### Why I did it
Added yang model for policer table
Fixes #9742 and #9743
#### How I did it
Creating yang model for policer

#### How to verify it
Added UT to verify the yang model

The configuration schema for policer is added in the pull request sonic-net/sonic-swss#2144
@dgsudharsan dgsudharsan deleted the policer_yang branch March 9, 2023 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Included in 202111 Branch Request for 202111 Branch For PRs being requested for 202111 branch YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[yang-models] POLICER table missing
5 participants