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

[Tarek Saad <[e-mail]>]

Hi Benjamin,

Thank you very much for the thorough review and for the valuable comments. We have uploaded revision -11 that attempts to address those comments. Please see inline for detailed responses. Let us know if there are further outstanding comments.

On 8/20/19, 1:27 PM, "Benjamin Kaduk via Datatracker" <[e-mail]> wrote:

    Benjamin Kaduk has entered the following ballot position for
    draft-ietf-teas-yang-te-types-10: 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://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_iesg_statement_discuss-2Dcriteria.html&d=DwICaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=Kd6NW5ctLWr7GB646PVmMByvi8wQxPILpKhHvGQeRHY&m=hnPuRIqXqStfK232Nk2hmq9WsTD2edVNgVLywB7I9S8&s=-tmE5kZMwh42TgMAaC3M4xeGTxYdYNt5fZ7gnVI59m0&e=
 
    for more information about IESG DISCUSS and COMMENT positions.
    
    
    The document, along with other ballot positions, can be found here:
    https://urldefense.proofpoint.com/v2/url?u=https-3A__datatracker.ietf.org_doc_draft-2Dietf-2Dteas-2Dyang-2Dte-2Dtypes_&d=DwICaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=Kd6NW5ctLWr7GB646PVmMByvi8wQxPILpKhHvGQeRHY&m=hnPuRIqXqStfK232Nk2hmq9WsTD2edVNgVLywB7I9S8&s=RkaTEY5gt6bhkr_3YkKb0CkmkQyY4-GYjfuvWkkKVaQ&e= 
    
    
    
    ----------------------------------------------------------------------
    DISCUSS:
    ----------------------------------------------------------------------
    
    There's a couple points (describe further in the comment section) that
    I'd like to get a bit more clarity on:
    
    (1) the semantics of admin-group hex strings of length smaller than 11
[TS]: RFC6991 defines the hex-string as stream of octets - separated by colon. The below are acceptable representations that result in same value (bit-position 0 is set).
01
00:01
00:00:00:01

     (2) the units for the te-bandwidth numerical values (and in what cases
    no units are needed), and for performance-metrics-*-way-bandwidth values
[TS]: The unit is technology dependent. For example for packet, the example we give in the description is inline with the type bandwidth-ieee-float32 defined in RFC8294. We can clarify this further in description if needed.

    (3) whether threshold-accelerated-advertisement's use of relative vs.
    absolute values requires any special handling
[TS]: these are absolute values and inline with the rfc7810#section-5 and rfc7471#section-5

    (4) whether we want to specify any requirements/error-handling/etc. when
    there is potential for "nonsense" configuration, such as a
    max-constraint being configured as numerically smaller than a
    min-constraint
[TS]: We have some checks where it was needed. Implementations can also make additional restrictive checks with a deviations if needed.
    
    (5) what the "variation" in the te-packet-types module's leafs is
    intending to measure.
[TS]: the per link delay variation as defined in RFC7823.    
    
    ----------------------------------------------------------------------
    COMMENT:
    ----------------------------------------------------------------------
    
    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.
    
[TS]: Agreed. For 'suppression-interval', it is detailed in rfc7810#section-6. We will update the description/reference for this.

    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.
[TS]: updated.
    
    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.
[TS]: this was discussed on the WG mail list, and agreement was to keep TE node ID as dotted-quad 4 octets. We updated the description as below.. https://mailarchive.ietf.org/arch/msg/teas/bP5VurFZKQVx3hNWxUVz6alRIGg
We can update description to:
OLD:
       The identifier is represented as 32-bit unsigned integer in
       the dotted-quad notation.
NEW:
       The identifier is represented as 4 octets in dotted-quad notation.    
       te-metric:
    
          A type representing the TE link metric as defined in [RFC3785].
    
    (editorial) the phrase "link metric" does not appear in RFC 3785.
[TS]: Sure. 
OLD:
A type representing the TE <link> metric as defined in [RFC3785].
 
NEW:
A type representing the TE metric as defined in [RFC3785].
    
    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.
[TS]: update to: 
NEW:
       The ietf-te-packet-types module covers the common types and groupings
       that are specific to packet technology.
    
       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.
[TS]: update text to:
NEW: A YANG grouping that contains the generic performance metrics and additional packet specific metrics.
    
    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?

[TS]: number is stored as 32-bits. As indicated earlier:
01
00:01
00:00:00:01 all indicate bit-position 0 is set and can internally be represented as 0x00000001

    
      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".
[TS]: The RFCs refer to it as "anomaly or anomalous". We updated the description to indicate this.
    
      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)?
