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

["Adrian Farrel" <[e-mail]>]

Good work, Ahmed.
 
Very many thanks for the changes and for the clear response to my review.
 
Best,
Adrian
 
From: Ahmed Bashandy [mailto:[e-mail]] 
Sent: 11 June 2018 22:55
To: [e-mail]; [e-mail]; 'SPRING WG List'
Cc: [e-mail]
Subject: Re: [spring] WG Last Call for draft-ietf-spring-segment-routing-mpls-13
 
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
   o  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.
 
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 
 
   o  The 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
   o  The label value MUST NOT be identical to or within the range of
      any reserved label value or range [reserved-MPLS], respectively.
NEW
   o  The label value MUST NOT come from the range of special purpose
      labels [RFC7274].
#Ahmed: Agreed and done


 
---
 
2.3 ditto
 
OLD
   o  Every 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
   o  Every 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
 
   o  If 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
 
   o  Apply 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...
 
   o  Apply 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.