Re: [CCAMP] FW: I-D Action: draft-ietf-ccamp-l1csm-yang-03.txt

[Leeyoung <[e-mail]>]

Hi Tom,



The draft has been updated yesterday. Please see https://datatracker.ietf.org/doc/draft-ietf-ccamp-l1csm-yang/?include_text=1

See also the attached diff file that compares versions 4 and 5.



Please see inline for my specific comment to your comment. Let us know if there are any other concerns you may have.



Thanks.

Young

-----Original Message-----
From: t.petch [mailto:[e-mail]]
Sent: Tuesday, June 26, 2018 11:31 AM
To: Leeyoung <[e-mail]>; [e-mail]
Subject: Re: [CCAMP] FW: I-D Action: draft-ietf-ccamp-l1csm-yang-03.txt



Thanks for fixing the indentation; much easier to comment on!



Two major thoughts.



First, how attached are you to l1, that is letter l followed by digit 1 ?  I know it makes sense but I also know many contexts in which that would not be allowed because the probability of confusion is so great, with the digit being read as the letter i or letter l or the letter being read as a digit  or ...  I think that this needs careful consideration; I was making mistakes reading the I-D on the comfort of a proper PC with good resolution etc (but with oldish eyes:-(



YL>> We can discuss this issue during the WG meeting in Montreal. I don't have much preference on this.



Second, references need some work.  You identify the YANG modules as version 1.1 but have an Informative Reference - must be Normative - to YANG 1.0; confusing.  The Security Considerations are not the current one;  e.g. you should reference RESTCONF and SSH. Might I suggest looking at

draft-ietf-ccamp-mw-yang-06 ? And the section does reference RFC6241 and

RFC6536 which I cannot see in the Normative References



YL>> I think we have addressed theses references issues.



s.1    controller via a Netconf[RFC6020] interface.

No; NETCONF is not RFC6020



YL>> Corrected.



In the identity coding func you reference clauses 49, 82 and such like - these need reference clauses to the relevant specification in the YANG module.  This probably applies to most of these definitions (although one comprehensive reference to any one document would suffice).



YL>> We put the MEF reference in the YANG module as well as in the Reference Section from which all the terms and the original references for specific clauses can be checked.



You need YANG reference clauses for

  import ietf-yang-types

and

  import ietf-l1-mef-service-types {



YL>> This was already done in Section 1.4. Let us know if this is what you meant.



Likewise I recognise e.g. 'one way severely errored second' as a technical term of the ITU-T; if that is what you intend, then these all need a reference to the current ITU-T Recommendation.  If not, more explanation is needed



YL>> It is indeed ITU-T term. The MEF document referenced has the original ITU-T references.



You need YANG reference clauses for

  import ietf-yang-types

and

  import ietf-l1-mef-service-types {



YL>> I think this is the same comment as above.



Besides this,

- the running footing tells me that this is valid until 2028.



YL>> I am not sure which one you are pointing. If this is not fixed with the revision, please let us know which one you are referring to.



-s 1.4 references [This I-D]

Suggest RFC XXXX (and add a note up front to the RFC Editor asking them to replace XXXX with the number assigned to the RFC)



YL>> We corrected. See Section 1.4 for this.



-  grouping protocol-coding-optical_interface {

    description  "describes <p,c,o>";

p c o are not expanded in the YANG module - they need to be



YL>> We added description for each term.



-  grouping subscriber-l1vc-sls-service-attribute {

    description

      "The value of the Subscriber L1VC SLS (Service Level

        Specification) Service Attribute expressed in a 3-tuple

        of the form.";

leaves me asking what form does the 3-tuple take?



YL>> We added comment that the 3-tuple is <p,c,o>.



-      leaf time-interval {

        type int64;

        units seconds;

        description "a time interval (e.g., 1 month) ...

int64 is a lot of seconds and anyway cannot be used to express a month since a month is a number of different lengths.  If a month is a sensible time (seems long to me) then change it to 28 day and give the value for 28 day in second and perhaps provide a default as that.  And how about a range limiting this to something sensible (lest a typo sets it to 2028 years)



YL>> We added an example with 28 days and changed int64 to int 16.



-      leaf subscriber-l1vc-ep-id-1 { type string;

        description "subscriber end point ID";  }



      leaf subscriber-l1vc-ep-id-2 { type string;

        description "subscriber end point ID"; Spot the difference - I can't!



YL>> we added "subscriber end point ID for one end" and "subscriber end point ID for the other end" to differentiate these two terms.

-  container l1cs

This is the sort of identifier I expect to be frequently misspelt!



YL>>  I would stick to this for now until we have discussion in the f2f meeting.



Enough for now



Tom Petch



----- Original Message -----

From: "t.petch" <[e-mail]<mailto:[e-mail]>>

To: "Leeyoung" <[e-mail]<mailto:[e-mail]>>; <[e-mail]<mailto:[e-mail]>>

Sent: Saturday, June 23, 2018 9:07 PM

Subject: Re: [CCAMP] FW: I-D Action: draft-ietf-ccamp-l1csm-yang-03.txt





> Young

>

> The most significant comment, for me, is the one about the

indentation;

> there came a point where I gave up trying to follow the I-D because it

> was so hard, so fix that and likely more comments will follow:-)

>

> Tom Petch

>

> ----- Original Message -----

> From: "Leeyoung" <[e-mail]<mailto:[e-mail]>>

> To: <[e-mail]<mailto:[e-mail]>>

> Sent: Thursday, June 21, 2018 11:43 PM

>

> > Hi WG,

> >

> > This revision had some error in the date field in the yang model. So

> it is corrected; Tom's comments will be incorporated in the next

version

> (04).

> >

> > Thanks,

> > Young

> >

> > -----Original Message-----

> > From: CCAMP [mailto:[e-mail]] On Behalf Of

> [e-mail]<mailto:[e-mail]>

> > Sent: Thursday, June 21, 2018 5:35 PM

> > To: [e-mail]<mailto:[e-mail]>

> > Cc: [e-mail]<mailto:[e-mail]>

> > Subject: [CCAMP] I-D Action: draft-ietf-ccamp-l1csm-yang-03.txt

> >

> >

> > A New Internet-Draft is available from the on-line Internet-Drafts

> directories.

> > This draft is a work item of the Common Control and Measurement

Plane

> WG of the IETF.

> >

> >         Title           : A Yang Data Model for L1 Connectivity

> Service Model (L1CSM)

> >         Authors         : G. Fioccola

> >                           K. Lee

> >                           Y. Lee

> >                           D. Dhody

> >                           D. Ceccarelli

> > Filename        : draft-ietf-ccamp-l1csm-yang-03.txt

> > Pages           : 24

> > Date            : 2018-06-21

> >

> > Abstract:

> >    This document provides a YANG data model for Layer 1 Connectivity

> >    Service Model (L1CSM).

> >

> >

> >

> >

> > The IETF datatracker status page for this draft is:

> > https://datatracker.ietf.org/doc/draft-ietf-ccamp-l1csm-yang/

> >

> > There are also htmlized versions available at:

> > https://tools.ietf.org/html/draft-ietf-ccamp-l1csm-yang-03

> > https://datatracker.ietf.org/doc/html/draft-ietf-ccamp-l1csm-yang-03

> >

> > A diff from the previous version is available at:

> > https://www.ietf.org/rfcdiff?url2=draft-ietf-ccamp-l1csm-yang-03

> >

> >

> > Please note that it may take a couple of minutes from the time of

> submission until the htmlized version and diff are available at

> tools.ietf.org.

> >

> > Internet-Drafts are also available by anonymous FTP at:

> > ftp://ftp.ietf.org/internet-drafts/

> >

> > _______________________________________________

> > CCAMP mailing list

> > [e-mail]<mailto:[e-mail]>

> > https://www.ietf.org/mailman/listinfo/ccamp

> >

> > _______________________________________________

> > CCAMP mailing list

> > [e-mail]<mailto:[e-mail]>

> > https://www.ietf.org/mailman/listinfo/ccamp

>

> _______________________________________________

> CCAMP mailing list

> [e-mail]<mailto:[e-mail]>

> https://www.ietf.org/mailman/listinfo/ccamp