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, [email protected] 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:*[email protected] [mailto:[email protected]]
*Sent:* Thursday, May 24, 2018 7:14 PM
*To:* SPRING WG List
*Cc:* [email protected]
*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.
_______________________________________________
spring mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/spring
