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