Subject: Re: [bess] AD Review of draft-ietf-bess-evpn-etree-09

From: "Ali Sajassi (sajassi)" <[email-masked]>
Date: Wed, 10 May 2017 00:51:40 +0000

Hi Alvaro,

Thank you  for your thorough review and comments. I have addressed all of them. Please refer inline for details on each. I just posted a new rev (rev10) that covers all the comment resolutions addressed here.

https://www.ietf.org/id/draft-ietf-bess-evpn-etree-10.txt

From: "Alvaro Retana (aretana)" <[email-masked]<mailto:[email-masked]>>
Date: Tuesday, April 4, 2017 at 2:37 PM
To: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, Thomas Morin <[email-masked]<mailto:[email-masked]>>
Subject: AD Review of draft-ietf-bess-evpn-etree-09
Resent-From: <[email-masked]<mailto:[email-masked]>>
Resent-To: Cisco Employee <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>
Resent-Date: Tuesday, April 4, 2017 at 2:37 PM

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?

Because RFC7387 is informational RFC (and the framework RFC) and the idnits complains of a downref: Normative reference to an informational RFC

M2. There is no reference to E-Tree.  Please add a Normative one.

Added a reference for E-TREE definition in MEF.

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).

Changed “MAY” to “can”.

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…?

It is cleaned up.

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.

Removed the replicated/redundant text

M5.1. BTW, that is a “valid MPLS label”?  How would a receiver recognize it?  Please add a reference to avoid confusion.

Added a brief description of “valid MPLS label” and provided the reference

M6. Definition of the E-TREE Extended Community

M6.1. Only one Flag is defined.  What about the others?  Please set up a registry.

If needed in the future, we will setup a registry.

M6.2. [Minor] Please put a Figure number and heading for the community format.

Done.

M6.3. It looks like the Reserved fields are set to 0.  What should happen if the receiver gets something else?

Added a sentence that “the receiver should ignore the reserved bits”

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!

For MPLS label, changed it to “the transmitter SHOULD set it to zero and the receiver SHOULD ignore it”.

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.

For Leaf-Inication flag, change it to "the transmitter SHOULD set it to zero and the receiver SHOULD ignore it"

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?

Added “the route should be ignored"

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).

Modified the 1st para to make it explicit:
"This draft defines a new Composite tunnel type by introducing a new  "Composite Tunnel' bit in the Tunnel Type field and adding a MPLS label to the Tunnel Identifier field of PMSI Tunnel attribute as detailed below. This draft uses all other remaining fields per existing definition."

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]

The description in section 3.3.1 is consistent with this section 5.2. Basically, Composite Tunnel type, as its name implies consist of two tunnels: a P2MP tunnel in the transmit direction and a MP2P tunnel in the receive direction. The MP2P tunnel in the receive direction is used by Leaf PE devices for their BUM traffic transmission. The "ingress replication tunnel type” is not valid because for that we don’t need composite tunnel type!!
I added the following sentence to the 1st paragraph to clarify it  more:
"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 for the BUM traffic."

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!

OK, changed it to "Composite Tunnel bit".

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.

Added a sentence that it should be treated as malformed per section 5 of [RFC6514].

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.

Now, using “Composite Tunnel bit” everywhere.

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.

The ranges “0xFB-0xFE” and 0x0F have been replaced with “0x7B-0x7E” and 0x7F.

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]

Done.

M8.4. What is 0x7F reserved for?  It doesn’t seem to be used in this document.  Why a different registration procedure?

0x7F just replaces 0x0F - i.e.  No new 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").

Done.

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].

Done and we’ll do.

P2.1.  Please also expand: DF, BUM…

Done.

[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.

Done.

P4. “E-TREE for VPLS”  Please add an Informative reference to rfc7796.

Done.

P5. “…new BGP Extended Community for leaf indication as shown later in this document.”  Please include a forward reference to Section 5.1.

Done.

P6. “MAC mobility procedures”  What are those?  Please add a reference.

Done.

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.

Done.

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.

Done.

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…”.

Done.

P10. s/EVPN MAC Advertisement route/MAC/IP Advertisement Route     Please be specific!

Done.

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!

Done.

P12. Section 5.3 doesn’t exist.

Corrected.

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.

They are for two different solutions – one for EVPN and the other for PBB-EVPN. It is done intentionally for ease of reference.

P14. “This document defines two new BGP Extended Community for EVPN.”  I only see one.

It used to be two :-) I corrected it.

P15. A Normative reference to rfc4360 is needed in 5.1.

Done.

P16. There’s no need to the reference to rfc7153 because you’re referring to the registry itself.

It doesn’t hurt :-)

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.

Done. Also added 6514 to the list.

P18. There is no reference to rfc5226.

Done.

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.

I did an editorial pass through this section to improve its readability.

N2. s/ each of its MAC/IP Advertisement route/each of its MAC/IP Advertisement routes

Done.

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…

