Skip to content

Commit 511bc28

Browse files
committed
lib: implement callout_when so TCP timers can fire
callout_when() existed only as an empty body in ff_stub_14_extra.c, a file of link-only stubs for FreeBSD 14 symbols whose defining sources this library does not compile. kern_timeout.c is one of those: it is replaced by ff_kern_timeout.c, which reimplements the callwheel but never carried callout_when across. That silently disabled every TCP timer. tcp_timer_activate() computes a deadline into &tp->t_timers[which] by calling this function, then asks tcp_timer_next() for the earliest pending one. With nothing ever written, the entries kept their initial SBT_MAX, tcp_timer_next() reported that no timer was pending, and the arming path fell through to callout_stop(). Every request to start a retransmit, persist, delayed-ack or keepalive timer stopped it instead. Because FreeBSD 14 drives all five from one callout per connection, a single missing function disabled all of them at once. Implement it in ff_kern_timeout.c, next to the callwheel it feeds, and drop the stub. The stub file states that its contents are link-only, that reaching one at runtime means an unsupported path, and that bodies must not be hand-edited because the file is generated -- so a working implementation cannot live there. A stub for callout_when must not be regenerated into it. The implementation follows callout_when() in sys/kern/kern_timeout.c, less two parts that depend on machinery this library does not build: - Upstream anchors a hardclock-driven callout to the last hardclock edge, read from per-CPU state maintained by kern_clocksource.c. That file is not compiled here, so there is nothing to read and sbinuptime() is used throughout. The deadline can therefore be up to one tick later than upstream would compute, and callouts armed within the same tick are not batched. - Upstream derives a precision floor from C_PRELGET(flags) so the scheduler can coalesce callouts with overlapping tolerance. This callwheel is tick-granular with no sub-tick slack to trade, and the only caller passes precision 0, so the caller's value is passed through unchanged. A second fix is required with it, because the first one exposes it. callout_reset_sbt_on() divided its sbintime by tick_sbt to index the tick-based callwheel. That is correct for a duration and wrong for the absolute deadline tcp_timer_next() passes with C_ABSOLUTE: dividing a deadline yields uptime-in-ticks, scheduling the callout an uptime into the future, and past roughly 24 days of uptime at hz=1000 the tick count exceeds INT_MAX and wraps negative. ff_callout_delay_ticks() subtracts the current uptime when the deadline is absolute, saturates SBT_MAX to INT_MAX, treats an already-past deadline as one tick, and rounds up so a callout cannot fire early. Relative callers keep the previous arithmetic exactly. Impact before the fix: a connection died on its first lost segment. Nothing retransmitted it, so snd_una never advanced, the congestion window stayed full of unacknowledged data, tcp_output() computed len=0 indefinitely, and the send buffer could never drain -- writes returned EAGAIN for as long as the process lived. Small responses never exposed this, because a few hundred bytes never put enough in flight to lose any. Testing: - Reproducer: one TLS connection transferring a 1 MB object through a userspace proxy pinned to a single lcore. Before the fix the transfer stalled at a repeatable byte count (364788 with a 512 KB send buffer) and then hung, every subsequent write returning EAGAIN. After the fix the transfer completes and a single connection sustains about 297 MB/s. - Instrumented the arming path to confirm retransmit, delayed-ack and keepalive callouts are entered into the callwheel and fire. Before the fix the callwheel stayed empty. - Rebuilt lib/ clean under its own -Werror -Wmissing-prototypes -Wstrict-prototypes flags, and confirmed with nm that callout_when and ff_callout_delay_ticks have exactly one definition each after the move. - Environment: DPDK 24.11.6, vmxnet3 under VMware bound with uio_pci_generic, one lcore, load generated from a separate host. Testing was only possible on vmxnet3 in a virtual machine. The change is in generic timer code rather than anything driver specific, but it has not been exercised on a physical NIC. Signed-off-by: Lijun Wang <83639177+lijunwangs@users.noreply.github.com>
1 parent 34065f1 commit 511bc28

3 files changed

Lines changed: 82 additions & 7 deletions

File tree

‎freebsd/sys/callout.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,10 @@ void _callout_init_lock(struct callout *, struct lock_object *, int);
9393
#define callout_pending(c) ((c)->c_iflags & CALLOUT_PENDING)
9494
int callout_reset_tick_on(struct callout *, int, void (*)(void *),
9595
void *, int, int);
96+
int ff_callout_delay_ticks(sbintime_t, int);
9697
#define callout_reset_sbt_on(c, sbt, pr, fn, args, cpu, flags) \
97-
callout_reset_tick_on((c), (sbt)/tick_sbt, (fn), (args), (cpu), (flags))
98+
callout_reset_tick_on((c), ff_callout_delay_ticks((sbt), (flags)), \
99+
(fn), (args), (cpu), (flags))
98100
#define callout_reset_sbt(c, sbt, pr, fn, arg, flags) \
99101
callout_reset_sbt_on((c), (sbt), (pr), (fn), (arg), -1, (flags))
100102
#define callout_reset_sbt_curcpu(c, sbt, pr, fn, arg, flags) \

