Skip to content

[YANG] Fix QoS scheduler validation for PIR-only configurations - #29314

Open
yizhenzha wants to merge 1 commit into
sonic-net:masterfrom
yizhenzha:fix-yang-qos-scheduler
Open

[YANG] Fix QoS scheduler validation for PIR-only configurations#29314
yizhenzha wants to merge 1 commit into
sonic-net:masterfrom
yizhenzha:fix-yang-qos-scheduler

Conversation

@yizhenzha

@yizhenzha yizhenzha commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Why I did it

The current sonic-scheduler.yang model requires CIR whenever PIR is configured. This incorrectly prevents valid PIR-only scheduler profiles, including profiles used for port egress shaping. CIR and PIR should only come in pair in a theoretical two rate policer model, which is not exactly how SONiC QoS scheduler is implemented at runtime.

At runtime, SONiC maps CIR and PIR to separate SAI scheduler attributes:

  • CIR maps to the minimum bandwidth rate.
  • PIR maps to the maximum bandwidth rate.

These rates can be configured independently. CIR-only provides a minimum bandwidth allocation without a local maximum, while PIR-only provides a maximum shaping rate without a minimum bandwidth guarantee.

Requiring a dummy CIR for a PIR-only profile is not an appropriate workaround. Some SAI implementations do not support a minimum bandwidth rate on a port-bound scheduler and reject such a configuration.

The YANG model should therefore allow CIR and PIR to be absent or configured independently. When both values are present, PIR must still be greater than or equal to CIR.

How I did it

  • Removed the YANG constraint requiring CIR to be configured whenever PIR is configured.
  • Made the PIR >= CIR validation conditional on CIR being present.
  • Preserved the rejection of configurations where both rates are configured and PIR < CIR.

The resulting behavior is:

CIR PIR Result
Not configured Not configured Valid (same as before)
Configured Not configured Valid (same as before)
Not configured Configured Valid (changed by this PR, before it is not valid)
Configured Configured, PIR >= CIR Valid (same as before)
Configured Configured, PIR < CIR Invalid (same as before)

How to verify it

  1. Only configure PIR for a scheduler and confirm YANG rejects it on the unpatched model:
$ redis-cli -n 4 hset "SCHEDULER|port_scheduler" type STRICT pir 625000000
(integer) 2
$ redis-cli -n 4 hset "PORT_QOS_MAP|Ethernet0" scheduler port_scheduler
(integer) 1
$ sudo config save -y
$ sudo config reload -y
sonic_yang(3):Data Loading Failed:failed to parse data tree: pir can't be configured without cir.: Data path: /sonic-scheduler:sonic-scheduler/SCHEDULER/SCHEDULER_LIST[name='port_scheduler']/pir
/etc/sonic/config_db.json fails YANG validation! Error: Data Loading Failed
Aborted!
  1. Apply the patched YANG model and confirm the same PIR-only config now loads cleanly, and the scheduler and its PIR binding reach the ASIC via orchagent (SCHEDULER table created, applied to the port).
# 1. Reload invoked, patched YANG loads clean
02:14:38.075678  NOTICE wjh-config: 'reload' executing with command: config reload -y
02:14:38.294737  INFO sonic_yang: modd successfully

# 2. Validator translates and evaluatesthe patched schema
02:14:38.420548  INFO sonic_yang: xlateConfigDBtoYang sonic-scheduler:...:SCHEDULER
02:14:38.420616  INFO sonic_yang: xlaEDULER_LIST
02:14:38.422948  INFO sonic_yang: Try to load Data in the tree
(This is the exact step that threw Dabe configured without cir. in thebaseline run 32 seconds earlier — here it passes silently and moves on to the next module.)

# 3. Validation passed — reload proceeds to actually apply config
02:14:38.475908  NOTICE config: 'relo
02:14:57.207980  NOTICE config: 'reload' restarting services...
(~19s gap = normal container stop/songrator/target-restart work, same as any reload.)

# 4. orchagent picks up the reloaded CONFIG_DB and creates the scheduler object
02:15:56.241070  NOTICE swss#orchagenreated [SCHEDULER:port_scheduler]
02:15:56.242328  NOTICE swss#orchagent: :- handleSchedulerTable: Created [SCHEDULER:scheduler.0]
02:15:56.243383  NOTICE swss#orchagenreated [SCHEDULER:scheduler.1]

# 5. orchagent binds the scheduler to t failed in the original bug
02:15:56.289135  NOTICE swss#orchagent: :- handlePortQosMapTable: Applied QoS maps to ports
02:15:57.256423  NOTICE swss#orchagen times: [handlePortQosMapTable: Applied QoS maps to ports]    

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

  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • 202608

Tested branch

  • master

Description for the changelog

Allow CIR and PIR to be configured independently in SONiC scheduler profiles.

Link to config_db schema for YANG module changes

https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md#scheduler

Remove the rule that rejects PIR when CIR is absent, and make the
PIR >= CIR comparison conditional on CIR being configured. CIR and
PIR map to independent SAI scheduler attributes (minimum and maximum
bandwidth rate respectively) and are not required to be configured
together; requiring a dummy CIR for a PIR-only profile (e.g. a port
egress shaper) is not appropriate, and some SAI implementations
reject a minimum bandwidth rate on a port-bound scheduler.

Update SCHEDULER_PIR_NO_CIR to expect success now that pir no longer
requires cir. Add SCHEDULER_PIR_ONLY_PORT_SHAPER covering the
port-shaper use case (STRICT scheduler with pir only), and
SCHEDULER_PIR_EQUAL_CIR covering the pir == cir boundary when both
are configured.

Signed-off-by: Yizhen Zhang <evazha@nvidia.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@dgsudharsan dgsudharsan added the YANG YANG model related changes label Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

YANG YANG model related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants