Early Review of draft-ietf-teas-nrp-yang-05
review-ietf-teas-nrp-yang-05-yangdoctors-early-wills-2026-05-25-00

Thank you for producing a draft that gives a good textual description of the
model.  I have some comments which I have grouped by severity, and am looking
forward to seeing the next revision.

HIGH

1. The ietf-nrp module imports ietf-vpn-common but is missing a normative
reference to RFC 9181.  Also update the module dependency overview in Section 4
(line 821 of the document) to mention RFC 9181

2. The ietf-nrp module imports draft-ietf-spring-srv6-yang and
draft-ietf-teas-yang-topology-filter, but the references are informative.  They
need to be normative.

3. Some of the data in the examples in Appendix B is incorrect:

3a. Lines 2657, 2711, 2657 and 2711: a "mode" leaf is specified, but nrp-device
does not have a mode leaf.

3b. Lines 2539, 2587: "nrp-id" is a uint32.

3c. Line 2546: "srv6-derived" is a Yang case, and should not be in the JSON. 
"srv6-sid" should be directly under "ipv6".

3d. Lines 2553, 2592: typo - change "phb-profile:" to "phb-profile":

3e. Line 2556: don't quote the number.

3f. Lines 2558-60: Malformed JSON.

3g. Line 2590: "bw-value" is a Yang case.  This line should read
"maximum-bandwidth": "10000"

3h. Line 2592: "EF" is an unresolved leafref - it has no matching
qos-profiles/phb-profile-identifier/id, so this example does not stand alone.

3i. Line 2597: Similarly, "te-topology-filter1" is an unresolved leafref.

4. Lines 364 and 397 refer to "policy-profiles", but in the model it is called
"qos-profiles".

MEDIUM

5. Optional selector mechanisms are not modeled with YANG features. The text
says implementations must support at least one selector type, but the module
exposes IPv4, IPv6 HBH, SRv6, MPLS MNA/PSD, and ACL selectors unconditionally. 
Consider modeling optional capabilities with feature/if-feature.

6. Line 2619: The example configures ietf-interfaces type values using
iana-if-type:ethernetCsmacd, but the document does not reference iana-if-type.

LOW

7. ietf-nrp-device copyright year is stale.

8. Line 1294: The "igp-congruent" presence container can be configured without
any child leaves.  Is this correct?  The description sort of implies that
"multi-topology-id" and "algo-id" are mandatory.

9. Line 1438: The "nrp-policy" list only requires the "name" leaf.  All the
other leaves are optional.  Should mandatory/must constraints be added?  I
particularly wonder whether "nrp-id" should be mandatory.

10. Line 1720: "nrp-attachment-type" contains only operational state and could
be marked as "config false" for clarity.

11. Lines 1376, 1438, 1920: Key/name leaves use an unconstrained string, so
empty strings are valid for nrp-policy/name, phb-profile-identifier/id, and
topology-group/group-id.  Add length "1..max" or a stricter typedef if empty
identifiers are invalid.