[mpls] Mahesh Jethanandani's Discuss on draft-ietf-mpls-mldp-yang-16: (with DISCUSS and COMMENT)

[Mahesh Jethanandani via Datatracker <[e-mail]>]

Mahesh Jethanandani has entered the following ballot position for
draft-ietf-mpls-mldp-yang-16: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-mpls-mldp-yang/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Section 12.2, RFC 7431 in informative references:

2889 >    [RFC7431]  Karan, A., Filsfils, C., Wijnands, IJ., Ed., and B.
2890 >               Decraene, "Multicast-Only Fast Reroute", RFC 7431,
2891 >               DOI 10.17487/RFC7431, August 2015,
2892 >               <https://www.rfc-editor.org/info/rfc7431>.

RFC 7431 is listed as an informative reference, but in my view, it is used
normatively in the YANG module. The ietf-mpls-mldp-extended module defines
the mldp-mofrr feature and the multicast-only-frr configuration container,
both of which carry an explicit reference statement pointing to RFC 7431:

1891 >      feature mldp-mofrr {
1892 >        description
1893 >          "This feature indicates that the system supports mLDP
1894 >           Multicast only FRR (MoFRR).";
1895 >         reference
1896 >           "RFC7431: Multicast-Only Fast Reroute";
1897 >      }

