Re: [spring] WG Last Call for draft-ietf-spring-segment-routing-mpls-13

[Ahmed Bashandy <[e-mail]>]

I uploaded version 15 on Oct/23 to address comments.

Thanks a lot for the comments

See my reply "#Ahmed"


On 5/24/18 10:40 AM, [e-mail] wrote:
>
> Hi authors,
>
> I’ve reviewed the document.
>
> It looks good, thanks for your effort.
>
> Please find below some comments, nothing major.
>
> Thanks for considering them as part of the WG LC.
>
> Thanks,
>
> Regards,
>
> Bruno
>
> ---
>
> Somewhere in Abstract/1. Introduction/2. MPLS Instantiation of Segment 
> Routing
>
> MPLS Instantiation of Segment Routing is sometimes referred to with 
> two different names: MPLS-SR or SR-MPLS which seems sub-optimal.
>
> [I.D.-ietf-spring-segment-routing] uses the term SR-MPLS. May this 
> term should also be used at least once in this document.
>
> ---
>
> §2.2
>
> "using the SRGB of the node"
>
> Please expand SRGB on first used. (in -13, this term is expand on §2.3 
> which is after)
>
#Ahmed: Done
>
> ---
>
> §2.3
>
> OLD: Just like SRGB, the SRLB need not be a single
>
>    contiguous range of label, except the SRGB MUST only be used to
>
>    instantiate global SIDs into the MPLS forwarding plane.
>
> NEW:
>
> Just like SRGB, the SRLB need not be a single
>
>    contiguous range of label.
>
> (simplify the sentence. The rule is already stated before in this section)
>
#Ahmed: I used a more general (and IMO more precise) wording
>
> ---
>
> [SRLB]
>
> "Hence it is
>
>    specified the same way and follows the same rules SRGB is specified
>
>    above in this sub-section."
>
> Sentence is not crystal clear and could probably be rephrased.
>
> More importantly, the sentence is incorrect: the SRLB does not follow 
> the last SRGB rules ( "The list of label ranges MUST only be used to 
> instantiate global SIDs into the MPLS forwarding plane")
>
#Ahmed: I used a more general (and IMO more precise) wording
>
> ------
>
> §2.4
>
> "0 =< I < size."
>
> "size = Lh(1)- Ll(1) + 1 + Lh(2)- Ll(2) + 1 + ... + Lh(k)- Ll(k) + 1"
>
> Algorithm looks good to me. Yet starting the index at 0 for SID index, 
> and 1 for SRGB sub-blocks may be slightly misleading. "Lh(1)- Ll(1)" 
> seems only defined in this document so an option would be to start 
> with "Lh(0)- Ll(0)" (this would impact the algo: :s/j = 1/ j = 0)
>
> Up to you.
>
#Ahmed: I left it as it is :)
>
> ------
>
> §2.5
>
> OLD: MPLS Incoming Label MAP
>
> NEW: MPLS Incoming Label Map
>
#Ahmed: Corrected
>
> (as per https://tools.ietf.org/html/rfc3031#section-3.11 )
>
> --
>
> OLD: o  (Endpoint, Color) representing an SR policy [I.D. 
> filsfils-spring-segment-routing-policy]
>
> NEW: o  an SR policy [I.D.-ietf-spring-segment-routing]
>
> (Otherwise, this would likely require a _Normative_ reference to I.D. 
> filsfils-spring-segment-routing-policy which would significantly delay 
> the RFC publication of this document. While 
> [I.D.-ietf-spring-segment-routing is already in the RFC editor queue 
> and does define a SR policy. )
>
#Ahmed: This has become rfc8402. I overlooked this one and I will make 
it refer to rfc8402 in version 16
>
> --
>
> OLD:
>
>    o  (Prefix, Routing Instance, Topology, Algorithm), where a topology
>
>       is identified by a set of links with metrics. For the purpose of
>
>       incoming label collision resolution, the same numerical value
>
>       SHOULD be used on all routers to identify the same set of links
>
>       with metrics.
>
> (It's not crystal clear what "numerical value" refers to, given the 
> list (Prefix, Routing Instance, Topology, Algorithm))
>
> NEW1:
>
>    o  (Prefix, Routing Instance, Topology, Algorithm), where a topology
>
>       is identified by a set of links with metrics. For the purpose of
>
>       incoming label collision resolution, the same Topology numerical 
> value
>
>       SHOULD be used on all routers to identify the same set of links
>
>       with metrics.
>
#Ahmed: I used this wording (thanks)
>
> NEW2:
>
>    o  (Prefix, Routing Instance, Topology, Algorithm). A Topology
>
>       identifies a set of links with metrics. For the purpose of
>
>       incoming label collision resolution, the same Topology numerical 
> value
>
>       SHOULD be used on all routers to identify the same set of links
>
>       with metrics.
>
> -----
>
> §2.5.1
>
> "       o All addresses are represented in 128 bits as follows
>
>             . IPv6 address is encoded natively
>
>             . IPv4 address is encoded in the most significant bits and
>
>                the remaining bits are set to zero
>
>        o All prefixes are represented by 128.
>
>             . A prefix is encoded in the most significant bits and the
>
>                remaining bits are set to zero.
>
>             . The prefix length is encoded before the prefix"
>
> "All prefixes are represented by 128."
>
> 128 what? Could be bits, but this does not fit with the addition of 
> the prefix length.
>
> "The prefix length is encoded before the prefix"
>
> using a field of what size?
>
#Ahmed: I corrected this to be (128+8)
>
> Do we (really) need 2 different encodings for prefix and address? 
> Especially since the rest of the algorithm does not make the difference:
>
>    "o  Encode the remaining set of FECs as follows
>
>        o Prefix, Routing Instance, Topology, Algorithm: (Prefix Length,
>
>          Prefix, SR Algorithm, routing_instance_id, Topology)"
>
#Ahmed: It is just that conceptually an address and a prefix are 
different and they are usually handled quite differently by routing 
protocols
>
> ---
>
> §2.5.2. Redistribution between Routing Protocol Instances
>
> "   o  If the receiving instance's SRGB is the same as the SRGB of origin
>
>       instance, THEN
>
>        o the index is redistributed with the route
>
>    o  Else
>
>        o the index is not redistributed and if needed it is the duty of
>
>          the receiving instance to allocate a fresh index relative to
>
>          its own SRGB"
>
> With a strict interpretation of this rule, when one increases the size 
> of the SRGB, all indexes from redistributed prefixes will be withdrawn 
> because it's unlikely that we can guaranty that the change of both 
> SRGB be atomic.
>
> Could this be accommodated? e.g. by only using the SRBG below the 
> index ? i.e SRGB [0;index] or just the mapped label in both SRGB?
>
#Ahmed: I think this is a very fine corner case. But I am open to any 
suggestion
>
> ---
>
> §2.6
>
> OLD:
>
> In the general case, the ingress node may not have exactly have the
>
>    same data of the receiving node, so the result may be different.
>
> NEW:
>
> In the general case, the ingress node may not have exactly the
>
>    same data of the receiving node, so the result may be different.
>
#Ahmed: I used that wording
>
> ----
>
> §2.10
>
> OLD
>
>    This document covers the calculation of outgoing label for the top
>
>    label only. The case where outgoing label is not the top label and is
>
>    part of a stack of labels that instantiates a routing policy or a
>
>    traffic engineering tunnel is covered in other documents such as
>
> [I.D.filsfils-spring-segment-routing-policy].
>
> This may be understood as requiring a _normative_ reference for 
> [I.D.filsfils-spring-segment-routing-policy], which would 
> significantly delay the RFC publication of this document.
>
> Proposed NEW1:
>
>    This document covers the calculation of outgoing label for the top
>
>    label only. Other cases are out of scope of this document.
>
> Or NEW2:
>
>   This document covers the calculation of outgoing label for the top
>
>    label only. The case where outgoing label is not the top label and is
>
>    part of a stack of labels that instantiates a routing policy or a
>
>    traffic engineering tunnel is out of scope of this document.
>
#Ahmed: I changed the wording a little bit such that 
[I.D.filsfils-spring-segment-routing-policy] is an example
>
> -- 
>
> §2.10.1
>
> "   Suppose an MCC on a router "R0" determines that PUSH or CONTINUE
>
>    operation is to be applied to an incoming packet whose active SID is
>
>    the global SID "Si" ..."
>
>  My undertanding is that "whose" in "whose active SID" refers to the 
> the "incoming packet". This seems to imply that the incoming packet is 
> (already) labeled.
>
> - This does not match the example used in the following paregraph, 
> where the incoming packet is IP.
>
>  "An example of a method to determine the SID
>
>    "Si" for PUSH operation is the case where IS-IS [I-D.ietf-isis-
>
>    segment-routing-extensions] receives the prefix-SID "Si" sub-TLV
>
>    advertised with prefix "P/m" in TLV 135 and the destination address
>
>    of the incoming IPv4 packet is covered by the prefix "P/m"."
>
>  - Also, you can't simply perform a PUSH on a received label packet. 
> You first need to POP or SWAP the incoming label.
>
> May be proposed NEW:
>
>    Suppose an MCC on a router "R0" determines that a PUSH or CONTINUE
>
>    operation is to be applied to an incoming packet, related to the 
> global SID "Si" ...
>
#Ahmed: Modified the text as suggested
>
> -----
>
> §2.10.1
>
> OLD: If the neighbor "N" does not support SR or "I" does not satisfy
>
>       the inequality specified in Section 2.4 for the SRGB of the
>
>       neighbor "N"
>
> May be a more generic sentence:
>
> NEW   If the neighbor "N" does not support SR or advertises a SRGB 
> invalid or too small,
>
#Ahmed: Modified as suggested
>
>  -----
>
> §2.10.1
>
> OLD:
>
>        o If it is possible to send the packet towards the neighbor "N"
>
>           using standard MPLS forwarding behavior as specified in
>
>           {RFC3031] and [RFC3032], then forward the packet. The method
>
>           by which a router decides whether it is possible to send the
>
>           packet to "N" or not is beyond the scope of this document. For
>
>           example, the router "R0" can use the downstream label
>
>           determined by another MCC, such as LDP [RFC5036], to send the
>
>           packet.
>
> The fall back to LDP seems like a mandated behavior. If so, the 
> behavior seems under-specified. In particular "out of scope of this 
> document" does not seem like a valid argument.
>
#Ahmed: In general LDP is not mandated. The statement says use MPLS. How 
and what protocol is used to determine what to do with the incoming 
label(s) (if any) and what is (are) the outgoing label(s) is totally 
outside the scope of this document.
>
> Possible NEW:
>
> "       o If this neighbor "N" advertises a valid label for the same 
> FEC via another MCC (e.g. LDP [RFC5036]), then forward the packet to 
> N, using the label advertised by this MCC.
>
> ----
>
> 2.11.2. Forwarding Behavior Corresponding to CONTINUE Operation for 
> Local SIDs
>
>   "A local SID on a router "R0" corresponds to a local label such as an
>
>    IGP adj-SID. In such scenario, the outgoing label towards a next-hop
>
>    "N" is determined by the MCC running on the router "R0"and the
>
>    forwarding behavior for CONTINUE operation is identical to swap
>
>    operation [RFC3032] on an MPLS label"
>
> I'm not seeing a case where an IGP adj-SID would have "CONTINUE" as a 
> forwarding behavior.
>
> And the SR architecture requires a NEXT operation:
>
> "      When a node binds an Adj-SID to a local data-link L, the node MUST
>
>        install the following FIB entry:
>
>       Incoming Active Segment: V
>
>       Ingress Operation: NEXT
>
>       Egress Interface: L"
>
> https://tools.ietf.org/html/draft-ietf-spring-segment-routing-15#section-3.4
>
> A solution would be to simply remove "such as an IGP adj-SID"
>
#Ahmed: I agree and I removed adj-SID
>
> ---
>
> §3 IGP Segment examples
>
> This section, also useful, is quite long and not normative for this 
> STD track doc. An option may be to move it to appendix.
>
> Up to you.
>
#Ahmed: Agreed and moved to appendix
>
> ---
>
> §3.1
>
> "R2 is the next-hop along the shortest path towards R8. By applying
>
>    the steps in Section 2.8 the local label downloaded to R1's FIB
>
>    corresponding to the global SID index 8 is 1008 because the SRGB of
>
>    R2 is [1000,5000] as shown in Figure 2."
>
>  may be
>
> OLD:  local label downloaded to R1's FIB
>
> NEW:  local outgoing label downloaded to R1's FIB
>
>  (because I would assume that by default, label downloaded in the FIB 
> is probably the incomig label one)
>
#Ahmed Changed "local" to "outgoing"
>
>  --
>
> OLD: owned by the R8
>
> NEW1: OLD: owned by R8
>
> NEW2: OLD: owned by the node R8
>
#Ahmed: Done
>
> --
>
> "The packet arrives at router R2. Because the top label 1008
>
>    corresponds to the IGP SID "8", which is the prefix-SID attached to
>
>    the prefix 192.0.2.8/32 owned by the R8, then the instruction
>
>    associated with the SID is "forward the packet using all ECMP/UCMP
>
>    interfaces and all ECMP/UCMP next-hop(s) along the shortest path
>
>    towards R8". "
>
> IGP prefix-SID are typically forwarded along the ECMP shortest path. 
> I'm not sure where "UCMP" is coming from. Also "along the shortest 
> path" seems to be incompatible with UCMP.
>
#Ahmed: I changed the wording to "shortest/useable"
>
> --
>
> "Because R3
>
>    is the penultimate hop, R3 applies NEXT operation then sends the
>
>    packet to R8."
>
>    "penultimate hop" is a required but not sufficient reason. You are 
> assuming that R8 asked for PHP, but I'm not seeing this assumption 
> stated in the text.
>
#Ahmed:
I changed to wording to use  "assume"
>
> ---
>
> "The NEXT operation results in popping the outer label
>
>    and sending the packet as a pure IPv4 packet to R8. The
>
>    In conclusion, "
>
> OLD: The
>
> NEW:
>
#Ahmed: In many places, I refer to each operation using "the".
>
> ---
>
> §3.2 & §3.3 & §3.4 & §3.5
>
> " Using the
>
>    calculation techniques specified in Section 2.10 and 2.11 the
>
>    resulting label stack starting from the topmost label is <1002, 9001,
>
>    1008>."
>
> Actually section 2.10 only defines the calculation technique for the 
> top label:
>
>    "This document covers the calculation of outgoing label for the top
>
>    label only. The case where outgoing label is not the top label and is
>
>    part of a stack of labels that instantiates a routing policy or a
>
>    traffic engineering tunnel is covered in other documents such as
>
> [I.D.filsfils-spring-segment-routing-policy]."
>
> https://tools.ietf.org/html/draft-ietf-spring-segment-routing-mpls-13#section-2.10
>
>    Hence this document does not define the behavior for this example 
> 2. You would need to either extend this doc to cover the push of a 
> stack of label/SID or remove these examples 2, 3, 4, 5
>
#Ahmed Done
>
> ----
>
> [RFC8174] to be added to normative references.
>
#Ahmed: Done
>
> *From:*[e-mail] [mailto:[e-mail]]
> *Sent:* Thursday, May 24, 2018 7:14 PM
> *To:* SPRING WG List
> *Cc:* [e-mail]
> *Subject:* WG Last Call for draft-ietf-spring-segment-routing-mpls-13
>
> Hello Working Group,
> This email starts a Working Group Last Call on 
> draft-ietf-spring-segment-routing-mpls-13 [1] which is considered 
> mature and ready for a final working group review.
> Please read this document if you haven't read the most recent version 
> yet, and send your comments to the list, no later than *June 08*.
> As a reminder, this document had already passed a WGLC more than a 
> year ago on version -06 [2], had been sent to the AD but then returned 
> to the WG.
> Since then, the document has significantly changed, so please read it 
> again. In particular, it now includes the resolution in case of 
> incoming label collision. Hence it killed 
> draft-ietf-spring-conflict-resolution.
> Both co-chairs co-author this document, hence:
> - Shraddha has agreed to be the document shepherd. Thank you Shraddha.
> - Martin, our AD, has agreed to evaluate the WG consensus.
> Thank you,
> Bruno, Rob
> [1] https://tools.ietf.org/html/draft-ietf-spring-segment-routing-mpls-13
> [2] 
> https://mailarchive.ietf.org/arch/msg/spring/STiYsQJWuVXA1C9hK4BiUnyMu7Y
> _________________________________________________________________________________________________________________________
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> This message and its attachments may contain confidential or privileged information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.
> _________________________________________________________________________________________________________________________
>
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
>
> This message and its attachments may contain confidential or privileged information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.