The FlowQueue-CoDel Packet Scheduler and Active Queue Management Algorithm
draft-ietf-aqm-fq-codel-06

Note: This ballot was opened for revision 05 and is now closed.

Search Mailarchive

Alissa Cooper Yes

Spencer Dawkins Yes

Comment (2016-03-16 for -05)
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?

(Stephen Farrell) Yes

Comment (2016-03-17 for -05)
Good stuff. Thanks.

(Martin Stiemerling) Yes

(Jari Arkko) No Objection

Alia Atlas No Objection

Comment (2016-03-17 for -05)
I think it would be useful to have a reference to the Linux implementation ("current" version and pointer).

Deborah Brungard No Objection

Ben Campbell No Objection

Benoit Claise No Objection

Comment (2016-03-16 for -05)
- Is the following really necessary: 

   In this document, these words will appear with that interpretation
   only when in ALL CAPS.  Lower case uses of these words are not to be
   interpreted as carrying [RFC2119] significance.

- section 6
   While FQ-CoDel has been shown in many scenarios to offer significant
   performance gains, there are some scenarios where the scheduling
   algorithm in particular is not a good fit. 

Gains compared to?

- From J├╝rgen's OPS DIR review:
The working draft still says this:

  and we encourage such implementations be widely deployed

It is unclear what 'we' is. This is something I think that needs to be
fixed since people will come up with different interpretation of such
a recommendation. (In a scientific paper, it would be clear that 'we'
refers to the authors but in documents coming out of IETF WGs, the
notion of what is 'we' is not so clear anymore.

(Joel Jaeggli) No Objection

(Barry Leiba) No Objection

Terry Manderson No Objection

Alvaro Retana No Objection