MIME-Version: 1.0
References: <3B421669-2E05-44A3-85A2-7DE36EFB182F@gmail.com>
In-Reply-To: <3B421669-2E05-44A3-85A2-7DE36EFB182F@gmail.com>
From: Marisol <marisol.ietf@gmail.com>
Date: Tue, 28 Jul 2026 12:20:05 +0200
Message-ID: 
 <CAO+xsemwncNraF0pzNtmG_cC5nf3AtjQCsXxT=COToqwZ05L+A@mail.gmail.com>
To: Acee Lindem <acee.ietf@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Message-ID-Hash: 4GDVN65PPTSH4ZTA3BTA3HN33CECJO5I
CC: draft-ietf-ivy-entitlement-inventory@ietf.org, ops-ads@ietf.org,
 ivy@ietf.org, YANG Doctors <yang-doctors@ietf.org>
Precedence: list
Subject: =?utf-8?q?=5BIvy=5D_Re=3A_YANG_Doctors_Early_Review_for_=22A_YANG_Module_for?=
 =?utf-8?q?_Entitlement_Inventory=22_-_draft-ietf-ivy-inventory-entitlement-?=
 =?utf-8?q?04?=
Archived-At: 
 <https://mailarchive.ietf.org/arch/msg/ivy/e8rl78n92H6mzM1dWQI5kPAId1U>

We want to thank you Acee, on behalf of the authors, for the detailed revie=
w.

Really appreciate it!,
In the next coming days we will work on it addressing all the points
that you are highlighting.
Many thanks,
Marisol

On Mon, Jul 27, 2026 at 7:56=E2=80=AFPM Acee Lindem <acee.ietf@gmail.com> w=
rote:
>
> Document: draft-ietf-ivy-inventory-entitlement
> Reviewer: Acee Lindem
> Review Date: 2026-07-27
> IETF LC End Date: N/A (Early Review)
> Intended Status: Proposed Standard.
>
> This is YANG doctor early review on the YANG data module
> ietf-entitlement-inventory.yang. The YANG data module includes augments
> to the network-inventroy.yang YANG module to expose the entitlements
> for network elements in the network inventory. Additionally, the data
> module includes the entitlement restrictions and relationships between
> entitlements (i.e., the hierarchy).
>
> The draft also includes a description of the framework for entitlements.
> Interestly, the provisioning of entitlements is outside the scope of the
> draft and all the data nodes in the module are read-only. Finally, the
> draft contains a large number of examples (8) for various entitlement
> use cases.
>
> The ietf-network-entitlement.yang data module augments the
> ietf-network-inventory.yang data module and is well-structured. As this
> is an early review, there are a number of issues that need to be fixed
> prior to WG last call. I've characterized these by the type of issue.
>
> Formating issues:
>
>  1. All the YANG trees are formatted without line constraints and
>     exceed the supported RFC page length. Please reformat with
>
>         pyang -f tree --tree-line-length 66 ietf-network-inventory.yang
>
>  2. Many of the JSON example lines also exceed 69, the RFC maximum.
>
>  3. There are a number of unprintable characters in the draft. I've tried
>     to removed these in the nits.
>
> RFC 9907 Issues:
>
>   1. Missing References for imported YANG data modules:
>
>      RFC 7950 =E2=80=94 the module is yang-version 1.1. RFC 6020 is cited
>                 correctly the registry.
>      RFC 9911 =E2=80=94 ietf-yang-types is imported for date-and-time.
>      RFC 8340 =E2=80=94 tree diagrams in sections 3.2, 3.3, 3.5, and 3.7.=
1.
>      RFC 7951 =E2=80=94 all of section 4 is JSON encoding.
>      RFC 6241/8040/8446 for the security boilerplate.
>
>    2. Similiarly, the imported modules should contain YANG
>       reference statements for the corresponding drafts for RFCs.
>
>    3. The YANG description statement and the YANG revision statement
>       should reference the current version of the draft. Also, the
>       revision statement should a YANG refernce statement. There are
>       plenty of examples. Also, no need to have revision
>       statements for revisions prior to publication - just update the
>       single revision.
>
>    4. Reference to [BaseInventory] draft should be normative rather
>       than informative.
>
>    5. Need <CODE BEGINS> / <CODE ENDS> for ietf-network-entitlements.yang
>       source code.
>
>    6. The "Security Considerations" doesn't follow the template in
>       RFC 9907. Also, the data paths of the sensitive leaves need
>       to be listed.
>
>    7. Drop the "-t" on "entitlement-state-t". While this is sometimes
>       done in C, it shouldn't be done in YANG for typedefs.
>
>
> YANG Module Problems and Suggestions:
>
>    1. The installed-entitlements/entitlement/entitlement-id references a
>
>            type leafref {
>                path "/inv:network-inventory/ei:entitlements"
>                   + "/ei:entitlement/ei:entitlement-id";
>             }
>
>       Since require-instance is true, an entitlement can't be returned
>       unless the corresponding network-element exists. Consider
>       "require-instance false;".
>
>
>    2. Same probllem entitlement-attachment/assets/{elements,components}
>       since the assets can't be returned unless both the network-element
>       and the compoent exists. Consider "require-instance false;".
>
>            type leafref {
>                        path "/inv:network-inventory"
>                           + "/inv:network-elements"
>                           + "/inv:network-element"
>                           + "[inv:ne-id=3Dcurrent()/../network-element]"
>                           + "/inv:components/"
>                           + "inv:component/inv:component-id";
>             }
>
>    3. Should "max-value" and "current-value" be uint64 rather than uint32
>       for extendibility to restrictions with units that can exceed 4.2G?
>
>    4. Section 5.2 says:
>
>         Network elements SHOULD generate notifications when installed
>         entitlements are approaching expiration.  The notification timing=
 and
