Subject: [Last-Call] Genart last call review of draft-ietf-6lo-fragment-recovery-08

From: Peter Yee via Datatracker <[email-masked]>
Date: Sat, 01 Feb 2020 16:01:11 -0800

Reviewer: Peter Yee
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-6lo-fragment-recovery-08
Reviewer: Peter Yee
Review Date: 2020-02-01
IETF LC End Date: 2020-01-30
IESG Telechat date: Not scheduled for a telechat

Summary: This draft is a clearly-written Standards Track specification for how
to do fragment recovery in a 6LoWPAN mesh network.  The document has some minor
issues and numerous nits that should be addressed prior to publication. [Ready
with Issues]

Apologies for the timely review. Sigh. ;-)

Major issues:  None

Minor issues:

Page 9, 1st paragraph after Figure 1, 1st sentence: how does the sender ensure
the full reception of the datagram?  It can assume that to be the case, but
there's no further mechanisms available to it to "ensure" anything beyond that.

Page 13, 4th full paragraph, 1st sentence: the receiver is instructed send an
RFRAG Acknowledgment without regard to whether the sender wishes to protect the
datagram, in contradiction to the 2nd sentence in the 4th preceding paragraph
(which starts on the bottom of page 12).  Try to rationalize the two
paragraphs.  Also, what constitutes a "short" timer?  How is an implementer to
decide what's reasonable?

Page 14, 2nd full paragraph, 1st sentence: how is a "reasonable" amount of time
calculated?

Nits/editorial comments:

General:

Change "en-route" to "en route".

Append a comma after each occurence of "i.e.".

Specific:

Page 2, Section 1, 1st sentence: change "in" to "on".  Append "of a" after
"order".

Page 3, 1st full paragraph, 2nd sentence: change "to reassemble" to
"reassembling".

Page 3, 1st full paragraph, 3rd sentence: change "to use" to "using".

Page 3, 2nd full paragraph, 4th sentence: insert "on" before "pages".

Page 3, 2nd full paragraph, last sentence: change "can not" to "cannot".

Page 4, 1st partial paragraph, 1st full sentence: change "to a" to "in".

Page 4, section 2.2, 3rd paragraph, 1st sentence: insert "the" before
"application".

Page 6, section 3, 1st paragraph, last sentence: change "fragment" to
"fragments".

Page 6, section 3, 2nd paragraph, 1st sentence: change "to modify" to
"modifying".

Page 6, section 3, 2nd paragraph, 2nd sentence: change "It results" to "This
requires".  Change "whcih" to "which".

Page 6, section 3, 2nd paragraph, 3rd sentence: change "to modify" to
"modifying".

Page 6, section 3, 3rd paragraph: change "consistently" to "consistent". 
Append a comma after "[RFC6282]".

Page 7, section 4.1, 1st paragraph, 1st sentence: consider eliding "At the time
of this writing, ".  If the other draft's final position can be clarified
before publication of this draft, it's redundant.  If it's not clear at time of
publication of this draft than it's a guess.

Page 7, section 4.1, 1st paragraph, 2nd sentence: change "to happen" to
"happening".

Page 7, section 4.2, 1st paragraph, 1st sentence: insert "an" before
"Inter-Frame".  Consider whether "Inter-Frame Gap" should be written the same
as subsequent usage ("InterFrameGap") and if you choose not to do so, perhaps
make it "inter-frame gap" since it is a concept, not a name.

Page 7, section 4.2, 1st paragraph, 2nd sentence: consider changing
"progressed" to "processed" or "completed processing of".

Page 7, section 4.3, 1st paragraph, 1st sentence: delete the comma after
"Routing Header".

Page 8, section 5, 2nd paragraph, last sentence: change "It" to "the" and
insert an "i" before "s".

Page 8, section 5.1, 1st sentence: change "on" to "with respect to".  Change "-
native -" to "(native)".

Page 10, RFRAG Header names: try to capitalize these consistently amongst
themselves or at least consistently with Figure 1.

Page 10: Fragment_size, 1st sentence: change "10 bit" to "10-bit".

Page 10, Fragment_size, 2nd sentence: append a comma after "octet".

Page 10, Sequence, 1st sentence: change "5 bit" to "5-bit".

Page 10, Fragment_offset: change "16 bit" to "16-bit".

Page 10, Fragment_offset, 4th bullet item, 1st sentence: append a comma after
"6.1.1.".

