Repository navigation
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 the OpenConfig IPsec model by introducing a new operational state counter. This change provides better visibility into packet drops occurring during inbound SPI/SAD lookups, allowing for more granular troubleshooting of IPsec connection issues while maintaining backward compatibility. 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 the 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 counterproductive. 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 updates the "openconfig-security-ipsec.yang" module to version 0.2.0 and introduces a new "ingress-sa-failure" counter leaf to track packets dropped due to invalid security associations or SPIs in IPv4 IPsec connections. The review feedback correctly identifies that this counter is missing from the corresponding IPv6 IPsec connection counters. To ensure consistency across IPv4 and IPv6 telemetry, it is recommended to add the "ingress-sa-failure" leaf to the IPv6 counters container as well.
| leaf ingress-sa-failure { | ||
| type yang:counter64; | ||
| mandatory true; | ||
| description | ||
| "Count of packets dropped due to invalid security | ||
| association or SPI."; | ||
| } |
There was a problem hiding this comment.
The ingress-sa-failure counter has been added to the IPv4 IPsec connection counters, but it is missing from the IPv6 IPsec connection counters (under /oc-ni:network-instances/oc-ni:network-instance/oc-sec:security/ipsec/ipv6/connections/connection/state/counters). To ensure consistency across IPv4 and IPv6 IPsec telemetry, this leaf should also be added to the IPv6 counters container.
Vendor Implementation Comparison
Pursuant to the repository style guide, here is a comparison of the ingress-sa-failure leaf with industry implementations:
| Network Operating System | Command / Path | Description | Documentation URL |
|---|---|---|---|
| Cisco IOS-XR | show crypto ipsec sa |
Displays SA statistics including packets dropped due to no matching SA / invalid SPI. | Cisco IOS-XR Crypto Command Reference |
| Juniper JunOS | show security ipsec security-associations statistics |
Shows statistics for security associations, including "No SA" drop counters. | Juniper JunOS IPsec SA Statistics |
| Nokia SR Linux | /state/ipsec/statistics |
Tracks IPsec global and SA-specific statistics, including drops due to missing SA or invalid SPI. | Nokia SR Linux IPsec State |
| Arista EOS | show ipsec security-association |
Displays IPsec SA details and packet drop counters for SA lookup failures. | Arista EOS IPsec TOI |
| NVIDIA Cumulus Linux | swanctl --stats / ip xfrm state count |
Shows XFRM state statistics, including error/drop counters for SA lookup failures. | NVIDIA Cumulus Linux IPsec VPN |
| SONiC | ip xfrm state count |
Tracks Linux XFRM framework counters, including SA lookup failures. | SONiC IPsec User Guide |
Since all six major implementations support tracking drops due to missing or invalid SAs, this change meets the minimum OpenConfig guidelines for vendor-neutral modeling.
Please add the following block to the IPv6 counters container (around line 522 in the original file):
leaf ingress-sa-failure {
type yang:counter64;
mandatory true;
description
"Count of packets dropped due to invalid security
association or SPI.";
}References
- Compare the yang changes with the documentation of implementations of network operating systems from Cisco IOS-XR, Nokia SR Linux, Juniper JunOS/JunOS Evolved, Arista EOS, NVIDIA Cumulus Linux and SONiC, and generate a comparison table. (link)
There was a problem hiding this comment.
Added the IPv6 counter
ingress-sa-failure counter to the IPv4 IPsec connection countersingress-sa-failure counter to IPsec connection counters
|
/gcbrun |
dplore
left a comment
There was a problem hiding this comment.
LGTM. OC Operators review on Oct 20,2026.
|
No major YANG version changes in commit 1ed9665 |
Change Scope
finds no matching active Security Association (RFC 4301 §5.2).
Tree View
module: openconfig-security-ipsec augment /oc-ni:network-instances/oc-ni:network-instance/oc-sec:security: +--ro ipsec +--ro ipv4 | +--ro connections | +--ro connection* [name] | +--ro state | +--ro counters | +--ro replay-failure yang:counter64 | +--ro integrity-failure yang:counter64 | +--ro encryption-failure yang:counter64 + | +--ro ingress-sa-failure yang:counter64 +--ro ipv6 | +--ro connections | +--ro connection* [name] | +--ro state | +--ro counters | +--ro replay-failure yang:counter64 | +--ro integrity-failure yang:counter64 | +--ro encryption-failure yang:counter64 + | +--ro ingress-sa-failure yang:counter64