IETF Last Call Review of draft-ietf-mpls-mldp-yang-16
review-ietf-mpls-mldp-yang-16-yangdoctors-lc-clarke-2026-03-26-00

I am been asked to re-review this document on behalf of YANG Doctors.  My
earlier assessment was it was on the right track, and a few of the points I
raised have been addressed.  However, there are still issues in rev -16, though
it's overall in good shape.

The mldp-binding-label-state-attributes and mldp-ext-binding-label-peer-state
groupings contain leafs with relative XPaths, but neither describe the contexts
where these groupings are applicable as required by RFC9907.

You define two presence containers for ipv4 and ipv6 with the description
"Enables IP...unless the 'enabled' leaf is set to 'false'".  This is confusing
with respect to what 9907 calls for.  The presence definition should clear
state what happens when the container is created.  Maybe just simplify these
descriptions to "Enables IP...mLDP support."  That said, I find it odd to have
both a presence container and an enabled flag.  Maybe these containers don't
need to be presence at all and just rely on the enabled leaf?

RFC 9907 Section 4.8 requires that you have the following in all modules'
descriptions:

> "All revisions of IETF and IANA published modules can be found at the 'YANG
Parameters' registry group: https://www.iana.org/assignments/yang-parameters."

RFC 9907 Section 3.7.1 updates the security template.  The template now
requires:

> "This section is modeled after the template described in Section 3.7.1 of
[RFC9907]."

Though, in your case it's kind of a rough model.  You do need to incorporate
more of the template (e.g., mention QUIC and RFC9000).

RFC 9907 Section 3.8.3.1 now requires the YANG Module Name registration
template to include

Maintained by IANA? Y or N

In this case, I'd think adding the following in your IANA Considerations for
the modules:

Maintained by IANA? N

I noticed some unprefixed path elements in your augments.  For example:

     // IPv6 bindings state
     augment "/rt:routing/rt:control-plane-protocols/"
       + "rt:control-plane-protocol/ldp:mpls-ldp/ldp:global/mldp:mldp/"
       + "mldp:address-families/ipv6/roots/root/bindings" {

You need to have mldp: in front of the ipv6, roots, root, and bindings elements.

// IPv6 config
     augment "/rt:routing/rt:control-plane-protocols/"
       + "rt:control-plane-protocol/ldp:mpls-ldp/ldp:global/mldp:mldp/"
       + "mldp:address-families/ipv6" {
       description "Augmentation for mLDP IPv6 configuration";
       uses mldp-ext-per-af-config-attributes;
     }

Was another one I found.  I think those were the only two.

I noticed an inconsistency between IPv4 and IPv6.  IPv6 bindings augmentation
containers explicitly carry "config false", but the IPv4 containers do not.  I
imagine they should be the same, yes?

As for nits:

You have two instances of the typo exchnaged.  Operatiobal appears multiple
times, and you misspelled copyright as copytight in your latest module
revisions.