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 protected]> wrote: > > 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 _______________________________________________ lisp mailing list [email protected] https://www.ietf.org/mailman/listinfo/lisp
