Re: [bess] AD Review of draft-ietf-bess-evpn-vpws-07

[Sami Boutros <[e-mail]>]

Hi Alvaro,

Thanks for your review. Please have a look at attached draft w/ most of the comments addressed.

Please see comments inline Sami:

Thanks,

Sami
From: "Alvaro Retana (aretana)" <[e-mail]<mailto:[e-mail]>>
Date: Wednesday, January 25, 2017 at 8:39 PM
To: "[e-mail]<mailto:[e-mail]>" <[e-mail]<mailto:[e-mail]>>
Cc: "[e-mail]<mailto:[e-mail]>" <[e-mail]<mailto:[e-mail]>>, Jeffrey Zhang <[e-mail]<mailto:[e-mail]>>, "[e-mail]<mailto:[e-mail]>" <[e-mail]<mailto:[e-mail]>>
Subject: AD Review of draft-ietf-bess-evpn-vpws-07
Resent-From: <[e-mail]<mailto:[e-mail]>>
Resent-To: <[e-mail]<mailto:[e-mail]>>, <[e-mail]<mailto:[e-mail]>>, <[e-mail]<mailto:[e-mail]>>, <[e-mail]<mailto:[e-mail]>>, <[e-mail]<mailto:[e-mail]>>, Sami Boutros <[e-mail]<mailto:[e-mail]>>, <[e-mail]<mailto:[e-mail]>>, <[e-mail]<mailto:[e-mail]>>
Resent-Date: Wednesday, January 25, 2017 at 8:39 PM

Dear authors:

Hi!

I just finished reading this document.  Please take a look at my comments below – I think they should be easy to resolve.  I will start the IETF Last Call when the comments have been addressed and a new revision has been published.

Thanks!

Alvaro.


Major:

M1. There are 8 authors listed on the front page.  Following the guidelines in the RFC Style Guide [RFC7322], we want the list to be at most 5.  Please work among yourselves to reduce the number of authors.  Alternatively, you can just list an Editor or two.


M2. From the Introduction: “Unlike EVPN where Ethernet Tag ID in EVPN routes are set to zero…in EVPN-VPWS, Ethernet tag ID in the Ethernet A-D route MUST be set to a valid value in all the service interface types.”  Zero is a valid value for the Ethernet Tag ID.

Sami: I will change the text from “a valid value” —> “non zero value”.

Section 3. (BGP Extensions) says that “the Ethernet Tag ID 32-bit field is set to the 24-bit VPWS service instance identifier”, but I couldn’t find a place where the document told me what a valid value is.  IOW, how can the “MUST” be enforced if there’s no clear indication of what is valid?  OTOH, any specification wants the fields to contain valid values, obviously!  What happens if the value is not valid?    BTW, all this is to say that without a proper explanation (which probably doesn’t belong in the Introduction), the whole paragraph is superfluous.

Sami: A valid value will be non-zero 24-bit, I will change the text to "non-zero 24-bit VPWS service instance identifier"


M3. The Introduction says that “For EVPL service, the Ethernet frames transported over an MPLS/IP network SHOULD remain tagged with the originating VID and any VID translation is performed at the disposition PE.”, later the same (or at least something similar) is mentioned in Section 2.1. (VLAN-Based Service Interface): “the Ethernet frames transported over an MPLS/IP network SHOULD remain tagged with the originating VID, and a VID translation MUST be supported in the data path and MUST be performed on the disposition PE.”  Please keep the normative language in one place – I am not fond of normative language in the Introduction; note that even though the result should be the same, the text is different (the “MUSTs” are used in 2.1).

Sami: Will change the introduction to say MUST be

M3.1. [minor] It is not clear in the text that EVPL service corresponds to VLAN-based service.  Please clarify that.  In fact, some of the flow of the document feel disjoint because slightly different terminology is used and no hint of how it all ties together is presented; mapping EPL/EVPL to the Service Interfaces, which are only mentioned in Section 2 (and very briefly in the Introduction – see M2).

