Subject: [lisp] AD Review of draft-ietf-lisp-6834bis-09

From: Alvaro Retana <[email-masked]>
Date: Wed, 13 Apr 2022 11:54:40 -0700

Dear authors/WG:

While I appreciate the small number of changes from rfc6834 and the
original intent (from 2018) of processing this document quickly, but
it requires a non-trivial amount of work.  It is now going for
Proposed Standard, so we need to be more rigorous.

I put detailed comments inline (below).  There are two main themes
that I want to highlight here:

(1) Duplication and overlap

   Several parts of this document describe the same or related behaviors.
   This issue has resulted in duplicate descriptions and specifications, not
   all of them saying the same thing. :-(  The problem extends beyond this
   document into both rfc6830bis and rfc6833bis.  I pointed and cross-
   referenced the sections as best as I could below.

   In general, please specify the behavior only once.  Doing so in multiple
   places can be confusing and, as in this case, result in the specifications
   not being in sync.

   Note that to address this point we may need to update rfc6830bis and
   rfc6833bis.  I would prefer to see as much of the specification related to
   Map-Versioning as possible (all?) in this document.  Removing pieces and
   referencing this document shouldn't require removing them from the RFC
   Editor's queue -- and they both Normatively depend on this document anyway.

   One specific action is to remove the text from §13.2/rfc6830bis.  Some of
   it is already covered in this document -- the remaining pieces, which I
   believe are only the last couple of paragraphs, can find a better home.

(2) Document Structure

   As I started reading this document, I found myself going back to rfc6830bis
   to look for packet formats -- and I wasn't even past the Introduction!  I
   then realized that some of the information was in this document but an
   unexpected order.  Some actionable comments:

   (1) Simplify the Introduction; there's too much detail.  For example, it
   mentions the location of the Map-Version in excruciating detail ("the 24
   low-order bits of the first longword of the LISP header"), but the
   description is wrong (see more inline).  There are explanations about the
   topics later on but no forward references.  Instead, paint a high-level
   picture and tell the reader where to find the details.

   (2) Beyond the Introduction, the document is ordered in a cumbersome way:
   The location and format of the Map-Version are presented in §6, but both
   §4/§5 describe how the versions are handled.  It would be much better for
   the reader if §6 were moved forward.  This is my suggested ordering:

   6.  LISP Header and Map-Version Numbers
   7.  Map Record and Map-Version (this could be a subsection)
   4.  EID-to-RLOC Map-Version Number
   5.  Dealing with Map-Version Numbers

   Also, I suggest moving §8 to an Appendix.

As we have a constrained timeline for this document, I would like to
start the IETF Last-Call by the end of this month.  I have a lot of
comments, but I don't think they will be hard to address.

Please reply inline to this message to expedite my review of any
updates.  Also, if you think talking would clear things up faster,
feel free to find time on my calendar:
https://doodle.com/mm/alvaroretana/book-a-time

Padma:  As Shepherd, I'll need you to please update the writeup when
the final version is ready.

Thanks!!

Alvaro.

[Line numbers from idnits.]

...
13	Abstract

15	   This document describes the LISP (Locator/ID Separation Protocol)
16	   Map-Versioning mechanism, which provides in-packet information about
17	   Endpoint ID to Routing Locator (EID-to-RLOC) mappings used to
18	   encapsulate LISP data packets.  The proposed approach is based on
19	   associating a version number to EID-to-RLOC mappings and the
20	   transport of such a version number in the LISP-specific header of
21	   LISP-encapsulated packets.  LISP Map-Versioning is particularly
22	   useful to inform communicating Ingress Tunnel Routers (ITRs) and
23	   Egress Tunnel Routers (ETRs) about modifications of the mappings used
24	   to encapsulate packets.  The mechanism is optional and transparent to
25	   implementations not supporting this feature, since in the LISP-
26	   specific header and in the Map Records, bits used for Map-Versioning
27	   can be safely ignored by ITRs and ETRs that do not support or do not
28	   want to use the mechanism.

[minor] s/proposed//g
This mechanism is now on the standards track, so it is not "proposed"
any more. :-)

...
93	1.  Introduction

[] As I mentioned above, please simplify this section.  I left
comments below, but I think that most of the text can be changed for a
high-level description.

...
113	   When Map-Versioning is used, LISP-encapsulated data packets contain
114	   the version number of the two mappings used to select the RLOCs in
115	   the outer header (i.e., both source and destination RLOCs).  These
116	   version numbers are encoded in the 24 low-order bits of the first
117	   longword of the LISP header and indicated by a specific bit in the
118	   flags (first 8 high-order bits of the first longword of the LISP
119	   header).  Note that not all packets need to carry version numbers.

[major] "version numbers are encoded in the 24 low-order bits of the
first longword of the LISP header"

Wow!  That's not an easy reference to follow!

I went to find the "LISP Header" and found this definition in rfc6830bis:

   LISP Header:   LISP header is a term used in this document to refer
      to the outer IPv4 or IPv6 header, a UDP header, and a LISP-
      specific 8-octet header that follow the UDP header and that an ITR
      prepends or an ETR strips.

If the LISP Header includes the "outer IPv4 or IPv6 header, a UDP
header, and a LISP-specific 8-octet header" then the "24 low-order
bits of the first longword" maps to something different. :-(

Instead, it would be easier for everyone to simple show the header
with the V-bit set (from rfc6830bis):

     0 x 0 1 x x x x
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |N|L|E|V|I|R|K|K|  Source Map-Version   |   Dest Map-Version    |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |                 Instance ID/Locator-Status-Bits               |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

[major] "indicated by a specific bit in the flags (first 8 high-order
bits of the first longword of the LISP header)."

