Re: [RTG-DIR] Rtgdir last call review of draft-ietf-lisp-gpe-04

[Fabio Maino <[e-mail]>]

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:[e-mail]]
>> Sent: 15 August 2018 19:15
>> To: Adrian Farrel; [e-mail]
>> Cc: [e-mail]; [e-mail]; [e-mail]
>> 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
>>>