Subject: Re: [Teas] binary wasRe: Yangdoctors early review of draft-ietf-teas-yang-te-types-03 (was -01)

From: Jan Lindblad <[email-masked]>
Date: Fri, 1 Feb 2019 10:44:00 +0100

Tarek,

Very good! I'm particularly happy that you introduced ordered-by user on the lists I mentioned, and with good descriptions. I am still a little worried that quite a few people will mistake the function of the numeric keys, because many are used to that numeric rule names implies that they are sorted in ascending order. You state very clearly that this is not the case in the description, but not all people read descriptions. I don't insist that you change this, however. Just a minor reflection on my part.

The must expressions on label-start and -end could be simplified. They are making the same tests twice now, but this is not a major problem. If you are interested, I could help you simplify this.

Thank you,
/jan
--
Jan Lindblad, [email-masked], +46 702855728
Solutions Architect, Business Development, Tail-f
Tail-f is now a part of Cisco

> On 31 Jan 2019, at 15:02, Tarek Saad (tsaad) <[email-masked]> wrote:
> 
> Hi Jan and Tom,
>  
> Thank you much for the review and additional comments. We've uploaded version -04 and -05 of the draft that attempts to address your comments.
> Please see inline [TS2] for resolution to raised comments inline below.
>     
>     ----- Original Message -----
>     From: "Jan Lindblad" <[email-masked] <mailto:[email-masked]>>
>     To: "Tarek Saad (tsaad)" <[email-masked] <mailto:[email-masked]>>
>     Cc: <[email-masked] <mailto:[email-masked]>>;
>     <[email-masked] <mailto:[email-masked]>>; <[email-masked] <mailto:[email-masked]>>;
>     <[email-masked] <mailto:[email-masked]>>
>     Sent: Monday, January 21, 2019 10:10 AM
>     Subject: Re: [Teas] Yangdoctors early review of
>     draft-ietf-teas-yang-te-types-03 (was -01)
>     
>     
>     Tarek, team,
>     
>     > Hi Jan
>     >
>     > Thanks again for your thorough review and comments. We have uploaded
>     version -03 of the draft that addresses your comments. Please see Inline
>     [TS] for the resolution action taken.
>     > Please let us know if you have any further comments.
>     
>     Very good to see progress. There are still a few things I think we
>     should discuss. Look for [janl2] inline below.
>     
>     
>     > From: Jan Lindblad <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>
>     > Date: Monday, November 5, 2018 at 4:20 AM
>     > To: Tarek Saad <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>
>     > Cc: "[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>"
>     <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>,
>     "[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>"
>     <[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>>, "[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>" <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>,
>     "[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>" <[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>>
>     > Subject: Re: Yangdoctors early review of
>     draft-ietf-teas-yang-te-types-01
>     > Resent-From: <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>
>     > Resent-To: Tarek Saad <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>,
>     <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>,
>     <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>,
>     <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>,
>     <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>,
>     <[email-masked] <mailto:[email-masked]> <mailto:[email-masked] <mailto:[email-masked]>>>, <[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>>, <[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>>, <[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>>, <[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>>, Lou Berger <[email-masked] <mailto:[email-masked]>
>     <mailto:[email-masked] <mailto:[email-masked]>>>
>     > Resent-Date: Monday, November 5, 2018 at 4:19 AM
>     >
>     > Tarek,
>     >
>     > See my comments below [janl].
>     >
>     >>     General questions:
>     >>
>     >>     #1: There are many locations in the YANG talking about an "ERO
>     subobject index"
>     >>     (and once RRO index, record route subobject). What is this, and
>     how is it
>     >>     supposed to be used? The document is silent on this matter, and I
>     have seen
>     >>     modules with problems around numeric index leafs much like this
>     earlier. Are
>     >>     these numbers stable, i.e. remains the same forever?
>     >>
>     >> [TS]: The list of explicit route objects defines a TE path (e.g.
>     hops), where the index is used as a key to identify a specific entry in
>     the list. A lower index implies the TE path traverses this entry
>     earlier. We will clarify the description and remove mention of
>     “subobject” in the next update to the document.
>     >
>     > [janl] Very good re description changes.
>     >
>     > I'm a little worried that numeric indices will end up in the same
>     quagmire+impasse where ACLs have been the last decades. Isn't there a
>     risk that people will want to insert entries between two consecutive
>     indices? Is "renumber" an operation that someone might ask for on these
>     indices?
>     > [TS]: The list is ordered by the key (index).. We’ve added a note in
>     the description that entries with lower index are visited first by the
>     path. It is understandable that the list may have to be reconfigured if
>     an entry can not be fit in between two existing ones. Note, a change in
>     the configured ERO list is generally handled as non-destructive event to
>     an existing/provisioned tunnel (e.g. handled as make-before-break).
>     >
>     >       leaf index {
>     >         type uint32;
>     >         description
>     >           "Route object entry index. A lower index indicates
>     >            path traverses the hop earlier than the higher index
>     >            hop(s)";
>     
>     [janl2] Thanks for the clarification. My strong advice in the situation
>     you describe would be to change the lists with a uint32 index key to an
>     ordered-by user list with a string key.
> [TS2]: we discussed this amongst the authors. We have agreed that ordered-by user is a good option to define ordering of the entries in the list. We also agreed to leave the uint32 index as a distinguisher of entries only – as opposed to a string key.
>     
>     Several decades' experience attempting to automate management of ACLs
>     keyed by integers speak quite strongly against that solution. The IETF
>     standard YANG modules for ACLs have now abandoned this approach and
>     adopted ordered-by user lists with string keys.
>     
>     Let's say a manager has pushed a path to a set of devices. Now it needs
>     to configure a re-route around a network problem. This means replacing
>     one of the configured legs with five legs. Now, if they are keyed by
>     integers, this may make it necessary to renumber rules to make this
>     re-route fit. Of course an "experienced" manager may leave some room in
>     between rules so that this renumbering doesn't happen so often. The
>     problem is that if it could ever happen, whoever programs the manager
>     still needs to write (and test) the code for this. Then there are
>     references to these rules (exclude, include) that also need to be
>     updated if this happens. More code, more test. And if a human goes in
>     and makes this change manually on a device, the manager no longer has
>     any clue as to what happened. All it sees is a bunch of new rules with
>     new rule names; and a bunch of rules with the same names, many of which
>     have different content; it has no way of learning the intent. What has
>     been added, changed and removed? When nothing is stable, reconciliation
>     of a change becomes pretty much impossible. ACL management is
>     notoriously inefficient today, largely due to this seemingly small (and
>     completely unnecessary) problem with the management approach.
>     
>     The fix is simple. Make sure rule identities do not also double as
>     sequencing information. If identifiers are also used to convey important
>     configuration information, so that existing identifiers must change
>     whenever the intent is adjusted, that leads to an expensive solution. In
>     YANG, there is a mechanism that is made for this situation. That's
>     order-by user lists. In such lists, the order of entries is controlled
>     by the user (operator, programmer) without relying on the key value(s)
>     to determine that order. The user can insert any number of rules before
>     or after any other, or first or last in the list. The user can move
>     entries around without changing their names.
>     
>     This makes the device interface significantly easier to use for both
>     operators and programmers. While integer rule names are still possible,
>     the string type is better as some folks might falsely believe the an
>     integer key would be used as a sorting metric.
>     
>     To fix this in the model, the lists currently keyed by leaf index would
>     look something like this instead:
>     
>       list example-route-state {
>         ordered-by user;
>         key name;
>         uses record-route_state;
>         ...
>       }
> [TS2]: thanks for the detailed explanation and sharing your experience on this. We agreed “ordered-by-user” is acceptable to define the order of items in the list and index key is just to distinguish the different entries with no additional ordering meaning. We’ve added ordered-by-user to the route-object lists using index to address this.
>  
> NEW:
>         list path-route-object {
>           key index;
>           ordered-by user;
>           description
>             "List of route objects either returned by the computation
>              engine or actually used by an LSP";
>           leaf index {
>             type uint32;
>             description
>               "Route object entry index. The index is used to
>                identify an entry in the list. The order of entries
>                is defined by the user without relying on key values";
>  
>  
>     
>     Where the record-route_state looks like this:
>     
>       grouping record-route_state {
>         description
>           "The record route grouping";
>         leaf name {
>           type string;
>           description
>             "Record route hop name. The name is used to
>              identify an entry in the list. Records listed
>              earlier in the list means the path traverses it earlier";
>         }
>     
>     I'm not sure I understand the author's intent perfectly, but my
>     impression is that some of the lists keyed by index are meant to refer
>     to route-objects in other lists. Like list route-object-include-object
>     entries would only make sense when they point to (use the same index as)
>     an existing route-object. Is that so? Any such reference should be
>     modeled using the leafref type, and a path pointer to the list that
>     contains relevant entries. E.g. like this:
>     
>         list route-object-include-object {
>           key name;
>           description
>             "List of explicit route objects to be included
>              in path computation";
>           leaf name {
>             type leafref {
>               path "/path/to/the/rule/name"; // fill in which list we're
>     pointing to here
>             }
>             description
>               "Route object entry name. Points out a route object
>                in list xxxx that will be included in the path.";
>           }
>     
>     Let me know if this makes sense. Happy to discuss and/or explain
>     further.
>     
>     >>     #2: There are few leafs (5) with default values given, and none
>     with mandatory.
>     >>     Probably needs to increase before we get to last call.
>     >> [TS]: In general, the team tried to avoid setting defaults unless it
>     is strictly dictated by another RFC/standard. The team will review again
>     to see if any was missed and will take action.
>     >
>     > [janl] That's all fine. Just remember to describe what happens when a
>     leaf has no value every time there is no default or mandatory. Easy to
>     forget, experience shows. Missing this causes non-interoperability in
>     the standard.
>     > [TS]: the team has opted to assign defaults to all optional leafs in
>     the latest revision of the draft.
>     
>     [janl2] Very good.
>     
>     >>     #3: There are several choices in the module that are meant to be
>     augmented with
>     >>     additional cases. In many instances, this is explicitly spelled
>     out, very good.
>     >>     If this is meant to happen in all choices, it would be nice to
>     point this out
>     >>     in every instance. Also, if there are any specific assumptions or
>     >>     considerations to keep in mind when augmenting in a new
>     technology, please note
>     >>     that in the description as well.
>     >> [TS]: yes, we will update as necessary.
>     >
>     > [janl] Great!
>     >
>     >>     Issues and nits:
>     >>
>     >>     #4: Unclear data type
>     >>     419:
>     >>       typedef admin-group {
>     >>         type binary {
>     >>           length 4;
>     >>
>     >>     What is the format of this binary? If this is always a 4-byte
>     binary, wouldn't
>     >>     a numeric type be more user friendly, e.g. uint32?
>     >> [TS]: the standard defines admin group as 4-octet binary (9 -
>     Administrative group (4 octets) ), and for extended-admin-group as
>     series of those. We are also defining a union to encompass both as
>     admin-groups below. Do you still have concerns on this?
>     >>
>     >>   typedef admin-groups {
>     >>     type union {
>     >>       type admin-group;
>     >>       type extended-admin-group;
>     >>     }
>     >>     description "TE administrative group derived type";
>     >>   }
>     >
>     > [janl] I'm have two concerns. The first is that I don't know what the
>     format of the type is. How do I know how to construct one of these
>     values, or interpret an existing one? Is this a randomly chosen value?
>     The other concern is how an operator would enter this value. The binary
>     type is not the most user friendly of types. A numeric representation in
>     the management interface might make better sense.
>     > [TS]: RFC3630 and RFC5305 defines admin-groups (aka affinity colors)
>     as binary flags that are user specified (e.g. bit-position X is for RED,
>     and bit-position Y is for GREEN). So, binary type would be closest to
>     this representation. On other hand, RFC7591 states that a binary value
>     is represented as a JSON string -- base64 encoding
>     (https://tools.ietf.org/html/rfc7951#section-6.6 <https://tools.ietf.org/html/rfc7951#section-6.6>
>     <https://tools.ietf.org/html/rfc7951#section-6.6 <https://tools.ietf.org/html/rfc7951#section-6.6>>). Let us know if there
>     are still concerns on this.
>     
>     [janl2] The YANG binary type is mostly used for large binary chunks that
>     an operator would never type, e.g. a 2048-bit RSA key or a new software
>     image. As I understand, the admin-group type is used for something an
>     operator might configure or read. If so, I would suggest changing this
>     type. If this is a 4-octet value, an uint32 would work nicely, would it
>     not? "2149847551" Or a dotted-quad? "128.36.17.255" Or maybe you could
>     invent a more human friendly interface, e.g. a string of zeros and ones
>     with dashes at strategic positions?
>     "10000000-00100100-00010001-11111111"
>     
> [TS2]: OK, we’ve reviewed this and we agreed hexstring type is easier for users to pass/respresent such information. We’ve made this change to existing leafs with binary type to use hexstring.
>  
> NEW:
>  
>   typedef admin-group {
>     type yang:hex-string {
>       /* 01:02:03:04 */
>       length "1..11";
>     }
>     description
>       "Administrative group/Resource class/Color representation in
>        hex-string type.";
>     reference "RFC3630 and RFC5305";
>   }
>  
>     If you leave this as type binary, at least don't expect operators to be
>     able to enter these values with reasonable effort. The binary type is
>     not meant for this.
>     
>     I noticed you also modeled the leaf as-number as type binary. This is
>     probably not a good idea. Suggest using uint16 or uint32.
>  
> [TS2]: fixed this to use:
> NEW:  
>           leaf as-number {
>             type inet:as-number;
>             mandatory true;
>             description "The AS number";
>           }
>  
>     >>     #5: identifier in container with optional leafs
>     >>     1496:
>     >>       grouping te-topology-identifier {
>     >>
>     >>     The name suggests this is used as an identifier, but all the
>     leafs are
>     >>     optional. This is not typical. They are also in a container,
>     precluding them
>     >>     from being used as list keys. Is that as intended?
>    >> [TS]: this container is not meant to be reused for a list, rather
>     contains ID(s) that together can be used to lookup a specific topology
>     that the TE tunnel is using. We can clarify this in the description
>     >
>     > [janl] Hmm, ok. Still not sure what happens if you omit some value(s),
>     but I guess I should wait to see your clarification.
>     > [TS]: we have addressed this by specifying defaults for the optional
>     leafs.
>     
>     [janl2] Nice.
>     
>     >>     #6: Optional -id leafs again
>     >>     1700:
>     >>               leaf node-id {
>     >>               leaf link-tp-id {
>     >>     1768:
>     >>             leaf address {
>     >>     1783:
>     >>             leaf node-id {
>     >>             leaf link-tp-id {
>     >>
>     >>     Leafs that appear to be used as identifiers are optional
>     >> [TS]: such IDs identify a specific node, link, or address in the
>     network but in our applications we do not to use them as keys in such
>     lists. For example list explicit-route-objects defines a separate index
>     leaf to be used as key -- since, a path may contain the same node-id or
>     address repeated multiple times (looping path)..
>     >
>     > [janl] So what happens if you omit specifying some of them?
>     > [TS]: This was reworked and added a mandatory to leafs that are must,
>     and added defaults to optional leafs.
>     
>     [janl2] ok.
>     
>     >>     #7: binary length in bits?
>     >>     1731:
>     >>                leaf as-number {
>     >>                 type binary {
>     >>                   length 16;
>     >>     1773:
>     >>             leaf ip-flags {
>     >>               type binary {
>     >>                 length 8;
>     >>     1805:
>     >>               leaf label-flags {
>     >>                 type binary {
>     >>                   length 8;
>     >>
>     >>     It appears to me the modeler might have thought the length is
>     given in bits.
>     >>     The value of length is in bytes, however.
>     >> [TS]: good catch, thanks. We'll correct it to indicate number of
>     octets as RFC6020 defines.
>     >>
>     >>     #8: Must expression copy paste
>     >>     1852:
>     >>         container label-end {
>     >>           must "not(../label-end/te-label/direction) or "
>     >>             + "not(te-label/direction) "
>     >>             + "or ../label-end/te-label/direction =
>     te-label/direction" {
>     >>
>     >>     This must expression appears to have been copied from
>     label-start. In any case,
>     >>     it always evaluates to true and has no effect.
>     >> [TS]: thanks, we can see the bug in the expression as is, and this
>     will be fixed/updated.
>     >>
>     >>         container label-end {
>     >>           must "not(../label-start/te-label/direction) or "
>     >>             + "not(te-label/direction) "
>     >>             + "or ../label-start/te-label/direction =
>     te-label/direction" {
>     >
>     > [janl] This revised must statement is superfluous. The must statement
>     on container label-start already covers this case.
>     > [TS]: addressed
>     
>     [janl2] This may not work as intended anymore. The must expression is
>     fine in itself, but now the direction leaf has a default (forward),
>     which means it will always exist with one value or another. The case
>     that the   not(.... /direction)   expressions test for will therefore
>     never happen. Only the last part (direction = direction) will be
>     relevant, and I'm not sure that's what you wanted.
>  
> [TS2: I’ve updated the check to accommodate the default case.
>  
> NEW:
>     container label-start {
>       must "(not(../label-end/te-label/direction) and" +
>                     " not(te-label/direction))"
>         + " or "
>         +  "(../label-end/te-label/direction = te-label/direction)"
>         + " or "
>         +  "(not(te-label/direction) and" +
>                     " (../label-end/te-label/direction = 'forward'))"
>         + " or "
>         +  "(not(../label-end/te-label/direction) and" +
>                     " (te-label/direction = 'forward'))" {
>         error-message
>           "label-start and label-end must have the same direction.";
>       }
>  
> <snip>
>     container label-end {
>       must "(not(../label-start/te-label/direction) and" +
>                     " not(te-label/direction))"
>         + " or "
>         +  "(../label-start/te-label/direction = te-label/direction)"
>         + " or "
>         +  "(not(te-label/direction) and" +
>                     " (../label-start/te-label/direction = 'forward'))"
>         + " or "
>         +  "(not(../label-start/te-label/direction) and" +
>                     " (te-label/direction = 'forward'))" {
>         error-message
>           "label-start and label-end must have the same direction.";
>       }    
>  
>     >>     #9: Unclear bit field
>     >>     1885:
>     >>         leaf range-bitmap {
>     >>           type binary;
>     >>           description
>     >>             "When there are gaps between label-start and label-end,
>     >>              this attribute is used to specify the positions
>     >>              of the used labels.";
>     >>         }
>     >>
>     >>     Need more information on how to interpret this leaf. Which bits
>     map to what,
>     >>     and what does the bit field values 0 and 1 indicate?
>     >> [TS]: ]: Each bit-position in the bitmap will map to an offset from
>     the label-start. For example, if label-start=16000 and bitmap=0x11, then
>     labels={16000, 16004} are relevant. We will clarify this in the next
>     update.
>     >
>     > [janl] Very good. Since you are using type binary and refer to the
>     contents using integers, you should also specify how you map those
>     integer values to bit positions in the binary. E.g. big-endian?
>     > [TS]: We have clarified this in the description of the leaf that it is
>     big-endian representation
>     
>     [janl2] Good that you added a description, but it doesn't make me much
>     wiser, I'm afraid. Even if there is a reference to RFC7951 (which is
>     only applicable to RESTCONF, btw), the new description doesn't describe
>     what the values mean (even when encoded according to 7951s6.6).
>     
>     Try to describe the algorithm I, as operator or network automation
>     programmer, should use to figure out what values to use here. The
>     encoding is also important, but if this is something operators should
>     enter, the binary type isn't what you want. If you pick a different
>     type, the encoding will probably fall out automatically.
>  
> [TS2]:  I added the below description and example to describe the algorithm.
>  
> NEW:
>     leaf range-bitmap {
>       type yang:hex-string;
>       description
>         "When there are gaps between label-start and label-end,
>          this attribute is used to specify the positions
>          of the used labels. This is represented in big-endian as
>          hex-string.
>  
>          Each bit-position in the range-bitmap hex-string maps to a
>          label in the range derived from the label-start.
>  
>          For example, assuming label-start=16000 and range-bitmap=0x01000001,
>          then:
>           - bit-position(0) is set, and the corresponding mapped label
>             from the range is: 16000 + (0 * label-step) or
>             16000 for default label-step=1.
>           - bit-position(24) is set, and the corresponding mapped label
>             from ihe ranage is: 16000 + (24 * label-step) or
>             16024 for defautl label-step=1";
>     }
>   }
>  
> Regards,
> Tarek
>   
>     Best Regards,
>     /jan
>     
>     
>     >
>     > Regards,
>     > Tarek
>     >>     #10: Canonical representation
>     >>     67:
>     >>       typedef te-bandwidth {
>     >>
>     >>     The type is based on a string with a pattern allowing hex
>     characters and an
>     >>     upper or lowercase P. Since the pattern allows multiple
>     representations of the
>     >>     same underlaying value (0x1p10 presumably means the same as
>     0x1p0xa and
>     >>     0x1P0XA) the question comes up if there is a canonical
>     representation of this
>     >>     value, e.g. using all lowercase and all hex, or if the string
>     must be
>     >>     remembered exactly as given by the client. The description could
>     answer this
>     >>     question.
>     >> [TS]: We will update the description to indicate more strict
>     canonical form (all upper case).
>     >>
>     >>     #11: Mix of upper and lowercase
>     >>     The module specifies many enumeration and identity values. Some
>     are all
>     >>     lowercase. Some are all uppercase. The principle of least
>     astonishment suggests
>     >>     to pick one and stick with it. YANG recommendations suggest to
>     use all
>     >>     lowercase when in doubt.
>     >>
>     >>       typedef te-link-direction {
>     >>       typedef te-label-direction {
>     >>       typedef te-hop-type {
>     >>       identity LSP_METRIC_TYPE {
>     >>       identity LSP_METRIC_RELATIVE {
>     >>       identity LSP_METRIC_ABSOLUTE {
>     >>       identity LSP_METRIC_INHERITED {
>     >> [TS]: we will update to all lower-case to align with YANG
>     recommendations.
>     >
>     > [janl] Great!
>     >
>     > /jan
>     
>     
>     
>     
>     ------------------------------------------------------------------------
>     --------
>     
>     
>     > _______________________________________________
>     > Teas mailing list
>     > [email-masked]
>     > https://www.ietf.org/mailman/listinfo/teas
>     >

---

From: "Zhenghaomian (Zhenghaomian, Optical Technology Research Dept)" <[email-masked]>
Date: Sun, 3 Feb 2019 08:58:43 +0000

Hi, Tarek and Authors,

Another comments to the module in -05, regarding the lsp-state-type:

We are now having quite a few identities (path-computing, path-computation-ok, etc.). It is proposed that there are still some states missed given the whole life cycle of LSP, so we propose to add a few more identities including  ‘lsp-tear-down-ok’, ‘lsp-tear-down-failed’, ‘lsp-modifying’, ‘lsp-modify-ok’, ‘lsp-modify-failed’, to complete the LSP state definition.

Or, do we merge the above states into existing ones?

Thank you.

Best wishes,
Haomian

发件人: Teas [mailto:[email-masked]] 代表 Tarek Saad (tsaad)
发送时间: 2019年1月31日 22:03
收件人: tom petch <[email-masked]>; Jan Lindblad <[email-masked]>
抄送: [email-masked]; [email-masked]; [email-masked]
主题: Re: [Teas] binary wasRe: Yangdoctors early review of draft-ietf-teas-yang-te-types-03 (was -01)

Hi Jan and Tom,

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

    From: "Jan Lindblad" <[email-masked]<mailto:[email-masked]>>

    To: "Tarek Saad (tsaad)" <[email-masked]<mailto:[email-masked]>>

    Cc: <[email-masked]<mailto:[email-masked]>>;

    <[email-masked]<mailto:[email-masked]>>; <[email-masked]<mailto:[email-masked]>>;

    <[email-masked]<mailto:[email-masked]>>

    Tarek, team,

    > From: Jan Lindblad <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>

    > To: Tarek Saad <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>

    > Cc: "[email-masked] <mailto:[email-masked]><mailto:[email-masked]%20%3cmailto:[email-masked]%3e>"

    <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>,

    "[email-masked]

    <mailto:[email-masked]>"

    <[email-masked]

    <mailto:[email-masked]>>, "[email-masked]

    <mailto:[email-masked]>" <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>,

    "[email-masked] <mailto:[email-masked]><mailto:[email-masked]%20%3cmailto:[email-masked]%3e>" <[email-masked]

    <mailto:[email-masked]>>

    > Resent-From: <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>

    > Resent-To: Tarek Saad <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>,

    <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>,

    <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>,

    <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>,

    <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>,

    <[email-masked] <mailto:[email-masked]<mailto:[email-masked]%20%3cmailto:[email-masked]>>>, <[email-masked]

    <mailto:[email-masked]>>, <[email-masked]

    <mailto:[email-masked]>>, <[email-masked]

    <mailto:[email-masked]>>, <[email-masked]

    <mailto:[email-masked]>>, Lou Berger <[email-masked]

    <mailto:[email-masked]>>

    subobject index"

    how is it

    have seen

    earlier. Are

    indices?

        ordered-by user;

        key name;

        uses record-route_state;

        ...

      }

NEW:

        list path-route-object {

          key index;

          ordered-by user;

          description

          leaf index {

            type uint32;

            description

        description

        leaf name {

          type string;

          description

        }

          key name;

          description

             in path computation";

          leaf name {

            type leafref {

    pointing to here

            }

            description

          }

    further.

    with mandatory.

    the standard.

    [janl2] Very good.

    augmented with

    out, very good.

    point this out

    technology, please note

    binary, wouldn't

    (https://tools.ietf.org/html/rfc7951#section-6.6

    <https://tools.ietf.org/html/rfc7951#section-6.6>). Let us know if there

NEW:

  typedef admin-group {

    type yang:hex-string {

      /* 01:02:03:04 */

      length "1..11";

    }

    description

       hex-string type.";

  }

    not meant for this.

NEW:

          leaf as-number {

            type inet:as-number;

            mandatory true;

          }

    leafs are

    precluding them

    leafs.

    [janl2] Nice.

    [janl2] ok.

    given in bits.

    te-label/direction" {

    will be fixed/updated.

    te-label/direction" {

NEW:

    container label-start {

        + " or "

        + " or "

        + " or "

        error-message

      }

<snip>

    container label-end {

        + " or "

        + " or "

        + " or "

        error-message

      }

    map to what,

    update.

NEW:

    leaf range-bitmap {

      type yang:hex-string;

      description

         hex-string.

         then:

    }

  }

Regards,

Tarek

    Best Regards,

    /jan

    characters and an

    representations of the

    0x1p0xa and

    representation of this

    must be

    answer this

    are all

    astonishment suggests

    use all

    recommendations.

    --------

    > [email-masked]<mailto:[email-masked]>

---

From: tom petch <[email-masked]>
Date: Tue, 5 Feb 2019 14:47:28 +0000

Tarek

Getting there .

You will need to have a reference to the new I-D references in the body
of the I-D else you will get unused references.  One way to do this is
to have a Section 4.1 This module references [RFC3272]. [....  I have
not gone through and seen how many this applies to but imagine it is
most of them.

Also you have G.8031 but G808.  Not wrong, but...

      import ietf-routing-types { prefix "rt-types";
now has the right RFC but the wrong title; should be
Common YANG Data Types for the Routing Area

Tom Petch

----- Original Message -----
From: "Tarek Saad (tsaad)" <[email-masked]>
Sent: Thursday, January 31, 2019 2:10 PM

> Hi Tom,
>
> Thank again for your review comments below. We've uploaded version
04/-05 which attempts to address these comments.
> See inline [TS] for resolution.
>
> -----Original Message-----
> From: tom petch <[email-masked]>
> Date: Wednesday, January 23, 2019 at 12:13 PM
> To: Jan Lindblad <[email-masked]>, Tarek Saad <[email-masked]>
> Cc: "[email-masked]" <[email-masked]>,
"[email-masked]"
<[email-masked]>, "[email-masked]"
<[email-masked]>, "[email-masked]" <[email-masked]>
> Subject: [Teas] not a Yangdoctor review of
draft-ietf-teas-yang-te-types-03
>
>     Tarek
>
>     The YANG modules have lots of references - good - but they are not
in
>     the I-D references - not good.
>
>     My list is
>
>     3272
>     4202
>     4328
>     4657
>     5817
>     6004
>     6205
>     6511
>     7139
>     7308
>     7551
>     7571
>     7579
>     7951
>     G.808
>     G.8031
>     G.8131
>     G.873.1
>
> [TS]: thanks. I've added the missing references and they should show
in the I-D references now.
>
>     s.3.1 I would find more usable if the types were in an order I
could
>     recognise, such as alphabetical
>
> [TS]: OK, I tried an attempt to sort the typedefs alphabetically.
>
>       import ietf-routing-types { prefix "rt-types";
>     reference "RFC6991: Common YANG Data Types";
>     perhaps RFC8294 is intended
>
> [TS]: corrected to RFC8294
>
>     "   defined in ietf-network.yang, to help user to understand ""
>     might benefit from a reference - is this
>     draft-ietf-i2rs-yang-network-topo?
>
> [TS]: added reference RFC8345.
>
>     /"Then index of the label/ "The index of the label /
>
> [TS]: fixed typo.
>
>               container tiebreakers {
>                 description
>                   "The list of tiebreaker criterion to apply
>                    on an equally favored set of paths to pick best";
>                 list tiebreaker {
>                   description
>                   "The list of tiebreaker criterion to apply
>                      on an equally favored set of paths to pick best";
>     One description is perhaps enough
>
> [TS]: removed/updated redundant description.
>
>     uses path-objective-function_config;
>     using _ is not wrong but is discouraged, mixing _ with - in a
label more
>     so
>
> [TS]: OK, we have moved away from using "_" in the naming.
>
>     /This document registers a YANG module/
>     This document registers two YANG modules/
>
> [TS]: fixed typo.
>
>        name: ietf-te-types namespace:
urn:ietf:params:xml:ns:yang:ietf-te-
>        types prefix: ietf-te-types reference: RFC3209
>        name: ietf-te-packet-types namespace:
>        urn:ietf:params:xml:ns:yang:ietf-te-packet-types prefix:
ietf-te-
>        packet-types reference: RFC3209
>
>     Perhaps /3209/XXXX/
>
> [TS]: fixed.
>
> Regards,
> Tarek
>
>     Tom Petch
>
>     From: "Jan Lindblad" <[email-masked]>
>     To: "Tarek Saad (tsaad)" <[email-masked]>
>     Cc: <[email-masked]>;
>     <[email-masked]>; <[email-masked]>;
>     <[email-masked]>

---

From: "Tarek Saad (tsaad)" <[email-masked]>
Date: Tue, 5 Feb 2019 20:11:59 +0000

Thanks, Tom. Please see inline below.

-----Original Message-----
From: tom petch <[email-masked]>
Date: Tuesday, February 5, 2019 at 9:47 AM
To: Tarek Saad <[email-masked]>, Jan Lindblad <[email-masked]>
Cc: "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>
Subject: Re: [Teas] not a Yangdoctor review of draft-ietf-teas-yang-te-types-03

    Tarek
    
    Getting there .
    
[TS]: unfortunate that the tool will flag those as warning as the I-D YANG module clearly references them.. I am wondering if there is a chance or a plan to enhance the IETF idnit tool to parse the YANG module for references and silence such warnings?

    Also you have G.8031 but G808.  Not wrong, but...
[TS]: G.8031 is already listed in references. G808 is still relevant. I've moved both to informative references as per other recommendations.

[TS]: thank you. I'll take care of this one in the next update.

Regards,
Tarek

    Tom Petch
    
    ----- Original Message -----
    From: "Tarek Saad (tsaad)" <[email-masked]>
    Sent: Thursday, January 31, 2019 2:10 PM
    
    in
    could
    label more
    ietf-te-

---

From: tom petch <[email-masked]>
Date: Thu, 7 Feb 2019 11:42:20 +0000

----- Original Message -----
From: "Tarek Saad (tsaad)" <[email-masked]>
Sent: Tuesday, February 05, 2019 8:11 PM

> [TS]: unfortunate that the tool will flag those as warning as the I-D
YANG module clearly references them.. I am wondering if there is a
chance or a plan to enhance the IETF idnit tool to parse the YANG module
for references and silence such warnings?

Well no, that would be wrong.

A YANG module needs references, for import statements and to tell the
user where to go for more information on the objects, their use and so
on.

A YANG module must be plain text so cannot contain the usual HTML/XML
style anchors that an I-D/RFC has.

Documents referenced from within an RFC must appear in the References of
that RFC.

Hence
 - anything referenced in the YANG module must be in the References of
the RFC
- anything referenced in the YANG module must have a HTML/XML style
reference somewhere in the RFC outside the YANG module.

Where such references do not crop up in the existing text of the memo
outside the YANG module, then the practice is to add a
Section X.1
before the YANG module saying words to the effect that this YANG module
imports from [. and references [....

So the tool wants enhancing to generate more warnings, not less, when
the contents of a a YANG reference clause do not appear in the Reference
section of the I-D.

HTH

I was unclear about my comment on G808; having the reference is find but
I would prefer G.808; I note that Deborah's e-mail used G.808 even when
referring to the 'G808' in the I-D so I think G.808 is to be preferred.

Tom Petch

> [TS]: G.8031 is already listed in references. G808 is still relevant.
I've moved both to informative references as per other recommendations.

---

From: "Tarek Saad (tsaad)" <[email-masked]>
Date: Thu, 7 Feb 2019 15:03:29 +0000

Hi Tom,

>>     reference somewhere in the RFC outside the YANG module.

[TS]: I don’t mind if it is a common practice to follow it.. However, I’d note as can be seen in the uploaded ver -05, I am able to have all references mentioned in YANG module show up in the references section of the I-D without explicitly needing cross-reference them in the I-D text outside of the YANG module. FWIW, kramdown-rfc2629 allows to list those at the top so they automatically get added to the references section.

Also, since I see (some) IETF tools are already parsing the I-D YANG module contents (e.g. for replacing RFC tags with HTML’zed URL links – see below), I would have thought it would be possible to determine such warnings were inaccurate..

“  == Unused Reference: 'RFCXXXX' is defined on line 3780, but no explicit

     reference was found in the text”

E.g.:

Liu, et al.                  Standards Track                    [Page 8]
________________________________
<https://tools.ietf.org/html/rfc8294#page-9>
RFC 8294<https://tools.ietf.org/html/rfc8294>                 Routing Area YANG Types           December 2017

      revision 2017-12-04 {
        description "Initial revision.";
        reference
          "RFC 8294<https://tools.ietf.org/html/rfc8294>: Common YANG Data Types for the Routing Area.
           Section 3<https://tools.ietf.org/html/rfc8294#section-3>.";
     }

     /*** Identities related to MPLS/GMPLS ***/

    identity mpls-label-special-purpose-value {
       description
         "Base identity for deriving identities describing
          special-purpose Multiprotocol Label Switching (MPLS) label
          values.";
       reference
         "RFC 7274<https://tools.ietf.org/html/rfc7274>: Allocating and Retiring Special-Purpose MPLS
          Labels.";
     }

     identity ipv4-explicit-null-label {
       base mpls-label-special-purpose-value;
       description
         "This identity represents the IPv4 Explicit NULL Label.";
       reference
         "RFC 3032<https://tools.ietf.org/html/rfc3032>: MPLS Label Stack Encoding.  Section 2.1<https://tools.ietf.org/html/rfc8294#section-2.1>.";
     }

Regards,

Tarek

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

From: Teas <[email-masked]> on behalf of tom petch <[email-masked]>

Date: Thursday, February 7, 2019 at 6:42 AM

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

    >     You will need to have a reference to the new I-D references in the

    body

    >     of the I-D else you will get unused references.  One way to do

    this is

    >     to have a Section 4.1 This module references [RFC3272]. [....  I

    have

    >     not gone through and seen how many this applies to but imagine it

    is

    on.

    that RFC.

    Hence

    the RFC

    Section X.1

    section of the I-D.

    HTH

    Tom Petch

    >     > Thank again for your review comments below. We've uploaded

    version

    >     >     The YANG modules have lots of references - good - but they

    are not

    >     > [TS]: thanks. I've added the missing references and they should

    show

    >     >     s.3.1 I would find more usable if the types were in an order

    I

    >     > [TS]: OK, I tried an attempt to sort the typedefs

    alphabetically.

    >     >     "   defined in ietf-network.yang, to help user to understand

    ""

    >     >                    on an equally favored set of paths to pick

    best";

    >     >                      on an equally favored set of paths to pick

    best";

    >     >     using _ is not wrong but is discouraged, mixing _ with - in

    a

    >     >     <[email-masked]>;

    <[email-masked]>;

    Teas mailing list

    [email-masked]

---

From: Italo Busi <[email-masked]>
Date: Thu, 7 Feb 2019 15:24:27 +0000

Hi  Tom,

Regarding your comment:

"I was unclear about my comment on G808; having the reference is find but I would prefer G.808; I note that Deborah's e-mail used G.808 even when referring to the 'G808' in the I-D so I think G.808 is to be preferred."

The draft references G.808 whenever it is possible

However, there are a couple of items which are common across multiple technologies but missing in G.808:
- signal-fail-of-protection which has higher priority than the FS command
- failure-of-protocol

In this case, the set of technology-specific Recommendations (G.8031, G.8131 and G.873.1) has been referenced instead of G.808

@Tarek: I have just noted the following nit in G.873.1 reference which can be easily fixed when you publish the next draft update:

OLD
   [G.873.1]  "G.8131 : Linear protection switching for MPLS transport
              profile", July 2014,

NEW
   [G.873.1]  "G.873.1 : Optical transport network (OTN):
             Linear protection", October 2017,

Italo

-----Original Message-----
From: tom petch [mailto:[email-masked]] 
Sent: giovedì 7 febbraio 2019 12:42
To: Tarek Saad (tsaad) <[email-masked]>; Jan Lindblad <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: [Teas] not a Yangdoctor review of draft-ietf-teas-yang-te-types-03

----- Original Message -----
From: "Tarek Saad (tsaad)" <[email-masked]>
Sent: Tuesday, February 05, 2019 8:11 PM

body
this is
have
is
> [TS]: unfortunate that the tool will flag those as warning as the I-D
YANG module clearly references them.. I am wondering if there is a chance or a plan to enhance the IETF idnit tool to parse the YANG module for references and silence such warnings?

A YANG module needs references, for import statements and to tell the user where to go for more information on the objects, their use and so on.

A YANG module must be plain text so cannot contain the usual HTML/XML style anchors that an I-D/RFC has.

Documents referenced from within an RFC must appear in the References of that RFC.

Hence
 - anything referenced in the YANG module must be in the References of the RFC
- anything referenced in the YANG module must have a HTML/XML style reference somewhere in the RFC outside the YANG module.

Where such references do not crop up in the existing text of the memo outside the YANG module, then the practice is to add a Section X.1 before the YANG module saying words to the effect that this YANG module imports from [. and references [....

So the tool wants enhancing to generate more warnings, not less, when the contents of a a YANG reference clause do not appear in the Reference section of the I-D.

HTH

I was unclear about my comment on G808; having the reference is find but I would prefer G.808; I note that Deborah's e-mail used G.808 even when referring to the 'G808' in the I-D so I think G.808 is to be preferred.

Tom Petch

version
are not
show
I
alphabetically.
""
best";
best";
a
<[email-masked]>;

---

From: "Ryoo, Jeong-dong " <[email-masked]>
Date: Fri, 8 Feb 2019 01:38:23 +0000

Italo and all,

G.808 has a sentence about the priority in clause 3.2.8.10 as follows:

"For the case where an APS protocol is in use, an SF on the protection transport entity (over which the APS signal is routed) has priority over the forced switch."

Therefore, G.808 can be used for "signal-fail-of-protection."

For "failure-of-protocol," G.8031, G.8131 and G.873.1 should be referenced.

Best regards,

Jeong-dong

________________________________
보낸 사람 : "Italo Busi" <[email-masked]>
보낸 날짜 : 2019-02-08 00:25:04 ( +09:00 )
받는 사람 : tom petch <[email-masked]>, Tarek Saad (tsaad) <[email-masked]>, Jan Lindblad <[email-masked]>
참조 : [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>
제목 : Re: [Teas] not a Yangdoctor review of draft-ietf-teas-yang-te-types-03

Hi Tom,

Regarding your comment:

- failure-of-protocol

OLD

profile", July 2014,

NEW

Italo

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

To: Tarek Saad (tsaad) ; Jan Lindblad

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

From: "Tarek Saad (tsaad)"

> From: tom petch

body

this is

have

is

Hence

HTH

Tom Petch

> From: "Tarek Saad (tsaad)"

version

> > From: tom petch

> > To: Jan Lindblad , Tarek Saad

> > Cc: "[email-masked]" ,

> , "[email-masked]"

> , "[email-masked]"

are not

show

I

alphabetically.

""

best";

best";

a

> > From: "Jan Lindblad"

> > To: "Tarek Saad (tsaad)"

> > Cc: ;

> > ;

;

Teas mailing list

[email-masked]