Page 10, Fragment_offset, 4th bullet item, 2nd sentence: delete "as".

Page 11, section 5.2, 1st paragraph, 2nd sentence: change "one to one" to
"121".  No, actually I mean "one-to-one".  ;-)

Page 11, 1st paragraph after Figure 3: change "a" to "an" before "RFRAG".

Page 12, RFRAG Acknowledgment Bitmap, 2nd sentence: delete the first instance
of "that".

Page 12, RFRAG Acknowledgment Bitmap, 3rd sentence: delete the first instance
of "that".  Change the comma to a semicolon.

Page 12, section 6, 1st paragraph, 2nd sentence: delete the extraneous space
before "with".

Page 12, section 6, 1st paragraph, last sentence: change "associated to" to
"associated with".

Page 12, section 6, 2nd paragraph, 1st sentence: insert "the" before "6LoWPAN".

Page 12, section 6, 2nd paragraph, 5th sentence: insert "the" before "6LoWPAN".

Page 13, 1st partial paragraph, 1st full sentence: expand the use of "ARQ" here
and put "ARQ" in parenthesis, or put "(ARQ)" before a prior use of
"Ack-Request".  Append "for" after "out".  Change "amount" to "number".

Page 13, 2nd full paragraph, 2nd sentence: change "preceeding" to "preceding".

Page 13, 2nd full paragraph, last sentence: change "round trip" to "round-trip".

Page 13, 3rd full paragraph, last sentence: append "the sender" after "inform".
 Change "process" to "processing".

Page 13, 4th full paragraph, 1st sentence: change "a" to "an" after "sends". 
Insert "to" before "abort".  Consider changing "reasonable time" to "the
duration of the timer".  Also, this is one really long sentence.  Perhaps you
could break it down?

Page 14, 1st full paragraph, 1st sentence: change "round robin" to
"round-robin".

Page 14, section 6.1, 1st paragraph: change "Fragment" to "fragment".

Page 14, section 6.1, 2nd paragraph, 2nd sentence: change "Switching" to
"switching".

Page 14, section 6.1, 2nd paragraph, 3rd sentence: delete "In more details, "
and change the first "the" to "The".

Page 14, section 6.1, 2nd paragraph, 4th sentence: append "receiving" after
"Upon".

Page 14, section 6.1, 2nd paragraph, 6th sentence: change "that" to "which".

Page 14, section 6.1, 2nd paragraph, last sentence: change "at" to "in".

Page 14, section 6.1.1: change "Upon" to "Receiving" or perhaps append
"receiving" after "Upon".

Page 14, section 6.1.1., 1st paragraph, 2nd sentence: change "to" to "with"
after "associated".  Append "address" after each "MAC".

Page 14, section 6.1.1., 1st paragraph, 3rd sentence: append "receiving" after
"Upon".

Page 15, 1st partial paragraph: change "whereas" to "where".

Page 15, 1st full paragraph, 1st sentence: change "to match" to "matching".

Page 15, 1st full paragraph, 2nd sentence: change "Reverse" to "reverse".

Page 15, 1st full paragraph, 3rd sentence: change "to match" to "matching".

Page 15, 2nd full paragraph, 3rd sentence: insert "determine whether to" before
"abort".

Page 15, section 6.1.2: Change "Upon" to "Receiving" or append "receiving"
after "Upon".

Page 15, section 6.1.2, 1st paragraph, 1st sentence: append "receiving" after
"Upon".

Page 15, section 6.1.2, 2nd paragraph: change "if" to "If".

Page 15, section 6.1.2, 2nd bullet item: insert "is" before "set".

Page 16, 1st partial paragraph, 1st full sentence: change "in" to "is".

Page 16, section 6.2: change "Upon" to "Receiving" or append "receiving" after
"Upon".  Drop "the" if you like.

Page 16, section 6.2, 1st paragraph, 1st sentence: append "receipt of" after
"Upon".

Page 16, section 6.3, 1st paragraph: append a comma after "sequence".

Page 17, 1st paragraph: change "to" to "with" after "associated".

Page 17, 2nd paragraph, 3rd sentence: change "frowarded" to "forwarded".

Page 18, OptARQTimeOut: append "of time" after "amount".

Page 18, MaxFragRetries: change "Fragment" to "fragment".

Page 18, MaxDatagramRetries: change "Datagram" to "datagram".