Please replace that with a specific mention to the V-bit.

121	   When an ITR (Ingress Tunnel Router) encapsulates a data packet, with
122	   a LISP header containing the Map-Version numbers, it puts in the
123	   LISP-specific header two version numbers:

125	   1.  The version number assigned to the mapping used to select the
126	       source RLOC (contained in the EID-to-RLOC Database).

128	   2.  The version number assigned to the mapping used to select the
129	       destination RLOC (contained in the EID-to-RLOC Map-Cache).

[major] Please use the names in the packet formats: indicate which is
the "Source Map-Version" and which the "Dest Map-Version".

...
154	3.  Definitions of Terms

156	   This document uses terms already defined in the main LISP
157	   specification ([I-D.ietf-lisp-rfc6830bis],
158	   [I-D.ietf-lisp-rfc6833bis]).  Here, we define the terms that are
159	   specific to the Map-Versioning mechanism.  Throughout the whole
160	   document, Big Endian bit ordering is used.

[major] Please avoid redefining terms that are already defined
elsewhere.  Changes in either (even if editorial) may cause the
documents to fall out of sync.  If you want to call attention to
specific terms, please point at the original definition or a specific
section.

For example:

   Map-Version Number:  as defined in §7.

162	   Map-Version number:  An unsigned 12-bit integer is assigned to an
163	     EID-to-RLOC mapping, excluding the value 0 (0x000).

[major] Beyond being a 12-bit integer, this definition doesn't say
what the Map-Version number is.  §7 contains a much better
description.

165	   Null Map-Version:  The 12-bit null value of 0 (0x000) is not used as
166	     a Map-Version number.  It is used to signal that no Map-Version
167	     number is assigned to the EID-to-RLOC mapping.

[] Suggestion>
   Null Map-Version:  A Map-Version number with a value of 0x000 (zero),
      used to signal that no Map-Version number is assigned to the
      EID-to-RLOC mapping (Section 4.1).

169	   Source Map-Version number:  This Map-Version number of the EID-to-
170	     RLOC mapping is used to select the source address (RLOC) of the
171	     outer IP header of LISP-encapsulated packets.

[major] This definition describes what the Source Map-Version number
is used for, but not what it is.  Also, the definition in §6 is
different from the one here -- it is close, but not the same.  Please
define terminology in only one place.  It seems to me that leaving
this definition up to §6 would be best.

173	   Destination Map-Version number:  This Map-Version number of the EID-
174	     to-RLOC mapping is used to select the destination address (RLOC) of
175	     the outer IP header of LISP-encapsulated packets.

[major] The packet format in rfc6830bis uses "Dest Map-Version"
instead of "Destination Map-Version".  Please use the same terminology
in the packet format and throughout the document.

[major] Same comment as above: it describes what it's used for, not
what it is.  There's a different definition in §6.

177	4.  EID-to-RLOC Map-Version Number

179	   The EID-to-RLOC Map-Version number consists of an unsigned 12-bit
180	   integer.  The version number is assigned on a per-mapping basis,
181	   meaning that different mappings have a different version number,
182	   which is also updated independently.  An update in the version number
183	   (i.e., a newer version) consists of incrementing by one the older
184	   version number.

[major] "incrementing by one"

There's an exception in the case where V1+1 would result in 0, right?
Please mention it.