2008 >        container multicast-only-frr {
2009 >          if-feature mldp-mofrr;
2010 >          description
2011 >            "Multicast-only FRR (MoFRR) policy";
2012 >          reference
2013 >            "RFC7431: Multicast-Only Fast Reroute";

An implementation must consult RFC 7431 to implement the mldp-mofrr feature
correctly. A reference statement in a YANG feature or container that points
to the specification a conforming implementation is required to follow is, by
definition, a normative reference. RFC 7431 must be moved to the normative
references list.

---

Section 10.1.1, Writable Nodes:

2676 >    There are several data nodes defined in this YANG module that are
2677 >    writable/creatable/deletable (i.e., config true, which is the
2678 >    default).  These data nodes may be considered sensitive or vulnerable
2679 >    in some network environments.  Write operations (e.g., edit-config)
2680 >    to these data nodes without proper protection can have a negative
2681 >    effect on network operations.
2682 >
2683 >    For mLDP, the ability to modify MPLS mLDP configuration may allow the
2684 >    entire MPLS mLDP domain to be compromised including forming LDP
2685 >    adjacencies and/or peer sessions with unauthorized routers to mount a
2686 >    massive Denial-of-Service (DoS) attack.
2687 >
2688 >    In particular, the subtrees and data nodes that are sensitive and
2689 >    vulnerable are same as captured in [RFC9070] section 10.1.1

RFC 9907 Section 3.7.1 requires the security template to explicitly enumerate
writable data nodes introduced by this document that could be disruptive if
abused. The concern is that Section 10.1.1 defers entirely to RFC 9070 Section
10.1.1, which lists only LDP-specific nodes; it does not list any of the new
writable nodes that this document introduces.

The base mLDP module introduces writable nodes including, at minimum:
  - mldp:enabled (enables/disables all of mLDP)
  - mldp:p2mp/enabled and mldp:mp2mp/enabled (capability enablement)
  - mldp:make-before-break/* (switchover-delay and timeout timers)
  - mldp:ipv4/enabled (address-family enable/disable)

The extended module adds additional writable nodes, including:
  - mldp-ext:hub-and-spoke/enabled and mldp-ext:node-protection/* (capabilities)
  - mldp-ext:multicast-only-frr/prefix-list and mldp-ext:recursive-fec/prefix-list
  - mldp-ext:mldp-disable (forwarding nexthop interface control)


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Section 12.1, normative reference to RFC 8407:

2860 >    [RFC8407]  Bierman, A., "Guidelines for Authors and Reviewers of
2861 >               Documents Containing YANG Data Models", BCP 216, RFC 8407,
2862 >               DOI 10.17487/RFC8407, October 2018,
2863 >               <https://www.rfc-editor.org/info/rfc8407>.

RFC 8407 has been obsoleted by RFC 9907. In my view, the normative reference
should be updated to RFC 9907. Several RFC 9907 requirements (noted below in
subsequent comments and in the YANG Doctors review by Joe Clarke, thanks Joe) are not
fully satisfied in the current revision.

---

Section 9.1, module description statement:

1159 >      description
1160 >        "This YANG module defines the essential components for the
1161 >         management of Multiprotocol Label Switching (MPLS) Multipoint
1162 >         LDP (mLDP).
...

Section 9.2, module description statement:

1782 >      description
1783 >        "This YANG module defines the extended components for the
1784 >         management of Multiprotocol Label Switching (MPLS) Multipoint
1785 >         LDP (mLDP).

Per RFC 9907 Section 4.8, each module description must include text directing
readers to the IANA YANG Parameters registry (i.e., a statement along the
lines of "The YANG module in this document conforms to the NMDA as described
in RFC 8342. This module is available from the IANA YANG Parameters registry").
Neither module description includes this required registry pointer. I would
suggest adding the standard registry reference text to the description
statement of both modules.

---

Section 9.1, grouping mldp-binding-label-state-attributes (and the equivalent
grouping mldp-ext-binding-label-peer-state in Section 9.2):

1457 >          leaf peer {
1458 >            type leafref {
1459 >              path
1460 >                "../../../../../../../../../../../ldp:peers/ldp:peer/"
1461 >              + "ldp:lsr-id";

Both groupings use deeply nested relative XPath expressions. Per RFC 9907
Section 4.1, a grouping that uses relative XPath paths must document in its
description the precise schema tree context in which it is intended to be
instantiated, so that the XPath evaluation is unambiguous. Neither grouping
currently includes that context description. I would suggest adding a brief
statement to each grouping description specifying its intended instantiation
point.

---

Section 11, IANA Considerations — YANG module registration:

2737 >    +============+===================================+======+===========+
2738 >    | Name       | Namespace                         |Prefix| Reference |
...

Per RFC 9907 Section 3.8.3.1, the YANG Module Names registration template must
include a "Maintained by IANA?" field (value "N" for modules that are not
IANA-maintained). Neither registration entry in Table 3 includes this field.
I would suggest adding the missing field to both entries.

---

Section 3 and the narrative text throughout: I agree with the OPSDIR reviewer
(Ran Chen, 2026-05-06) that the document would benefit from an Operational
Considerations section. draft-ietf-opsawg-rfc5706bis recommends including
such a section before Security Considerations for documents introducing new
protocol state. mLDP introduces observable protocol state (roots, bindings,
capabilities) and new configuration knobs (MBB timers, address-family enables,
prefix-list policies) that are clearly manageable. While rfc5706bis is not yet
enforceable, I would suggest the authors consider adding at least a brief
Operational Considerations section addressing deployment, fault visibility, and
key configuration considerations.

----------------------------------------------------------------------
NIT
----------------------------------------------------------------------

All comments below are about very minor potential issues that you may
choose to address in some way - or ignore - as you see fit. Some were
flagged by automated tools (via
https://github.com/larseggert/ietf-reviewtool) so there will likely
be some false positives. There is no need to let me know what you did
with these suggestions.

Section 9.1, revision 2026-02-03, description:

1188 >          description
1189 >            "Fixing copytight and authors before publication";

s/copytight/copyright/

The same typo appears at the corresponding revision in Section 9.2:

1812 >          description
1813 >            "Fixing copytight and authors before publication";

s/copytight/copyright/

---

Section 9.1, operational state augmentation, container p2mp description:

1499 >          container p2mp {
1500 >            description
1501 >              "Operatiobal state information for the point-to-multipoint
1502 >               capability of a peer";

s/Operatiobal/Operational/

The same misspelling appears in Section 9.2:

2184 >          container hub-and-spoke {
2185 >            description
2186 >              "Operatiobal state information for the peer's
2187 >               hub-and-spoke-multipoint capability.";

2198 >          container node-protection {
2199 >            description
2200 >              "Operatiobal state information for the peer's node-protection
2201 >              capability";

s/Operatiobal/Operational/ (both instances)

---

Section 9.1, grouping mldp-binding-label-state-attributes, list peer description:

1453 >            description
1454 >              "List of peers with bindings exchnaged (sent and/or
1455 >               received)";

s/exchnaged/exchanged/

The same typo appears in Section 9.2:

1933 >          description
1934 >            "List of peers with bindings exchnaged (sent and/or
1935 >             received)";

s/exchnaged/exchanged/

---

Section 3.1, Table 1 header:

306 >       | Opaque Type | Variant     | Opaque specifc Key | RFCs      |

s/specifc/specific/

---

Section 3.1, Table 1:

296 >       *  Transit Birdir

s/Birdir/Bidir/

---

Section 6.2.1:

1017 >    instead, the node treats it as a opaque binary stream

s/a opaque/an opaque/