[Lsr] Robert Wilton's No Objection on draft-ietf-lsr-yang-isis-reverse-metric-04: (with COMMENT)

[Robert Wilton via Datatracker <[e-mail]>]

Robert Wilton has entered the following ballot position for
draft-ietf-lsr-yang-isis-reverse-metric-04: No Objection

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/blog/handling-iesg-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-lsr-yang-isis-reverse-metric/



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

Hi Chris,

Thanks for this YANG module.

Nit:
- Copyright statement in the YANG module should presumably be updated to 2021,
to match the revision entry.

A few comments on the YANG model:
- For the interface config, reverse-metric turns up twice in the path.  You
could perhaps drop it from the
  grouping so that it only appears once?
- Would it be helpful to make the top level reverse-metric container have
presence?  This might make more
  sense if constraints are ever added to validate that a metric is specified at
  the top level, or under at least one of the levels.
- Am I right in assuming that that the level-1/level-2 config is effectively
hierarchical and would override
  the config under the reverse-metric grouping?  E.g., is it allowed to specify
  a metric at the top level, and the whole-lan flag only under the level-1?  If
  so, would it be helpful to document this hierarchical behaviour in the
  description for the fields?
- There is a default assigned to exclude-te-metric, but no default assigned to
whole-lan and allow-unreachable.
  If the config is not hierarchical, then should these all have defaults? If
  the config is hierarchical then perhaps they should not have any defaults,
  and the use statement for the top level reverse-metric grouping should refine
  them with default values?  Assuming that the descriptions make their
  hierarchical nature clear?

Security Considerations:
Would it also be helpful to include a reference back to the security
considerations of the base ISIS YANG module, since the concerns that apply to
metrics there would seem to mostly also apply here.

References:
- Probably need to add XML and JSON references or otherwise change the requests
to edit-config or RESTCONF requests. - XML reference can be as per RFC 8342,
JSON should probably be to RFC 7951.

A.1.  Example Enable XML
Suggest retitling to: Enablement Example Using XML YANG Instance Data"

A.2.  Example Use XML
Suggest retitling to: "Usage Example using XML YANG Instance Data"

A.3.  Example JSON
Suggest retitling to: "Usage Example using JSON YANG Instance Data"

Thanks,
Rob