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

Request Review of draft-ietf-bess-evpn-prefix-advertisement
Requested rev. no specific revision (document currently at 11)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2018-04-10
Requested 2018-03-27
Authors Jorge Rabadan, Wim Henderickx, John Drake, Wen Lin, Ali Sajassi
Draft last updated 2018-05-04
Completed reviews Rtgdir Telechat review of -10 by Geoff Huston (diff)
Secdir Last Call review of -10 by Barry Leiba (diff)
Genart Last Call review of -10 by Elwyn Davies (diff)
Assignment Reviewer Barry Leiba
State Completed
Review review-ietf-bess-evpn-prefix-advertisement-10-secdir-lc-leiba-2018-05-04
Reviewed rev. 10 (document currently at 11)
Review result Has Issues
Review completed: 2018-05-04

Review
review-ietf-bess-evpn-prefix-advertisement-10-secdir-lc-leiba-2018-05-04

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”?