Re: Rtgdir last call review of draft-ietf-lisp-gpe-04

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

 



Hi Adrian,
here is the updated draft that should address all your comments: https://tools.ietf.org/html/draft-ietf-lisp-gpe-05.

Rfcdiff pointer: https://goo.gl/bMbRvC

Please let me know if you have any other suggestion.

Thanks again,
Fabio

On 8/15/18 11:30 AM, Adrian Farrel wrote:
Hi Fabio,

That additional text is helpful, thanks.

Adrian

-----Original Message-----
From: Fabio Maino [mailto:fmaino@xxxxxxxxx]
Sent: 15 August 2018 19:15
To: Adrian Farrel; rtg-dir@xxxxxxxx
Cc: lisp@xxxxxxxx; ietf@xxxxxxxx; draft-ietf-lisp-gpe.all@xxxxxxxx
Subject: Re: Rtgdir last call review of draft-ietf-lisp-gpe-04

Hi Adrian,
thanks for such a detailed review.

I went through your comments and I can incorporate all of them into a
new version of the draft.

Wrt the reduction in size of the Map-Versioning and Nonce fields, I
could add in Section 3, right after the definition of the encoding of
those fields, the following:

The encoding of the Nonce field in LISP-GPE, compared with the one
used in RFC6830bis for the LISP data plane encapsulation, reduces the
length of the nonce from 24 to 16 bits. As per RFC6830bis, ITRs are
required to generate different nonces when sending to different RLOCs,
but the same nonce can be used for a period of time when encapsulating
to the same ETR. The use of 16 bits nonces still allows  an ITR to
determine to and from reachability for up to 64k RLOCs at the same time.

Similarly, the encoding of the Source and Dest Map-Version fields,
compared with RFC6830bis, is reduced from 12 to 8 bits. This still
allows to associate 256 different versions to each EID-to-RLOC mapping
to inform commmunicating ITRs and ETRs about modifications of the
mapping.


Either Deborah, Joel, or Luigi: if you could please confirm that it is
ok to publish a new version of the draft at this point, I'll update it
right away.

Thanks,
Fabio




On 8/9/18 9:05 AM, Adrian Farrel wrote:
Reviewer: Adrian Farrel
Review result: Has Issues

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The
Routing Directorate seeks to review all routing or routing-related drafts as
they pass through IETF last call and IESG review, and sometimes on special
request. The purpose of the review is to provide assistance to the Routing ADs.
For more information about the Routing Directorate, please see
?http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs, it would
be helpful if you could consider them as normal review comments. I believe
that
this review comes between WG publication and the start of IETF last call - you
may wish to discuss with your AD whether to treat these comments separately
or
as part of IETF last call.

Document: draft-ietf-lisp-gpe-04.txt
   Reviewer: Adrian Farrel
   Review Date: 9-August-2018
   IETF LC End Date: No known
   Intended Status: Standards Track

Summary
I have significant concerns about this document and recommend that the
Routing
ADs discuss these issues further with the authors. The issues are not
substantially technical in nature, but do indicate the need for significant
reworking of the text. I have tried to make suggestions for new text.

Comments:

This document specifies an alternate LISP header format that can be used to
allow LISP to carry payloads other than IP. A new capabilities flag is defined
so that routers know whether this new format is supported, and a new flag in
the header itself indicates when the new format is in use.

The document is clear and readable, but has some issues of presentation that
could close a few potential misunderstandings and thus improve implmentation
prospects.

No attempt is made in the document to explain how/why the reduction in size
of
some standard LISP header fields is acceptable. For example, if
implementations
of this spec can safely operate with a 16 bit Nonce or 8 bit Map-Versions, why
does 6830/6830bis feel the need for 24 and 12 bit fields rspectively?

===Major Issues===

Section 3 has a mix of minor and leess minor issues...

