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
