Early Review of draft-ietf-teas-rfc8776-update-14
review-ietf-teas-rfc8776-update-14-yangdoctors-early-clarke-2024-11-15-00

I have been asked to review this draft on behalf of the YANG Doctors.  This
draft defines two YANG modules: ietf-te-types and ietf-te-packet-types.  Both
define identities, typedefs, and groups.  Neither define data nodes in use by
themselves alone.  I think the draft and modules are on the right track.  I
especially enjoyed the plethora of references throughout.  For the most part,
someone using these modules will have excellent guides as to how to use the
objects.

However, I found a few issues.  First, the indentation and formatting are off. 
I recommend a good "pyang -f yang" on these to normalize the YANG structure. 
Second, the latest revision in each (2020-06-10) doesn't match the file name of
2024-10-17.  The revision statements themselves make reference to the original
8776 where I think you should change that to RFC XXXX to prompt the RFC Ed to
change it when this new draft is published.

One other thing that generally applies to both modules: there are several leafs
that specific delay.  The description indicates the delay is measured in
microseconds, but there is no "units" attribute.  I think making units explicit
in the leaf definition would be valuable.  Similarly, I see several packet-loss
leafs where the max is described as 50.331642%.  I think a "percent" units
would be useful on those.

On to per-module items:

ietf-te-types:

For the first typedef srlg, expand SRLG in the description.  You do this below
for another node, but I think it would be helpful here.

For typedef te-metric, please provide a more detailed description.

For leaf one-way-delay-offset, what unit is this measured in?  And can you add
that as a "units" attribute?  Also, add units for measured-interval,
advertised-interval, etc.

In list route-object-include-exclude, you use the explicit-route-hop grouping,
but you augment that to put in slrg.  Why?  Why not just have this case in the
type choice globally?  It seems odd to augment one's own module.  You do this
same thing in the path-route-exclude-objects grouping below.

In leaf upper-bound, you have a typo: s/Specificied/Specified/

Module ietf-te-packet-types:

Type: s/Enginneering/Engineering/

For Security Considerations, I think it would be useful to flesh out specifics
around those groupings or grouping leafs that could be more sensitive it would
be helpful to those that consume these modules.