Re: [yang-doctors] Yangdoctors early review of draft-ietf-mpls-msd-yang-02

[Jan Lindblad <[e-mail]>]

Yingzhen,

> Please see my questions below inline about the "mandatory" statement.

Certainly.

>>>> 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?
>>>  
>>> [Yingzhen]: It's meaningless if there is a msd-value without a msd-type. Theoretically
>>> they should always show up as a pair. These are read-only data, so I don't think any
>>> control is needed.
>> 
>> Since you confirm my understanding that both values are needed for an entry to make sense, I would actually suggest that you mark both as   mandatory true;   The mandatory concept is also useful for config false data. In this case it means the server MUST return these values for each entry. This will make it abundantly clear to all that every entry needs to have both. Users that are using this model can file a bug report to their vendors etc if they see something else, rather than a drawn out conversation about what is allowed in this list.
> 
> [Yingzhen]: although both the msd-type and msd-value are needed for an entry to make sense, it's possible for a router to not return any entry. My understanding of a leaf to be mandatory means the leaf has to exist with a value. Please correct me if I'm wrong.

No, not exactly. If you make msd-type and msd-value mandatory, that means that each entry (if any) in the list must have both. Which is, I believe, what everyone expects. So it would be nice to write this down in the model.

// I think you want:

  grouping msd-type-value {
    description
      "Grouping for MSD type and value.";
    leaf msd-type {
      mandatory true;
      type identityref {
        base iana-msd-types:msd-base-mpls;
      }
      description
        "MSD types. The MSD type is defined in IANA IGP
         MSD-Types registry.";
    }
    leaf msd-value {
      mandatory true;
      type uint8;
      description
        "MSD value, in the range of 0-255.";
    }
  }

If you don't have the mandatory true statements, it would, for example, be considered valid from a YANG perspective for an implementation to return all possible msd-type-values but leave some of them without an msd-value. If someone complained, the implementor could claim to implement RFC xxxx (this document). So if you mean that both msd-type and msd-value have to be returned for each entry in the list (if any), I think marking them mandatory is a good thing.

If you wanted to say something about how many entries there must be in the list, you would use min-elements and/or max-elements. E.g. you could say min-elements 1; on the list to require at least one entry. But this is not what you want in this case.

// I think you don't want:

      list node-msds {
        key "msd-type";
        uses msd-type-value;
        // To require at least one entry
        min-entries 1;
        description
          "List of different types of MSDs of the node. A type of
           Node MSD is the smallest same type link MSD supported by
           the node.";
      }


I hope this makes sense. Happy to clarify/discuss.

Best Regards,
/jan