[6lo] Review of draft-ietf-6lo-fragment-recovery-03

"Carles Gomez Montenegro" <carlesgo@entel.upc.edu> Fri, 07 June 2019 15:57 UTC

Return-Path: <carlesgo@entel.upc.edu>
X-Original-To: 6lo@ietfa.amsl.com
Delivered-To: 6lo@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 93B2B1200B7 for <6lo@ietfa.amsl.com>; Fri, 7 Jun 2019 08:57:30 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.197
X-Spam-Level:
X-Spam-Status: No, score=-4.197 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_NONE=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Hrj4WQu6tmT0 for <6lo@ietfa.amsl.com>; Fri, 7 Jun 2019 08:57:27 -0700 (PDT)
Received: from violet.upc.es (violet.upc.es [147.83.2.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id D00A312006D for <6lo@ietf.org>; Fri, 7 Jun 2019 08:57:26 -0700 (PDT)
Received: from entelserver.upc.edu (entelserver.upc.es [147.83.39.4]) by violet.upc.es (8.14.4/8.14.4/Debian-4.1ubuntu1) with ESMTP id x57FvLxj008843; Fri, 7 Jun 2019 17:57:21 +0200
Received: from webmail.entel.upc.edu (webmail.entel.upc.edu [147.83.39.6]) by entelserver.upc.edu (Postfix) with ESMTP id 1EF291D53C1; Fri, 7 Jun 2019 17:57:21 +0200 (CEST)
Received: from 131.111.5.141 by webmail.entel.upc.edu with HTTP; Fri, 7 Jun 2019 17:57:21 +0200
Message-ID: <0dbe57929fb7cc5c07f48fdba9f9df46.squirrel@webmail.entel.upc.edu>
Date: Fri, 7 Jun 2019 17:57:21 +0200
From: "Carles Gomez Montenegro" <carlesgo@entel.upc.edu>
To: pthubert@cisco.com
Cc: 6lo@ietf.org
User-Agent: SquirrelMail/1.4.21-1.fc14
MIME-Version: 1.0
Content-Type: text/plain;charset=iso-8859-1
Content-Transfer-Encoding: 8bit
X-Priority: 3 (Normal)
Importance: Normal
X-Virus-Scanned: clamav-milter 0.100.2 at violet
X-Virus-Status: Clean
X-Greylist: ACL matched, not delayed by milter-greylist-4.3.9 (violet.upc.es [147.83.2.51]); Fri, 07 Jun 2019 17:57:21 +0200 (CEST)
Archived-At: <https://mailarchive.ietf.org/arch/msg/6lo/3deoZlrLDAPTB4-t0-n42SwdG5A>
Subject: [6lo] Review of draft-ietf-6lo-fragment-recovery-03
X-BeenThere: 6lo@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Mailing list for the 6lo WG for Internet Area issues in IPv6 over constrained node networks." <6lo.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/6lo>, <mailto:6lo-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/6lo/>
List-Post: <mailto:6lo@ietf.org>
List-Help: <mailto:6lo-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/6lo>, <mailto:6lo-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 07 Jun 2019 15:57:31 -0000

Hi Pascal,

Please find below my review of draft-ietf-6lo-fragment-recovery-03.

In my humble opinion, this is a very readable document. In addition, the
document provides the motivations for some decisions made, which I
appreciated while reading the document.

Below you can find my comments and/or suggestions. They are mostly
editorial, although some of them are technical.

I hope this helps.

Cheers,

Carles


-------------- Review --------------

- Page 2: "in all cases" can be removed

- Page 2: "a firmware upgrades" -> perhaps remove "a" (or apply other
                                                       changes)

- Page 3: "10Kbytes" -> "10 kbyte"

- Page 3: "requires to reassemble the full packet at each hop".

          The sentence should probably be modified. At least, "requires" is
          probably not the right verb here. As e.g. stated in
          draft-ietf-6lo-minimal-fragment: "it may be beneficial for a
          forwarder not to have to reassemble each packet in its entirety
          before forwarding it.  This has always been possible with the
          original fragmentation design of RFC4944".

- Page 3: "... and trigger the retransmission of the full datagram".

          Perhaps "... potentially triggering" the retransmission..." (since
          datagram retransmission will only happen if an upper layer
          protocol requires so).

- Section 2.2: "in the following documents" --> "in the following document"

- Section 2.4: "miss-associated" -> "misassociated"

- Section 2.4, last paragraph: "MLPS" --> "MPLS"

- Section 3, first paragraph: "A new format for fragment is introduces"

          --> "A new format for fragments is introduced"

- Section 3, last paragraph: "... consistently with in Section 2"

          --> "... consistently with Section 2"

- Section 4.1, 2nd paragraph: "the fragment number".

          This term appears here for the first time. Perhaps some forward
          reference might help the reader.

- Section 4.2, 2nd paragraph: "is required protect"

          --> "is required to protect"

- Section 4.2, 2nd paragraph: "progressing alon"

          --> "progressing along"

- Section 4.3: "The compression of the Hop Limit, of the source and
  destination addresses"

          --> Perhaps mention that these are IPv6 header fields (even if it
              might seem obvious)?

- Section 4.3, 1st paragraph: "datagram_size"

          --> Perhaps add a forward reference to section where this field is
              defined?

- Section 5, 2nd paragraph: "the node that is the source MAC address"

          --> "the node that corresponds to the source MAC address"

- Section 5, 2nd paragraph: "the selected tag"

          --> Perhaps better sticking to "Datagram_tag"

- The line above Fig. 1: "page" --> "Page" (as defined in RFC 8025)

- Section 5, last paragraph of intro: "datagram-tag" -> "Datagram_tag"

- Several instances: "recomposition" --> "reassembly" ?

- Section 5.1. The text before Fig. 2 uses terms such as "the sequence
  field" that have not yet been introduced. Perhaps indicate they are
  introduced later (e.g. in Fig. 2), or some other solution.

- Section 5.1, 2nd paragraph: "seem be" --> "seem to be"

- Page 9: "Fragment_size" field. I was wondering whether this field is
  actually needed. Is this used just in case a lower layer technology uses
  padding?

- Page 9: "this field indicates the datagram_size of the compressed
  datagram"

         --> "... of the *potentially* compressed datagram" (or some similar
             approach), as perhaps in some (rare) cases, a packet will
             not be compressed?

- Page 10, first lines: "may store" --> "may be stored"

- Page 10, 2nd bullet: "When the Sequence is not 0, this field indicates the
  offset of the fragment **in the compressed form**."

         --> I was wondering what is the meaning of "in the compressed form"
             in the previous sentence. Which is the expected representation
             of the offset of the fragment?

- End of 5.1: "If the fragment cannot be forwarded or routed, then an abort
  RFRAG-ACK is sent back to the source."

         --> Perhaps add a forward reference where the concept of "abort
             RFRAG-ACK" is defined.

- Page 11: there is no explicit text about "Datagram_tag" in the RFRAG
  Acknowledgment subsection, below Figure 5.

- Page 12, 1st paragraph. The terms "All 0's" and "All 1's" may be well
  known for LPWANners. :) But I was wondering if perhaps it might be clearer
  to say something like "A bitmap with all bits set to 0...", etc.

