Re: [Teas] Benjamin Kaduk's Discuss on draft-ietf-teas-yang-te-types-12: (with DISCUSS and COMMENT)

[Tarek Saad <[e-mail]>]

Hi Benjamin,

Thanks again. We are OK to make the suggested change to remove ambiguity as below. We are OK either uploading rev -13 (once tool reopens) or address it as
an RFC-Editor note..

OLD:
      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.
           The Most Significant Byte (MSB) is the farthest to the
           left in the byte sequence.";

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.
           The Most Significant Byte (MSB) is the farthest to the
           left in the byte sequence. Leading zero bytes in the configured
           value may be omitted for brevity. ";

Regards,
Tarek (for authors)

On 11/12/19, 7:22 PM, "Teas on behalf of Benjamin Kaduk via Datatracker" <[e-mail] on behalf of [e-mail]> wrote:

    Benjamin Kaduk has entered the following ballot position for
    draft-ietf-teas-yang-te-types-12: Discuss
    
    When responding, please keep the subject line intact and reply to all
    email addresses included in the To and CC lines. (Feel free to cut this
    introductory paragraph, however.)
    
    
    Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
    for more information about IESG DISCUSS and COMMENT positions.
    
    
    The document, along with other ballot positions, can be found here:
    https://datatracker.ietf.org/doc/draft-ietf-teas-yang-te-types/
    
    
    
    ----------------------------------------------------------------------
    DISCUSS:
    ----------------------------------------------------------------------
    
    Thank you for the updates in the -12; they're some good improvements!
    I think I am still looking for a little more clarity on one point, though it
    can probably be addressed with an RFC-Editor note:
    
    (1) the semantics of admin-group hex strings of length smaller than 11
    
    In the -12 we now have:
    
      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.
           The Most Significant Byte (MSB) is the farthest to the
           left in the byte sequence.";
    
        reference "RFC3630 and RFC5305";
      }
    
    This description still feels potentially ambiguous as to how to interpret
    a byte sequence with fewer than four bytes -- does "MSB" mean "MSB of the
    input string" or "MSB of the full-width value"?
    
    I suggest adding  "Leading zero bytes in the configured value may be omitted for brevity."
    (but anything with similar clarifying value would be fine).
    
    
    ----------------------------------------------------------------------
    COMMENT:
    ----------------------------------------------------------------------
    
    [comment section from -10 preserved unchanged, for convenience]
    
    In general it's hard to examine many of these types and see whether they
    are fit for purpose without some sense of how they would be used, but of
    course following a chain of references for all of the types in question
    wouldu be tedious to the point of exhaustion.  I'm reluctant to start
    getting into examples (since such a list would necessarily be
    incomplete), but if you want one, the 'suppression-interval' leaf has a
    very terse description, that will presumably be supplemented by some
    text in some other document about what exactly is getting suppressed and
    when, but if I were to try to implement from just this document I'd be
    doing a lot of guessing.
    
    Section 3
    
       types for TE generic types and ietf-te-packet-types for packet
       specific types.  Other technology specific TE types are outside the
    
    nits: "packet-specific" and "technology-specific" to hyphenate the
    compound adjectives.
    
    Section 3.1
    
       te-node-id:
    
          A type representing the identifier for a node in a TE topology.
          The identifier is represented as 32-bit unsigned integer in the
          dotted-quad notation.  This attribute MAY be mapped to the Router
    
    Why is te-node-id IPv4-centric?  E.g., RFC 6119 describes an "IPv6 TE
    Router ID" TLV as well as the "TE Router ID" TLV that we reference here
    for the node ID.
    Furthermore, either it's a 32-bit unsigned integer or it's represented
    in the dotted-quad notation, but the dotted-quad notation doesn't really
    count as a 32-bit unsigned integer.
    
       te-metric:
    
          A type representing the TE link metric as defined in [RFC3785].
    
    (editorial) the phrase "link metric" does not appear in RFC 3785.
    
    Section 3.2
    
       The ietf-te-packet-types module covers the common types and groupings
       specific packet technology.
    
    nit: I think there's a word missing here.
    
       performance-metrics-attributes-packet:
    
          A YANG grouping for the augmentation of packet specific metrics to
          the generic performance metrics grouping parameters.
    
    nit(?) I can't tell which way the augmentation is supposed to go, just
    from this text.
    
    Section 4
    
      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";
    
    RFC 5305 says to treat this as essentially a bitmask.  What are the
    semantics when I only supply a string of (say) length 1 or 2?
    
      typedef performance-metrics-normality {
           [...]
        reference
          "RFC7471: OSPF Traffic Engineering (TE) Metric Extensions.
           RFC8570: IS-IS Traffic Engineering (TE) Metric Extensions.
           RFC7823: Performance-Based Path Selection for Explicitly
           Routed Label Switched Paths (LSPs) Using TE Metric
           Extensions";
    
    None of these three references describes "normal" or "abnormal".
    
      typedef te-bandwidth {
        type string {
          pattern
            '0[xX](0((\.0?)?[pP](\+)?0?|(\.0?))|'
          + '1(\.([\da-fA-F]{0,5}[02468aAcCeE]?)?)?[pP](\+)?(12[0-7]|'
          + '1[01]\d|0?\d?\d)?)|0[xX][\da-fA-F]{1,8}|\d+'
          + '(,(0[xX](0((\.0?)?[pP](\+)?0?|(\.0?))|'
          + '1(\.([\da-fA-F]{0,5}[02468aAcCeE]?)?)?[pP](\+)?(12[0-7]|'
          + '1[01]\d|0?\d?\d)?)|0[xX][\da-fA-F]{1,8}|\d+))*';
        }
    
    Can the document shepherd please report what validation has been done
    for this pattern?
    
           For packet switching type, a float number is used, such as
           0x1p10.
    
    What kind of units are implied for this (and the other types)?
    
      typedef te-metric {
        type uint32;
        description "TE link metric";
        reference "RFC3785";
    
    [same comment about "link metric" and 3785]
    
      typedef te-node-id {
        type yang:dotted-quad;
        description
          "A type representing the identifier for a node in a TE
           topology.
           The identifier is represented as 32-bit unsigned integer in
           the dotted-quad notation.
    
    [same comment about uint32 vs. dotted-quad]
    
      typedef te-oper-status {
        [...]
    
    Is there some way (a grouping?) to deduplicate the admin and operational
    status enumerations?  They seem to be nearly identical in terms of the
    range of possible values.
    
      identity se-style-desired {
        [...]
    
    (et seq) The rtgdir's comments about whether 'base' should be present
    seems relevant for these as well.
    
      identity oob-mapping-flag {
    [....]
      identity entropy-label-capability {
    
    Are all of the flags based on lsp-attributes-flags supposed to be
    "desired" in the description (these aren't)?
    
      identity oam-mep-entity-desired {
        base lsp-attributes-flags;
        description "OAM MEP entities desired";
    
    We probably should expand Maintenance Entity Group End Point (and MIP,
    below).
    
      identity loopback-desired {
        base lsp-attributes-flags;
        description
          "This flag indicates a particular node on the LSP is
           required to enter loopback mode.  This can also be
    
    Is there a mismatch between "desired" and "required"?
    
      identity rtm-set-desired {
        base lsp-attributes-flags;
        description
          "Residence Time Measurement (RTM) attribute flag";
    
    Is there a mismatch regarding "desired" in the identity name vs.
    description?
    
      identity link-protection-type {
        description "Base identity for link protection type.";
      }
      identity link-protection-unprotected {
        base link-protection-type;
    
    [I assume the WG had some good discussion about identities vs.
    enumeration and don't want to second-guess that.]
    
      identity association-type-recovery {
        base association-type;
        description
          "Association Type Recovery used to association LSPs of
           same tunnel for recovery";
    
    nit: the grammar seems weird, here; maybe a missing word?
    
      identity path-locally-computed {
        base path-computation-method;
        description
          "indicates a constrained-path LSP in which the
          path is computed by the local LER";
        reference "RFC3209";
    
    In line with the gen-art reviewer's comments, a section ref might be
    useful here.
    
      identity path-externally-queried {
        base path-computation-method;
        description
          [...]
          to the external source to aid computation); and the path that is
          returned by the external source is not required to provide a
          wholly resolved path back to the originating system - that is to
          say, some local computation may also be required";
    
    nit: "provide a wholly resolved path back to the originating system"
    could potentially be read ambiguously about whether "back to the
    originating system refers to the path or where the (partial) result is
    returned to.
    
      identity te-tunnel-type {
        [...]
    
    There is no need for a mp2p or mp2mp tunnel type?
    
      identity tunnel-state-type {
        description
          "Base identity for TE tunnel states";
    
    This is just "tunnel state" not "operational state"?
    
      identity lsp-state-type {
        description
          "Base identity for TE LSP states";
    
    Is there a state machine for which transitions between states based on
    this identity are allowed, or is it basically just a full mesh?
    
      identity path-invalidation-action-drop-tear {
        base path-invalidation-action-type;
        description
          "TE path invalidation action tear";
    
    Is there a reference for this (or the other
    path-invalidation-action-type)?  My naive reading wants to complete it
    to "teardown" but I have low confidence that is correct.
    
      identity path-metric-optimize-includes {
        [...]
      identity path-metric-optimize-excludes {
        base path-metric-type;
        description
          "A metric that optimizes the number of excluded resources
           specified in a set";
    
    optimizes to minimize or maximize it?
    
      grouping performance-metrics-one-way-bandwidth {
        [....]
    
    What are the units on the bandwidth values herein?  bits per second?
    
          container threshold-accelerated-advertisement {
            description
              "When the difference between the last advertised value and
               current measured value exceed this threshold, anomalous
               announcement will be triggered.";
            uses performance-metrics-thresholds;
    
    Are we reusing the performance-metrics-threshold container, which up
    until now has been used for absolute measurements, to hold the
    corresponding relative values (differences)?
    
      grouping optimization-metric-entry {
        [...]
        leaf weight {
    
    Where is the sense of the 'weight' leaf defined?  That is, to larger
    weight values get more traffic or less traffic?
    
        container path-srlgs-names {
            [...]
            leaf-list names {
              type string;
              description "List named SRLGs";
    
    nit: missing "of"?
    
      grouping generic-path-disjointness {
        description "Path disjointness grouping";
        leaf disjointness {
          type te-path-disjointness;
          description
            "The type of resource disjointness.
             Under primary path, disjointness level applies to
             all secondary LSPs. Under secondary, disjointness
             level overrides the one under primary";
    
    What does "under" mean?  It seems like it has something to do with the
    position in the hierarchy where this grouping gets instantiated, which
    is potentially subtle.
    
        container path-properties {
          config false;
          [...]
          container path-route-objects {
            description
              "Container for the list of route objects either returned by
               the computation engine or actually used by an LSP";
            list path-route-object {
              key index;
              ordered-by user;
    
    (How do 'config false' and 'ordered-by user' interact?)
    
    Section 5
    
       grouping performance-metrics-attributes-packet {
         [...]
         uses te-types:performance-metrics-attributes {
           augment performance-metrics-one-way {
             leaf one-way-min-delay {
             [...]
             leaf one-way-max-delay {
    
    What kind of sanity or error checking is there for the nodes in this
    model, e.g., if max delay is configured to be smaller than min delay?
    
             leaf one-way-delay-variation {
               type uint32 {
                 range 0..16777215;
               }
               description "One-way delay variation in micro seconds.";
    
    What does "variation" mean?  Deviation from mean?  Variance?  Mean
    absolute error?
    
             leaf one-way-packet-loss {
               type decimal64 {
                 fraction-digits 6;
                 range "0 .. 50.331642";
    
    Is there a reason for the oddly specific upper limit?
    I also can't quite convince myself from the RFC 7950 grammar whether the
    quotes on the range are valid and/or needed.
    
    I also am unsure as to how useful the "normality" is for some of these,
    like the packet loss and delay-variation values.
    
    [same comments about two-way metrics as one-way metrics, I think.
    Also the -packet variants.]
    
    Section 7
    
    We probably could say more here if we want, though I'm as-yet decided on
    whether we should.
    That is, things like editing  the config could cause traffic to be
    disallowed, routed on suboptimal paths, routed through locations more
    easily targeted for other purposes, etc., and reading config/status
    could reveal information about network topology, which sites might be
    most sensitive as targets for subsequent attacks, etc..  It's less clear
    if there's much that would allow for identification of specific
    users/customers or their traffic, though.  Maybe some of the DSCP stuff,
    in some cases.
    
    Section 10.2
    
    A question for the responsible AD: do references that are used in
    "reference" statements need to be normative references?  (E.g., RFC
    3471.)
    
    
    _______________________________________________
    Teas mailing list
    [e-mail]
    https://www.ietf.org/mailman/listinfo/teas