>
>       However, the YANG module doesn't include definitiions for notificat=
ions.
>
>    5. Rename "parent-entitlement-uid" to "parent-entitlement-id" consiste=
nt
>       with the referenced leaf. Fix in json example as well.
>
>    6. The identity hierarchy of "basic-capability-class" and
>       "basic-capability-description" is awkward. Why isn't it
>       "basic-capability-list" rather than "basic-capability-description"?
>
>           *Basic capability class*: The module defines basic-capability-
>       description as a simple capability class using only identifiers
>       and descriptions.  This supports implementations that present
>       capabilities as straightforward lists.
>
>    7. For the holder leaf lists, organization-names/organizations and
>       user-names/users, do we really need an enclosing containter? I'm no=
t
>       sure what it buys you. If needed, maybe organizations/organization
>       and users/user for the leaf lists.
>
>    8. If "universal-access" is true for entitlement attachments, does thi=
s
>       mean that holders and assets cannot be specified? What are the inte=
nded
>       semantics?
>
> Other Issues:
>
>    1. Section 3.5 says "This point requires further exploration in future
>       instances of this document."
>
>    2. Section 2 includes:
>
>            *  ToBeUpdated(TBU) Open Issue for the IVY WG, to include:
>
>          <<Update Glossary under Network Inventory draft, [BaseInventory]=
.  We
>          need at least formal definitions of "capability" and "entitlemen=
t".>>
>
>        However, it seems these are already convered in the document.
>
>    3. In Section 3.6.4:
>
>        When a capability lists multiple supporting entitlements, the
>        entitlement-state/allowed field MUST reflect the combined effect o=
f
>        all required entitlements.  If any required entitlement is missing=
,
>        expired, or revoked, the allowed leaf should be false.
>
>      Since there is "MUST reflect", shouldn't it be "the allowed leaf SHO=
ULD
>      be false."? Uppercase "SHOULD"?
>
>    4. The tree disagrams in sections 3.2, 3.3, and 3.5 are snipped withou=
t
>       including the  augment line from the full tree. It would be clearer=
 to
>       include the augment line.
>
> Editorial suggestions:
>
> 183c183
> <    Network elements provide capabilities=E2=80=9A i.e., functions relat=
ed to
> ---
> >    Network elements provide capabilities, i.e., functions related to
> 288c288
> <    rights to use that capability=E2=80=94even if access to the device i=
tself is
> ---
> >    rights to use that capability, even if access to the device itself i=
s
> 589c589
> <    entitlement=E2=80=94whether active or not.
> ---
> >    entitlement, whether active or not.
> 961c961
> <    when necessary=E2=80=94specifically when each component has its own =
set of
> ---
> >    when necessary, specifically when each component has its own set of
> 1052c1052
> <    Other licenses should be openly constrained to a geographic location=
.
> ---
> >    Other licenses should be specifically constrained to a geographic lo=
cation.
> 1069c1069
> <    Some entitlements are inherently associated with a holder, such as
> ---
> >    Some entitlements are inherently associated with a holder, such as a=
n
> 1074c1074
> <    holders, such as people under a jurisdiction or a geographical area.
> ---
> >    holders, such as people under a jurisdiction or in a geographical ar=
ea.
> 1195c1195
> <    expired, or revoked, allowed should be false.  The in-use field
> ---
> >    expired, or revoked, the allowed leaf should be false.  The in-use l=
eaf
> 1260c1260
> <         Copyright (c) 2025 IETF Trust and the persons identified as
> ---
> >         Copyright (c) 2026 IETF Trust and the persons identified as
> 1423c1423
> <                 capabilities list exist and the information system
> ---
> >                 capabilities list entry exists and the information syst=
em
> 1521c1521
> <                   current allowed state. this container
> ---
> >                   current allowed state. This container
> 1523c1523
> <                   An empty list of supporting-entitlement means
> ---
> >                   An empty list of supporting-entitlements means
> 1662,1664c1662,1664
> <                "Stock Keeping Unit - vendor's catalog/ordering number
> <                 for this entitlement. Used for procurement and asset
> <                 management integration.";
> ---
> >                "Stock Keeping Unit (SKU) - vendor's catalog/ordering
> >                 number for this entitlement. Used for procurement and
> >                 asset management integration.";
> 1756c1756
> <                 statements due to XPath 1.0 limitations, management
> ---
> >                 statements due to XPath 1.1 limitations, management
> 2429c2429
> <                       "description": "Maximum OSPF neighbor adjacencies=
, just to give an example :)",
> ---
> >                       "description": "Maximum OSPF neighbor adjacencies=
",
> 4443c4443
> <    handling is implementation-specific but SHOULD provide sufficient
> ---
> >    handling are implementation-specific but SHOULD provide sufficient
>
> Thanks,
> Acee
>
>
> _______________________________________________
> Ivy mailing list -- ivy@ietf.org
> To unsubscribe send an email to ivy-leave@ietf.org