Re: fq_codel: fix CoDel drop scheduling
Bjorn Ketelaars <[email protected]>
| Newsgroups | gmane.os.openbsd.tech |
|---|---|
| Message-ID | <[email protected]> |
On Wed 29/07/2026 08:23, Bjorn Ketelaars wrote:
> Hi,
>
> The diff below fixes a few details in fq_codel's CoDel dequeue path.
>
> The interval table starts at interval / sqrt(1), while cd->drops is
> one-based when control_law() is called. Use drops - 1 as the table index
> and assert that the counter is non-zero.
>
> When checking the non-starvation backlog during dequeue, exclude the
> candidate packet. The packet is still physically queued until
> deq_commit(), but the CoDel decision is about the backlog remaining
> after this packet is dequeued.
>
> Finally, enter dropping when now is equal to cd->start. cd->start marks
> the time at which the observation interval has elapsed.
>
> Comments / ok?
Ping, diff enclosed again for your convenience:
diff --git sys/net/fq_codel.c sys/net/fq_codel.c
index eb49c18aa5c..4179ee5bacd 100644
--- sys/net/fq_codel.c
+++ sys/net/fq_codel.c
@@ -334,7 +334,8 @@ control_law(struct codel *cd, struct codel_params *cp, int64_t rts)
{
unsigned int idx;
- idx = min(cd->drops, nitems(codel_intervals) - 1);
+ KASSERT(cd->drops > 0);
+ idx = min(cd->drops - 1, nitems(codel_intervals) - 1);
cd->next = rts + cp->intervals[idx];
}
@@ -366,8 +367,9 @@ codel_next_packet(struct codel *cd, struct codel_params *cp, int64_t now,
return (NULL);
}
+ KASSERT(cd->backlog >= m->m_pkthdr.len);
if (now - m->m_pkthdr.ph_timestamp < cp->target ||
- cd->backlog <= cp->quantum) {
+ cd->backlog - m->m_pkthdr.len <= cp->quantum) {
/*
* The minimum delay decreased below the target, reset
* the current observation interval.
@@ -385,7 +387,7 @@ codel_next_packet(struct codel *cd, struct codel_params *cp, int64_t now,
* next packet.
*/
cd->start = now + cp->interval;
- } else if (now > cd->start) {
+ } else if (now >= cd->start) {
*drop = 1;
}
return (m);