Subject: AD review of draft-ietf-l2vpn-evpn

From: "Adrian Farrel" <[email-masked]>
Date: Fri, 18 Jul 2014 23:56:03 +0100

Goodness, but there's a long and complicated document. But I think 
you have made it as clear and concise as it could possibly have been.
Good job!

I have done my AD review and found no substantive issues. I do,
however, have a little pile of nits. Actually, quite a large heap.
Nothing to worry about, but if you could clean them up i think it
would improve the document still further.

The only topics that need real attention are those related to IANA.

Let me know how you get on, and please object if my comments are wrong.

Thanks,
Adrian

===

It would be best to move the Introduction to be the first section in
the document.

---

Section 5

   Ethernet segments have an
   identifier, called the "Ethernet Segment Identifier" (ESI) which is
   encoded as a ten octets integer.

It would help if you said "...in line format with the most significant
octet sent first."

---

Section 5

   In general, an Ethernet segment MUST have a non-reserved ESI that is
   unique network wide

"In general" is not really consistent with "MUST"

---

Do you want an IANA registry to track the values of the Type field of
the ESI?                                     

---                                          

There is some mixing of "octet" and "byte" in the document. This creates
the impression that you mean something different by the two words.

---

Could you expand DF on first use. You have it in 8.3.

---

Section 6

You use "Ethernet Tag ID", "Ethernet Tag", and "Ethernet Tag Identifier"
interchangeably. It would be helpful to use just one term and to check
usage in the rest of the document.

---

Section 6.1

   In such
   scenarios, the Ethernet frames transported over 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.

I think you should add under what circumstances the frames MAY be re-
tagged with a different VID (or s/SHOULD/MUST). You don't need a
detailed explanation, but a guide to the implementer/operator.

---

Do you want IANA to create a registry and track the Route Types defined
for the EVPN NLRI in Section 7?

---

Section 7.1 and onwards...

I know "RD" is a term of art in the context of BGP, but could you
please expand RD it on first use rather than leaving that to 8.2.1.

(All the forward references to later sections are good, thanks.)

---

A small inconsistency between sections 7 and 8. In the figures in
Section 7 you have "MPLS Label" and "MPLS Label1" etc. In the text
in Section 8 you have "MPLS label" etc. When you refer to the fields
you need to match the case. When you refer to the concept of an MPLS
label, you can (of course) use normal case.

---

Are you sure that the ESI Label extended community and subtypes don't
need IANA intervention here?

---

It would be nice if 7.5 included a hint as to what an "ESI label" is.

---

In 7.10

   If a PE uses RT-Constrain, the PE SHOULD advertise all such RTs using
   RT Constraints.

Is this a general restatement of RFC 4684 (if so add "As described in 
[RFC4684]...") or new guidance for implementers of this spec (if so, 
what is the reason for SHOULD? is there a MAY to counter it?)

---

8.1.1

   The Ethernet Segment Identifier MUST be set to the ten octet ESI
   identifier described in section 5.

Would that be the ESII? :-)

---

8.2.1 has "MANDATORY" I guess you are inventing a 2119 term to counter-
point "OPTIONAL". Please use "REQUIRED."

---

In Section 13.1

   In certain
   environments the source MAC address MAY be used to authenticate the
   CE and determine that traffic from the host can be allowed into the
   network.

Want to hint which environments they would be. Possibly more important,
want to say in which environments this would be a damn fool idea?

---

14.1.2

   The MPLS label stack to send the packets to PE1 is the MPLS LSP stack
   to get to PE1 and the EVPN label advertised by PE1 for CE1's MAC.

and

   The MPLS label stack to send packets to PE2 is the MPLS LSP stack to
   get to PE2 and the MPLS label in the Ethernet A-D route advertised by
   PE2 for <ES1, VLAN1>, if PE2 has not advertised MAC1 in BGP.

It *should* be perfectly obvious to the implementer, but perhaps you
should say what order the labels appear on the stack since "and" is non-
specific.

---

Section 18

I wish you would add a reference to 4385 and use that control word with
the various fields set to zero. This would keep us from increasing the
number of different control word definitions in the wild. I think that 
the impact on your spec would be zero.

---

Section 21 should be renamed "Contributors"

---

I think RFC 2119 is a normative reference.

---

From: "Ali Sajassi (sajassi)" <[email-masked]>
Date: Mon, 21 Jul 2014 04:55:35 +0000

Adrian,

Thanks very much for your review. I will incorporate your comments into
the next rev. For more details, please refer inline ...

Regards,
Ali

>It would help if you said "...in line format with the most significant

Done.

>"In general" is not really consistent with "MUST"

Will change "MUST" to "SHOULD"

>Do you want an IANA registry to track the values of the Type field of

We don't anticipate any other ESI type besides the ones mentioned here.

>the impression that you mean something different by the two words.

Will make it consistent.

>Could you expand DF on first use. You have it in 8.3.

Will do.

>usage in the rest of the document.

Will do.

>detailed explanation, but a guide to the implementer/operator.

The sentence before this says:
"If the VLAN is represented
   by different VIDs on different PEs, then each PE needs to perform VID
   translation for frames destined to its attached CEs."

I thought this description is clear enough but I will try to make it more
clear.

>for the EVPN NLRI in Section 7?

We'll look into it.

>please expand RD it on first use rather than leaving that to 8.2.1.

Sure, we'll do.

>label, you can (of course) use normal case.

Agreed.

>need IANA intervention here?

We have registered these values with IANA. We will reflect that in IANA
section.

>It would be nice if 7.5 included a hint as to what an "ESI label" is.

Agreed.

>what is the reason for SHOULD? is there a MAY to counter it?)

I'll add RFC4684 reference.

>Would that be the ESII? :-)

Nice catch :-)

>point "OPTIONAL". Please use "REQUIRED."

Agreed.

>want to say in which environments this would be a damn fool idea?

We'll do :-)

>should say what order the labels appear on the stack since "and" is non-

OK.

>the impact on your spec would be zero.

We'll do.

>Section 21 should be renamed "Contributors"

We'll do.

>I think RFC 2119 is a normative reference.

OK.