[Isis-wg] Comments on - draft-ietf-isis-segment-routing-extensions-03

[Uma Chunduri <[e-mail]>]

Dear Authors,

Though it's bit delayed from side to get these out because of unavoidable reasons thought to
post my comments any ways on "03" version for future reference.


Comments:
=========

1. Section 2
   "The 'Prefix SID'
   MUST be unique within a given IGP domain (when the L-flag is not
   set).  The 'Prefix SID' MUST carry an index (when the V-flag is not
   set) that determines the actual SID/label value inside the set of all
   advertised SID/label ranges of a given router."

   With the above its possible to advertise a "globally unique and absolute" prefix SID.
   But this is negated below and there is nothing called globally unique and absolute prefix SID.
   Perhaps L flag is not required and V flag is sufficient to represent the local nature of the SID.
   It's better to correct the same.

2. Section 2

   " P-Flag: no-PHP flag.  If set, then the penultimate hop MUST NOT
         pop the Prefix-SID before delivering the packet to the node
         that advertised the Prefix-SID."

   Inconsistent with OSPF definition, which could be ok but I would agree with Acee's original comment which led OSPF draft
   to say that as NP flag.

3. Section 2

   "
      *  A 4 octet index defining the offset in the SID/Label space
         advertised by this router using the encodings defined in
         Section 3.1.  In this case the V and L flags MUST be unset.

      *  A 3 octet local label where the 20 rightmost bits are used for
         encoding the label value.  In this case the V and L flags MUST
         be set."

    This is a big change and has some impact on how we configure things today.
    Is this because it's clear that globally agreeable ranges can't be configured in a multi-vendor deployment?
    It's good to write some explanation on why?

4. Section 2.2.1

         "F-Flag: Address-Family flag.  If unset, then the Adj-SID refers
         to an adjacency with outgoing IPv4 encapsulation.  If set then
         the Adj-SID refers to an adjacency with outgoing IPv6
         encapsulation."

   As base IS-IS TLVs would have MT-ID as and when needed to represent the topology/AF, I don't see the need for this.

   Also I see towards the end of section 2.2.1 -
   "
      The F-flag is used in order for the router to advertise the
      outgoing encapsulation of the adjacency the Adj-SID is attached
      to.  Use cases of the use of the F-flag are described in
      [I-D.filsfils-spring-segment-routing-use-cases]."

   Why MTID of the base TLVs (22/222/etc..) won't cover outgoing encapsulation??

5. Section 2.2.1
         "S-Flag.  Set Flag.  When set, the S-Flag indicates that the
         Adj-SID refers to a set of adjacencies (and therefore MAY be
         assigned to other adjacencies as well)."

A forward reference (same document or other document) would be helpful on how to use this.
Ideally "procedures" should explain the advertisement and processing og this in the same document.

6.  Section 2.2.1
    "      *  A 16 octet IPv6 address.  In this case the V flag MUST be set.
         The L flag MUST be unset if the IPv6 address is globally
         unique."

7. Section 2.2.1 -

   "      *  A 16 octet IPv6 address.  In this case the V flag MUST be set.
         The L flag MUST be unset if the IPv6 address is globally
         unique."

   All of a sudden this document introduces IPv6 data plane (or non-mpls) semantics here.
   It's good to cover some background in the introduction and rationale on why only for ADJ-SIDs this
   is required should be explained. Currently there is no explanation more than the reference as below
   and 6man-sr-header drafts.

   Section 2.2.2 -

   "An ipv6 address SID is encoded in 16 octets (IPv6 Adj-SID is defined
   in [I-D.previdi-6man-segment-routing-header])."

8. Section 2.4
   for the first 2 bullets, it should include the previously agreed upon reference,
    which details the usage scenarios of binding SIDs -
    https://tools.ietf.org/html/draft-bowers-spring-advertising-lsps-with-sr-00

9. Section 2.4.1
   "      F-Flag: Address Family flag.  If unset, then the Prefix FEC
      carries an IPv4 Prefix.  If set then the Prefix FEC carries an
      IPv6 Prefix."

    It's good to put MT-ID, as for ISIS it's more of MT ID (RFC 5120) which represents AF too.

10. Section 2.4
    "   o  The SID/Label Binding TLV may also be used in order to advertise
      prefixes to SID/Label mappings.  This functionality is called the
      'Mapping Server' and it's used when, in a heterogeneous network,
      not all nodes are capable of advertising their own SIDs/Labels.
      When the SID/Label Binding TLV is used by the Mapping Server in
      order to advertise prefix to SID/label mappings, the index/label
      MUST include the Prefix-SID SubTLV (Section 2.1)."

     It should have a forward reference to section 2.4.5, where mapping server prefix-SID is kind of attempted to be addressed.


11. Section 2.4.5

    "The Prefix-SID sub-TLV (suggested value 3) is defined in Section 2.1
   and contains the SID/index/label value associated with the prefix and
   range. "

Reference should be corrected to Section 2.4.
Advertising node, receiving node in SR domain and the receiving boarder node in SR domain processing should be detailed.

12. Section 3.1
      I see an example of index sid - 0 as valid value
      do we have to make it valid ?? Can this be invalid -
      Just feeling uncomfortable to receive an index "0" for prefix SID (error checks on the received data id difficult).

     Also example can be modified not to have index 0


   " SRGB =  [101, 200]
            [1001, 2000]
            [501, 600]

      The indexes span multiple ranges:

         index=1   means label 101
         ...
         index 100  means label 200
         index 101 means label 1001
         index 200 means label 2000
         ...
         index 201 means label 501
         ..."

    If authors strongly feel 0 is a valid index and can be advertised in the LSP, it's fine.

13. As we have https://tools.ietf.org/html/draft-ginsberg-isis-prefix-attributes-01 , which defines some of the flags (N flag for example) defined here it's better to harmonize this and deprecate some of the flags from SR draft (per offline discussion).
--
Uma C.