[Bug 276890] Getting fq_codel correct on inbound shaping

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=276890

--- Comment #4 from Dave Taht <dave.taht@gmail.com> ---
Assuming I am looking at a correct source tree:

http://fxr.watson.org/fxr/source/netpfil/ipfw/dn_sched_fq_codel.c#L330

Logging an overflow event twice, and to the console, while under stress, is not
a good idea.

  if (mainq->ni.length > schk->cfg.limit) { D("over limit");
  331                 /* find first active flow */
  332                 for (maxidx = 0; maxidx < schk->cfg.flows_cnt; maxidx++)
  333                         if (si->flows[maxidx].active)
  334                                 break;
  335                 if (maxidx < schk->cfg.flows_cnt) {
  336                         /* find the largest sub- queue */
  337                         for (i = maxidx + 1; i < schk->cfg.flows_cnt;
i++) 
  338                                 if (si->flows[i].active &&
si->flows[i].stats.length >
  339                                         si->flows[maxidx].stats.length)
  340                                         maxidx = i;
  341                         codel_drop_head(&si->flows[maxidx], si);
  342                         D("maxidx = %d",maxidx);
  343                         drop = 1;
  344                 }
  345         }

I would delete both Ds here. Even then there are two things we ended up doing
in the linux version - 1) We added the drop_batch facility (and optimized it)
to drop up to 64 packets at a time from the head (all but the last - it helps
to always deliver at least the last packet in the queue).  

It was *very* expensive under a packet flood to hit this limit, search the flow
list, then drop a single packet.

2) Also merely dropping the packet without also telling the AQM to drop harder
on its own led to persistent hitting of this spot. So we also incremented the
codel count variable on this drop in the expectation the AQM would eventually
catch up.

3) It is possible to keep extra state around to always track the fattest queue
(see fq_codel_fast) and eliminate this non-O(1) search, at the cost of tracking
the fattest queue elsewhere. The expectation is that in normal use, this
routine is rarely hit.

-- 
You are receiving this mail because:
You are the assignee for the bug.