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

            Bug ID: 298731
           Summary: dummynet: config_aqm() "flowset busy" check rejects
                    CoDel/PIE on masked schedulers unconditionally and
                    silently strips AQM on flowset reconfiguration
           Product: Base System
           Version: CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: kern
          Assignee: [email protected]
          Reporter: [email protected]

## Component

kern (sys/netpfil/ipfw/ip_dummynet.c). Reaches every dummynet user: ipfw
`pipe`/`queue`, dnctl(8), and pf `dnpipe`/`dnqueue`.

## Environment

FreeBSD 15.1-RELEASE-p3 kernel as shipped by OPNsense 26.7.4_1
(stable/26.7-n283949), `dummynet.ko` + `ipfw.ko`, dnctl(8). dnctl is ipfw's
dummynet front end, so every command below is `ipfw <same args>` on a stock
system. The code cited is unchanged between that kernel and `main`; it dates
from the 2016 AQM import.

## Summary

`config_aqm()` refuses to (re)configure CoDel/PIE on a flowset when it is
"busy". The busy test is `locate_scheduler(nfs->sched_nr)->siht != NULL`
(ip_dummynet.c:1681 and 1705-1706). That predicate is wrong in three ways, a
fourth defect makes the wrong path the common one, and none of it reaches
userland.

1. Masked schedulers can never take AQM. For a scheduler with a mask, `siht` is
a hash table allocated when the scheduler is created (`schk_new`, line
882-885), so it is never NULL and the flowset is "busy" from birth. Two
configurations hit this:
   - `pipe N config ... mask ... codel|pie`: `config_sched()` moves the pipe's
mask onto the FIFO scheduler `N + DN_MAX_ID` (lines 1890-1908), so it is the
pipe's own internal flowset that is refused. (Queues attached to such a pipe
are not affected: they attach to scheduler `N`, which `config_sched` created
without a mask, lines 1792-1797.)
   - `sched N config ... mask ...` followed by `queue M config sched N
codel|pie`: the queue's flowset is refused.
   Reproduced idle with zero traffic, `si_count` 0, for both CoDel and PIE.

2. Changing any parameter of a flowset whose scheduler has an instance destroys
its AQM. For flowsets on a MULTIQUEUE scheduler (queues under wf2q+, qfq, ...)
the per-queue AQM state lives in the flowset's own queues (`q_new` line 377-378
inits it, `dn_delete_queue` line 400-401 frees it), but the busy test looks at
the shared scheduler's instances, so traffic on any sibling queue counts. In
the changed-parameters path `config_fs()` has already called `fsk_detach(fs,
DN_DETACH|DN_DESTROY)` (line 1699), destroying every queue of this flowset and
deconfiguring its AQM (`aqm_cleanup_deconfig_fs`, 714-743, from `fsk_detach`
line 774). Then `fs->fs = *nfs` (1701) copies the userland flags, which include
`DN_IS_AQM` (userland sets it at sbin/ipfw/dummynet.c:1593), `fs->aqmfp` is set
to NULL (1703), and `config_aqm()` (1705) is refused. Result: `DN_IS_AQM` set,
`aqmfp == NULL`. The data path keys on `aqmfp` (ip_dn_io.c:502-503), so the
flowset tail-drops. Changing a queue's weight, size or buckets while its pipe
carries traffic therefore silently disables AQM on it.

3. Unchanged parameters: new AQM parameters are silently ignored. In the
unchanged path (1671-1684) a busy flowset keeps its old AQM configuration;
`target`/`interval`/`ecn` changes are dropped.

4. For masked flowsets, an identical re-apply takes path 2. `fsk_attach()` sets
`DN_QHT_HASH` in `fs->fs.flags` when the flowset has a mask (1324-1327).
Userland never sends that flag (no reference in sbin/ipfw/dummynet.c), so
`bcmp(&fs->fs, nfs, ...)` at 1671 can never match for a masked flowset.
Re-issuing the same `queue N config pipe P mask ... codel` line while the pipe
has an instance destroys that queue's flows and its AQM. This is what a config
reload does on OPNsense and pfSense, which re-run their generated dnctl/ipfw
rules over the live objects on every filter reload.

