Subject: [Gen-art] Genart telechat review of draft-ietf-6lo-fragment-recovery-12

From: Peter Yee via Datatracker <[email-masked]>
Date: Sun, 16 Feb 2020 13:51:39 -0800

Reviewer: Peter Yee
Review result: Ready with Nits

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 wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

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

Document: draft-ietf-6lo-fragment-recovery-12
Reviewer: Peter Yee
Review Date: 2020-02-16
IETF LC End Date: None
IESG Telechat date: 2020-02-20

Summary: This revision mostly covers the items I raised during my initial
review.  Some remain and there are some new nits to be addressed.  (Ready with
Nits)

Major issues: None

Minor issues: None

Nits/editorial comments:

Page 8, section 5, 2nd paragraph, last sentence: Change “It results” to “The
result is”.

Page 12, RFRAG Acknowledgment Bitmap, 2nd sentence: delete “that”.

Page 12, RFRAG Acknowledgment Bitmap, 3rd sentence: delete “that”.  Change the
comma to a semicolon.

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

Page 13, 1st full paragraph, 1st sentence: change “TimeOut” to “Time Out”.

Page 13, 5th full paragraph, 1st sentence: change “in” to “on”.

Page 13, 5th full paragraph, 3rd sentence: change “abort” to “aborts”.

Page 14, 3rd paragraph, 1st sentence: change “a same” to one of “any”, “any
single”, or something similar.  These suggestions are based on the assumption
that fragments from disparate datagrams are not intermingled, otherwise you’ll
need some other description of what you want to do in order to allow fragments
to get a few hops away.

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

Page 15, section 6.1.1, 1st paragraph, 3rd sentence: append “receiving” after
“Upon”.  Consider changing “a” to “the” before “first”.

Page 15, section 6.1.1, 2nd paragraph, 1st sentence: change “to match” to
“matching”.

Page 16, 2nd paragraph, 2nd sentence: change “in” to “is”.

Page 16, section 6.3, 1st sentence: change “sequence” to “Sequence”. 
Personally, I find the mixing of cases between these header fields
inconsistent.  I’d prefer you choose one style and use it consistently.

Page 18, section 7, 1st paragraph, 3rd sentence: change “This” to “These”.

Page 18, section 7, 2nd paragraph, 2nd sentence: insert a space between “10”
and “Kbps”.

Page 18, section 7, 3rd paragraph, 2nd sentence: append a comma after “window
size”.

Page 18, section 7.1, 2nd paragraph, 1st sentence: change “a same” to “the
same”.

Page 18, section 7.1, 2nd paragraph, 2nd sentence: change “a same” to “the
same”.

Page 18, section 7.1, inter-frame gap, 1st sentence: change “a minimum” to “the
minimum”.

Page 18, section 7.1, inter-frame gap, 2nd sentence: change “a same” to “the
same” in both places in the sentence.  Overall, I couldn’t quite parse this
sentence.  “may be subject to receive while transmitting” left me guessing as
to what exactly you wanted to convey here.  Please rewrite this sentence.

Page 19, 1st paragraph, 2nd sentence: append a comma after “(Fragment_Size)”.

Page 19, 1st paragraph, 3rd sentence: change “node” to “nodes”.  Delete “the”
before “fluidity”.

Page 19, OptFragmentSize, 2nd sentence: change “more” to “greater”.

Page 19, OptFragmentSize, 4th sentence: change “On” to “For”.  Change “enable”
to “account for”.  Delete “of” before “the Hop Limit”.

Page 19, OptFragmentSize, 5th sentence: change “On” to “For”.

Page 19, OptWindowSize, last sentence: change “in” to “on”.

Page 20, OptARQTimeOut, 1st sentence: change “starting point of the value” to
“initial value”.  Change “that” to “which”.  Insert “the” before “amount”. 
Change “a next” to “the next”.

Page 20, OptARQTimeOut, 2nd sentence: change “more” to “greater”.

Page 20, MaxARQTimeOut, 2nd sentence: change “in” to “on”.

Page 20, first paragraph after MaxDatagramRetries: change “to perform” to “of
performing”.  Change the comma to a semicolon.  Insert “see” before “more”.

Page 20, UseECN: append a comma after “MaxFragmentSize”.

Page 21, section 7.2, 1st sentence: change “amount” to “number”.  Delete “the”
before “Window_Size”.  Change “OptDatagramSize” (this term does not exist
elsewhere in the draft) to “OptFragmentSize”.

Page 22, section 10, 2nd sentence:  Append a comma after “Bormann”.

Page 22, section 10, 3rd sentence: change “ot” to “to”.

---

From: "Pascal Thubert (pthubert)" <[email-masked]>
Date: Tue, 18 Feb 2020 15:21:28 +0000

Many thanks for your review Peter!

I applied all your typo fix suggestions that are not discussed below.

> that fragments from disparate datagrams are not intermingled, otherwise
> you’ll need some other description of what you want to do in order to allow
> fragments to get a few hops away.

You're right, that can be any frame. Proposed change to:
"
 When a single frequency is used by contiguous hops, the sender should insert a delay between the frames (e.g., frames carrying fragments) that are sent to the same next hop. The delay should cover multiple transmissions so as to let a frame progress a few hops and avoid hidden terminal issues.
"

> to what exactly you wanted to convey here.  Please rewrite this sentence.

Actually I did a few lines ahead but forgot to erase those : ) There's also section 4.2.
So why nit just erase the weird sentence? We then get

"
   An implementation must control the rate at which it sends packets
   over the same path to allow the next hop to forward a packet before
   it gets the next.  In a wireless network that uses the same frequency
   along a path, more time must be inserted to avoid hidden terminal
   issues between fragments (more in Section 4.2).

   This is controlled by the following parameter:

   inter-frame gap:  Indicates the minimum amount of time between
      transmissions.  The inter-frame gap protects the propagation of
      one transmission before the next one is triggered and creates a
      duty cycle that controls the ratio of air time and memory in
      intermediate nodes that a particular datagram will use.
"

> Page 19, OptFragmentSize
Delete “of” before “the Hop Limit”.

Unsure: The Hop Limit is a field that can be compressed at the first hop and will need to be expanded later.

Many thanks for your care, Peter. I looked up and fixed other occurrences of the similar French and I hope the doc reads better now. 

If you agree with the points above I'll submit a new round asap, maybe together with another review.

All the best,

Pascal

---

From: Alissa Cooper <[email-masked]>
Date: Wed, 19 Feb 2020 15:40:16 -0500

Peter, thanks for your review. Pascal, thanks for your response. I entered a No Objection ballot.

Best,
Alissa

> On Feb 18, 2020, at 10:21 AM, Pascal Thubert (pthubert) <[email-masked]> wrote:
> 
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art