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

["Alvaro Retana (aretana)" <[e-mail]>]

On 2/16/17, 3:57 PM, "Alvaro Retana (aretana)" <[e-mail]<mailto:[e-mail]>> wrote:


On 2/3/17, 4:41 PM, "Sami Boutros" <[e-mail]> wrote:



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

> addressed.



Looking at the published version -08.



I have some answers to your answers below.  I also see that there may be other changes as a result of the discussion on this thread.  I’ll wait for an update before starting the IETF Last Call.



Thanks!



Alvaro.





…



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



[a] the new text is not necessarily better – the normative language added doesn’t always do what it should, for example: “A given PE MAY have thousands of EVPLs configured.”  That is really still a statement, not an option (“MAY”) given as part of the solution.



Yes, please remove this section.





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



[a] Ok, but you still didn’t mention how the 24-bit value is to be aligned in the 32-bit field.  I’m guessing there will be some 0-padding, but will that the at the beginning or the end?





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?



[a] The Control Flags field is 16 bits long, you’re only defining 3 bits.    If someone else in the future wants to use one of those other bits, what should be the criteria for assignment: first come first served, do you think they should at least have an RFC, should that be standards track??



As it is right now, IANA won’t know what to do if anyone else wants do use any of the other bits in the future.



Note that MBZ doesn’t preclude the bits being used in the future for something else.





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.



[a] Not forwarding any traffic means that the route is ignored and not used, right?  Should it be discarded?  Maybe phrase the resulting action in terms of the route and not the forwarding of traffic…





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”



[a] Yes, but what are the units?  0x0001 means what?  I would assume bytes, but please be specific.





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?



[a] No, VPWS.  I think rfc4664 is called out at some point – please reference it when VPWS is first mentioned in the Introduction.



BTW, please move it to Informative to avoid a downref.





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!



[a] How about this:  https://www.mef.net/Assets/Technical_Specifications/PDF/MEF_6.1.pdf  ??



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.



[a] We still need a reference to rfc4360 – at least in Section 3.1 where the new community is defined.



You did add a reference to rfc7153, but it is not used in the text. ☹  There’s no point in having it if it isn’t used!









P10. Please add Figure numbers/names.



Sami: Done.



[a] Maybe it’s just me, but I don’t see them.  Note that the figures in 3.1 seem to run into each other w/out names/numbers.



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.



[a] There are still several places where the P bit, B bit or C bit are used.





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.



[a]  Just a suggestion:



OLD>

   Both services are supported by using the per EVI Ethernet A-D route

   which contains an Ethernet Segment Identifier, in which the customer

   ES is encoded, and an Ethernet Tag, in which the VPWS service

   instance identifier is encoded.  I.e., for both EPL and EVPL

   services, a specific VPWS service instance is identified by a pair of

  per EVI Ethernet A-D routes which together identify the VPWS service

   instance endpoints and the VPWS service instance.



NEW>

   For both EPL and EVPL

   services, a specific VPWS service instance is identified by a pair of

   per EVI Ethernet A-D routes which together identify the VPWS service

   instance endpoints and the VPWS service instance.



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.