Last Call Review of draft-ietf-bess-evpn-prefix-advertisement-10
review-ietf-bess-evpn-prefix-advertisement-10-secdir-lc-leiba-2018-05-04-00

The "issues" I call out below are minor, and if the working group thinks they
aren't worth dealing with, I'll not be offended nor lose any sleep.

— Section 1 —
I’m sure that all these terms are defined in the normative references, and ’tis
a small thing, but it would sure help a non-expert reader if this list of terms
included, for each term, a citation to the RFC that defines it.  I hope you’ll
consider adding that; thanks.

[Follow-up; I finally found “Tenant System” defined in RFC 7365, which is not
in your references at all.  Please don’t make your readers work that hard, and
please consider beefing up the references and citations to definitions.]

— Section 2.1 —

   If the term Tenant System (TS) is used to designate a physical or
   virtual system identified by MAC and maybe IP addresses, and
   connected to a BD by an Attachment Circuit, the following
   considerations apply:

I find the wording “if the term Tenant System is used” to be odd.  Are you
really saying (maybe you are) that the application of the considerations
depends on whether or not we *call* it a Tenant System?  Or whether or not it
*is* a Tenant System?  From the definition I found for “Tenant System” I can
see that maybe this can go either way.  But if we’re talking about the latter,
I’d use wording more like, “The following considerations apply to Tenant
Systems (TS) that are physical or virtual systems identified by MAC and maybe
IP addresses and connected to BDs by Attachment Circuits:” (cast as plural,
because the considerations use plurals).


— Section 3.1 —

I initially couldn’t figure out, as I was reading this, how you’d know whether
you’re dealing with v4 or v6 addresses, and, therefore, how to interpret the
lengths of the IP Prefix and GW IP Address fields.  I finally got to it seven
bullets down, where you say, “The total route length will indicate the type of
prefix”.    Maybe someone already expert in this would find this OK, but to me
it was too much work to sort it out, when I think it could be made clearer like
this:

NEW
   An IP Prefix Route Type for IPv4 has the Length field set to 34
   and consists of the following fields:

    +---------------------------------------+
    |      RD   (8 octets)                  |
    +---------------------------------------+
    |Ethernet Segment Identifier (10 octets)|
    +---------------------------------------+
    |  Ethernet Tag ID (4 octets)           |
    +---------------------------------------+
    |  IP Prefix Length (1 octet, 0 to 32)  |
    +---------------------------------------+
    |  IP Prefix (4 octets)                 |
    +---------------------------------------+
    |  GW IP Address (4 octets)             |
    +---------------------------------------+
    |  MPLS Label (3 octets)                |
    +---------------------------------------+

   An IP Prefix Route Type for IPv6 has the Length field set to 58
   and consists of the following fields:

    +---------------------------------------+
    |      RD   (8 octets)                  |
    +---------------------------------------+
    |Ethernet Segment Identifier (10 octets)|
    +---------------------------------------+
    |  Ethernet Tag ID (4 octets)           |
    +---------------------------------------+
    |  IP Prefix Length (1 octet, 0 to 128) |
    +---------------------------------------+
    |  IP Prefix (16 octets)                |
    +---------------------------------------+
    |  GW IP Address (16 octets)            |
    +---------------------------------------+
    |  MPLS Label (3 octets)                |
    +---------------------------------------+

   The total route length will indicate the type of IP Prefix (34 for
   IPv4 or 58 for IPv6) and the type of GW IP Address. The IP Prefix
   and GW IP Address are always both IPv4 or both IPv6; mixing the
   two is not allowed.

   […and then follow with the explanations of the fields…]
END

Do you agree that that makes things clearer?

— Section 3.2 —

   o If either the ESI or GW IP are non-zero, then one of them is the
     Overlay Index, regardless of whether the Router's MAC Extended
     Community is present or the value of the Label.

Should that say “then the non-zero one is the Overlay Index”?