Hello all,

I performed a detailed review of section 3 (YANG module), and I have some 
suggestions. I apologize to my co-authors for not raising these issues sooner 
(I was more focused on the other sections).

1. Upload the YANG module to 
https://github.com/YangModels/yang/tree/master/standard/ietf/DRAFT

Now that this module is WG draft, I assume this is appropriate, and it will 
increase visibility with the YANG community.

2. For data set members specified as "implementation-specific" in 1588-2008, 
use the data type from clause 15.

The "implementation-specific" text refers to use of the data inside the 
implementation, and does not refer to management. Management needs a data type, 
and 1588-2008 provides this for its own management protocol (clause 15). This 
module should use the data type from clause 15, since YANG is another form of 
management. As a side note, the revision of 1588 is clarifying this issue, so 
use of the 1588-2008 clause 15 type will align in the future.

Details:
- current-ds-entry.offset-from-master: Use TimeInterval as type (see suggestion 
#3, and 15.5.3.4.1.2)
- current-ds-entry.mean-path-delay: Use TimeInterval as type (see suggestion 
#3, and 15.5.3.4.1.3)
- time-properties-ds-entry.current-utc-offset: Use int16 (see 15.5.3.6.1.1)

3. Create and use a grouping for each data type in 1588-2008 subclause 5.3 
(derived data type specifications).

1588-2008 specifies a "struct" for many types, and the data set members use 
those types. We should repeat this practice in the YANG module for consistency, 
and to reduce duplication.

Details:
- Create a grouping for TimeInterval of 5.3.2, and use that grouping for:
        * current-ds-entry.offset-from-master
        * current-ds-entry.mean-path-delay
        * port-ds-entry.peer-mean-path-delay
        * transparent-clock-port-ds-entry.peer-mean-path-delay
- Create a grouping for ClockIdentity of 5.3.4, and use that grouping for:
        * default-ds-entry.clock-identity
        * parent-ds-entry.grandmaster-identity
        * transparent-clock-default-ds-entry.clock-identity
- Create a grouping for PortIdentity of 5.3.5, and use that grouping for:
        * parent-ds-entry.parent-port-identity
        * port-ds-entry.port-identity
        * transparent-clock-port-ds-entry.port-identity
- Create a grouping for ClockQuality of 5.3.7, and use that grouping for:
        * default-ds-entry.clock-quality
        * parent-ds-entry.grandmaster-clock-quality

4. Update the description of instance-list.

It is correct to say that each instance represents a distinct domain, but not 
necessarily a distinct domain-number. To be clearer we could replace the 
description with: 

"List of one or more PTP datasets in the device, one for each domain (see IEEE 
1588-2008 subclause 6.3). Each PTP dataset represents a distinct instance of 
PTP implementation in the device (i.e. distinct Ordinary Clock or Boundary 
Clock)."

5. Create and use a typedef for each Enumeration data type in 1588-2008.

1588-2008 uses the data type Enumeration8 (or 16) for data that references a 
table of name/number pairs. As the 1588 standard evolves with each revision, 
new name/number pairs will be added.
 
The current draft uses a YANG enumeration for:
        * port-ds-entry.delay-mechanism
        * transparent-clock-default-ds-entry.delay-mechanism
Since 1588-2008 has a single table for both, in the YANG we should create a 
typedef for this, and use that typedef for both data set members.

The current draft uses YANG "uint8" for other enumerations in 1588-2008. We 
should consider creating typedefs with an enumeration for those as well:
        * port-ds-entry.port-state
        * time-properties-ds-entry.time-source

Other modules seem to use YANG "identityref" for enumerations (e.g. draft 
802.1Q modules), but I admit to ignorance as to why. I would like to understand 
the advantages/disadvantages of enumeration and identityref, to help ensure 
that we are using the best type for 1588-2008.

Rodney

_______________________________________________
TICTOC mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/tictoc

Reply via email to