I later found that the last paragraph in §4.1 explains this exception.
Please point to if from here, or move the paragraph to this section.
Note that the text in §4.1 normatively requires (MUST) the increment,
but the description here doesn't use normative language.  Please be
consistent.

...
212	   Map-version numbers are assigned to mappings by configuration.  The
213	   initial Map-Version number of a new EID-to-RLOC mapping SHOULD be
214	   assigned randomly, but it MUST NOT be set to the Null Map-Version
215	   value (0x000), because the Null Map-Version number has a special
216	   meaning (see Section 4.1).

[major] "Map-version numbers are assigned to mappings by
configuration.  The initial Map-Version number of a new EID-to-RLOC
mapping SHOULD be assigned randomly..."

This sounds like a contradiction: configure, but make it random.  I
assume that you mean that manual configuration may be the exception.

Suggestion>
   The initial Map-Version number of a new EID-to-RLOC mapping SHOULD be
   assigned randomly; it MUST NOT be set to the Null Map-Version value
   (0x000) (see Section 4.1).  Optionally, the initial Map-Version number
   may be configured.

...
224	4.1.  The Null Map-Version

[] The comments below indicate that there are multiple places that
overlap with this section.  The text from this section may be
relocated (and clarifying actions taken) to make the document clearer.
This section could then be eliminated.

   Paragraph 1 could be moved to §6.

   Paragraph 2 could be moved to §5/§5.1/§5.2.

   Paragraph 3 could be moved to §7.

   Paragraph 4 could be moved to §4.

226	   The value 0x000 (zero) is not a valid Map-Version number indicating
227	   the version of the EID-to-RLOC mapping.  Such a value is used for
228	   special purposes and is named the Null Map-Version number.

230	   The Null Map-Version MAY appear in the LISP-specific header as either
231	   a Source Map-Version number (cf.  Section 5.2) or a Destination Map-
232	   Version number (cf.  Section 5.1).  When the Source Map-Version
233	   number is set to the Null Map-Version value, it means that no map
234	   version information is conveyed for the source site.  This means that
235	   if a mapping exists for the source EID in the EID-to-RLOC Map-Cache,
236	   then the ETR MUST NOT compare the received Null Map-Version with the
237	   content of the EID-to-RLOC Map-Cache.  When the Destination Map-
238	   Version number is set to the Null Map-Version value, it means that no
239	   map version information is conveyed for the destination site.  This
240	   means that the ETR MUST NOT compare the value with the Map-Version
241	   number of the mapping for the destination EID present in the EID-to-
242	   RLOC Database.

[major] "Null Map-Version MAY appear"

s/MAY/may

This is a statement of fact, not a Normative statement.

[nit] s/cf.  //g

[minor] Please put a forward reference to §5.1/§5.2 where "MUST NOT
compare" is used above.  §5.1/§5.2 don't use "compare" as part of
their specification, but pointing to them should keep us from changing
the language here.

244	   The other use of the Null Map-Version number is in the Map Records,
245	   which are part of the Map-Request, Map-Reply, and Map-Register
246	   messages (defined in [I-D.ietf-lisp-rfc6833bis]).  Map Records that
247	   have a Null Map-Version number indicate that there is no Map-Version
248	   number associated with the mapping.  This means that LISP-
249	   encapsulated packets destined to the EID-Prefix referred to by the
250	   Map Record MUST either not contain any Map-Version numbers (V bit set
251	   to 0) or, if they contain Map-Version numbers (V bit set to 1), then
252	   the destination Map-Version number MUST be set to the Null Map-
253	   Version number.  Any value different from zero means that Map-
254	   Versioning is supported and MAY be used.

[] See comments in §7 about the requirements in this paragraph.

[nit] s/(defined in [I-D.ietf-lisp-rfc6833bis])/[I-D.ietf-lisp-rfc6833bis]/g

[major] "Any value different from zero means that Map-Versioning is
supported and MAY be used."

256	   The fact that the 0 value has a special meaning for the Map-Version
257	   number implies that, when updating a Map-Version number because of a
258	   change in the mapping, if the next value is 0, then the Map-Version
259	   number MUST be incremented by 2 (i.e., set to 1 (0x001), which is the
260	   next valid value).

[] See my comment in §4 about this exception.

262	5.  Dealing with Map-Version Numbers
...
272	   To each mapping, a version number is associated and changes each time
273	   the mapping is changed.  Note that Map-Versioning does not introduce
274	   new problems concerning the coordination of different ETRs of a
275	   domain.  Indeed, ETRs belonging to the same LISP site must return for
276	   a specific EID-prefix the same mapping, including the same Map-
277	   Version number.  This is orthogonal to whether or not Map-Versioning
278	   is used.  The synchronization problem and its implications on the
279	   traffic are out of the scope of this document.

