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);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.