- If a datagram is carried by 20 fragments, and all fragments are
  successfully received, is the FULL bitmap the only way to indicate so? Or
  would a bitmap with the first 20 bits set to 1 and the remaining bits set
  to 0 also work?

- Section 6, 3rd line: "a one or more fragments" -> "one or more fragments"

- Section 6, 4th line: "a standalone header"

        --> Does this mean "without a payload"? If yes, perhaps something
            along these lines could be added for extra clarity.

- Section 6, end of 1st paragraph: "by reversing the MPLS operation"

        --> "MPLS-like"?

- Section 6, 2nd paragraph: "Te maximum number" --> "The maximum number"

- Section 6, 2nd paragraph: "the 6LoWPAN endpoint that reassembles
  the packets at 6LoWPAN level (the receiver)"

        --> Swap with "it" in the previous sentence?

- Section 6, 3rd paragraph: "a ARQ timer" --> "an ARQ timer"

- Section 6, 3rd paragraph. A reader may wonder which are the names and
  values for the parameters for number of retries, timers, etc. Perhaps a
  reference to Section 7 could be added.

- Page 13, 2nd paragraph: "last fragment of a series"

        --> Is this the same as "Last fragment of a window"? If yes, perhaps
            use "window" (which has been introduced before).

- Page 13, 2nd paragraph: "RFRAG Acknowledgment Request" and
  "acknowledgment request" are not defined terms, whereas "Ack-Request bit"
  is defined.

- Page 13, 2nd paragraph, last sentence: does "it" refer to "Delaying the
  acknowledgment" or to the "round trip delay computation" ?

- In some instances, "cancel" is used. I was wondering if "abort" refers to
  the same. If yes, then perhaps just stick to one term?

- Page 14. "alternate routes not possible" -> "alternate routes are not
  possible"

- I understand that the document has been designed with Route-Over in mind,
  but I was wondering if the document might also work in Mesh-Under. Any
  thoughts?

- Page 15, last paragraph: "to the previous router"

      --> "to the previous node" (since perhaps the previous node could be a
           host?)

- Section 6.2, 2nd paragraph. "until" -> "Until"

- Section 6.2, 3rd paragraph. If the minimal MTU that decreases is in one of
  the intermediate hops (i.e. not the first hop), how does the sender detect
  such event?

- Section 7.1, 1st paragraph: "as echoing ECN should always be on"

      --> "as echoing ECN is always on" (or is the opposite possible?)

- Section 7.1, 1st paragraph: "to the sender that is in control"

      --> "to the sender, which is in control"

- Section 7.1. Should there be default values for the parameters defined?

- Section 7.1. There are two UseECN entries!

- Section 7.2. "The management system should monitor..."

     --> Any hint on how this could be done? E.g. each node self-monitors
         its performance? An external NMS system collects such information
         and makes decisions?

- Section 8. Some of the content of Section 7 in
https://tools.ietf.org/html/draft-ietf-core-cocoa-03 may apply here.

- Page 22: "all fragments are resent" --> "all fragments would need to be
  resent..."

- Page 22, 4th paragraph. "Mesh Addressing" and "Mesh-Under" are mentioned.
  Since the document focuses on Route-Over, are these needed?

- I understand that the document has been designed with Route-Over in mind,
  but I was wondering if the document might also enable the same
  functionality for Mesh-Under (not sure if with perhaps minor adaptations).
  Any thoughts?