[] See my comment in §7 about leaving this out of scope and the
requirement from rfc6833bis.

...
304	   If one or both of the above conditions do not hold, the ETR can send
305	   a Map-Request either to make the ITR aware that a new mapping is
306	   available (see Section 5.1) or to update the mapping in the local
307	   EID-to-RLOC Map-Cache (see Section 5.2).

[major] Should this behavior be Normative (MUST/SHOULD)?  The text
currently says that "the ETR can send"; if the text doesn't require
(or at least recommend) an action then it looks like this document
just talks about carrying extra numbers around that don't serve a
specific purpose.  I believe that §10 makes a good case for an action
to be specified.

309	5.1.  Handling Destination Map-Version Number

311	   When an ETR receives a packet, the Destination Map-Version number
312	   relates to the mapping for the destination EID for which the ETR is
313	   an RLOC.  This mapping is part of the ETR EID-to-RLOC Database.
314	   Since the ETR is authoritative for the mapping, it has the correct
315	   and up-to-date Destination Map-Version number.  A check on this
316	   version number can be done, where the following cases can arise:

[major] "A check on this version number can be done..."  Same comment
as above: Should this behavior be Normative (MUST/SHOULD)?

...
323	   2.  The packet arrives with a Destination Map-Version number greater
324	       (i.e., newer) than the one stored in the EID-to-RLOC Database.
325	       Since the ETR is authoritative on the mapping, meaning that the
326	       Map-Version number of its mapping is the correct one, this
327	       implies that someone is not behaving correctly with respect to
328	       the specifications.  In this case, the packet carries a version
329	       number that is not valid; otherwise, the ETR would have the same
330	       number, and the packet SHOULD be silently dropped.

[minor] The "otherwise" in this last sentence gives the impression
that the packet is dropped "otherwise" (i.e. when the version is not
greater).

Suggestion>
  In this case, the packet carries a version number that is not valid
  and the packet SHOULD be silently dropped.

[major] When is it ok for the packet not to be dropped?  Why is this
action recommended and not required?

332	   3.  The packets arrive with a Destination Map-Version number smaller
333	       (i.e., older) than the one stored in the EID-to-RLOC Database.
334	       This means that the ITR sending the packet has an old mapping in
335	       its EID-to-RLOC Map-Cache containing stale information.  The ETR
336	       MAY choose to normally process the encapsulated datagram
337	       according to [I-D.ietf-lisp-rfc6830bis]; however, the ITR sending
338	       the packet has to be informed that a newer mapping is available.
339	       This is done with a Map-Request message sent back to the ITR.
340	       The Map-Request will either trigger a Map-Request back using the
341	       Solicit-Map-Request (SMR) bit or it will piggyback the newer
342	       mapping.  These are not new mechanisms; how to use the SMR bit or
343	       how to piggyback mappings in Map-Request messages is already
344	       described in [I-D.ietf-lisp-rfc6833bis].  One feature introduced
345	       by Map-Version numbers is the possibility of blocking traffic not
346	       using the latest mapping.  Indeed, after a certain number of
347	       retries, if the Destination Map-Version number in the packets is
348	       not updated, the ETR MAY drop packets with a stale Map-Version
349	       number while limiting the rate of Map-Request messages.  This is
350	       because either the ITR is refusing to use the mapping for which
351	       the ETR is authoritative, or (worse) it might be some form of
352	       attack.

[major] (Related to the last comment in §5.) "the ITR sending the
packet has to be informed that a newer mapping is available"

"has to be informed" sounds like a requirement.  Should this action be
Normative?

[minor] If you're going to point at rfc6833bis anyway, don't go into
unnecessary detail.

OLD>
       This is done with a Map-Request message sent back to the ITR.
       The Map-Request will either trigger a Map-Request back using the
       Solicit-Map-Request (SMR) bit or it will piggyback the newer
       mapping.  These are not new mechanisms; how to use the SMR bit or
       how to piggyback mappings in Map-Request messages is already
       described in [I-D.ietf-lisp-rfc6833bis].

NEW>
   This is done with a Map-Request message sent back to the ITR, as
   specified in [I-D.ietf-lisp-rfc6833bis].

[major] "after a certain number of retries...the ETR MAY drop packets"

If this is a Normative action, then the number of retries should be
indicated or at least suggested.  §5.3/rfc6833bis mentions this:

   Map-Requests MUST be rate-limited to 1 per second per EID-prefix.
   After 10 retransmits without receiving the corresponding Map-Reply
   the sender MUST wait 30 seconds.

