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

From: "Carles Gomez Montenegro" <[email-masked]>
Date: Fri, 7 Jun 2019 17:57:21 +0200

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?

---

From: "Pascal Thubert (pthubert)" <[email-masked]>
Date: Tue, 11 Jun 2019 16:32:52 +0000

Many Thanks Carles!

Please see below

done

done

Separated the 10 and spelled out kilobytes. See https://en.wikipedia.org/wiki/Kilobyte

The sentence is about the "original design" specifically to contrast with minimal.

Minimal fragment updates that design as you mention. The next sentence says so.

I think we're good there.

done

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

done

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

done

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

done

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

done

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

done

You right. I changed to a sequence number, which does not need to be defined.

>           --> "is required to protect"

done

done

>               might seem obvious)?

Sure, done

>               defined?

Added in terminology

"That owns the source MAC address " is OK I think

>           --> Perhaps better sticking to "Datagram_tag"

done

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

done

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

done

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

done

>   introduced later (e.g. in Fig. 2), or some other solution.

What about

...

   The format of the fragment header is shown in Figure 2.

    It is the same for all fragments. The format has a length and an offset, as

    well as a sequence field. This would be redundant if the offset was computed

    as the product of the sequence by the length, but this is not the case.

...

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

Removed, see above

>   padding?

No, fragments can be of different sizes, in particular to adapt to variation of MTU.

Unsurprisingly, this showed up with Laurent's review : )

>              not be compressed?

You are correct, I fixed here and there. Here fixed to

              In this specification, if the packet is compressed then the size and offset

    of the fragments are expressed on the Compressed Form of the packet form as

    opposed to the uncompressed - native - packet form.

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

done

>              of the offset of the fragment?

I added  "Compressed Form" of the datagram to the terminology

Actually added all the below:

                 <t hangText="6LoWPAN endpoints">

                  The LLN nodes in charge of generating or expanding a 6LoWPAN header

        from/to a full IPv6 packet.

                            The 6LoWPAN endpoints are the points where fragmentation and reassembly

        take place.

        </t>

                 <t hangText="Compressed Form">

                  This specification uses the generic term Compressed Form to refer to

        the format of a datagram after the action of <xref target="RFC6282"/>

        and possibly <xref target="RFC8138"/> for RPL <xref target="RFC6550"/>

        artifacts.

        </t>

       <t hangText="datagram_size:">

        The size of the datagram in its Compressed Form before it is fragmented.

        The datagram_size is expressed in a unit that depends on the MAC layer

        technology, by default a byte.

        </t>

       <t hangText="fragment_offset:">

        The offset of a particular fragment of a datagram in its Compressed Form.

        The fragment_offset is expressed in a unit that depends on the MAC layer

        technology and is by default a byte.

        </t>

       <t hangText="datagram_tag:">

        An identifier of a datagram that is locally unique to the Layer-2 sender.

        Associated with the MAC address of the sender, this becomes a globally

        unique identifier for the datagram.

                  </t>

       <t hangText="RFRAG:">Recoverable Fragment

                  </t>

       <t hangText="RFRAG-ACK:">Recoverable Fragment Acknowledgement

                  </t>

       <t hangText="RFRAG Acknowledgment Request:">An RFRAG with the

       Acknowledgement Request flag ("X" flag) set.

                  </t>

       <t hangText="All 0's:">Refers to a bitmap with all bits set to zero.

                  </t>

       <t hangText="All 1's:">Refers to a bitmap with all bits set to one.

                  </t> >

>              RFRAG-ACK" is defined.

done

>   Acknowledgment subsection, below Figure 5.

added

      datagram_tag:   16 bits; an identifier of the datagram that

       is locally unique to the sender.

>   to say something like "A bitmap with all bits set to 0...", etc.

You right  😊  😊  😊  added a definition in the terminology

>   to 0 also work?

You want the full bitmap because the intermediate hops do not need to remember the sequences.

A full bitmap makes it easy to them. That means done, clean up.

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

done

>             along these lines could be added for extra clarity.

added

>         --> "MPLS-like"?

Yes, done

done

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

Done

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

Or just "a timer"

>   reference to Section 7 could be added.

done

>             use "window" (which has been introduced before).

done

>   is defined.

Added to terminology

Hard to fix. Proposal:

    Because it might defeat the round trip delay computation, delaying the

    acknowledgment should be configurable and not enabled by default.

Generalized to "abort"

>   possible"

Fixed to

     As a consequence, the next fragments can only follow the path that was set

      up by the first fragment and cannot follow an alternate rout

>   thoughts?

Probably does, yes. No need to forward fragments, so all the MPLS-like / reverse MPLS is moot.

>            host?)

Reworded as

   To achieve this, each hop that performed an MPLS-like operation on fragments

    reverses that operation for the RFRAG_ACK by sending a frame from the next

    hop to the previous hop as known by its MAC address in the VRB.

    The datagram_tag in the RFRAG_ACK is unique to the receiver and is enough

    information for an intermediate hop to locate the VRB that contains the

    datagram_tag used by the previous hop and the Layer-2 information associated

    to it (interface and MAC address).

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

Done

>   such event?

PMTUD I guess. Or configuration change. I added:

Note that Path MTU Discovery is out of scope for this document.

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

done

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

Done

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

Hard, depends on the tech and number of hops.

> - Section 7.1. There are two UseECN entries!

fixed

Open to suggestions : )

Excellent suggestion, added a link.

>   resent..."

done

> - Page 22, 4th paragraph. "Mesh Addressing" and "Mesh-Under" are

> mentioned.

>   Since the document focuses on Route-Over, are these needed?

I'd say yes as a comparison point. This is annex anyway.

>   Any thoughts?

Well I guess it could be used there too. The fragment forwarding is not useful but the ARQ might be.

If the group wants we could add a section.

Many thanks Carles, that was huge! I'll be publishing 04 now.

Pascal