Add packet trimming counters#1458
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances network observability by introducing a set of packet trimming counters across various OpenConfig YANG models. The changes provide detailed metrics on packets that undergo trimming, including those successfully transmitted and those dropped, at the interface, integrated circuit, and QoS queue levels. This update ensures backward compatibility while expanding the data model to support more granular monitoring of network device behavior related to packet processing. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces new YANG definitions for packet trimming counters across various OpenConfig modules. It updates openconfig-interfaces, openconfig-platform-integrated-circuit, openconfig-platform-pipeline-counters, and openconfig-qos-interfaces to include these new counter types at port, switch, and queue levels, respectively. The review comments highlight several typos in the revision descriptions and suggest grammatical improvements for the descriptions of the new counter leaves.
2464c3d to
9d626f4
Compare
|
Discussed at OC Operators Meeting March 31, 2026: Can we add information (maybe updating the descriptions for the existing packet counters) about how these |
9d626f4 to
3c76ad8
Compare
Updated the description for /interfaces/interface/state/counters/out-discards and /qos/interfaces/interface/output/queues/queue/state/dropped-pkts |
|
/gcbrun |
|
No major YANG version changes in commit ae35af8 |
3c76ad8 to
1f3ef98
Compare
|
/gcbrun |
1f3ef98 to
a4229cc
Compare
|
/gcbrun |
1 similar comment
|
/gcbrun |
|
/gcbrun |
ElodinLaarz
left a comment
There was a problem hiding this comment.
Discussed at the OC Operators Meeting April 21, 2026:
Left a couple comments on the leaf descriptions / expectations for how trimmed packets interact with the existing leaves.
8b42fcc to
ded3478
Compare
|
/gcbrun |
ded3478 to
df15b7b
Compare
|
/gcbrun |
1 similar comment
|
/gcbrun |
|
/gcbrun |
|
Can we get the unresolved comments addressed-- other than that, it looks good to me. |
Resolved. Thanks |
|
Reviewed in May 12, 2026 OC Operators call. Setting last call to May 19, 2026 |
| } | ||
| } | ||
|
|
||
| grouping trim-pkts-counters-top { |
There was a problem hiding this comment.
Defining trim-pkts-counters-top / trim-pkts-common-counters-top in openconfig-platform-integrated-circuit.yang and then importing them from openconfig-interfaces.yang and openconfig-qos-interfaces.yang seems like awkward layering.
Packet trimming is a QoS feature, and its config already lives in qos (/qos/packet-trim/ in openconfig-qos.yang rev 2026-03-25). A more natural home is openconfig-qos-elements.yang, or a dedicated openconfig-packet-trim.yang submodule of qos.
| description | ||
| "Packet trimming counters for entire device."; | ||
| container state { | ||
| description |
There was a problem hiding this comment.
The IC-level trim container and the grouping it uses are inconsistent. The description says:
Total number of trimmed packets is not included as it can be calculated by sum the sent trimmed packets and dropped trimmed packets.
But trim-pkts-common-counters-top contains { trim-pkts, dropped-trim-pkts }, the total is included, and the missing counter is out-trim-pkts -> "The number of packets that were trimmed and successfully transmitted").
Suggest exposing the same three counters at all three levels (interface, queue, IC). NVIDIA hardware will expose all 3 counters at all 3 levels. The missing out-trim-pkts in IC would need to be handled by a vendor specific augmentation otherwise.
| description | ||
| "Structural grouping for packet trimming counters. | ||
| Counters collected at port level, queue level and | ||
| switch level."; |
There was a problem hiding this comment.
port level, queue level and switch level
The "switch" level statement is inaccurate. The counters are added to "integrated-circuit" component. Multi asic devices will have multiple records.
Change Scope
Platform Implementations
Tree View
module: openconfig-interfaces +--rw interfaces +--rw interface* [name] +--ro state | +--ro counters | | +--ro in-octets? oc-yang:counter64 | | +--ro in-pkts? oc-yang:counter64 | | +--ro in-unicast-pkts? oc-yang:counter64 | | +--ro in-broadcast-pkts? oc-yang:counter64 | | +--ro in-multicast-pkts? oc-yang:counter64 | | +--ro in-errors? oc-yang:counter64 | | +--ro in-discards? oc-yang:counter64 | | +--ro out-octets? oc-yang:counter64 | | +--ro out-pkts? oc-yang:counter64 | | +--ro out-unicast-pkts? oc-yang:counter64 | | +--ro out-broadcast-pkts? oc-yang:counter64 | | +--ro out-multicast-pkts? oc-yang:counter64 | | +--ro out-discards? oc-yang:counter64 | | +--ro out-errors? oc-yang:counter64 | | +--ro last-clear? oc-types:timeticks64 | | +--ro in-unknown-protos? oc-yang:counter64 | | +--ro in-fcs-errors? oc-yang:counter64 | | x--ro carrier-transitions? oc-yang:counter64 | | +--ro interface-transitions? oc-yang:counter64 | | +--ro link-transitions? oc-yang:counter64 | | +--ro resets? oc-yang:counter64 + | | +--ro out-trim-pkts? oc-yang:counter64 + | | +--ro trim-pkts? oc-yang:counter64 + | | +--ro dropped-trim-pkts? oc-yang:counter64 module: openconfig-platform +--rw components +--rw component* [name] +--rw integrated-circuit | +--ro oc-ppc:pipeline-counters | +--ro oc-ppc:packet | | +--ro oc-ppc:interface-block | | | +--ro oc-ppc:state | | | +--ro oc-ppc:in-packets? oc-yang:counter64 | | | +--ro oc-ppc:out-packets? oc-yang:counter64 | | | +--ro oc-ppc:in-bytes? oc-yang:counter64 | | | +--ro oc-ppc:out-bytes? oc-yang:counter64 + | | +--ro oc-ppc:trim + | | +--ro oc-ppc:state + | | +--ro oc-ppc:trim-pkts? oc-yang:counter64 + | | +--ro oc-ppc:dropped-trim-pkts? oc-yang:counter64 module: openconfig-qos +--rw qos +--rw config +--ro state +--rw interfaces | +--rw interface* [interface-id] | +--rw output | +--rw config | | +--rw buffer-allocation-profile? -> ../../../../../buffer-allocation-profiles/buffer-allocation-profile/config/name | | +--rw multicast-buffer-allocation-profile? -> ../../../../../buffer-allocation-profiles/buffer-allocation-profile/config/name | | +--rw unicast-buffer-allocation-profile? -> ../../../../../buffer-allocation-profiles/buffer-allocation-profile/config/name | +--ro state | | +--ro buffer-allocation-profile? -> ../../../../../buffer-allocation-profiles/buffer-allocation-profile/config/name | | +--ro multicast-buffer-allocation-profile? -> ../../../../../buffer-allocation-profiles/buffer-allocation-profile/config/name | | +--ro unicast-buffer-allocation-profile? -> ../../../../../buffer-allocation-profiles/buffer-allocation-profile/config/name | +--rw classifiers | | +--rw classifier* [type] | | +--rw type -> ../config/type | | +--rw config | | | +--rw name? -> ../../../../../../../classifiers/classifier/config/name | | | +--rw type? enumeration | | +--ro state | | | +--ro name? -> ../../../../../../../classifiers/classifier/config/name | | | +--ro type? enumeration | | +--rw terms | | +--ro term* [id] | | +--ro id -> ../state/id | | +--ro state | | +--ro id? -> ../../../../../../../../../classifiers/classifier[name=current()/../../../../config/name]/terms/term/config/id | | +--ro matched-packets? oc-yang:counter64 | | +--ro matched-octets? oc-yang:counter64 | +--rw queues | | +--rw queue* [name] | | +--rw name -> ../config/name | | +--rw config | | | +--rw name? string | | | +--rw queue-management-profile? -> ../../../../../../../queue-management-profiles/queue-management-profile/config/name | | +--ro state | | +--ro name? string | | +--ro queue-management-profile? -> ../../../../../../../queue-management-profiles/queue-management-profile/config/name | | +--ro max-queue-len? oc-yang:counter64 | | +--ro avg-queue-len? oc-yang:counter64 | | +--ro transmit-pkts? oc-yang:counter64 | | +--ro transmit-octets? oc-yang:counter64 | | +--ro dropped-pkts? oc-yang:counter64 | | +--ro dropped-octets? oc-yang:counter64 | | +--ro ecn-marked-pkts? oc-yang:counter64 | | +--ro ecn-marked-octets? oc-yang:counter64 | | +--ro ecn-selected-pkts? oc-yang:counter64 | | +--ro ecn-selected-octets? oc-yang:counter64 + | | +--ro out-trim-pkts? oc-yang:counter64 + | | +--ro trim-pkts? oc-yang:counter64 + | | +--ro dropped-trim-pkts? oc-yang:counter64