Would 10 reties be appropriate in this case?

[major] "ETR MAY drop packets"

Why is this an optional action and not required?  What should the ETR
take into account when deciding if the packets are to be dropped or
not?

354	   The rule in the third case MAY be more restrictive.  If the mapping
355	   has been the same for a period of time as long as the Time To Live
356	   (TTL) (defined in [I-D.ietf-lisp-rfc6833bis]) of the previous version
357	   of the mapping, all packets arriving with an old Map-Version SHOULD
358	   be silently dropped right away without issuing any Map-Request.  Such
359	   action is permitted because if the new mapping with the updated
360	   version number has been unchanged for at least the same time as the
361	   TTL of the older mapping, all the entries in the EID-to-RLOC Map-
362	   Caches of ITRs must have expired.  Hence, all ITRs sending traffic
363	   should have refreshed the mapping according to
364	   [I-D.ietf-lisp-rfc6833bis].  If packets with old Map-Version numbers
365	   are still received, then either someone has not respected the TTL or
366	   it is a form of spoof/attack.  In both cases, this is not valid
367	   behavior with respect to the specifications and the packet SHOULD be
368	   silently dropped.

[major] s/MAY be more restrictive/may be more restrictive
This is just a statement of fact, not a normative one.

[minor] "the Time To Live (TTL) (defined in
[I-D.ietf-lisp-rfc6833bis]) of the previous version"

I wasn't sure if you meant that the TTL was defined in rfc6833bis or
if a specific value was defined there.  But after looking at
rfc6833bis I am guessing that you mean something like this:

OLD>
   If the mapping has been the same for a period of time as long as
   the Time To Live (TTL) (defined in [I-D.ietf-lisp-rfc6833bis])
   of the previous version of the mapping, ...

NEW>
   If the Record TTL of the previous mapping has already expired, ...

[major] "all packets...SHOULD be silently dropped"

Why is this action recommended and not required?  When is it ok for
the the packets to not be dropped?

[minor] s/In both cases, this is not valid behavior with respect to
the specifications and the packet SHOULD be silently dropped./In both
cases, this is not valid behavior.

Please don't repeat the specification.

370	   LISP-encapsulated packets with the V-bit set, when the original
371	   mapping in the EID-to-RLOC Database has the version number set to the
372	   Null Map-Version value, MAY be silently dropped.  As explained in
373	   Section 4.1, if an EID-to-RLOC mapping has a Null Map-Version, it
374	   means that ITRs, using the mapping for encapsulation, MUST NOT use a
375	   Map-Version number in the LISP-specific header.

[major] "MAY be silently dropped" -- "As explained in Section 4.1..."

Please look at the related comments in §4.1/§7 about the required
behavior.  BTW, the reference should be to §7.

If there is a required (MUST) behavior in §4.1/§7, why is the behavior
specified here only optional (MAY)?  What should the ETR take into
account when making the decision?

Also, please just refer to other sections and don't repeat normative
language here.

377	   For LISP-encapsulated packets with the V-bit set, when the original
378	   mapping in the EID-to-RLOC Database has the version number set to a
379	   value different from the Null Map-Version value, a Destination Map-
380	   Version number equal to the Null Map-Version value means that the
381	   Destination Map-Version number MUST be ignored.

[] I find this sentence (and the first one in the previous paragraph)
hard to read.

Suggestion>
   If the mapping in the EID-to-RLOC Database has a non-zero version
   number, LISP-encapsulated packets with the V-bit set and a Null
   Map-Version in the Dest Map-Version field MUST be ignored.

[minor] "MUST be ignored"

I'm assuming that ignoring a packet is the same as dropping it, right?
 The other instances talked about dropping, please be consistent.

383	5.2.  Handling Source Map-Version Number
...
391	   1.  The packet arrives with the same Source Map-Version number as
392	       that stored in the EID-to-RLOC Map-Cache.  This is the correct
393	       regular case.  The ITR has in its EID-to-RLOC Map-Cache an up-to-
394	       date copy of the mapping.  No further actions are needed.

[minor] s/ITR/ETR

396	   2.  The packet arrives with a Source Map-Version number greater
397	       (i.e., newer) than the one stored in the local EID-to-RLOC Map-
398	       Cache.  This means that the ETR has in its EID-to-RLOC Map-Cache
399	       a stale mapping that needs to be updated.  A Map-Request SHOULD
400	       be sent to get the new mapping for the source EID.  This is a
401	       normal Map-Request message sent through the mapping system and
402	       MUST respect the specifications in [I-D.ietf-lisp-rfc6833bis],
403	       including rate-limitation policies.

