Hi Ladislav,

Thank you for your review and useful feedback.

These are fixed in 
https://github.com/o-pylypenko/draft-ietf-opsawg-discardmodel/pull/58 and will 
be incorporated in the next release.

cheers

John

From: Ladislav Lhotka via Datatracker <[email protected]>
Date: Thursday, 8 January 2026 at 10:58
To: [email protected] <[email protected]>
Cc: [email protected] 
<[email protected]>, [email protected] <[email protected]>
Subject: draft-ietf-opsawg-discardmodel-10 early Yangdoctors review

CAUTION: This email originated from outside of the organization. Do not click 
links or open attachments unless you can confirm the sender and know the 
content is safe.



Document: draft-ietf-opsawg-discardmodel
Title: Information and Data Models for Packet Discard Reporting
Reviewer: Ladislav Lhotka
Review result: Ready with Issues

I've done an early review of version -03 of this draft, see
https://datatracker.ietf.org/doc/review-ietf-opsawg-discardmodel-03-yangdoctors-early-lhotka-2024-08-28/.
The authors addressed most of my comments and suggestions in the present
version.

**** General comments

The document now contains two YANG modules:

- ietf-packet-discard-reporting-sx contains two dozen groupings with components
of the schema for detailed packet loss statistics as well as several YANG
features and identities. This module then also defines a YANG structure [RFC
8791] that defines a complete hierarchical schema. This module – or rather the
structure – represents the information model (IM).

- ietf-packet-discard-reporting (the data model, DM) then augments three
existing modules (ietf-interfaces, ietf-logical-network-element and
ietf-routing) with relevant packet loss statistics from the IM.

This separation is useful and can indeed aid implementing the same data
structures in other protocols such as IPFIX. I would recommend to put the
"packet-discard-reporting" structure to a separate module. This way,
implementations that want to use the ietf-packet-discard-reporting module (and
thus have to import ietf-packet-discard-reporting-sx) needn't parse the
structure.

**** Specific comments

- The ietf-packet-discard-reporting module augments
"/rt:routing/rt:control-plane-protocols/rt:control-plane-protocol" with the
contents of the "plr-sx:control-plane" grouping. The augment target is a
"config true" data node and the grouping doesn't contain "config false", so the
augmented statistics are also "config true" (rw), which is probably wrong.

- What is the purpose of defining "alias" groupings that only use another
grouping and nothing else? This applies to the following pairs of groupings:
l2-traffic/basic-frames-bytes and l3-traffic/ip.

- I assume "all" identity means "either ipv4 or ipv6 address family". If it is
so, I'd suggest to use the following hierarchy of identities:

  address-family
    ip
      ipv4
      ipv6

  The intermediate "ip" identity would replace "all". This is more flexible
  because "ipv4" and "ipv6" are also derived from "ip" (unlike "all", and it
  also allows for adding other address families.






Amazon Data Services UK Limited. Registered in England and Wales with 
registration number 09959151 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.


_______________________________________________
OPSAWG mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to