OLD
     This document defines the following changes to the LISP header in
     order to support multi-protocol encapsulation:

     P Bit:  Flag bit 5 is defined as the Next Protocol bit.  The P bit
        MUST be set to 1 to indicate the presence of the 8 bit next
        protocol field.

        P = 0 indicates that the payload MUST conform to LISP as defined
        in [I-D.ietf-lisp-rfc6830bis].  Flag bit 5 was chosen as the P bit
        because this flag bit is currently unallocated.

     Next Protocol:  The lower 8 bits of the first 32-bit word are used to
        carry a Next Protocol.  This Next Protocol field contains the
        protocol of the encapsulated payload packet.

        LISP uses the lower 24 bits of the first word for either a nonce,
        an echo-nonce, or to support map-versioning
        [I-D.ietf-lisp-6834bis].  These are all optional capabilities that
        are indicated in the LISP header by setting the N, E, and the V
        bit respectively.

        When the P-bit and the N-bit are set to 1, the Nonce field is the
        middle 16 bits.

        When the P-bit and the V-bit are set to 1, the Version field is
        the middle 16 bits.

        When the P-bit is set to 1 and the N-bit and the V-bit are both 0,
        the middle 16-bits are set to 0.

        This document defines the following Next Protocol values:

        0x1 :  IPv4

        0x2 :  IPv6

        0x3 :  Ethernet

        0x4 :  Network Service Header [RFC8300]

          0                   1                   2                   3
          0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
         |N|L|E|V|I|P|K|K|        Nonce/Map-Version      | Next Protocol |
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
         |                 Instance ID/Locator-Status-Bits               |
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                                LISP-GPE Header

NOTES
     - It would be helpful to put the figure higher up
     - The use of "MUST" for the P-bit is attenuated wrongly
     - Need to be consistent on "P Bit" or "P-bit" or "P bit"
     - There looks to be a problem in the case of map version. The base
       spec has 12 bits each for source and dest map-version, so this doc
       needs to describe how the reeduced 16 bits is split (presumably not
       12 and 4).
     - You need a pointer to the IANA registry for next protocol
NEW
     This document defines two changes to the LISP header in order to
     support multi-protocol encapsulation: the introduction of the P-bit
     and the definition of a Next Protocol field.  This is shown in
     Figure 1 and described below.

          0                   1                   2                   3
          0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
         |N|L|E|V|I|P|K|K|        Nonce/Map-Version      | Next Protocol |
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
         |                 Instance ID/Locator-Status-Bits               |
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                           Figure 1 : The LISP-GPE Header

     P-Bit:  Flag bit 5 is defined as the Next Protocol bit.

        If the P-bit is clear (0) the LISP header conforms to the
        definition in [I-D.ietf-lisp-rfc6830bis].

        The P-bit is set to 1 to indicate the presence of the 8 bit Next
        Protocol field.

     Next Protocol:  The lower 8 bits of the first 32-bit word are used to
        carry a Next Protocol.  This Next Protocol field contains the
        protocol of the encapsulated payload packet.

        In [I-D.ietf-lisp-6834bis], LISP uses the lower 24 bits of the
        first word for a nonce, an echo-nonce, or to support map-
        versioning.  These are all optional capabilities that are
        indicated in the LISP header by setting the N, E, and V bits
        respectively.

        When the P-bit and the N-bit are set to 1, the Nonce field is the
        middle 16 bits (i.e., encoded in 16 bits, not 24 bits).  Note that
        the E-bit only has meaning when the N-bit is set.

        When the P-bit and the V-bit are set to 1, the Version fields use
        the middle 16 bits: the Source Map-Version uses the high-order 8
        bits, and the Dest Map-Version uses the low-order 8 bits.

        When the P-bit is set to 1 and the N-bit and the V-bit are both 0,
        the middle 16-bits MUST be set to 0 on transmission and ignored on
        receipt.

        This document defines the following Next Protocol values:

        0x1 :  IPv4

        0x2 :  IPv6

        0x3 :  Ethernet

        0x4 :  Network Service Header [RFC8300]

        The values are tracked in an IANA registry as described in Section
        5.