[major] "Map-Request SHOULD be sent"

Why is this action recommended and not required?  When is it ok for
the ETR to not send a Map-Request (in response to this event)?

[minor] The last sentence, specially the part requiring respect for
rfc6833bis is not needed.  Please take it out.

405	   3.  The packet arrives with a Source Map-Version number smaller
406	       (i.e., older) than the one stored in the local EID-to-RLOC Map-
407	       Cache.  Such a case is not valid with respect to the
408	       specifications.  Indeed, if the mapping is already present in the
409	       EID-to-RLOC Map-Cache, this means that an explicit Map-Request
410	       has been sent and a Map-Reply has been received from an
411	       authoritative source.  Assuming that the mapping system is not
412	       corrupted, the Map-Version in the EID-to-RLOC Map-Cache is the
413	       correct one, while the one carried by the packet is stale.  In
414	       this situation, the packet MAY be silently dropped.

[] "Assuming that the mapping system is not corrupted"

I don't think this text adds anything to the specification -- just
opens up questions of mapping system consistency, etc.. which we don't
want to address in this document.  Please take that part out.

[major] "the packet MAY be silently dropped"

Why is this action optional and not required?  The rest of the
paragraph talks about how this "case is not valid with respect to the
specifications" -- I then don't understand why it would only be
optional to drop the packet.

...
419	   For LISP-encapsulated packets with the V-bit set, if the Source Map-
420	   Version number is the Null Map-Version value, it means that the
421	   Source Map-Version number MUST be ignored.

[] There is related text in §4.1.  Please specify actions only once.

423	6.  LISP Header and Map-Version Numbers

425	   In order for the versioning approach to work, the LISP-specific
426	   header has to carry both the Source Map-Version number and
427	   Destination Map-Version number.  This is done by setting the V-bit in
428	   the LISP-specific header as specified in [I-D.ietf-lisp-rfc6830bis].
429	   When the V-bit is set and the N bit is reset (0), the low-order 24
430	   bits of the first longword are used to transport both the source and
431	   destination Map-Version numbers.  In particular, the first 12 bits
432	   are used for the Source Map-Version number and the second 12 bits for
433	   the Destination Map-Version number.

[minor] "... as specified in [I-D.ietf-lisp-rfc6830bis].  When the
V-bit is set and the N bit is reset (0), the low-order 24 bits of the
first longword are used to transport both the source and destination
Map-Version numbers.  In particular, the first 12 bits are used for
the Source Map-Version number and the second 12 bits for the
Destination Map-Version number."

rfc6830bis also shows (§5.3) that the E-bit is not set.  That makes
this description incomplete.  Instead of trying to repeat what
rfc6830bis already says, simply point at it.  Also, a picture is worth
a thousand words -- in this case it can replace cumbersome
descriptions:

Suggestion>
   ... as specified in [I-D.ietf-lisp-rfc6830bis].  The Source
   Map-Version and Dest Map-Version are included as shown in
   Figure x.

435	   Below is an example of a LISP header carrying version numbers.

[major] s/LISP header/LISP-specific header

Note the definition of "LISP header" in rfc6830bis:

   LISP Header:   LISP header is a term used in this document to refer
      to the outer IPv4 or IPv6 header, a UDP header, and a LISP-
      specific 8-octet header that follow the UDP header and that an ITR
      prepends or an ETR strips.

437	        0                   1                   2                   3
438	        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
439	       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
440	     / |N|L|E|V|I|R|K|K|  Source Map-Version   |Destination Map-Version|
441	   LISP+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
442	     \ |                 Instance ID/Locator Status Bits               |
443	       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

[minor] Please add a figure number/title to this and all the figures.

[minor] s/Locator Status Bits/Locator-Status-Bits/g

...
450	   Destination Map-Version number (12 bits):  Map-Version of the mapping
451	     used by the ITR to select the RLOC present in the "Destination
452	     Routing Locator" field.  Section 5.1 describes how to set this
453	     value on transmission and handle it on reception.

[minor] The description in §5.1 says that "the Destination Map-Version
number relates to the mapping for the destination EID for which the
ETR is an RLOC."  I know that this description is equivalent to "the
mapping used by the ITR to select the RLOC present in the "Destination
Routing Locator" field", but they are different.  It might be worth
spending a paragraph or two in the Introduction summarizing how LISP
works.  Or, even better, put an Informative reference to
lisp-introduction.