Page 19, section 7.2, 1st sentence: delete "the" before "both".  Change the
first "OptWindowSize" to "OptDatagramSize".

Page 19, section 7.2, last sentence: change "a same" to "the same".

Page 19, section 8, 2nd paragraph, 2nd sentence: change "reducing" to "being
reduced".

Page 19, section 8, 2nd paragraph, last sentence: delete "ot" [sic].  Change
"clean" to "cleaning".

Page 19, section 8, 3rd paragraph, 3rd paragraph, 2nd sentence: change "to
realign" to "realigning".  Append a comma after "fragment".

Page 20, section 10, 1st sentence: append a comma after "Watteyne".

Page 20, section 10, 2nd sentence: append a comma after "Bormann".

Page 25, Appendix B, 1st paragraph, last sentence: change "a" to "an" before
"LLN".

Page 25, individual requirements: append a colon after the name of each
requirement, e.g., "Number of fragments:".

Page 25, Minimum acknowledgment overhead: insert "a" before "data".

Page 26, Appendix C: change "On" to "on".

Page 26, Appendix C, 2nd paragraph, 2nd sentence: change "to control" to
"controlling".  Delete the comma after "fragments".

Page 26, Appendix C, 4th paragraph, 3rd sentence: delete "an" before
"end-to-end".

Page 26, Appendix C, 4th paragraph, last partial sentence: change "TimeSlotted"
to "Time-Slotted".

---

From: "Pascal Thubert (pthubert)" <[email-masked]>
Date: Tue, 4 Feb 2020 17:08:33 +0000

Hello Peter:

Many thanks for your review! 

I published 09 so the next reviewers will not have to suffer from my nits, and we can progress from there if my proposals are not satisfactory.
The delta is available here, please see below for more:
https://www.ietf.org/rfcdiff?url2=draft-ietf-6lo-fragment-recovery-09
Looking at it I found that the addition of address after MAC had a side effect, I'll fix it next.

> Page 9, 1st paragraph after Figure 1, 1st sentence: how does the sender
> ensure the full reception of the datagram?  It can assume that to be the case,
> but there's no further mechanisms available to it to "ensure" anything
> beyond that.

I added text per Erik's review on this in 6.1.2:
"
   [I-D.ietf-6lo-minimal-fragment] indicates that the receiving endpoint
   stores "the actual packet data from the fragments received so far, in
   a form that makes it possible to detect when the whole packet has
   been received and can be processed or forwarded".  How this is
   computed in implementation specific but relies on receiving all the
   bytes up to the datagram_size indicated in the first fragment.  An
   implementation may receive overlapping fragments as the result of
   retries after an MTU change.
"
Is this OK? 

> RFRAG Acknowledgment without regard to whether the sender wishes to
> protect the datagram, in contradiction to the 2nd sentence in the 4th
> preceding paragraph (which starts on the bottom of page 12).  Try to
> rationalize the two paragraphs.  Also, what constitutes a "short" timer?  How
> is an implementer to decide what's reasonable?

This really depends on the MAC layer and depth of the mesh. Reasonable is an end-to-end turn around trip delay.
What about adding ' e.g., in the order of an average turn around trip delay in the network' ?
The result would read as follows:

"
   When all the fragments are received, the receiving endpoint
   reconstructs the packet, passes it to the upper layer, sends a RFRAG
   Acknowledgment on the reverse path with a FULL bitmap, and arms a
   short timer, e.g., in the order of an average round-trip delay
   in the network, to absorb fragments that are still in flight for that
   datagram without creating a new state and abort the communication if
   it keeps going on beyond a reasonable time.
"

> 
> Page 14, 2nd full paragraph, 1st sentence: how is a "reasonable" amount of
> time calculated?

This is indicated in the sentence, time for a packet to progress a few hops. 
I can change "reasonable" to something clearer. does this look better?

"

 When a single frequency is used by contiguous hops, the sender should
   insert a delay between fragments of a same datagram that covers
   multiple transmissions so as to let a fragment progress a few hops
   and avoid hidden terminal issues.

"

> Page 12, RFRAG Acknowledgment Bitmap, 3rd sentence: delete the first
> instance of "that".  Change the comma to a semicolon.

 But I guess we'll find them in the next round.

> Page 26, Appendix C, 4th paragraph, last partial sentence: change

Well, that's how 6TiSCH spells it, so I'd rather leave it as is.

Whao many thanks again, Peter, for your careful review!