---

Section 4 must describe the error case when a LISP-GPE capable router
sets the P-bit on a packet to a non LISP-GPE capable router. So...

OLD
     When encapsulating IP packets to a non LISP-GPE capable router the P
     bit MUST be set to 0.
NEW
     When encapsulating IP packets to a non LISP-GPE capable router the P-
     bit MUST be set to 0.  That is, the encapsulation format defined in
     this document MUST NOT be sent to a router that has not indicated
     that it supports this specification because such a router would
     ignore the P-bit (as described in [I-D.ietf-lisp-rfc6830bis]) and so
     would misinterpret the other LISP header fields possibly causing
     significant errors.
END

---

4.1

Not your fault that RFC 8060 doesn't have a registry for bits in the
LCAF, but now you really need one or else future orthogonal specs risk
colliding with the g-bit.  A bit odd to add this in this document, but
not worth a bis on 8060.

===Minor Issues ===

Section 2

OLD
     The LISP header [I-D.ietf-lisp-rfc6830bis] contains a series of flags
     (some defined, some reserved), a Nonce/Map-version field and an
     instance ID/Locator-status-bit field.  The flags provide flexibility
     to define how the various fields are encoded.  Notably, Flag bit 5 is
     the last reserved bit in the LISP header.

          0                   1                   2                   3
          0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
         |N|L|E|V|I|R|K|K|            Nonce/Map-Version                  |
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
         |                 Instance ID/Locator-Status-Bits               |
         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                                  LISP Header
NOTES
     We need to be careful not to risk any confusion. At least, "some
     reserved" is an over-statement. But also we should not show a repeat
     of the Lisp header as that causes a duplicate definition.
NEW
     The LISP header is defined in [I-D.ietf-lisp-rfc6830bis] and contains
     a series of flags of which one (bit 5) is shown in that document as
     "reserved for future use".  The setting of the flag fields defined
     how the subsequent header fields are interpretted.
END

---

4.1
I don't think you should reproduce the Multiple Data-Planes LCAF Type
figue from 8060 here as it creates a duplicate definition.  The text
explanation of which bit is the g-bit shold be enough.

===Nits===

Abstract
OLD
     This document describes extending the Locator/ID Separation Protocol
     (LISP) Data-Plane, via changes to the LISP header, to support multi-
     protocol encapsulation.
NEW
     This document describes extentions to the Locator/ID Separation
     Protocol (LISP) Data-Plane, via changes to the LISP header, to
     support multi-protocol encapsulation.
END

---

1.
OLD
     LISP Data-Plane, as defined in in [I-D.ietf-lisp-rfc6830bis], defines
     an encapsulation format that carries IPv4 or IPv6 (henceforth
     referred to as IP) packets in a LISP header and outer UDP/IP
     transport.
NEW
     The LISP Data-Plane is defined in [I-D.ietf-lisp-rfc6830bis].  It
     specifies an encapsulation format that carries IPv4 or IPv6 packets
     (henceforth jointly referred to as IP) in a LISP header and outer
     UDP/IP transport.

---

1.1
Please use the new boilerplate...
     The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
     "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
"MAY", and
     "OPTIONAL" in this document are to be interpreted as described in BCP
     14 [RFC2119] [RFC8174] when, and only when, they appear in all
     capitals, as shown here.

---

1.2
Nothwithstanding the text in this section, abbreviations need to be
expanded either on first use or in this section.
I see:
- LCAF
- ETR
- ITR
- RLOC
- xTR

---

2.
s/As described in the introduction/As described in Section 1/
s/LISP is limited to carry IP payloads/LISP is limited to carrying IP payloads/

---

4.1
s/field as g bit/field as the g-bit/

---

8.1
Please add RFC 8174





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

  Powered by Linux