[Bug 276890] Getting fq_codel correct on inbound shaping

From: <bugzilla-noreply_at_freebsd.org>
Date: Sun, 25 Feb 2024 01:18:05 UTC
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=276890

--- Comment #5 from Dave Taht <dave.taht@gmail.com> ---

I am a little dubious about the "active" idea.

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

Also.

 if (!si->flows[idx].active ) {
  319                 STAILQ_INSERT_TAIL(&si->newflows, &si->flows[idx],
flowchain);
  320                 si->flows[idx].deficit = param->quantum;
  321                 si->flows[idx].cst.dropping = false;
  322                 si->flows[idx].cst.first_above_time = 0;
  323                 si->flows[idx].active = 1;
  324                 //D("activate %d",idx);
  325         }

In the linux version we do not touch the codel state variables at this phase at
all, but retain the previous settings. It may be that dropping and
first_above_time get set in roughly the same way in my version, but perhaps if
I describe the intent of what should happen, I too will understand this code
better. 

The idea of a flow going out of an "active" state is not that any of it´s state
needs to be reset. The overall target of fq_codel is to reduce the total delay
in all the queues to the target (usually 5ms). It maintains a cache of the last
"good" drop rate. 

If a single flow, out of dozens, has an arrival rate like this:

A                      A                    A                 A

And that is still too much relative to the other flows, it needs to get more
drops. 

Anyway, instead of checking for or maintaining an active or inactive "state" we
just check to see if queue length > 0.

Just saving my state here on this subtley. Also we use the global queue length
not the per queue length to turn off the global dropper, which I have to go
looking through here to see if it is correct.

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