Repository navigation
Conversation
TimeNowInMilliseconds multiplied xTaskGetTickCount() by 1000 in TickType_t before widening it to 64 bits. With a 32 bit tick count the product wrapped after 2^32 / 1000 ticks, about 71 minutes at 1 kHz, and the clock jumped back to 0. Widen the tick count before multiplying. The FREESCALE_FREE_RTOS and FREESCALE_KSDK_FREERTOS variants divided the tick count by configTICK_RATE_HZ / 1000, which is 0 below 1 kHz, and the 32 bit variant divided in float, which drops milliseconds once the tick count passes 2^24. Use the same integer conversion there. TLS 1.3 ticket ages and the DTLS 1.3 timers read this clock.
tls13.c kept two copies of the TimeNowInMilliseconds platform ladder, one returning word32 for WOLFSSL_32BIT_MILLI_TIME and one returning sword64, with the same doc comment repeated in most branches. Keep one ladder and pick the return type with a local macro. Move it next to LowResTimer in internal.c, so both platform clocks live in one place and the millisecond clock no longer depends on tls13.c. The sys/time.h include moves with it, and the TIME_OVERRIDES branch reuses the time_t and XTIME declarations of the LowResTimer branch above it. Each branch computes what both copies did, with three exceptions. The gettimeofday() and FUSION_RTOS branches convert tv_sec to the return type before multiplying, so the 32 bit variant no longer overflows a 32 bit long and gives the same result mod 2^32. The 32 bit MICROCHIP_TCPIP variants cast the tick count before dividing, as the 64 bit ones did, which gives the same value for their 32 bit tick counters. The LinuxKM branch keeps both bodies, since the 32 bit one uses the NMI safe wc_linuxkm_time_epoch_msecs().
|
There was a problem hiding this comment.
🟡 Changes recommended
The retransmission timer is not restarted after blocked output flushes, risking premature retransmission or timeout failure, and the threaded session-ticket test omits scheduled ACK processing needed for its timer assertions.
2 open issues
What changed in this PR
Implements wolfSSL’s OpenSSL-compatible DTLS timeout APIs so event loops can schedule retransmissions and DTLS 1.3 acknowledgments.
Changes:
- Adds per-connection retransmission and ACK timers with expiry handling, backoff, and reset logic.
- Consolidates millisecond clocks and updates platform-specific timing behavior.
- Adds DTLS timer tests covering expiry, blocked writes, ACKs, and suspended handshakes.
| File | Description |
|---|---|
wolfssl/internal.h |
Adds timer state and internal helper declarations. |
tests/api/test_dtls.h |
Registers the new timer test suite. |
tests/api/test_dtls.c |
Adds timer coverage and updates existing timeout tests. |
src/wolfio.c |
Uses shared timer-state detection for receive timeouts. |
src/tls13.c |
Removes clock implementations moved to internal.c. |
src/ssl_api_dtls.c |
Implements timeout queries and expired-timer handling. |
src/internal.c |
Adds timer helpers and consolidates platform clocks. |
src/dtls13.c |
Integrates retransmission and ACK timer management. |
cmake/config.in |
Exports HAVE_GETTIMEOFDAY to generated configuration. |
Review effort: Balanced
|
@claude review |
|
@wolfSSL-Fenrir-bot review balanced |
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11974
Scan targets checked: wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): cmake/config.in, tests/api/test_dtls.h
Coverage: 6 of 7 in-scope changed file(s) opened by the reviewer; not opened: src/tls13.c
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Balanced
wolfSSL_DTLSv1_get_timeout always returned 0 and reported the full timeout instead of the time left, and wolfSSL_DTLSv1_handle_timeout was a stub that did nothing. OpenSSL style event loops that wait for get_timeout and then call handle_timeout never retransmitted a lost flight. Track two timers on the object. The retransmission timer restarts whenever a handshake message is stored for retransmission (each DTLS 1.2 pool save and DTLS 1.3 retransmit record), whenever the flight is resent, either on a timeout or because the peer retransmitted or acked part of it, including a resend that hit WANT_WRITE, on every timeout backoff, and when a valid handshake message, or in DTLS 1.3 an ACK that leaves nothing to retransmit, resets a backed off timeout to its initial value. Restarting on every stored message rather than only on an empty list keeps a renegotiation or post-handshake flight from inheriting the start time of an older flight that is still kept. A DTLS 1.2 flight counts as sent only in the states DtlsMsgPoolSend accepts for a retransmission, so no timer runs while a flight is still being written or waits for an asynchronous operation. A DTLS 1.3 flight counts as sent while records are kept for it. get_timeout and handle_timeout report no retransmission timer while the handshake is suspended on asynchronous crypto, non-blocking OCSP or a chain verify callback, so a slow signature cannot trigger a retransmission of a partly sent flight. The DTLS 1.3 ack timer runs for a quarter of the timeout while received records wait for an ack that can be sent. It starts when the first of those records is seen and whenever the quick timeout flag is set again. DtlsTimersRunning() reports which timer runs and replaces the inline check in EmbedReceiveFrom, which keeps its behaviour. The timers use TimeNowInMilliseconds, which is now also built for DTLS, so DTLS 1.2 builds without session tickets, PSK or TLS 1.3 no longer count whole seconds. Whole seconds stay the fallback where TLS 1.3 tickets, PSK and DTLS 1.3 do not require a millisecond clock. With USER_TICKS, or on a platform that would land in the gettimeofday() default without HAVE_GETTIMEOFDAY, the library defines no TimeNowInMilliseconds and the timers count LowResTimer seconds instead. Ports with only a seconds clock, such as Xilinx or Nucleus, keep building, and a USER_TICKS application that supplies its own TimeNowInMilliseconds still links. The DEOS clock now reads the tick counter through a const volatile pointer like the DEOS LowResTimer, since DTLS 1.2 builds now compile it. The Zephyr clock skips k_cpu_idle() on native_posix and native_sim like the Zephyr LowResTimer, since idling there advances simulated time and the DTLS timers now read this clock. CMake now exports HAVE_GETTIMEOFDAY to config.h as autotools does. The ladder marks its whole second branches and DtlsTimerLowRes reports that to the timers, so internal.h needs no second copy of the ladder. With a whole second clock the timers round elapsed time down by one tick so a timer never expires early. The ack timer is not rounded down, since an early ACK only makes the peer resend the records it is missing. A timeout of 0 means no timeout, so no timer runs. get_timeout reports at most DTLS_TIMEOUT_MAX seconds, which also keeps the millisecond count from overflowing. DtlsTimerNowMs and DtlsTimerLowRes are visible to the tests and get WOLFSSL_API_PREFIX_MAP aliases. Fix the DTLS 1.3 quick timeout. Dtls13RtxTimeout cleared the flag when it sent the ACKs and then checked it, so a timeout taken after the quarter wait also retransmitted the flight and doubled the timeout. Dtls13RtxTimeout now takes the timer that expired. handle_timeout passes the timer it found expired. The receive path and wolfSSL_dtls_got_timeout do not know how long the caller waited, so with the quick timeout flag set they send only the ACKs while the buffered flight has waited less than half its timeout, and retransmit with backoff once it waited longer. Without a clock a quick timeout only sends the ACKs and the next full timeout retransmits. A quick wait stays under half the timeout and a full one reaches it. A whole second clock is rounded down as for the timers, so a full wait can read as a quick one and retransmit one timeout later. With nothing buffered to retransmit, a timeout with the flag set only sends the ACKs instead of backing off. Flushing the seen records also clears the flag, so a flag left over from the previous flight no longer turns the next full timeout into an ACK-only one. A timeout within DTLS13_MIN_RTX_INTERVAL of the last retransmission no longer doubles the timeout when Dtls13RtxSendBuffered holds the retransmission back. Dtls13RtxSendBuffered now uses Dtls13RtxWaitMs for that check, and a clock that stepped back counts as elapsed, so it no longer holds the retransmission back until the clock catches up. test_dtls_timeout clears the last retransmission time before its second timeout, which falls within that interval of the first one. test_dtls_drop_client_ack takes one timeout instead of two, since the first one no longer finds a flag left over from the handshake. get_timeout now returns 1 with the time left while a timer runs and 0 otherwise, and no longer fills in dtls_timeout when no timer runs, so callers must check the return value. The time left includes any wait before DTLS 1.3 may retransmit again. No timer runs while the object's own output is still pending, so handle_timeout cannot queue a second copy of a blocked write. handle_timeout returns 0 when no timer expired. For an expired retransmission timer it sends any pending ACKs and retransmits with backoff, and returns 1. For an expired DTLS 1.3 ack timer it only sends the ACKs, without retransmitting or backing off, and returns 1. It returns -1 on error, including when the timeout reached its maximum, the ACK hits WANT_WRITE or the timers cannot be read under WOLFSSL_RW_THREADED, which also sets the error, while get_timeout returns 0. At the maximum timeout DTLS 1.3 now sets SOCKET_ERROR_E, as DTLS 1.2 does, instead of -1. Both return 0 when NO_ASN_TIME leaves no clock. Under WOLFSSL_RW_THREADED the DTLS 1.3 ack timer state now sits behind the dtls13Rtx mutex. The read thread sets the ack timer start and the quick timeout flag with it held, the timeout handler reads and clears the flag and the pending ACK state with it held, and get_timeout reads the start in the same locked section as the flag, so it never pairs a set flag with a stale start.
5d8e35d to
031caf4
Compare


wolfSSL_DTLSv1_get_timeout always returned 0 with the full timeout and wolfSSL_DTLSv1_handle_timeout was a stub, so OpenSSL-style event loops never retransmitted a lost flight. This implements both on a per-object millisecond retransmission timer and a DTLS 1.3 ACK timer.
Changes
Testing