From: Bjorn Ketelaars <bket@openbsd.org>
Subject: Re: fq_codel: fix CoDel drop scheduling
To: tech@openbsd.org
Date: Thu, 13 Aug 2026 03:29:48 +0200

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);