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

[Sami Boutros <[e-mail]>]

Hi Alvaro,

Please see comments inline. I have submitted 09.



Thanks,

Sami

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

I removed the section.

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

I made the VPWS service instance identifier a 32-bit value in the new draft.

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

I added a registry in the IANA consideration.

This document creates a registry called "EVPN Layer 2 attributes
   Control Flags". New registrations will be made through the "RFC
   Required" procedure defined in [RFC5226].  

   Initial registrations are as follows:

     P      Advertising PE is the Primary PE.
     B      Advertising PE is the Backup PE.
     C      Control word [RFC4448] MUST be present 



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

Changed text to MUST discard the received route from the sender PE.

>
>… 
>
>> > 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”
>
> 
>
>Yes, but what are the units?  0x0001 means what?  I would assume bytes, but please be specific. 
>
> 

Changed “in octets” —> “in bytes”

>
>
>
>… 
>
>> > P1. Please add a reference for VPWS.
>
>> 
>
>> Sami: You mean PWE3 reference?
>
> 
>
>No, VPWS.  I think rfc4664 is called out at some point – please reference it when VPWS is first mentioned in the Introduction.
>
> 

This is what I did in the 08, I referenced it in the introduction.

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

Done.

>
> 
>
>> > 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!
>
> 
>
>How about this:  https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mef.net_Assets_Technical-5FSpecifications_PDF_MEF-5F6.1.pdf&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=IVzcTRLQdpta08L0b_y2zDkqvwJhRKMCAbX-2K-LV98&m=GH5FIfqtBUACPwx-LVV2v5zPrGcNzhCEjfj8-0-R2OI&s=5b19ceQDqdsz0TepqsV7daJoYm9uDMyco7BZ4NeICWU&e=   ??
>
> 

Thanks,
>
>
>
>…
>
>> > P9. There is no Reference to any of the Extended Communities RFCs: 4360, 7153, etc…
>
>> 
>
>> Sami: Done.
>
> 
>
>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!
>
> 
I changed the text as follow in 3.1 and added/removed the above references.

"This draft proposes a new extended community, defined below as per [RFC7432] in addition to the values specified in [RFC4360]"

>
> 
>
> 
>
>> > P10. Please add Figure numbers/names.
>
>> 
>
>> Sami: Done.
>
> 
>
>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.
>
> 
Sorry, fixed that too.
>
>
>
>> > 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.
>
> 
>
>There are still several places where the P bit, B bit or C bit are used.


Changed to Flag .

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

Ok.

>
>
>
>A couple more nits:
>
>
>
>s/control flags SHOULD not be set in/control flags SHOULD NOT be set in
>
>
>
>s/then the C Bit MUST not be set/then the C Flag MUST NOT be set
>
>

Done.

Thanks,

Sami
>