Re: Secdir last call review of draft-ietf-bess-evpn-prefix-advertisement-10

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



All good, and thanks, Jorge, for taking the time to make the changes.

Barry

On Fri, May 18, 2018 at 8:30 PM Rabadan, Jorge (Nokia - US/Mountain View) <jorge.rabadan@xxxxxxxxx> wrote:
Hi Barry,

Thank you very much for reviewing.
I addressed all your comments, see below.
Thanks a bunch!
Jorge


-----Original Message-----
From: Barry Leiba <barryleiba@xxxxxxxxxxxx>
Date: Friday, May 4, 2018 at 3:51 PM
To: "secdir@xxxxxxxx" <secdir@xxxxxxxx>
Cc: "draft-ietf-bess-evpn-prefix-advertisement.all@xxxxxxxx" <draft-ietf-bess-evpn-prefix-advertisement.all@xxxxxxxx>, "ietf@xxxxxxxx" <ietf@xxxxxxxx>, "bess@xxxxxxxx" <bess@xxxxxxxx>
Subject: Secdir last call review of draft-ietf-bess-evpn-prefix-advertisement-10
Resent-From: <alias-bounces@xxxxxxxx>
Resent-To: <jorge.rabadan@xxxxxxxxx>, <wim.henderickx@xxxxxxxxx>, <jdrake@xxxxxxxxxxx>, <wlin@xxxxxxxxxxx>, <sajassi@xxxxxxxxx>, <matthew.bocci@xxxxxxxxx>, <stephane.litkowski@xxxxxxxxxx>, <martin.vigoureux@xxxxxxxxx>, <db3546@xxxxxxx>, <aretana.ietf@xxxxxxxxx>, Zhaohui Zhang <zzhang@xxxxxxxxxxx>, <zzhang@xxxxxxxxxxx>
Resent-Date: Friday, May 4, 2018 at 3:51 PM

    Reviewer: Barry Leiba
    Review result: Has Issues

    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.
[JORGE] I added a few references. Hope it's better now.

    [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.]
[JORGE] added now.

    — 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).
[JORGE] I took your suggestion, thx

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

[JORGE] ok, done

    — 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”?
[JORGE] ok, good point, done




--
Barry
--
Barry Leiba  (barryleiba@xxxxxxxxxxxx)
http://internetmessagingtechnology.org/

[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux