Dear authors:
I just finished reading this document. In general, the document could benefit
from an editorial pass to improve readability; I put in some suggestions below,
but I’m sure I didn’t mention all. I don’t think that my comments (even the
Major ones) will be hard to resolve – most are around providing clarity and
consistency. I’ll wait for a revised draft before starting the IETF Last Call.
Thanks!
Alvaro.
Major:
M1. Both the Introduction and the Abstract mention that “this document
discusses how the functional requirements for E-Tree service can be easily
met…” It seems to me that RFC7387 is used as the building block for this
document. Why is it not a Normative reference?
M2. There is no reference to E-Tree. Please add a Normative one.
M3. In Section 2.2 (Scenario 2: Leaf OR Root site(s) per AC): “…then a single
RT per EVI MAY be used…” I think that “MAY” is out of place because it seems
to be expressing just a fact. Also, please keep the normative language where
the operation is defined (in this case in 3.1).
M4. Section 3.1 (Known Unicast Traffic) talks about how in the “multi-homing
scenario of section 2.2…the PE MAY advertise leaf indication along with the
Ethernet A-D per EVI route”. Given that the text later says that “in case of
discrepancy, the multi-homing for that pair of PEs is assumed to be in default
"root" mode”, I find the “MAY” not sufficient. If we want to prevent as many
discrepancies as possible, shouldn’t that be a “SHOULD” (or even a “MUST”)?
Given the local configuration, why would the PE not want to include the leaf
indication…ever…?
M5. In Section 5.1 (E-TREE Extended Community) there is redundant information
(first and last paragraphs). Among that redundant information there is no
consistency: “the Leaf Label field is set to a valid MPLS label” and “the Leaf
Label MUST be set to a valid MPLS label” – please be consistent and specify
things just once.
M5.1. BTW, that is a “valid MPLS label”? How would a receiver recognize it?
Please add a reference to avoid confusion.
M6. Definition of the E-TREE Extended Community
M6.1. Only one Flag is defined. What about the others? Please set up a
registry.
M6.2. [Minor] Please put a Figure number and heading for the community format.
M6.3. It looks like the Reserved fields are set to 0. What should happen if
the receiver gets something else?
M6.4. “…the Leaf-Indication flag MUST be set to one and Leaf Label is set to
zero. The received PE should ignore Leaf Label and only processes
Leaf-Indication flag.” What if the Leaf Label is not set to zero? The second
sentence says to ignore it, but the first one that it “MUST be…set to zero”.
Which one is it? Maybe both… Be specific!
M6.5. “…the Leaf Label MUST be set to a valid MPLS label and the
Leaf-Indication flag should be set to zero. The received PE should ignore the
Leaf-Indication flag.” Similar question for the Leaf-Indication bit.
M6.6. “A non-valid MPLS label when sent along with the Ethernet A-D per ES
route, should be logged as an error.” I’m assuming that not only the logging
is needed, but is the route ignored/discarded too?
M7. The PMSI Tunnel Attribute: Please be clear to the fact that the C bit is
being defined/introduced in this document (and not just start talking about it
as if it is well-known to every reader).
M7.1. It is not clear to me how the C bit is to be used. Section 5.2 says that
“the high-order bit of the tunnel type field (C bit - Composite tunnel bit) is
set while the remaining low-order seven bits indicate the tunnel type as
before.” But 3.3.1 says that the “new composite tunnel type is advertised by
the root PE to simultaneously indicate a P2MP tunnel in transmit direction and
an ingress-replication tunnel in the receive direction…”. Knowing, from 5.2
that when the C bit is set “Tunnel Types…0x06 'Ingress Replication' is
invalid”, then does the C bit have a set meaning or ??? [BTW, s/is/are]
M7.2. [Minor] In some places you talk about the “C bit” or “Composite tunnel
bit”, but later mention the “Composite flag”. I’m assuming it is the same
thing – please be consistent!
M7.3. “invalid tunnel type” RFC6514 talks about an “undefined tunnel type”.
Do you mean the same thing? If you do, please use the right terminology and
put a reference to rfc6514 here to remind people where the action is defined.
M8. Section 8.1 (Considerations for PMSI Tunnel Types).
M8.1. What should the name of the bit be? C bit, “composite tunnels” or
“Composite tunnel bit” – all 3 versions are used, and IANA will want specific
directions.
M8.2. “…by removing the entries for 0xFB-0xFE and 0x0F” The range between
0x80-0xFA also needs to be changed/updated. s/0x0F/0xFF. It may be clearer to
write that this document updates the range 0x80-0xFF.
M8.3. “…and replacing them by…” Please format the table so that spacing is
aligned (for better clarity for IANA). Also, please include in it all the
reallocated space; for example:
Value Meaning Reference
0x0B-0x7A Unassigned
0x7B-0x7E Reserved for Experimental Use [this document]
0x7F Reserved [this document]
0x80-0xFF Composite Tunnels [this document]
M8.4. What is 0x7F reserved for? It doesn’t seem to be used in this document.
Why a different registration procedure?
Minor:
P1. s/and RFC called "A Framework for E-Tree Service over MPLS
Network"./RFC7387 ("A Framework for Ethernet Tree (E-Tree) Service over a
Multiprotocol Label Switching (MPLS) Network").
P2. Please expand EVPN on first use. I know that this is a well-known term –
please consider talking to the RFC Editor (once this document gets to them) in
order to include “EVPN” in the “RFC Editor Abbreviations List” [1].
P2.1. Please also expand: DF, BUM…
[1] https://www.rfc-editor.org/materials/abbrev.expansion.txt
P3. The abbreviation for Ethernet Segment is introduced in Section 3.2…please
introduce it on first mention (Section 2) as it gets used before 3.2. Also,
the abbreviation for Attachment Circuit is introduced after AC has been used
several times. EC is used w/out definition.
P4. “E-TREE for VPLS” Please add an Informative reference to rfc7796.
P5. “…new BGP Extended Community for leaf indication as shown later in this
document.” Please include a forward reference to Section 5.1.
P6. “MAC mobility procedures” What are those? Please add a reference.
P7. Section 3.2.1 (BUM traffic originated from a single-homed site on a leaf
AC) starts by talking about “a special MPLS label” – even though there’s a
reference later to 5.1, please be explicit (writing something like this): “the
PE adds the Leaf Label advertised using the E-Tree Extended Community (Section
5.1)”. Simplifying the text will go a long way to making implementation
easier.
P8. “IRB use case is outside the scope of this document.” An Informative
reference to draft-ietf-bess-evpn-inter-subnet-forwarding would be nice.
P9. It would be nice to include a “map” of the document in the Introduction:
(something like) “Section 2 talks about blah…Section 3 defines…”.
P10. s/EVPN MAC Advertisement route/MAC/IP Advertisement Route Please be
specific!
P11. “To support multicast/broadcast from Root to Leaf sites, either a P2MP
tree rooted at the PE(s) with the Root site(s) or ingress replication can be
used.” References would be very nice!
P12. Section 5.3 doesn’t exist.
P13. Both Sections 3 and 4 mention the same things (other tagging mechanisms
and IRB) out of scope. It would be nice to mention common pieces of the
operation in a single place.
P14. “This document defines two new BGP Extended Community for EVPN.” I only
see one.
P15. A Normative reference to rfc4360 is needed in 5.1.
P16. There’s no need to the reference to rfc7153 because you’re referring to
the registry itself.
P17. The 'Updates: ' line in the draft header should list only the _numbers_ of
the RFCs which will be updated by this document (if approved); it should not
include the word 'RFC' in the list.
P18. There is no reference to rfc5226.
Nits:
N1. There are a lot of very long sentences… It would be nice if the ideas were
composed so that the overall document was easier to read. For an example, take
a look at the last paragraph in 2.2… No specific action required here, but it
would be nice to give it an editing pass.
N2. s/ each of its MAC/IP Advertisement route/each of its MAC/IP Advertisement
routes
N3. Another example of where the text is not as clear as it could be for a
specification is Section 3.2, where tagging MPLS frames is mandated (“the
MPLS-encapsulated frames MUST be tagged with an indication when they originated
from a Leaf AC”), but there is no indication on how to do this until 3
paragraphs later – a seemingly unrelated discussion is held in between…
N4. “we” is used a couple of times (outside the Acknowledgements) – please
change that to “this document” (or something similar).
N5. s/where and Ethernet Segment/where an Ethernet Segment
N6. s/draft/document
_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess
