Subject: Re: [TICTOC] Comments on YANG module of draft-ietf-tictoc-1588v2-yang

From: Jiangyuanlong <[email-masked]>
Date: Tue, 20 Dec 2016 08:56:02 +0000

Hi Rodney,

The V3.0 YANG module file ([email-masked]) was merged into: 
https://github.com/YangModels/yang/tree/master/standard/ietf/DRAFT
You can comment and update this module more conveniently.

If a stable new revision is in shape, then we can update the WG draft to Version 04.

Thanks,
Yuanlong

-----Original Message-----
From: TICTOC [mailto:[email-masked]] On Behalf Of Rodney Cummings
Sent: Thursday, December 01, 2016 7:14 AM
To: [email-masked]
Subject: [TICTOC] Comments on YANG module of draft-ietf-tictoc-1588v2-yang

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-masked]
https://www.ietf.org/mailman/listinfo/tictoc

---

From: Rodney Cummings <[email-masked]>
Date: Tue, 20 Dec 2016 15:45:54 +0000

Thanks Yuanlong,

I assume that you mean that you agree with suggestions 1-5 below. If so, and if others on the list agree, we can work offline to make the changes for Version 04.

Rodney

> From: Jiangyuanlong [mailto:[email-masked]]
> Sent: Tuesday, December 20, 2016 2:56 AM
> To: Rodney Cummings ; [email-masked]
> Subject: RE: Comments on YANG module of draft-ietf-tictoc-1588v2-yang
> 
> Hi Rodney,
> 
> 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
> Now that this module is WG draft, I assume this is appropriate, and it
> will increase visibility with the YANG community.
> 
> 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:
> 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:
> 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)."
> 
> 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.
> 
> 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:
> 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
>