[aqm] Spencer Dawkins' Yes on draft-ietf-aqm-fq-codel-05: (with COMMENT)

["Spencer Dawkins" <[e-mail]>]

Spencer Dawkins has entered the following ballot position for
draft-ietf-aqm-fq-codel-05: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-aqm-fq-codel/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Very nice work. I have some nit-ish questions I hope you'll consider, but
nothing blocking.

This text in the Introduction,

   The FQ-CoDel algorithm is a combined packet scheduler and AQM
   developed as part of the bufferbloat-fighting community effort.  It
   is based on a modified Deficit Round Robin (DRR) queue scheduler,
   with the CoDel AQM algorithm operating on each queue.
   
introduces both CoDel and DDR, but they don't have references until
Section 1.3. Maybe move that forward in the doc?

Perhaps this text:

   FQ-CoDel stochastically classifies incoming packets into different
   queues by hashing the 5-tuple of IP protocol number and source and
   destination IP and port numbers, perturbed with a random number
   selected at initiation time (although other flow classification
   schemes can optionally be configured instead). 
   
would benefit from a forward reference to Section 4.1.1?

Just as a nit, you have an "is is" in Section 3.

In this text:

   After having selected a queue from which to dequeue a packet, the
   CoDel algorithm is invoked on that queue.  This applies the CoDel
   control law, 
   
"CoDel control law" hasn't been introduced, and isn't included in Section
1.2. Could you provide a reference or pointer?

In this text:

   This parameter can be set only at load time since memory has to be
   allocated for the hash table in the current implementation.
   
if "in the current implementation" refers to "can be set only at load
time", 

   This parameter can be set only at load time in the current
implementation,
   since memory has to be allocated for the hash table.
   
might be clearer.

I wonder if 

   while CoDel itself can take a while
   to respond, fq_codel doesn't miss a beat.
   
is clear to non-native English language readers?

In this text:

   In the presence of queue management schemes that contain latency
   under load, 
   
"contain" means something like "limit" right? That wasn't what I
understood in my first parsing attempt.

I'm confused by this text:

   o  Packet fragments without a layer 4 header can be hashed into
      different bins than the first fragment with the header intact.
      This can cause reordering and/or adversely affect the performance
      of the flow.  Keeping state to match the fragments to the
      beginning of the packet, or simply putting all fragmented packets
      into the same queue, are two ways to alleviate this.
      
Is this "all fragmented packets", or "all packet fragments"?

If it's "all fragmented packets", don't you have to reassemble the
fragments? 

If it's "all packet fragments", the packet fragments would all be
transmitted in order, but could the fragments with headers and without
headers of a single packet be reordered?

But either way, I have a question :-) ...

Do we say things like 

   In this document we have documented the Linux
   implementation in sufficient detail for an independent
   implementation, and we encourage such implementations be widely
   deployed.

in Experimental RFCs?