In all cases nothing reaches userland: `config_fs()` discards `config_aqm()`'s
return value (1681, 1705) and `do_config()` only checks for a NULL flowset
(2136). `ipfw`/`dnctl` exit 0. The only trace is `D("Unable to configure
flowset, flowset busy!")` (1491) in dmesg. `ipfw queue show` does not even say
"droptail": sbin/ipfw/dummynet.c:510-514 sees `DN_IS_AQM` and asks the kernel
for parameters, the kernel skips the copyout when `aqmfp` is NULL
(ip_dummynet.c:1392), and the line ends after `pri 0` with nothing. That is why
this shows up downstream as "harmless log spam" (opnsense/core#1279, pfSense
redmine #8991) while the AQM is not applied.

## Observed (idle, si_count 0)

```
# dnctl pipe 60001 config bw 10Mbit/s mask dst-ip 0xffffffff codel; echo rc=$?
rc=0
# dnctl pipe 60001 show
60001:  10.000 Mbit/s    0 ms burst 0
q191073  50 sl. 0 flows (1 buckets) sched 125537 weight 0 lmax 0 pri 0
 sched 125537 type FIFO flags 0x1 256 buckets 0 active
    mask:  0x00 0x00000000/0x0000 -> 0xffffffff/0x0000
# dmesg | tail -1
[13641] config_aqm Unable to configure flowset, flowset busy!
```
Control, same command without `mask`:
```
q191074  50 sl. 0 flows (1 buckets) sched 125538 weight 0 lmax 0 pri 0  AQM
CoDel target 5ms interval 100ms NoECN
```
`pipe 60004 config bw 10Mbit/s mask dst-ip 0xffffffff pie`: same refusal, same
empty tail. `sched 60003 config type wf2q+ mask dst-ip 0xffffffff` then `queue
60003 config sched 60003 codel`: refused. `queue 60001 config pipe 60001 codel`
(queue under the masked *pipe*): accepted, shows `AQM CoDel`. Three commands,
three busy lines, every exit status 0.

## Observed under load

Setup: `pipe 60010 config bw 300Mbit/s`; `queue 60010 config pipe 60010 weight
10 codel`; `queue 60011 config pipe 60010 mask src-ip 0xffffffff weight 10
codel`; one `ipfw add queue` rule per queue matching a loopback TCP port; `dd
if=/dev/zero | nc` through each. Both queues show `AQM CoDel`, `si_count` 1,
`queue_count` 2.

```
# dnctl queue 60010 config pipe 60010 weight 10 codel target 20ms; echo rc=$?  
 (3: unchanged params)
rc=0
q60010  50 sl. 1 flows (1 buckets) sched 60010 weight 10 lmax 0 pri 0  AQM
CoDel target 5ms interval 100ms NoECN
# dnctl queue 60010 config pipe 60010 weight 20 codel; echo rc=$?              
 (2: changed weight)
rc=0
q60010  50 sl. 0 flows (1 buckets) sched 60010 weight 20 lmax 0 pri 0
# dnctl queue 60011 config pipe 60010 mask src-ip 0xffffffff weight 10 codel;
echo rc=$?   (4: identical re-apply)
rc=0
q60011  50 sl. 0 flows (256 buckets) sched 60010 weight 10 lmax 0 pri 0
    mask:  0x00 0xffffffff/0x0000 -> 0x00000000/0x0000
```
Three more busy lines in dmesg, exit status 0 throughout. `0 flows` on the last
two shows the in-flight queues were destroyed by `fsk_detach`. After the
transfers ended and the idle instance was drained (`si_count` back to 0, about
35 s), the identical `weight 20 codel` and masked re-apply commands both
succeeded and `AQM CoDel` returned, which confirms the instance test is the
only thing standing in the way.

## Seen in production

A router running the same kernel logged `config_aqm Unable to configure
flowset, flowset busy!` three times on 2026-09-18 during an ordinary OPNsense
shaper apply, and three of its six CoDel queues were left in the empty-tail
state above. Neither the GUI nor dnctl reported anything.

## Why the check exists

For a !MULTIQUEUE (FIFO) scheduler the queue is embedded in each scheduler
instance and its AQM status is initialised in `si_new` (545-551); for
MULTIQUEUE schedulers it is initialised per queue in `q_new` (377-378).
Reconfiguring AQM under existing queues would leave them with a NULL or foreign
`aqm_status`. Both AQMs already guard the NULL case by dropping
(`aqm_codel_enqueue` dn_aqm_codel.c:237-241, `aqm_pie_enqueue`
dn_aqm_pie.c:490-496), and PIE defers freeing its status through its own
callout (`aqm_pie_cleanup` dn_aqm_pie.c:645-676). So the check protects the
FIFO/unchanged case; it is over-broad everywhere else.

## Proposed fix

Minimal (correct the predicate and stop lying to userland):

- Count instances instead of testing the table pointer, exactly as the `show`
path already does at 981-982: `(s->sch.flags & DN_HAVE_MASK) ?
dn_ht_entries(s->siht) : (s->siht ? 1 : 0)`.
- For flowsets on a MULTIQUEUE scheduler, test the flowset's own queues instead
(pattern at 1070-1071: `(fs->fs.flags & DN_QHT_HASH) ? dn_ht_entries(fs->qht) :
(fs->qht ? 1 : 0)`).
- In the changed-parameters path (1705) the flowset has no queues left after
`fsk_detach(DN_DESTROY)`; for MULTIQUEUE schedulers pass `busy = 0`. For the
FIFO scheduler the `si+1` queues still exist, so keep the instance count there.
- Exclude `DN_QHT_HASH` (kernel-owned) from the `bcmp` at 1671, or have
userland set it, so an identical re-apply of a masked flowset is the no-op it
is for an unmasked one.
- Make `config_fs()` return the AQM failure to `do_config()` so `ipfw`/`dnctl`
exit non-zero, and on failure clear `DN_IS_AQM` so `show` at least says
`droptail`.

Complete (make reconfiguration safe rather than refused): `config_aqm()`
already calls `aqm_cleanup_deconfig_fs(fs)` before reconfiguring (1496-1499).
Adding the symmetric operation after a successful `aqmfp->config()`, i.e.
walking the existing queues (`si+1` per scheduler instance for !MULTIQUEUE,
`fs->qht` for MULTIQUEUE) and calling `aqmfp->init()` on each, would let the
busy check go away entirely and keep traffic flowing. PIE's deferred free (old
status freed by its callout, `q->aqm_status` set to NULL under `pst->lock_mtx`)
looks compatible with an immediate re-init, but that needs review by someone
familiar with the PIE locking.

## Workaround

In use on OPNsense via a patched dnctl.conf template: emit `queue N delete` /
`pipe N delete` before every `config` line, so each object is created with its
AQM in one command and there is never a busy flowset to reconfigure. Costs the
in-flight packets of that pipe once per reload.

## Context

Behaviour dates from the 2016 AQM import. OPNsense's shaper drives dummynet
through ipfw `queue` rules plus dnctl(8) (opnsense/core#10893: CoDel/PIE queues
running droptail after apply); pfSense's drives it through pf
`dnpipe`/`dnqueue` (redmine #8991). Since 14.0 pf hands packets to dummynet by
number (`pf.c`, `ip_dn_io.c` 906-908), and both front ends configure it through
the same `IP_DUMMYNET3` socket option, so the fix is needed in the kernel, not
in either front end.

---
This report was drafted with AI assistance (Claude); the code trace and the
live reproduction were reviewed by the submitter.

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

Reply via email to