Sami: In the introduction we have this text "[MEF] defines Ethernet Virtual Private Line (EVPL) service as p2p service between a pair of ACs (designated by VLANs) and Ethernet Private Line (EPL) service …” So, isn’t that sufficient?


M4. Section 1.2 is titled Requirements.  However, the list seems to include a combination of statements of fact (“EPL service access circuit maps to the whole Ethernet port”: this is pretty much the definition of EPL), solution-sounding lines (“Each VLAN individually (or <S-VLAN,C-VLAN> combination) will be considered to be an endpoint for an EVPL service”: not only does it sound like what the solution will do, but it is also the definition of EVPL), and statements that talk to the configuration and not what the technical solution described later can do (“A given PE could have thousands of EVPLs configured. It must be possible to configure multiple EVPL services within the same EVI.”: is there an actual scalability requirement?).     I would have expected actual requirements (for example: “EPL service access circuits MUST map to the whole Ethernet port”; normative language is not required) that I can then check against the solution – but it all sounds like a rehash of what was explained before.  ☹

Sami: Please have a look at the attached draft to see if you are OK with the section now, we can consider removing the section too.

M5. Section 3. (BGP Extensions) says that “This document proposes the use of the per EVI Ethernet A-D route to signal VPWS services. The Ethernet Segment Identifier field is set to the customer ES and the Ethernet Tag ID 32-bit field is set to the 24-bit VPWS service instance identifier.”  First of all [this is minor/a nit], if this document intends to be in the Standards Track then it is past the “proposing” phase, it is *specifying*.  As a specification, it is rather weak in some places; for example, RFC7432 says (as an example) that the “Ethernet Tag ID in all EVPN routes MUST be set to 0”: that is a strong statement of what is expected.  On the other hand, this document is modifying the behavior, but no Normative language is used.  [In general I don’t advocate for the use of Normative language everywhere, but in cases like this where the behavior is being changed from the base spec when using this extension, it seems necessary.]

M5.1. Section 3: “Ethernet Tag ID 32-bit field is set to the 24-bit VPWS service instance identifier” How should this be aligned into the larger field?

Sami: Changed the text to "This document specifies the use of the per EVI Ethernet A-D route to signal VPWS services. The Ethernet Segment Identifier field is set to the customer ES and the Ethernet Tag ID 32-bit field MUST be set to the 24-bit VPWS service instance identifier value."


M6. Section 3.1 (EVPN Layer 2 attributes extended community).

M6.1. For the P flag – “SHOULD be set to 1 for multihoming all-active scenarios”: in a multihoming all-active scenario, when would this flag not be set?  IOW, why is the “SHOULD” not a “MUST”.  A couple of paragraphs later: “…the PEs in the ES that are active and ready to forward traffic to/from the CE will set the P bit to 1”.  In the all-active scenario, is there a case where a PE would not be “active and ready”?  What does that mean?  Clarifying may justify the “SHOULD”.

Sami: changed the text to "MUST be set to 1 for multihoming all-active scenarios by  all active PE(s)."

M6.2. How should the other flags in the Control Flags field be assigned?  Please define a new registry and include the details in the IANA Considerations section.

Sami: We are already describing how the other control flags be assigned in the doc, we have 2 other Flags B and C, not sure why do we need a new registry?

M6.3. What should a remote PE do if it receives both the P and B flags set (or both unset)?  I know that in a single-active scenario they should not be on at the same time…but what should the receiver do?

Sami:  Added "In multihoming scenarios, both B and P flags MUST not be both set or both unset by a sender PE, and a receiving PE that receives an update with both B and P flags set or unset MUST not forward any traffic to the sender PE.”  Need to review this with other authors too.

M6.4. What happens if the B flag is set in the all-active scenario?   I know there was some discussion about this on the list – the document needs to explicitly talk about it.

Sami: Added text "B flag SHOULD not be set in the multihoming all-active scenario and MUST be ignored by receiving PE(s)."

