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

[Ahmed Bashandy <[e-mail]>]

Thanks a lot for the through review

I posted version 14 few minutes ago to address your comments

See my reply as #Ahmed


On 5/25/18 2:22 AM, Adrian Farrel wrote:
>
> Hi,
>
> I think it is well past time that we completed this foundation work
>
> and got an RFC published. So I support passing this draft to the AD
>
> again.
>
> However, we do need to clean it up first. Hopefully this won't be a
>
> lot of effort for the editors as nearly all I found were editorial
>
> nits.
>
> As a side comment, it isn't a success that section 2.5 is so large.
>
> Full one quarter of the document is dedicated to the edge case of
>
> collision avoidance. That said, I don't have a better solution.
>
> Thanks for the work.
>
> Adrian
>
> ====
>
> idnits isn't as clean as it could be. In particular:
>
> - 2.5.2.1 uses IP address 1.1.1.1 and should pick one from RFC 6890.
>
> - Missing normative reference for RFC 8174
>
> - [I-D.ietf-idr-segment-routing-te-policy] is not used
>
> - [I-D.ietf-ospf-ospfv3-segment-routing-extensions] and
>
> [I-D.ietf-ospf-segment-routing-extensions] have spurious spaces in
>
> the filenames
>
#Ahmed: Fixed
>

> ---
>
> You'll need to move the "Requirements Language" to be Section 1.1.
>
#Ahmed: Moved it to section 1.1
>
> ---
>
> Section 2
>
> s/link State IGPs/link state IGPs/
>
> s/Segment ID (SIDs)/Segment IDs (SIDs)/
>
#Ahmed: Fixed
>
> ---
>
> You use SRGB in 2.2 before you expand it in 2.3
>
#Ahmed: I expanded it at the beginning of Section 2.2
>
> ---
>
> 2.2
>
> oThe label value MUST be unique within the router on which the MCC
>
> is running. i.e. the label MUST only be used to represent the SID.
>
> I don't think you mean this. You have written that the label value has
>
> be unique which means that only one label value can be assigned to a
>
> SID.
>
> I think what you mean is that
>
> oThe label value MUST NOT be used for any other purpose on the
>
> router on which the MCC is running. I.e. the label MUST NOT be
>
> used to represent more than one SID or for any other forwarding
>
> purpose on the router.
>
#Ahmed: I used a slightly different wording than what you suggested. 
Here it is
The label value MUST be unique within the router on which the MCC is 
running. i.e. the label MUST only be used to represent the SID and MUST 
NOT be used to represent more than one SID or for any other forwarding 
purpose on the router.
>
> ---
>
> 2.2
>
> Unusual to have a normative reference to an IANA URL as you have for
>
> [reserved-MPLS]. Besides, the URL you give is 404.
>
> You would do better to reference RFC 7274.
>
#Ahmed: Agreed and done
>
> You might also fix the language and terminology, thus...
>
> OLD
>
> oThe label value MUST NOT be identical to or within the range of
>
> any reserved label value or range [reserved-MPLS], respectively.
>
> NEW
>
> oThe label value MUST NOT come from the range of special purpose
>
> labels [RFC7274].
>
#Ahmed: Agreed and done
>
> ---
>
> 2.3 ditto
>
> OLD
>
> oEvery range in the list of ranges specifying the SRGB MUST NOT
>
> cover or overlap with a reserved label value or range [reserved-
>
> MPLS], respectively.
>
> NEW
>
> oEvery range in the list of ranges specifying the SRGB MUST NOT
>
> cover or overlap with the range of special purpose labels
>
> [RFC7274].
>
#Ahmed: Agreed and Done
>
> ---
>
> 2.3
>
> oIf the SRGB of a node does not conform to the structure specified
>
> in this section or to the previous two rules, then this SRGB is
>
> completely ignored and the node is treated as if it does not have
>
> an SRGB.
>
> Can we fix this passive voice? Presume the intention is that the
>
> advertisement of this SRGB is ignored by any routing protocol speakers
>
> that receive it.
>
> Two points of clarification I think we should add:
>
> 1. The *whole* SRGB is ignored, not simply the range of labels in a set
>
> of ranges that is at fault
>
> 2. The term "ignore" in this case means that no SR action is taken, but
>
> the SRGB advertisement continues to be propagated within the routing
>
> protocol.
>
#Ahmed: the section defines the SRGB as a single entity consisting of a 
list of label ranges. Hence the SRGB is not divisible. The document 
clearly specifies what "ignore" means by saying "and the node is treated 
as if it does not have an SRGB.". How a routing protocol advertises the 
SRGB (e.g. by flooding, leaking between levels/areas) is beyond the 
scope of this document. I modified the statement slightly to indicate 
what does it mean "ignored"
>
> ---
>
> 2.3 (bottom of page 5)
>
> s/reserved label/special purpose label/
>
#Ahmed: renamed "reserved label" to special purpose label through out 
the document as you s
>
> ---
>
> 2.3
>
> s/contiguous range of label/contiguous range of labels/
>
#Ahmed: Done
>
> ---
>
> 2.5
>
> s/(e.g.,over/(e.g., over/
>
> s/map to the same incoming/maps to the same incoming/
>
> s/FECk} maps to the same/FECk} map to the same/
>
> s/non-zero algo/non-zero algorithm/
>
#Ahmed: Corrected or removed the statement containing the typo
>
> ---
>
> 2.5.1
>
> The plural of FEC is FECs not FEC's
>
#Ahmed: corrected the 3 occurrences
>
> ---
>
> 2.5.1
>
> 2. if more than one competing FEC remains after step 1, sort them and
>
> select the smallest numerical FEC value
>
> Is sorting required? Can we just have...
>
> 2. if more than one competing FEC remains after step 1, select the
>
> smallest numerical FEC value
>
#Ahmed: Agreed and modified accordingly
>
> ---
>
> 2.5.1
>
> An implementation may choose to implement additional
>
> s/may/MAY/
>
#Ahmed: Agreed and modified accordingly
>
> ---
>
> 2.5.2
>
> s/THEN/then/
>
#Ahmed: Agreed and modified accordingly
>
> ---
>
> 2.7
>
> s/plan./plane./
>
#Ahmed: Agreed and modified accordingly
>
> ---
>
> Terminology for adjacency SIDs needs to be harmonised.
>
> draft-ietf-spring-segment-routing has "IGP-Adjacency Segment" and
>
> "Adj-SID"
>
> This document has:
>
> adjacency SID (2.5.1)
>
> Adj-SID (2.7.3)
>
> adj-SID (2.7.3)
>
> IGP-adjacency-SID (2.8)
>
> IGP-Adj-SID (2.11.1)
>
> IGP adj-SID (2.11.2)
>
  #Ahmed: I changed all of them to adj-SID
