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

["Pascal Thubert (pthubert)" <[e-mail]>]

Many Thanks Carles!



Please see below





> -------------- Review --------------

>

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

>





done



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

>                                                        changes)

>



done



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

>



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



> - 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".

>



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.





> - 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).

>



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



>

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

>

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





done



>

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

>

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



done



>

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

>

>           This term appears here for the first time. Perhaps some forward

>           reference might help the reader.

>





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



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

>

>           --> "is required to protect"



done



>

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

>

>           --> "progressing along"

>



done



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



Sure, done



>

> - Section 4.3, 1st paragraph: "datagram_size"

>

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

>               defined?



Added in terminology



>

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

>

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

>



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





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

>

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



>

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



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



>

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



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

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



>

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





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



>

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



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

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



done



>

> - Page 11: there is no explicit text about "Datagram_tag" in the RFRAG

>   Acknowledgment subsection, below Figure 5.



added





      datagram_tag:   16 bits; an identifier of the datagram that

       is locally unique to the sender.



>

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



You right  😊  😊  😊  added a definition in the terminology





>

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



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



>

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



added





>

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

>

>         --> "MPLS-like"?



Yes, done



>

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

>



done





> - Section 6, 2nd paragraph: "the 6LoWPAN endpoint that reassembles

>   the packets at 6LoWPAN level (the receiver)"

>

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



Done



>

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



Or just "a 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.



done



>

> - 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).



done



>

> - Page 13, 2nd paragraph: "RFRAG Acknowledgment Request" and

>   "acknowledgment request" are not defined terms, whereas "Ack-Request bit"

>   is defined.



Added to terminology



>

> - Page 13, 2nd paragraph, last sentence: does "it" refer to "Delaying the

>   acknowledgment" or to the "round trip delay computation" ?

>

Hard to fix. Proposal:



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

    acknowledgment should be configurable and not enabled by default.





> - In some instances, "cancel" is used. I was wondering if "abort" refers to

>   the same. If yes, then perhaps just stick to one term?

>



Generalized to "abort"





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

>   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





>

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



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



>

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

>

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

>            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



>

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



PMTUD I guess. Or configuration change. I added:



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





>

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

>

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



done





>

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

>

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





>

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

>



Open to suggestions : )









> - Section 8. Some of the content of Section 7 in

> https://tools.ietf.org/html/draft-ietf-core-cocoa-03 may apply here.

>



Excellent suggestion, added a link.





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

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





>

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



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