[TS]: An answer was given earlier, let us know if that clarifies.
    
      typedef te-metric {
        type uint32;
        description "TE link metric";
        reference "RFC3785";
    
    [same comment about "link metric" and 3785]
[TS]: Addressed.
    
      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]
[TS]: Addressed.
    
      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.
[TS]: Yes, created a common type and referenced it the respective typedefs.
    
      identity se-style-desired {
        [...]
    
    (et seq) The rtgdir's comments about whether 'base' should be present
    seems relevant for these as well.
[TS]: thanks for pointing it out, added the missing base.
    
      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)?
[TS]: These identities simply indicate the LSP attributes. Every flag has its own semantics. The respective RFCs describe whether any action is to be taken by nodes along the LSP path.

      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).
[TS]: expanded.
    
      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"?
[TS]: yes, RFC7571 uses the term "required" (as we have in description). We renamed the identity to be in sync with the RFC.
OLD:
identity loopback-desired {
NEW:
identity loopback-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?
[TS]: RFC 8169 section 4.4 uses "requested" - we think this can be equated to desired.
<snip>
.. indicates to the egress node that RTM is requested
</snip>

updated:
NEW:
      identity rtm-set-desired {
        description
          "Residence Time Measurement (RTM) attribute flag requested";


    
      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.]
[TS]: yes, this was discussed and the choice was to pick identities over enums for extensibility. We used enums in places where extensions were not expected.
    
      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?
[TS]: NEW:
          "Association Type Recovery used to associate LSPs of
           same tunnel for recovery";
    
      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.
[TS]: Added references.
    
      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.
[TS]: reworded:
NEW:
..................................................................................... The path that is
returned by the external source may require further local
computation on the device.";
    

      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";
[TS]: no, none standardized for TE exists as of yet.
    
    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?
[TS]: we've only modelled possible state outputs. There is no standard state-machine. Implementations may execute a state-machine for transitions, but we did not think it needs to be covered by the model.
    
      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.
[TS]: renamed identity 'teardown' and added reference to RFC3209 section 2.5 talks about these general principles.
    
      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?
[TS]: Update to:
NEW:
          "A metric that optimizes to a maximum the number of excluded resources
           specified in a set";
    
      grouping performance-metrics-one-way-bandwidth {
        [....]
    
    What are the units on the bandwidth values herein?  bits per second?
[TS]: The type bandwidth-ieee-float32 is defined in RFC8294. The expected units are bytes per second (this is also inline with rfc3630 and rfc5305#section-3.6).
    
          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)?
[TS]: yes, but these are currently defined as absolute threshold values - not differences/relative.
    
      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?
[TS]: this weight is input to optimization function when trying to optimize for multiple metrics. A higher weight signifies a more important metric when trying to find optimal path. 
    
        container path-srlgs-names {
            [...]
            leaf-list names {
              type string;
              description "List named SRLGs";
    
    nit: missing "of"?
[TS]: updated.
    
      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.
[TS]: Reworded to clarify.
NEW:
            "The type of resource disjointness.
             When configured for a primary path, the disjointness level applies to
             all secondary LSPs. When configured for a secondary path, disjointness
             level overrides the one configured for the primary path";
    
        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?)
[TS]: We discussed this within the authors. We think ‘ordered-by user’ for config false leafs will be ignored -- and hence would fallback to the default (‘ordered-by system’) as per RFC6020. Note, we think this is a common behavior that affect NMDA model(s) when extracting applied configuration. Please correct our thinking if this is not true and we’ll try to address it.


    
    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?
[TS]: these are measured values over a time period and maintained as state. The minimum/maximum being over the last period.
    
             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?
[TS]: This attempts to model the parameters as described in RFC7823 (including delay variation/jitter).
    
             leaf one-way-packet-loss {
               type decimal64 {
                 fraction-digits 6;
                 range "0 .. 50.331642";
    
    Is there a reason for the oddly specific upper limit?
[TS]: this is described in https://tools.ietf.org/html/rfc7810#section-4.4 . Updated the description and reference to indicate this.
   “Link Loss: This 24-bit field carries link packet loss as a percentage
   of the total traffic sent over a configurable interval.  The basic
   unit is 0.000003%, where (2^24 - 2) is 50.331642%. “


    I also can't quite convince myself from the RFC 7950 grammar whether the
    quotes on the range are valid and/or needed.
[TS]: RF7950 expects a string for the range argument - so it's OK. Also, made it consistent across the modules
    
    I also am unsure as to how useful the "normality" is for some of these,
    like the packet loss and delay-variation values.
[TS]: RFC7823 suggests possible utility of those values.

Regards,
Tarek
    
    [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.)