>
> ---
>
> Some of the section names are overly long and can be cut down without
>
> risking much. This helps with layout and readability. Just a suggestion,
>
> but...
>
> OLD
>
> 2.1. Supporting Multiple Forwarding Behaviors for the Same Prefix
>
> 2.8. MPLS Label downloaded to FIB corresponding to Global and Local SIDs
>
> 2.10.1. Forwarding Behavior for PUSH and CONTINUE Operation for Global 
> SIDs
>
> 2.10.2. Forwarding Behavior for NEXT Operation for Global SIDs
>
> 2.11.1. Forwarding Behavior Corresponding to PUSH Operation on Local SIDs
>
> 2.11.2. Forwarding Behavior Corresponding to CONTINUE Operation for 
> Local SIDs
>
> NEW
>
> 2.1. Multiple Forwarding Behaviors for the Same Prefix
>
> 2.8. MPLS Label Downloaded to FIB for Global and Local SIDs
>
> 2.10.1. Forwarding for PUSH and CONTINUE of Global SIDs
>
> 2.10.2. Forwarding for NEXT Operation for Global SIDs
>
> 2.11.1. Forwarding for PUSH Operation on Local SIDs
>
> 2.11.2. Forwarding for CONTINUE Operation for Local SIDs
>
#Ahmed: Thanks for the advice. Modified as suggested
>
> ---
>
> For some reason the definite article is missing before a lot of
>
> instances of "FIB". For example, in 2.8...
>
> The label corresponding to the global SID "Si" represented by the
>
> global index "I" downloaded to FIB is used to match packets whose
>
> ---
>
> 2.10
>
> OLD
>
> This section specifies forwarding behavior, including the outgoing
>
> label(s) calculations corresponding to a global SID when applying
>
> PUSH, CONTINUE, and NEXT operations in the MPLS forwarding plane.
>
> 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].
>
> NEW
>
> This section specifies forwarding behavior, including the calculation
>
> of outgoing labels, that corresponds to a global SID when applying
>
> PUSH, CONTINUE, and NEXT operations in the MPLS forwarding plane.
>
> This document covers the calculation of the outgoing label for the
>
> top label only. The case where the 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].
>
#Ahmed: Modified as suggested
>
> ---
>
> 2.10.1
>
> s/{RFC3031]/[RFC3031]/
>
> ---
>
> 2.10.2 has
>
> oApply the instruction associated with the incoming label prior to
>
> popping
>
> This is a little ambiguous. Might be read that the instruction should be
>
> applied prior to popping. The text that follows des clarify, but perhaps
>
> rewrite as...
>
> oApply the instruction associated with the incoming label that has
>
> been popped
>
#Ahmed: Agreed and modified accordingly
>
> ---
>
> In order that ECMP descriptions work in the examples, Section 3 just
>
> needs a quick comment like.
>
> All links in this example have the same IGP metric.
>
#Ahmed: Added a phrase to the end of the second sentence in the section
>
> ---
>
> While it is largely OK to defer the discussion of manageability in
>
> Section 5 to draft-ietf-spring-segment-routing, it would make sense
>
> to make explicit reference to RFC 8287 because that is the key document
>
> that means no further OAM discussion is needed in this document.
>
> Probably just copy in the following paragraph
>
> SR OAM use cases for the MPLS data plane are defined in
>
> [I-D.ietf-spring-oam-usecase].SR OAM procedures for the MPLS data
>
> plane are defined in [RFC8287].
>
#Ahmed. Copied
>
> ---
>
> 8.
>
> Possibly Himanshu would prefer to be named as "Himanshu Shah".
>
#Ahmed: Thanks for catching that :)
>
> *From:*spring [mailto:[e-mail]] *On Behalf Of 
> *[e-mail]
> *Sent:* 24 May 2018 18:14
> *To:* SPRING WG List
> *Cc:* [e-mail]
> *Subject:* [spring] 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.