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:[email protected]]
Sent: Thursday, September 28, 2017 9:23 PM
To: Jiangyuanlong
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [netmod] draft-ietf-tictoc-1588v2-yang-05 - Concern over port 
identifier

Jiangyuanlong <[email protected]> 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:[email protected]]
Sent: Wednesday, September 27, 2017 7:11 PM
To: Jiangyuanlong
Cc: [email protected]; 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.
[email protected]<mailto:[email protected]>

________________________________
From: Jiangyuanlong [[email protected]]
Sent: 27 September 2017 08:05
To: Sean Condon
Cc: [email protected]<mailto:[email protected]>; 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:[email protected]] On Behalf Of Rodney
Cummings
Sent: Wednesday, September 27, 2017 1:24 AM
To: [email protected]<mailto:[email protected]>
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:[email protected]] On Behalf Of Sean Condon

Sent: Tuesday, September 26, 2017 10:38 AM

To: [email protected]<mailto:[email protected]>

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:[email protected]



_______________________________________________

TICTOC mailing list

[email protected]<mailto:[email protected]>

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

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

Reply via email to