Subject: [bess] RtgDir review: draft-ietf-bess-evpn-etree-12

From: Ravi Singh <[email-masked]>
Date: Wed, 16 Aug 2017 06:56:41 +0000

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see ​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft.

Document: draft-ietf-bess-evpn-etree-12
Reviewer: Ravi Singh
Review Date: 08/15/2017
Intended Status: Standards track

Summary: I have some concerns about this document that I think should be resolved before publication.

I've reviewed this draft.
Most of my comments fall under the category of "minor comments" and "nits".
The only major comments pertain to
a.       Sections 5.2/8.1: which appear like an overkill and could be considered for dropping from text.
b.      Need for a new section on the doc:
Include section to address this specifically: "Is (PBB-)EVPN being a *more efficient* implementation of E-Tree functions demonstrated?" This is relevant since the abstract makes a pitch for this draft addressing how PBB(-EVPN) implement E-tree functionality more efficientlky. However, it takes a fine-toothed comb to glean that info. Eg. First para in section 3.1. Nowhere else in the text has the efficiency aspect been explicitly addressed.  This aspect could use a section of its own.

Feedback:
1.       Abstract: very clear
2.       Section1 : Introduction:
a.       How about referring to "[RFC7432] is a solution for multipoint L2VPN services"  as EVPN more directly?
3.       Section 2: E-tree scenarios
a.       Section 2.1: for the sake of terminology clarification, please edit the text to not mention the acronym until the full-expansion has been listed. The draft does list the full-expansion. Just not before first using the acronym.
b.      Section 2.2: "
thus color MAC addresses via a "color" flag in a new extended community as detailed in section 3.1." should be referring to section 5.1 instead.
4.       Section 3: Operation of EVPN
a.       3.1: "Extended Community with a Leaf indication flag is introduced [section5.2]"  ->
"Extended Community with a Leaf indication flag is introduced [section 5.1]"
b.      3.3.1 (& section 5.2 as well): specifying (as this draft does) that the ingress PE X acting for a root entity, be able to dictate to a PE Y acting on behalf of a leaf entity, that PE Y should "ingress replication" or otherwise sounds excessive. Also, this draft does not make a strong enough case for the need for the modification to "PMSI Tunnel Attribute". Even if the foregoing was ignored, the draft does not address how PE X behaves should PE Y choose not to honor the setting of the "composite-tunnel bit".

5.       Section 4: Operation of PBB-EVPN: no specific comments. However, a little more detail would be useful to avoid having the reader repeatedly refer back to the PBB-EPVN RFC7623.

6.       Section 5: BGP encoding
a.       This looks like an unexpected section given the "abstract" and the "introduction" section.
The abstract and the introduction seem to indicate that no signaling changes are needed to make (PBB-)EVPN provide E-tree services. Abstract/introduction could use some rewording on this count.
b.      Minor typo:
5.1: "The received PE" -> "The receiving PE"

5.2: it would enhance readability if the flags were presented as
       0 1 2 3 4 5 6 7
      +-+-+-+-+-+-+-+-+
      |C  |reserved  |L|
      +-+-+-+-+-+-+-+-+
C = composite-tunnel bit

Anyway, as stated above this draft does not make a strong enough case for the need for the modification to "PMSI Tunnel Attribute". The value added by this modification to the PMSI tunnel-attribute seems marginal. A leaf

7.       Section 8: IANA considerations: it would be useful to include text about the name of "E-Tree Flags" registry in section 5.1 and correlate these 2 sections.
a.       Section 8.1: could be considered for dropping, in view of the previous comments on PMSI etc.
8.       Appendix A: too wordy. Could do with fewer better-chosen words to communicate the same message. Some minor singular/plural typos.

Regards
Ravi

---

From: <[email-masked]>
Date: Wed, 16 Aug 2017 12:19:50 +0200

Hi Ravi,

Many thanks for this review.

One question below...

> considered for dropping from text.
[...]

Can you expand on why you this this is overkill ?

Thank you,

-Thomas

_________________________________________________________________________________________________________________________

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.

---

From: "Ali Sajassi (sajassi)" <[email-masked]>
Date: Thu, 24 Aug 2017 05:27:06 +0000

Hi Ravi,

Thanks for your comments. Please see my replies inline.

From: Ravi Singh <[email-masked]<mailto:[email-masked]>>
Date: Tuesday, August 15, 2017 at 11:56 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]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Subject: RtgDir review: draft-ietf-bess-evpn-etree-12
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, August 15, 2017 at 11:56 PM

Hello,

Section 3.3.1 describes the need and the use for composite tunnel type described in 5.2/8.1. This has been discussed in length with your colleague Eric Rosen, Jeffrey Zhang, and John Drake. After discussing with them, if you still think it is “overkill”, then please elaborate as to why.

OK, the new section is 3.1 :-) It is not just the first paragraph of section 3.1 but the entire section that describes it - i.e., first paragraph says that if filtering is done on the ingress PE, it provides an efficient filtering and the rest of the section describes what it takes to do ingress filtering.

Feedback:
1.       Abstract: very clear

OK, changed it to "EVPN (RFC7432) is a solution …"

Done. Also added a new section to define acronyms/terminology.

b.      Section 2.2: "

Done. Thanks for the catch.

Already corrected.

Section 3.3.1 describes why ingress replication from leaf to roots would be sufficient and how to signal it efficiently. The co-authors as well as other reviewers (e.g., Eric Rosen, Jeffrey Zhang, and others listed in the ack section) have consensus on the current description in section 3.3.1.

Both RFC 7432 and RFC7623 are prerequistes to this RFC and it is assumed the reader has full knowledge of both.

Updated both abstract and introduction to say “a solution based on EVPN” instead of “EVPN”.

b.      Minor typo:
Corrected it.

C bit is in Tunnel Type field (and not the flag field).

Already addressed because of a similar comment (please refer to the attachment).

OK.

Thanks,
Ali

Regards
Ravi