‎lib/ff_kern_timeout.c‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ __FBSDID("$FreeBSD$");
5656
#include <sys/systm.h>
5757
#include <sys/bus.h>
5858
#include <sys/callout.h>
59+
#include <sys/limits.h> /* INT_MAX, for the saturated tick delay */
60+
#include <sys/time.h> /* tick_sbt, sbinuptime() */
5961

6062
/*
6163
* F-Stack: 14.0+ removed CALLOUT_LOCAL_ALLOC and CS_EXECUTING.
@@ -326,6 +328,83 @@ callout_get_bucket(int to_ticks)
326328
return (to_ticks & callwheelmask);
327329
}
328330

331+
/*
332+
* Compute the absolute deadline a callout should fire at.
333+
*
334+
* Follows callout_when() in sys/kern/kern_timeout.c, less two parts that
335+
* depend on machinery this library does not build. Upstream anchors a
336+
* hardclock-driven callout to the last hardclock edge, read from per-CPU
337+
* state maintained by kern_clocksource.c; that file is not compiled here, so
338+
* there is nothing to read and sbinuptime() is used throughout. Upstream also
339+
* derives a precision floor from C_PRELGET(flags) so the scheduler can batch
340+
* callouts with overlapping tolerance; this callwheel is tick-granular and has
341+
* no sub-tick slack to trade, so the caller's precision is passed through.
342+
*
343+
* This lived as an empty stub in ff_stub_14_extra.c, which silently disabled
344+
* every TCP timer: tcp_timer_activate() writes a deadline into
345+
* tp->t_timers[which] through this function, tcp_timer_next() then takes the
346+
* earliest, and with nothing ever written the entries kept their initial
347+
* SBT_MAX, so no timer looked pending and the arming path called
348+
* callout_stop() instead.
349+
*/
350+
void
351+
callout_when(sbintime_t sbt, sbintime_t precision, int flags,
352+
sbintime_t *sbt_out, sbintime_t *precision_out)
353+
{
354+
sbintime_t to_sbt;
355+
356+
if ((flags & (C_ABSOLUTE | C_PRECALC)) != 0) {
357+
*sbt_out = sbt;
358+
*precision_out = precision;
359+
return;
360+
}
361+
/* A hardclock-based timer cannot resolve finer than one tick. */
362+
if ((flags & C_HARDCLOCK) != 0 && sbt < tick_sbt)
363+
sbt = tick_sbt;
364+
365+
to_sbt = sbinuptime();
366+
/*
367+
* Saturate rather than wrap. Testing to_sbt + sbt directly would have to
368+
* overflow a signed type to find out, which is undefined; and a wrapped
369+
* result is negative, which reads as already due and spins the callout
370+
* instead of sleeping.
371+
*/
372+
if (SBT_MAX - to_sbt < sbt)
373+
to_sbt = SBT_MAX;
374+
else
375+
to_sbt += sbt;
376+
377+
*sbt_out = to_sbt;
378+
*precision_out = precision;
379+
}
380+
381+
/*
382+
* Convert what callout_reset_sbt_on() was handed into a delay in ticks, which
383+
* is what this callwheel is indexed by.
384+
*
385+
* The macro used to divide by tick_sbt unconditionally. That is right for a
386+
* duration and wrong for the absolute deadline tcp_timer_next() passes with
387+
* C_ABSOLUTE: dividing a deadline by tick_sbt yields uptime-in-ticks, so the
388+
* callout is scheduled an uptime into the future, and past roughly 24 days at
389+
* hz=1000 the tick count exceeds INT_MAX and wraps negative.
390+
*/
391+
int
392+
ff_callout_delay_ticks(sbintime_t sbt, int flags)
393+
{
394+
sbintime_t now;
395+
396+
if ((flags & C_ABSOLUTE) == 0)
397+
return ((int)(sbt / tick_sbt));
398+
399+
if (sbt == SBT_MAX)
400+
return (INT_MAX); /* "never", as the caller meant it */
401+
now = sbinuptime();
402+
if (sbt <= now)
403+
return (1); /* already due: next tick, never 0 or negative */
404+
/* Round up: truncating would fire the callout before its deadline. */
405+
return ((int)((sbt - now) / tick_sbt) + 1);
406+
}
407+
329408
void
330409
callout_tick(void)
331410
{

‎lib/ff_stub_14_extra.c‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,6 @@ void buf_ring_free(struct buf_ring *br, struct malloc_type *type)
146146

147147
}
148148

149-
void callout_when(sbintime_t sbt, sbintime_t precision, int flags, sbintime_t *sbt_out, sbintime_t *precision_out);
150-
void callout_when(sbintime_t sbt, sbintime_t precision, int flags, sbintime_t *sbt_out, sbintime_t *precision_out)
151-
{
152-
153-
}
154-
155149
vm_paddr_t dump_avail[16] = {0};
156150

157151
void fdescfree_adapt_use(struct proc *p);

0 commit comments

Comments
 (0)