Suggestion (for the Introduction)>
   [lisp-introduction] describes the architecture of the Locator/ID
   Separation Protocol.  It is expected that the reader is familiar
   with this introductory document.

...
460	7.  Map Record and Map-Version

462	   To accommodate the proposed mechanism, the Map Records that are
463	   transported in Map-Request/Map-Reply/Map-Register messages need to
464	   carry the Map-Version number as well.  For this purpose, the 12 bits
465	   before the EID-AFI field in the Record that describes a mapping are
466	   used (see [I-D.ietf-lisp-rfc6833bis] and reported here as an example.

[minor] "For this purpose, the 12 bits before the EID-AFI field in the
Record that describes a mapping are used (see
[I-D.ietf-lisp-rfc6833bis] and reported here as an example."

The description is again cumbersome ("12 bits before") -- simply point
at the figure.  Also, I'm not sure what the example part means.

Suggestion>
   For reference, the Map Register (specified in [I-D.ietf-lisp-rfc6833bis])
   is presented in Figure y.

468	        0                   1                   2                   3
469	        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
470	   +-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
471	   |   |                          Record TTL                           |
472	   |   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
473	   R   | Locator Count | EID mask-len  | ACT |A|      Reserved         |
474	   e   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
475	   c   | Rsvd  |  Map-Version Number   |       EID-Prefix-AFI          |
476	   o   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
477	   r   |                          EID-Prefix                           |
478	   d   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
479	   |  /|    Priority   |    Weight     |  M Priority   |   M Weight    |
480	   | L +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
481	   | o |        Unused Flags     |L|p|R|           Loc-AFI             |
482	   | c +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
483	   |  \|                             Locator                           |
484	   +-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

486	   Map-Version Number:  Map-Version of the mapping contained in the
487	     Record.  As explained in Section 4.1, this field can be zero (0),
488	     meaning that no Map-Version is associated to the mapping; hence,
489	     packets that are LISP encapsulated using this mapping MUST NOT
490	     contain Map-Version numbers in the LISP-specific header, and the
491	     V-bit MUST be set to 0.

[major] This description is not the same as what is specified in §4.1
or the definition in §5.4/rfc6833bis.  Please define this term only
once!

(A) "Section 4.1...hence, packets that are LISP encapsulated using
this mapping MUST NOT contain Map-Version numbers in the LISP-specific
header, and the V-bit MUST be set to 0."

The required actions here are different from the actions required in §4.1:

   This means that LISP-encapsulated packets destined to the EID-Prefix
   referred to by the Map Record MUST either not contain any Map-Version
   numbers (V bit set to 0) or, if they contain Map-Version numbers (V
   bit set to 1), then the destination Map-Version number MUST be set to
   the Null Map-Version number.  Any value different from zero means that
   Map-Versioning is supported and MAY be used.

Note that "MUST either" is not a valid construct because it doesn't
reflect a requirement, it indicates an option.

[B] §5.4/rfc6833bis contains a different definition of this field:

   Map-Version Number:  When this 12-bit value is non-zero, the Map-
      Reply sender is informing the ITR what the version number is for
      the EID record contained in the Map-Reply.  The ETR can allocate
      this number internally but MUST coordinate this value with other
      ETRs for the site.  When this value is 0, there is no versioning
      information conveyed.  The Map-Version Number can be included in
      Map-Request and Map-Register messages.  See Map-Versioning
      [I-D.ietf-lisp-6834bis] for more details.

The text in §5 is vague about the required coordination mentioned in
rfc6883bis.  In fact, it mentions this: "The synchronization problem
and its implications on the traffic are out of the scope of this
document."   This is a significant problem!

rfc6833bis requires (MUST) the coordination and points to this
document for more details.  However, this document declares the
problem out of scope.

To make it worst, rfc6830bis both leaves synchronization out of scope
*and* specifies a window for doing so: "synchronization protocol is
out-of-the-scope of this document, but MUST keep ETRs synchronized
within a 1 minute window."

How can the requirements in rfc6833bis/rfc6830bis be satisfied if the
mechanism is out of scope?

Personally, I am ok with the synchronization between ETRs to be out of
scope.  I need you to the come up with specific language that gets the
specs in sync.  That will require eliminating the normative language
and possibly updating rfc6830bis and rfc6833bis.

493	   This packet format works perfectly with xTRs that do not support Map-
494	   Versioning, since they can simply ignore those bits.

[nit] By "works perfectly" I assume you mean something less
"marketing" like "is backwards compatible".

496	8.  Benefits and Case Studies for Map-Versioning

[] I suggest this section be moved to an appendix.

...
502	8.1.  Map-Versioning and Unidirectional Traffic
...
521	   In the case of the ITR, the ITR is able to put both the source and
522	   destination version number in the LISP header, since the Source Map-
523	   Version number is in the ITR's database, while the Destination Map-
524	   Version number is in the ITR's cache.

[nit] NEW>
   An ITR is able to put both the source and destination version
   numbers in the LISP header since the Source Map-Version number
   is in its database while the Destination Map-Version number is
   in its cache.

526	   In the case of the ETR, the ETR simply checks only the Destination
527	   Map-Version number in the same way as that described in Section 5,
528	   ignoring the Source Map-Version number.

[nit] NEW>
   The ETR checks only the Dest Map-Version number as described
   in Section 5, ignoring the Source Map-Version number.

...
612	8.4.  Map-Version Additional Use Cases

[] This use case, and specifically "a lightweight implementation of
LISP" seems inline with the experimental version of this document, but
not the current form.  Please remove it.

If there is a document describing that "lightweight implementation"
then a reference can be added.

...
628	9.  Security Considerations

630	   Attackers can try to trigger a large number of Map-Requests by simply
631	   forging packets with random Map-Versions.  The Map-Requests are rate-
632	   limited as described in [I-D.ietf-lisp-rfc6833bis].  With Map-
633	   Versioning it is possible to filter invalid version numbers before
634	   triggering a Map-Request, thus helping to reduce the effects of DoS
635	   attacks.  However, it might not be enough to really protect from a
636	   DDoS attack.

[] Given what is specified here, are there other things that an
attacker could do?  For example, just thinking out loud, not sure if
these are possible:

- Can an attacker glean at headers and reuse the version numbers to
generate additional traffic (to overload a node, for example) with a
header that looks to be correct?

- If the network is configured to use Map-Versioning but a rogue node
decides to not use the V-bit, are there significant consequences?  I
don't think to, just asking.

- Is there any significant information (topology, cache content, for
example) that could be gleaned from knowing the Map-Versions at
different xTRs?  Besides the attack already mentioned, can an attacker
do anything else with this knowledge?

638	   Robustness of the Map-Versioning mechanism leverages on a trusted
639	   Mapping Distribution System.  A thorough security analysis of LISP is
640	   documented in [RFC7835].

[major] We shouldn't get into aspects of what "trusted Mapping
Distribution System" may be.  Instead of this paragraph, let's add a
pointer to the base specs as the initial paragraph of this section.

Suggestion>
   This document builds on the specification and operation of the
   LISP control and data planes.  The Security Considerations of
   [I-D.ietf-lisp-rfc6830bis] and [I-D.ietf-lisp-rfc6833bis] apply.

642	   As specified in [I-D.ietf-lisp-rfc6830bis], Map-Versioning MUST NOT
643	   be used over the public Internet and SHOULD only be used in trusted
644	   and closed deployments.

[major] Please don't specify things in two places!  If specified in
rfc6830bis and this section points there anyway there's no need to
even mention it.

646	10.  Deployment Considerations
...
692	   The same problem can, however, arise without Map-Versioning, for
693	   instance, if the two ITRs of Domain A send different Locator Status
694	   Bits.  In this case, either the traffic is disrupted if ETR B trusts
695	   the Locator Status Bits, or if ETR B does not trust the Locator
696	   Status Bits it will start sending Map-Requests to confirm each change
697	   in reachability.

[major] "ETR B trusts...ETR B does not trust"

Instead of talking about "trust", which may not convey the appropriate
intent, please talk about "verification" (using the language in
rfc6830bis) and how the ETR can verify and avoid disruptions.

699	   So far, LISP does not provide any specific synchronization mechanism
700	   but assumes that synchronization is provided by configuring the
701	   different xTRs consistently.  The same applies for Map-Versioning.
702	   If in the future any synchronization mechanism is provided, Map-
703	   Versioning will take advantage of it automatically, since it is
704	   included in the Record format, as described in Section 7.

[minor] Both §5/§7 talk about synchronization.  Please consolidate
this topic in one place.

[EoR -09]

---

From: Luigi Iannone <[email-masked]>
Date: Thu, 14 Apr 2022 17:50:38 +0200

Hi Alvaro,

Thanks for the great review.
I’ll try to propose un update by end of next week, so that we can have it ready by end of the month.

Ciao

L.

> On 13 Apr 2022, at 20:54, Alvaro Retana <[email-masked]> wrote:
> 
> Dear authors/WG:
> 
> 330	       number, and the packet SHOULD be silently dropped