Subject: [lisp] Rtgdir last call review of draft-ietf-lisp-gpe-04

From: Adrian Farrel <[email-masked]>
Date: Thu, 09 Aug 2018 09:05:55 -0700

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

---

From: Dino Farinacci <[email-masked]>
Date: Thu, 9 Aug 2018 09:15:40 -0700

> does 6830/6830bis feel the need for 24 and 12 bit fields rspectively?

Note, you misread RFC6830. The Map-Version field is 24-bits when the V bit is set. And is divided up like this:

    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |N|L|E|V|I|flags|  Source Map-Version   |   Dest Map-Version    |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |                 Instance ID/Locator-Status-Bits               |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Dino

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 9 Aug 2018 17:39:01 +0100

Hello Dino,

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

I don't think I missed that even slightly.
The Map-Version field is 24 bits and split into two 12 bit Map-Versions (source
and dest).
When the P-bit is set, the Map-Version is reduced to 16 bits and is (presumably)
split at 8 bits each for source and dest (see my new text).

So my question could be seen as:
Why does this document only need 8 bits for Source Map-Version when 6830 needed
12 bits?

Cheers,
Adrian

---

From: Dino Farinacci <[email-masked]>
Date: Thu, 9 Aug 2018 09:49:33 -0700

I missed “fields” versus “field”. Sorry.

Your question still stands. I do believe shortening the nonce IS NOT a good think. But I believe if the version needs to change, rollover is not an issue. The fact that it is different to the receiver is what is important.

In RFC6830, we used the entire field because it was available and we didn’t want to store both version and nonce at the same time when V and N bits were set. But if one favors a smaller nonce, we could use the
idea in GPE to the same field for both purposes. But that is yet another variant.

The Map-Versioning has been underutilized and it is a shame. Because it is a lighter weight than using SMRs. In fact, this email thread is motivating me to go off and implement Map-Versioning and is particularly useful for fast moving LISP mobile-nodes.

I am a bit off topic, but this is a decent discussion to have. Here are the tradeoffs between map-versioning and SMRs:

(1) SMRs send a Map-Request that has the new RLOC-set info. The receiver can use the information on blind faith or verify its integrity by doing a mapping system lookup. That is how it is spec’ed in RFC6830. Unverified version takes 1/2 RTT.

(2) Map-Versioning always requires 3/2s RTT but the trigger/signal comes in a data-packet. The receiver only knows there is a change but doesn’t have the new RLOC-set info, so it must do a mapping system lookup. That means Map-Versioning, by nature, is required to verification.

Now with have draft-ietf-lisp-pubsub come to fruition, there is less need for (1) but believe (2) is very useful because it doesn’t require the state in the map-server for subscribers. That is, good job Luigi/Damien for the Map-Versioning idea!

Dino

rspectively?

Note, you misread RFC6830. The Map-Version field is 24-bits when the V bit is
set. And is divided up like this:

Cheers,
Adrian

---

From: Fabio Maino <[email-masked]>
Date: Wed, 15 Aug 2018 11:15:17 -0700

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

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 15 Aug 2018 19:30:18 +0100

Hi Fabio,

That additional text is helpful, thanks.

Adrian

> -----Original Message-----
> From: Fabio Maino [mailto:[email-masked]]
> Sent: 15 August 2018 19:15
> To: Adrian Farrel; [email-masked]
> Cc: [email-masked]; [email-masked]; [email-masked]
> Subject: Re: Rtgdir last call review of draft-ietf-lisp-gpe-04
> 
> Hi Adrian,
> of

---

From: "BRUNGARD, DEBORAH A" <[email-masked]>
Date: Wed, 15 Aug 2018 18:24:53 +0000

Hi Fabio,

Yes, please update-
Deborah

-----Original Message-----
From: Fabio Maino <[email-masked]> 
Sent: Wednesday, August 15, 2018 2:15 PM
To: Adrian Farrel <[email-masked]>; [email-masked]
Cc: [email-masked]; [email-masked]; [email-masked]
Subject: Re: Rtgdir last call review of draft-ietf-lisp-gpe-04

Hi Adrian,

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

> to inform commmunicating ITRs and ETRs about modifications of the 

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:
> 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.
> ?https://urldefense.proofpoint.com/v2/url?u=http-3A__trac.tools.ietf.o
> rg_area_rtg_trac_wiki_RtgDir&d=DwIDaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=6UhGp
> W9lwi9dM7jYlxXD8w&m=PkWde6zu-kE9gzGQsc-9pYLwdcRrjIa2425Q4AJA-fg&s=_8An
> SBg0ZBNU-n2uz9kLjT6VwSTXHys-h_54ayvz9PA&e=
>
> 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.
>
> 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===
>
>     - You need a pointer to the IANA registry for next protocol NEW
>                                  LISP Header NOTES
> 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

---

From: Fabio Maino <[email-masked]>
Date: Wed, 15 Aug 2018 22:29:04 -0700

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:
>> Routing