Skip to content

Add two new enums for scheduler-policy priority. #1311

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add two new enums for scheduler-policy priority. #1311

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a388a8f
Add two new enums for scheduler-policy priority.
sallylsy May 22, 2025
a47f0d0
Add two new enums for scheduler-policy priority.
sallylsy May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion release/models/qos/openconfig-qos-elements.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@ submodule openconfig-qos-elements {
packets for transmission, including policer and shaper
functions";

oc-ext:openconfig-version "0.11.2";
oc-ext:openconfig-version "0.11.3";

revision "2025-05-22" {
description
"Add two new enums for scheduler-policy priority.";
Copy link

Choose a reason for hiding this comment

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

Can we specify the two new enums in the description? "Add WRR and DWRR enums ..."

reference "0.11.3";
Copy link

Choose a reason for hiding this comment

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

Following https://github.com/openconfig/public/blob/master/doc/semver.md#general-guidelines-for-versioning-openconfig-yang-modules, should this be 0.12.0?

}

revision "2023-10-13" {
description
Expand Down Expand Up @@ -1231,6 +1237,18 @@ submodule openconfig-qos-elements {
term - such that packets that arrive in the queue are
immediately serviced.";
}
enum DWRR {
description
"This scheduler term is considered as a deficit weighted
round robin term - such that packets that arrive in the queue are
serviced based on deficit counter with weighted round robin fashion.";
}
enum WRR {
Copy link
Contributor

Choose a reason for hiding this comment

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

With the existing model, it seems like setting priority was optional, and at least for our mappings, I think that we defaulted to WRR if STRICT priority wasn't explicitly set. Is the intention with this change that priority should always be set? Would it make sense to mark WRR as the default so that this change is backwards compatible from an existing implementation perspective (with the underlying assumption that other implementations map similarly)?

Copy link

Choose a reason for hiding this comment

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

It seems like OpenConfig prefers not to define defaults, at least according to #1165 (comment).

description
"This scheduler term is considered as a deficit weighted
Copy link
Contributor

@rgwilton rgwilton May 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Presumably "deficit" isn't intended here, and this should just be "weighted round robin"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

round robin term - such that packets that arrive in the queue are
serviced based on weighted round robin fashion.";
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any strong references that provide a strong definition of what WRR or DWRR actually means? My fear is that at least DWRR may end up being quite platform/hardware specific and if that is the case I'm thinking that we may need to pull that out in the description.

Copy link

Choose a reason for hiding this comment

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

+1.

opening a featureprofiles PR with a functional test README would also help clarify the behaviour of these new enum values.

For examples, see the following tests:

Copy link
Contributor

Choose a reason for hiding this comment

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

Aruba seems to also use the DWRR term: https://arubanetworking.hpe.com/techdocs/AOS-CX/AOSCX-CLI-Bank/cli_832x/Content/QoS_cmds/dwrr-queue.htm

description
"Priority of the scheduler within the scheduler policy.";
Expand Down