Hi all,

This is my YANG-Doctor Last-Call review of draft-ietf-mpls-msd-yang-02. This draft contains two small and focused modules. They seem to be in pretty good shape, but I do have a couple of issues and nits to discuss.

Issue #1: Keyless list

The list node-msds is keyless. This is legal for config false lists, but that something is legal does not necessarily make it a good idea. Is there a strong reason for not having a key here? Keyless lists are less efficient, as all clients will have to read the entire list every time. Unless there is a strong reason not to, I would suggest adding a key here. 

Maybe the msd-type could function as a key for this list, or will there ever be multiple msd entries of the same msd type? Multiple entries of the same msd-type are currently allowed, but I'm not sure what the interpretation of such a server response should be.

         list node-msds {
           leaf msd-type { ... }
           leaf msd-value { ... }
         }

Issue #2: Optional values

Both msd-type and msd-value are modeled as optional. They have no default and there is no description text explaining what it means if either of them is missing. Maybe they should be made mandatory, as I expect the list entry is pretty uninformative unless both are present?

Nit #3: Repetition

The msd-type and msd-value leaf definitions are repeated twice in the module (except for a one letter difference in one description that probably is not intentional). I would suggest placing them in a grouping that is referenced in each list to avoid the repetition.

           leaf msd-type {
             type identityref {
               base iana-msd-types:msd-base-mpls;
             }
             description
               "MSD type";
           }
           leaf msd-value {
             type uint8;
             description
               "MSD value, in the range of 0-255.";
           } 

Nit #4: Indentation

The indentation is a bit off. I'd suggest running the module through a pretty printer, like pyang -f yang.

Nit #5: Typo in reference

The revision statement reference has some typos.

     revision 2023-10-22 {
       description
         "Initial Version";
       reference
         "RFC XXXX: YA YANG Data Model for MPLS MSD..";
     }


Nit #6: Read only data and get-config

In section 5, security considerations, there is a paragraph:

   Some of the readable data nodes in the modules may be considered
   sensitive or vulnerable in some network environments.  It is thus
   important to control read access (e.g., via get, get-config, or
   notification) to these data nodes.

Since the data in these modules is read-only, it will never show up in get-config, so listing that operation may be somewhat superfluous.

Best Regards,
/jan