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

[Martin Bjorklund <[e-mail]>]

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?

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.)



/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_html_draft-2Dietf-2Dtictoc-2D1588v2-2Dyang-2D05&d=DwMFAw&c=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA&r=WA71sf2o7Dw7CbYhFt24DPjt3lJuupswWYdnboKbZ8k&m=jKHczVNLN-KuV2HRZkbEZY1SzlCD_AztkaWSccrqBI8&s=msh7A7_OgHZ1l65Dn6_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