M6.5. What units is the MTU in?  How is it encoded?

Sami: Added "L2 MTU (Maximum Transmission Unit) is a 2-octet value indicating the MTU in octets”

M6.6. “non-zero MTU SHOULD be checked against local MTU”  When is it ok not to check?  I’m wondering why this “SHOULD” is not a “MUST”, specially because the result of that check is a “MUST NOT”.

Sami: Changed to MUST.

M6.7. “As per [RFC6790]…the control word (C bit set) SHOULD NOT be used…”  Where in RFC6790 does it say that?  I searched for “control word”, but found nothing?  Also, this “SHOULD NOT” is in conflict with the definition of the C flag: “If set to 1, a Control word [RFC4448] MUST be present…”

Sami: changed text to "If a network uses entropy labels per [RFC6790] then the C Bit MUST not be set to 1 and control word MUST NOT be used when sending EVPN-encapsulated packets over a P2P LSP.”

Minor:

P1. Please add a reference for VPWS.

Sami: You mean PWE3 reference?

P2. The [MEF] reference didn’t work for me; on all tries the connection timed out.  Is it possible to find a more stable reference?

Sami: No clue here!

P3. There are several acronyms that won’t be familiar to the average reader and that need to be expanded on first use: ES, AD (A-D), EVI, VID, VNI, EP-LAN, EVP-LAN. I know that some of these are expanded in the Terminology Section, but in some cases that comes later in the text.

Sami: Done for what happen before terminology.

P4. The EVPN-VPWS term is introduced for the first time in the text at the bottom of page 3.  I take it that it refers to the solution presented in this document.  Please include a sentence at the top of the introduction to clarify – note that this tag could be useful in other places as well.

Sami: Please look at doc.

P5. “Ethernet tag field” (and not “Ethernet Tag ID”, which I’m assuming is the same thing) is used in several parts of the text.  Please be consistent.

Sami: Done.

P6. “VxLAN encap” is mentioned in the Introduction, and potential things about handling it…but there are no references and no additional mention anywhere else in the document.

P7. “mass withdraw” is mentioned in the Introduction (“…can be used for flow-based load-balancing and mass withdraw functions”),  in Section 4 (“…can be used for mass withdraw”), and finally Section 6.2 (the last section in the draft!) has a reference…  Please move it earlier in the document.

Sami: Done.

P8. S-VLAN, C-VLAN: expand and put a reference for them.

Sami: Done added to terminology.

P9. There is no Reference to any of the Extended Communities RFCs: 4360, 7153, etc…

Sami: Done.

P10. Please add Figure numbers/names.

Sami: Done.

P11. Section 3.1 (EVPN Layer 2 attributes extended community) defines 3 control *flags*, but they are referred to later in the text as “bits”.  Please be consistent.

Sami: Please have a look.

P12. Section 4 (Operation): s/with Next Hop attribute set/with the NEXT_HOP attribute set   Also, include an Informative reference to RFC4271.

Sami: Done.

P13. Section 6 (Failure Scenarios): “…the PE must withdraw all the associated Ethernet AD routes…”  Should that be a “MUST”?

Sami: Done.
P14. A reference to “[ietf-evpn-overlay]” appears in the Security Considerations, but there’s no Reference later on.


Nits:

N1. “Both services are supported by using…I.e., for both EPL and EVPL services…”  The second part of that explanation is a lot clearer, you might want to simplify by just leaving that part in.

Sami: I don’t get this.

N2. The introduction reads like a rushed summary of the solution, which results in potentially confusing text.  Suggestion: focus the Introduction on setting the stage/background – if you want to provide a summary of the solution (good idea!), then do it after the requirements.

Sami: I prefer to leave it as is.

N3. s/Ethernet Segment on a PE refer to/Ethernet Segment on a PE refers to

Sami: Done

N4. s/multi home…single home/multi homed…single homed

Sami: Done

N5. The text in Section 9 is misaligned.

Sami: Done.