Re: [netmod] draft-ietf-tictoc-1588v2-yang-05 - Concern over port identifier

[Robert Wilton <[e-mail]>]

Hi Yuanlong,

I agree with Martin, having a single key is a better data model.

Duplicating the port-number in the data model just makes it that little 
bit more annoying to use.

I think that Information models are often modeled using UML, which I'm 
not particularly familiar with, but seems to follow object orientated 
principles.

YANG is not object-orientated, instead seems to more closely resemble a 
document based "mixin" architecture.

Because of this difference, I think that there will naturally be 
differences between how something in best modeled in UML vs how it is 
best modeled in YANG.

For me, having the YANG model as simple, consistent, and as easy to 
understand as possible is more important that a very close mapping to a 
related information model.  So, in summary, my suggestion is to allow 
these models to diverge where necessary.

Thanks,
Rob


On 29/09/2017 10:22, Jiangyuanlong wrote:
> Martin,
>
> Thank a lot for your instantly reply. Please see my further comments inline.
>
> Cheers,
> Yuanlong
>
> -----Original Message-----
> From: Martin Bjorklund [mailto:[e-mail]]
> Sent: Thursday, September 28, 2017 9:23 PM
> To: Jiangyuanlong
> Cc: [e-mail]; [e-mail]; [e-mail]
> Subject: Re: [netmod] draft-ietf-tictoc-1588v2-yang-05 - Concern over port identifier
>
> Jiangyuanlong <[e-mail]> wrote:
>> Sean,
>>
>> My personal opinion is that it can work, but I would like to ask for
>> more opinions from our netmod experts;)
>>
>> Hi netmod experts,
>> Considering the following sample module, my-list is a list, and it
>> needs to use a leaf member "port-number" in my-port-container as a
>> key.
>> We now have 3 options:
>> 1.
>>    list my-list {
>>      key "port-number";
>>      leaf port-number {
>>         type uint16;
>>      }
>>
>>      container my-port-container {
>>          leaf port-number {
>>            type uint16;
>>          }
>>           leaf other-leaf {
>>             ...
>>           }
>>      }
>>    }
>> But this does not work for there is compiling error.
> Why wouldn't this work?
> [JY] The original intention of the 1588 info model is to use my-port-container.port-number as the key. The code actually parses, but we need to synchronize my-list.port-number and my-list.my-port-container.port-number all the while.
>
> I suspect you meant:
>
>     list my-list {
>       key "port-number";
>   
>       container my-port-container {
>           leaf port-number {
>            type uint16;
>           }
>            leaf other-leaf {
>              ...
>            }
>       }
>     }
>
>
>> 2.
>>    list my-list {
>>      key "port-number";
>>      leaf port-number {
>>         type uint16;
>>      }
>>      container my-port-container {
>>          leaf port-number {
>>              type leafref{
>>                path "../../port-number";
>>              }
>>          }
>>           leaf other-leaf {
>>             ...
>>           }
>>      }
>>    }
>> Option 2 can parse, though leafref in a sub-module seems not very
>> natural.
>>
>> 3.
>>    list my-list {
>>      key "port-number";
>>      leaf port-number {
>>         type leafref{
>>            path "../my-port-container/port-number";
>>         }
>>      }
>>      container my-port-container {
>>          leaf port-number {
>>            type uint16;
>>          }
>>           leaf other-leaf {
>>             ...
>>           }
>>      }
>>    }
>>
>> Option 3 can also parse, though leafref seems not a very natural key.
>> What is your favorite option? Or do you have any better schemes?
>
> Having a leafref like in option 2 or 3 is just redundant and a hassle for the client.  In order to create a a list entry, the client would have to first provide the port-number value once for the key, and then again the exact same value in the container:
>
>    <my-list>
>      <port-number>25</port-number>
>      <my-port-container>
>        <port-number>25</port-number>
>        <other-leaf>...</other-leaf>
>      </my-port-container>
>    </my-list>
>
> Note that both "port-number" MUST be given the exact same value by the client.
>
> In YANG, key leafs cannot be nested under containers.  I would simply have the key on the top of the list, and not in the container.
>
> (It seems this is what others have proposed as well in this thread.)
>
> [JY] Yes, I agree that this model is not very elegant, but the info model was published in IEEE 1588 already, people have been used to this info model for a long time and we would like to stick to it as far as we can.
> As an alternative, option 2 can be enhanced:
>     list my-list {
>       key "port-number";
>       leaf port-number {
>          type uint16;
>       }
>       container my-port-container {
>           leaf port-number {
>               type leafref{
>                 path "../../port-number";
>               }
>               config false;
>           }
>           leaf other-leaf {
>             ...
>         }
>       }
>     }
>
> In this way, the "config false" makes the leafref read-only, so that configuration doesn't need to write twice. How do you think about this scheme?
> Thanks again,
> Yuanlong
>
>
> /martin
>
>
>
>
>
>> Your opinions are very important for our revision of this draft.
>>
>> Thanks,
>> Yuanlong
>>
>>
>> From: Sean Condon [mailto:[e-mail]]
>> Sent: Wednesday, September 27, 2017 7:11 PM
>> To: Jiangyuanlong
>> Cc: [e-mail]; Rodney Cummings
>> Subject: RE: draft-ietf-tictoc-1588v2-yang-05 - Concern over port
>> identifier
>>
>> Thanks guys for your responses.
>>
>> I accept your points to keep the structure as aligned to the IEEE 1588
>> standard as possible.
>>
>> One alternate suggestion that I would make, is that the port-number
>> currently defined as leafref should be made the real attribute (i.e.
>> uint16) and that the port-number inside port-identity container should
>> be made in to the leaf ref (and set to mandatory true).
>>
>> The reason I say this is that
>>
>>    1.  XML models are usually navigated and constructed root-to-leaf, and
>>    the way it's portrayed at the moment is that port-number leafref
>>    points to something that may not exist at the time it is being
>>    defined. This makes it difficult to implement.
>>    2.  Also port-identity/port-number is not "mandatory true" meaning
>>    that it could be left out altogether. It's not valid for it to have a
>>    default value either since every item in the list will need a
>>    different identifier.
>>
>> With this suggestion the structure you require with port-identity
>> still exists, but now the implementation is more straightforward, and
>> the change will be transparent to an end user.
>>
>>
>> Best regards, Sean
>> =======================
>> Sean Condon
>> System & Software Architect
>> Frequency & Timing Division
>> Microsemi Inc.
>> [e-mail]<mailto:[e-mail]>
>>
>> ________________________________
>> From: Jiangyuanlong [[e-mail]]
>> Sent: 27 September 2017 08:05
>> To: Sean Condon
>> Cc: [e-mail]<mailto:[e-mail]>; Rodney Cummings
>> Subject: RE: draft-ietf-tictoc-1588v2-yang-05 - Concern over port
>> identifier EXTERNAL EMAIL
>>
>> Dear Sean,
>>
>>
>>
>> Thank you a lot for diving into the technical details of this module.
>> Just as Rodney said, it is a challenge of 1588 info model to YANG, and
>> we use the leafref of YANG to work around it.
>>
>> I would like to provide a little more backgrounds on the tradeoff:
>>
>>
>>
>> 1. It says in Sec. 7.5.2.1 in IEEE 1588-2008: portIdentity is a member
>> of the portDS data set. A portIdentity consists of two attributes, as
>>
>> follows:
>>
>> ⎯ portIdentity.clockIdentity
>>
>> ⎯ portIdentity.portNumber
>>
>> Furthermore, the "portDS.portIdentity" attribute is mentioned quite a
>> few times in 1588-2008, especially in Table 17 and under Table 61,
>> with a hint that assignment and comparison can be done on this member
>> directly, thus it seems to me portIdentity is an important and well
>> understood construct in the 1588 specification.
>>
>>
>>
>> 2. If we change portDS.portIdentity from a container to a grouping,
>> then there is no portIdentity for portDS and transparentclockPortDS in
>> the resulting tree diagram:
>>
>>
>>
>> module: ietf-ptp-dataset
>>
>>      +--rw instance-list* [instance-number]
>>
>>      |  +--rw instance-number       uint16
>>
>>      |  +--rw default-ds
>>
>>      |  |  +--rw two-step-flag?    boolean
>>
>>      |  |  +--rw clock-identity?   clock-identity-type
>>
>>      |  |  +--rw number-ports?     uint16
>>
>>      |  |  +--rw clock-quality
>>
>>      |  |  |  +--rw clock-class?                  uint8
>>
>>      |  |  |  +--rw clock-accuracy?               uint8
>>
>>      |  |  |  +--rw offset-scaled-log-variance?   uint16
>>
>>      |  |  +--rw priority1?        uint8
>>
>>      |  |  +--rw priority2?        uint8
>>
>>      |  |  +--rw domain-number?    uint8
>>
>>      |  |  +--rw slave-only?       boolean
>>
>>      |  +--rw current-ds
>>
>>      |  |  +--rw steps-removed?        uint16
>>
>>      |  |  +--rw offset-from-master?   time-interval-type
>>
>>      |  |  +--rw mean-path-delay?      time-interval-type
>>
>>      |  +--rw parent-ds
>>
>>      |  |  +--rw parent-port-identity
>>
>>      |  |  |  +--rw clock-identity?   clock-identity-type
>>
>>      |  |  |  +--rw port-number?      uint16
>>
>>      |  |  +--rw parent-stats?                                 boolean
>>
>>      |  |  +--rw observed-parent-offset-scaled-log-variance?   uint16
>>
>>      |  |  +--rw observed-parent-clock-phase-change-rate?      int32
>>
>>      |  |  +--rw grandmaster-identity?                         binary
>>
>>      |  |  +--rw grandmaster-clock-quality
>>
>>      |  |  |  +--rw clock-class?                  uint8
>>
>>      |  |  |  +--rw clock-accuracy?               uint8
>>
>>      |  |  |  +--rw offset-scaled-log-variance?   uint16
>>
>>      |  |  +--rw grandmaster-priority1?                        uint8
>>
>>      |  |  +--rw grandmaster-priority2?                        uint8
>>
>>      |  +--rw time-properties-ds
>>
>>      |  |  +--rw current-utc-offset-valid?   boolean
>>
>>      |  |  +--rw current-utc-offset?         int16
>>
>>      |  |  +--rw leap59?                     boolean
>>
>>      |  |  +--rw leap61?                     boolean
>>
>>      |  |  +--rw time-traceable?             boolean
>>
>>      |  |  +--rw frequency-traceable?        boolean
>>
>>      |  |  +--rw ptp-timescale?              boolean
>>
>>      |  |  +--rw time-source?                uint8
>>
>>      |  +--rw port-ds-list* [port-number]
>>
>>      |     +--rw clock-identity?                clock-identity-type
>>
>>      |     +--rw port-number                    uint16
>>
>>      |     +--rw port-state?                    port-state-enumeration
>>
>>      |     +--rw underlying-interface?          if:interface-ref
>>
>>      |     +--rw log-min-delay-req-interval?    int8
>>
>>      |     +--rw peer-mean-path-delay?          time-interval-type
>>
>>      |     +--rw log-announce-interval?         int8
>>
>>      |     +--rw announce-receipt-timeout?      uint8
>>
>>      |     +--rw log-sync-interval?             int8
>>
>>      |     +--rw delay-mechanism?               delay-mechanism-enumeration
>>
>>      |     +--rw log-min-pdelay-req-interval?   int8
>>
>>      |     +--rw version-number?                uint8
>>
>>      +--rw transparent-clock-default-ds
>>
>>      |  +--rw clock-identity?    clock-identity-type
>>
>>      |  +--rw number-ports?      uint16
>>
>>      |  +--rw delay-mechanism?   delay-mechanism-enumeration
>>
>>      |  +--rw primary-domain?    uint8
>>
>>      +--rw transparent-clock-port-ds-list* [port-number]
>>
>>         +--rw clock-identity?                clock-identity-type
>>
>>         +--rw port-number                    uint16
>>
>>         +--rw log-min-pdelay-req-interval?   int8
>>
>>         +--rw faulty-flag?                   boolean
>>
>>         +--rw peer-mean-path-delay?          time-interval-type
>>
>>
>>
>> In contrast to the original 1588 YANG tree diagram:
>>
>>     module: ietf-ptp-dataset
>>
>>         +--rw instance-list* [instance-number]
>>
>>         |  +--rw instance-number      uint16
>>
>>         |  +--rw default-ds
>>
>>         |  |  +--rw two-step-flag?    boolean
>>
>>         |  |  +--rw clock-identity?   clock-identity-type
>>
>>         |  |  +--rw number-ports?     uint16
>>
>>         |  |  +--rw clock-quality
>>
>>         |  |  |  +--rw clock-class?                  uint8
>>
>>         |  |  |  +--rw clock-accuracy?               uint8
>>
>>         |  |  |  +--rw offset-scaled-log-variance?   uint16
>>
>>         |  |  +--rw priority1?        uint8
>>
>>         |  |  +--rw priority2?        uint8
>>
>>         |  |  +--rw domain-number?    uint8
>>
>>         |  |  +--rw slave-only?       boolean
>>
>>         |  +--rw current-ds
>>
>>         |  |  +--rw steps-removed?       uint16
>>
>>         |  |  +--rw offset-from-master?  time-interval-type
>>
>>         |  |  +--rw mean-path-delay?     time-interval-type
>>
>>         |  +--rw parent-ds
>>
>>         |  |  +--rw parent-port-identity
>>
>>         |  |  |  +--rw clock-identity?   clock-identity-type
>>
>>         |  |  |  +--rw port-number?      uint16
>>
>>         |  |  +--rw parent-stats?        boolean
>>
>>         |  |  +--rw observed-parent-offset-scaled-log-variance? uint16
>>
>>         |  |  +--rw observed-parent-clock-phase-change-rate?    int32
>>
>>         |  |  +--rw grandmaster-identity?                       binary
>>
>>         |  |  +--rw grandmaster-clock-quality
>>
>>         |  |  |  +--rw clock-class?                  uint8
>>
>>         |  |  |  +--rw clock-accuracy?               uint8
>>
>>         |  |  |  +--rw offset-scaled-log-variance?   uint16
>>
>>         |  |  +--rw grandmaster-priority1?           uint8
>>
>>         |  |  +--rw grandmaster-priority2?           uint8
>>
>>         |  +--rw time-properties-ds
>>
>>         |  |  +--rw current-utc-offset-valid?   boolean
>>
>>         |  |  +--rw current-utc-offset?         int16
>>
>>         |  |  +--rw leap59?                     boolean
>>
>>         |  |  +--rw leap61?                     boolean
>>
>>         |  |  +--rw time-traceable?             boolean
>>
>>         |  |  +--rw frequency-traceable?        boolean
>>
>>         |  |  +--rw ptp-timescale?              boolean
>>
>>         |  |  +--rw time-source?                uint8
>>
>>         |  +--rw port-ds-list* [port-number]
>>
>>         |     +--rw port-number        -> ../port-identity/port-number
>>
>>         |     +--rw port-identity
>>
>>         |     |  +--rw clock-identity?   clock-identity-type
>>
>>         |     |  +--rw port-number?      uint16
>>
>>         |     +--rw port-state?          port-state-enumeration
>>
>>         |     +--rw underlying-interface? if:interface-ref
>>
>>         |     +--rw log-min-delay-req-interval?    int8
>>
>>         |     +--rw peer-mean-path-delay?          time-interval-type
>>
>>         |     +--rw log-announce-interval?         int8
>>
>>         |     +--rw announce-receipt-timeout?      uint8
>>
>>         |     +--rw log-sync-interval?             int8
>>
>>         |     +--rw delay-mechanism?     delay-mechanism-enumeration
>>
>>         |     +--rw log-min-pdelay-req-interval?   int8
>>
>>         |     +--rw version-number?                uint8
>>
>>         +--rw transparent-clock-default-ds
>>
>>         |  +--rw clock-identity?    clock-identity-type
>>
>>         |  +--rw number-ports?      uint16
>>
>>         |  +--rw delay-mechanism?   delay-mechanism-enumeration
>>
>>         |  +--rw primary-domain?    uint8
>>
>>         +--rw transparent-clock-port-ds-list* [port-number]
>>
>>            +--rw port-number           -> ../port-identity/port-number
>>
>>            +--rw port-identity
>>
>>            |  +--rw clock-identity?   clock-identity-type
>>
>>            |  +--rw port-number?      uint16
>>
>>            +--rw log-min-pdelay-req-interval?   int8
>>
>>            +--rw faulty-flag?                   boolean
>>
>>            +--rw peer-mean-path-delay?         time-interval-type
>>
>>
>>
>> I agree, assignment and comparison can still be done on clock-identity
>> and port-number directly (without a portIdentity construct) . But
>> people who are familiar with 1588-2008 may question where portIdentity
>> is gone.
>>
>>
>>
>> 3. I think leafref is a very general semantics thing in YANG language,
>> if any tools have problem with this feature, maybe we need to contact
>> with the tool’s developer to support it.
>>
>>
>>
>> Finally, more inputs from the WG are welcome.
>>
>>
>>
>> Thanks again,
>>
>> Yuanlong
>>
>>
>>
>>
>>
>> -----Original Message-----
>> From: TICTOC [mailto:[e-mail]] On Behalf Of Rodney
>> Cummings
>> Sent: Wednesday, September 27, 2017 1:24 AM
>> To: [e-mail]<mailto:[e-mail]>
>> Subject: Re: [TICTOC] draft-ietf-tictoc-1588v2-yang-05 - Concern over
>> port identifier
>>
>>
>>
>> Thanks Sean,
>>
>>
>>
>> Regarding your other comment on TLD... I agree.
>>
>>
>>
>> Regarding the comment below on port-identity, I agree that we need to
>> do it for practical reasons.
>>
>>
>>
>> In 1588-2008, there is a distinct dataset member for
>> portDS.portIdentity, which 5.3.5 specifies as using type PortIdentity:
>>
>>    struct PortIdentity {
>>
>>      ClockIdentity clockIdentity;
>>
>>      UInteger portNumber;
>>
>>    }
>>
>>
>>
>> If we interpret the 1588-2008 datasets as an information model, and
>> apply that as directly as possible to a YANG data model,
>> portDS.portIdentity is a container, which is what we have so far. That
>> introduces a challenge, because we want to use
>> portDS.portIdentity.portNumber as the key to the list of portDS's. We
>> solved that challenge with a leafref, but I'd agree that is ugly.
>>
>>
>>
>> Your proposal changes portDS.portIdentity from a container to a
>> grouping, so that it portDS.portIdentity.portNumber can be used as the
>> key without a leafref.
>>
>>
>>
>> The downside is that if/when a YANG management client tries to "get"
>> portDS.portIdentity, it doesn't work... there is no portIdentity to
>> get.
>>
>>
>>
>> Personally, I think that downside is worth it. One can argue that your
>> proposal still conforms to the 1588-2008 information model for
>> management, in that portDS.portIdentity still "exists" in a manner
>> that makes sense for YANG.
>>
>>
>>
>> Rodney
>>
>>
>>
>>
>>
>>
>>
>> From: TICTOC [mailto:[e-mail]] On Behalf Of Sean Condon
>>
>> Sent: Tuesday, September 26, 2017 10:38 AM
>>
>> To: [e-mail]<mailto:[e-mail]>
>>
>> Subject: [TICTOC] draft-ietf-tictoc-1588v2-yang-05 - Concern over port
>> identifier
>>
>>
>>
>> With reference to "YANG Data Model for IEEE 1588v2"
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_ht
>> ml_draft-2Dietf-2Dtictoc-2D1588v2-2Dyang-2D05&d=DwMFAw&c=I_0YwoKy7z5LM
>> TVdyO6YCiE2uzI1jjZZuIPelcSjixA&r=WA71sf2o7Dw7CbYhFt24DPjt3lJuupswWYdnb
>> oKbZ8k&m=jKHczVNLN-KuV2HRZkbEZY1SzlCD_AztkaWSccrqBI8&s=msh7A7_OgHZ1l65
>> Dn6_LhiDIXkXpFeiLGmKbKxsqXWw&e=
>>
>>
>>
>> I have a concern about the structure of the YANG, and how the lists
>> port-ds-list and transparent-clock-port-ds-list are indexed with a
>> leaf which is a leafref.
>>
>>
>>
>> The structure seems unnecessarily complex - port-number is represented
>> as a leaf directly beneath the list (to be used as key) and again
>> under the port-identity container. It is structured in a way that I
>> believe would make it difficult to work with some YANG tools in the
>> market.
>>
>>
>>
>> The purpose of port-identity container is not clear from the document
>> - it would achieve the same purpose if it was left out of
>> port-ds-entry and transparent-clock-port-ds-entry and instead the
>> grouping port-identity-grouping included directly.
>>
>>
>>
>> See the attached files as original, a modified version and as a patch
>> file.
>>
>>
>>
>> Sean Condon
>>
>> =======================
>>
>> Sean Condon
>>
>> System & Software Architect
>>
>> Frequency & Timing Division
>>
>> Microsemi Inc.
>>
>> mailto:[e-mail]
>>
>>
>>
>> _______________________________________________
>>
>> TICTOC mailing list
>>
>> [e-mail]<mailto:[e-mail]>
>>
>> https://www.ietf.org/mailman/listinfo/tictoc
> _______________________________________________
> netmod mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netmod