RE: [Gen-art] Genart last call review of draft-ietf-tictoc-1588v2-yang-09

[Rodney Cummings <[e-mail]>]

Hi Linda,

I'll attempt to address both of your comments, and my co-authors can chime in as needed.

Regarding use of enum...

I agree that identity would be preferable in order to allow augments and other modules to add
new values. The reason this module uses enum is to explicitly disallow that sort of addition.
The reason why is that the list of number/name pairs is published in the IEEE 1588 standard,
and the IEEE 1588 working group is the only entity that can add new values. The number for each
name is used for IEEE 1588 messages (not only YANG management), and that's why the 
1588 standard itself needs to enforce its own registration to avoid incompatibilities.

That being said, it is certainly possible that future editions of the IEEE 1588 standard will
add new number/name pairs to the list. When that occurs, IEEE 1588 will revise the YANG
module to align with the new 1588 edition, and that YANG revision will add new enum values
according to the requirements of RFC 7950 section 11, which states:
      An "enumeration" type may have new enums added, provided the old
      enums's values do not change.  Note that inserting a new enum
      before an existing enum or reordering existing enums will result
      in new values for the existing enums, unless they have explicit
      values assigned to them.
Since IEEE 1588 assigns explicit values, and IEEE 1588 cannot change old values
(i.e., cannot break existing products), this requirement is straightforward.

Therefore, I recommend retaining the current use of enum.

Regarding default-ds and current-ds...

Those specific terms are used in the IEEE 1588 standard document itself, including
use of the abbreviation "ds" for "data set". 1588's data sets are used by the
protocol itself (e.g. in messages), and also for management (thus in YANG). The terms
date back to the early 1990's, so most 1588 implementers just "know" what these terms
mean.

That being said, I agree that additional description in the YANG module would be
helpful.

I would say that the original intent for these data sets from a 
management perspective was:
- default-ds: configuration of local 1588 data for the instance
- current-ds: state of local 1588 data for the instance

Most of the other data sets in 1588 represent information learned remotely (from 
received messages), configuration/state of a port, or data for optional features.

Why do I say "original intent"? Unfortunately, the IEEE 1588 document did not
provide an explicit definition of "default" and "current", and the document
still doesn't provide such a definition. As with any standard, when a concept
is ambiguous, implementers sometimes add enhancements in unintended ways. In 
this case, it was possible for a 1588 implementer to add state data to default-ds, 
or add configuration data to current-ds. For YANG, that sort of addition might be
done in a vendor-specific augment in order to represent product features that
have existed for years. For example, we could potentially add "config false" to
current-ds, but if we do, there might be complaints from folks who ship a product
that configures current-ds, with justification based on the ambiguity in the 
1588 standard document itself.

That history is the reason why we effectively dodged addition of a better description
of default-ds and current-ds.

I'd appreciate your advice on this.

Rodney Cummings

> -----Original Message-----
> From: Linda Dunbar <[e-mail]>
> Sent: Tuesday, September 4, 2018 5:42 PM
> To: Linda Dunbar <[e-mail]>; [e-mail]
> Cc: [e-mail]; [e-mail];
> [e-mail]
> Subject: RE: [Gen-art] Genart last call review of draft-ietf-tictoc-
> 1588v2-yang-09
> 
> One more comment with the structure of the YANG Module:
> 
> The data model specified used several "enum" type, making it very
> difficult to expand in the future.
> 
> For example, "delay-mechanism-enumeration" currently has "e2e", "p2P", and
> "disabled". If you want to add one more value, the new data model is not
> backward compatible.
> 
> Should consider using "identity" and use "identityref". When expand in the
> future, data model is still backward compatible.
> 
> Linda Dunbar
> 
> -----Original Message-----
> From: Gen-art [mailto:[e-mail]] On Behalf Of Linda Dunbar
> Sent: Tuesday, September 04, 2018 5:30 PM
> To: [e-mail]
> Cc: [e-mail]; [e-mail];
> [e-mail]
> Subject: [Gen-art] Genart last call review of draft-ietf-tictoc-1588v2-
> yang-09
> 
> Reviewer: Linda Dunbar
> Review result: Almost Ready
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area Review
> Team (Gen-ART) reviews all IETF documents being processed by the IESG for
> the IETF Chair.  Please treat these comments just like any other last call
> comments.
> 
> For more information, please see the FAQ at
> 
> <https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__trac.ietf.org_trac_gen_wiki_GenArtfaq&d=DwIFAg&c=I_0YwoKy7z5LMTVdyO6YC
> iE2uzI1jjZZuIPelcSjixA&r=WA71sf2o7Dw7CbYhFt24DPjt3lJuupswWYdnboKbZ8k&m=4uF
> XFzSFUJzVZFHQNy9WTW6pJuumHOJ1hyKNAe-
> hPYM&s=4idNt4twmS2wCSQ2GRIHlrdgUd9wsckQx3u9H85y1XM&e=>.
> 
> Document: draft-ietf-tictoc-1588v2-yang-??
> Reviewer: Linda Dunbar
> Review Date: 2018-09-04
> IETF LC End Date: 2018-09-07
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary:
> This document specify the YANG data model for IEEE1588-2008.
> The document is written very clear. I have some questions, such as What is
> the relationship between Current-DS and Default-DS?
> It seems to be that the "default-ds" has most of the information for the
> clock.
> Is Current-ds simply supplement?
> 
> Major issues:
> 
> Minor issues:
> 
> Nits/editorial comments:
> 
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__www.ietf.org_mailman_listinfo_gen-
> 2Dart&d=DwIFAg&c=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA&r=WA71sf2o7Dw
> 7CbYhFt24DPjt3lJuupswWYdnboKbZ8k&m=4uFXFzSFUJzVZFHQNy9WTW6pJuumHOJ1hyKNAe-
> hPYM&s=9jRwKb0u3LDMczyKgP8Es7qIHh6Fil57BSmR0ZpgOy4&e=