I did an editorial pass through this section to improve its readability.

N4. “we” is used a couple of times (outside the Acknowledgements) – please change that to “this document” (or something similar).

Done.

N5. s/where and Ethernet Segment/where an Ethernet Segment

Done.

N6. s/draft/document

Done.

---

From: "Alvaro Retana (aretana)" <[email-masked]>
Date: Fri, 12 May 2017 19:18:33 +0000

On 5/9/17, 8:51 PM, "Ali Sajassi (sajassi)" <[email-masked]<mailto:[email-masked]>> wrote:

Ali:

Hi!

We’re on the right path, but not there yet.  Please see my comments inline below.

Thanks!

Alvaro.

…
> > 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?
>
> Because RFC7387 is informational RFC (and the framework RFC) and the idnits
> complains of a downref: Normative reference to an informational RFC

The status of an RFC is not an indicator of how it should be referenced – complaints from idnits is not an excuse.  IOW, there is nothing that prevents an Informational document to be referenced as Normative – we just need to take care of the process: not a big deal, but important.

Let me rephrase.  Is rfc7387 a document that “that must be read to understand or implement” [1] what is specified in this document?  Because of how rfc7387 is positioned in the Abstract/Introduction, it seems to me that the framework must be read to then understand how this document addresses it:

“                                             A solution

   framework for supporting this service in MPLS networks is proposed in

   RFC7387<https://tools.ietf.org/html/rfc7387> ("A Framework for Ethernet Tree (E-Tree) Service over a

   Multiprotocol Label Switching (MPLS) Network"). This document

   discusses how those functional requirements can be easily met with

   Ethernet VPN (EVPN) and how EVPN offers a more efficient

   implementation of these functions.”

[1] https://www.ietf.org/iesg/statement/normative-informative.html

> Added a reference for E-TREE definition in MEF.

This reference needs to be Normative – see above.

…
> If needed in the future, we will setup a registry.

Please set it up now.

…
> > 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]
>
> The description in section 3.3.1 is consistent with this section 5.2. Basically, Composite
> Tunnel type, as its name implies consist of two tunnels: a P2MP tunnel in the transmit
> direction and a MP2P tunnel in the receive direction. The MP2P tunnel in the receive
> direction is used by Leaf PE devices for their BUM traffic transmission. The "ingress
> replication tunnel type” is not valid because for that we don’t need composite tunnel
> type!!
> "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
> for the BUM traffic."

Let me see if I understand what you’re saying:

If the C-bit is set, then the receive direction always has an IR tunnel.  The type of the P2MP tunnel is defined by one of the other bits.  Is that right?

If so, are all the other bits valid (always)?  The text already says that 0x00/0x06 are invalid, but, for example, what about 0x07 (mLDP MP2MP LSP) or 0x0A (Assisted-Replication Tunnel [draft-ietf-bess-evpn-optimized-ir])?

How can you be sure that any other type besides 0x00/0x06 will be ok?

…
> 0x7F just replaces 0x0F - i.e.  No new registration procedure

0x0F is currently Unassigned, not Reserved.

…
> > 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.
>

Hmm...  I don’t see how this document Updates rfc6514….??

But if it does, you need to include some text about it in both the Abstract and the Introduction.  BTW, please add something about the Update to rfc7385 in the Introduction.

> > P18. There is no reference to rfc5226.

This one must be Normative.

---

From: "Ali Sajassi (sajassi)" <[email-masked]>
Date: Fri, 12 May 2017 23:15:35 +0000

Hi Alvaro,

Please see my comment resolutions inline. I have updated and posted a new rev:

https://www.ietf.org/id/draft-ietf-bess-evpn-etree-11.txt

From: "Alvaro Retana (aretana)" <[email-masked]<mailto:[email-masked]>>
Date: Friday, May 12, 2017 at 12:18 PM
To: Cisco Employee <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, Thomas Morin <[email-masked]<mailto:[email-masked]>>
Subject: Re: AD Review of draft-ietf-bess-evpn-etree-09

Ali:

Hi!

Thanks!

Alvaro.

…

“                                             A solution

It is a bit subjective in this case but to be safe and assuming the readers don’t have any background in E-TREE (which has been around for a long time – e.g., private VLAN), I have moved it to the normative section.

Done.

…

Please set it up now.

OK, I will set it up.

…

That’s correct. The remaining 7 bits indicate the type of tunnel.

 Basically composite tunnel type doesn’t make sense when ingress replication (0x06) is used or when tunnel info is not present (0x00). Other than that, it can be used with other tunnel types.

…

I meant to say 0xFF. Basically the existing experimental types 0xFB – 0xFE and reserved type of 0xFF has moved to new code points of: 0x7B – 0x7E and 0x7F.

…

On the 2nd thought, it doesn’t updated it because the whole purpose of C-bit is backward compatibility. So no updates to rfc6514. I removed it from the text.

Added a sentence to the introduction in last paragraph.

Done.