diff --git a/rtl837x_common.h b/rtl837x_common.h index 737a6829..9b601730 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -7,6 +7,14 @@ #define SYS_TICK_HZ 200 +/* Timers of the main loop count system ticks, not loop passes: a pass is as + * long as the packets it handles, and since #483 the loop no longer sleeps + * while packets wait, so passes can outrun the tick as well as lag it. `last` + * is the low byte of ticks when the timer last ran; the byte arithmetic wraps + * every 256 ticks, so a pass longer than 1.28 s loses time rather than + * running the timer 64 times to catch up. */ +#define TICKS_DUE(last, period) ((uint8_t)((uint8_t)ticks - (last)) >= (period)) + #define CPU_PORT 9 #define NUL '\0' diff --git a/rtlplayground.c b/rtlplayground.c index 80f599cb..cd1281f7 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -88,11 +88,13 @@ __xdata struct uip_eth_addr uip_ethaddr; volatile __xdata uint32_t ticks; volatile __xdata uint8_t sec_counter; volatile __xdata uint16_t sleep_ticks; -__xdata uint8_t stp_clock; +__xdata uint8_t stp_tick_last; +__xdata uint8_t tx_tick_last; __xdata uint8_t arp_age_secs; extern __xdata struct dhcp_state dhcp_state; -#define STP_TICK_DIVIDER 3 +#define STP_TICK_STEP (SYS_TICK_HZ / STP_HZ) +#define STP_CATCH_UP 8 /* Buffer for serial input, SBUF_SIZE must be power of 2 < 256 * Writing to this buffer is under the sole control of the serial ISR @@ -1075,6 +1077,9 @@ void handle_rx(void) void handle_tx(void) { + if (!TICKS_DUE(tx_tick_last, 1)) + return; + tx_tick_last = (uint8_t)ticks; for(uint8_t i = 0; i < UIP_CONNS; i++) { uip_periodic(i); if(uip_len > 0) { @@ -1265,13 +1270,12 @@ void idle(void) // Check UIP for packets to transmit handle_tx(); health_phase(HEALTH_PH_TX); - // If STP protocol enabled, decrease STP timers to trigger actions + // If STP protocol enabled, run its timers once per STP tick that has passed if (stp_enabled) { - if (!stp_clock) { - stp_clock = STP_TICK_DIVIDER; + uint8_t n = STP_CATCH_UP; + while (n-- && TICKS_DUE(stp_tick_last, STP_TICK_STEP)) { + stp_tick_last += STP_TICK_STEP; stp_timers(); - } else { - stp_clock--; } } health_phase(HEALTH_PH_STP); @@ -1597,7 +1601,7 @@ void check_and_flash_update_image(void) void main(void) { ticks = 0; - stp_clock = STP_TICK_DIVIDER; + stp_tick_last = (uint8_t)ticks; dhcp_state.state = DHCP_OFF; sbuf_ptr = 0; diff --git a/test/Makefile b/test/Makefile index e394aec4..ae3884ac 100644 --- a/test/Makefile +++ b/test/Makefile @@ -27,7 +27,8 @@ BUILDDIR = build TESTS = $(BUILDDIR)/test_cmd_editor \ $(BUILDDIR)/test_port_tables \ $(BUILDDIR)/test_page_json \ - $(BUILDDIR)/test_httpd_tx + $(BUILDDIR)/test_httpd_tx \ + $(BUILDDIR)/test_tick_gate # The table tests link the register mock and one environment file; stub/ # stands in for the two headers a firmware build generates. @@ -47,6 +48,9 @@ $(BUILDDIR)/test_cmd_editor: test_cmd_editor.c support.c $(SRCROOT)/cmd_editor.c $(BUILDDIR)/test_port_tables: test_port_tables.c $(TABLE_ENV) hw_mock.h | $(BUILDDIR) $(CC) $(CFLAGS) $(filter %.c,$^) -o $@ $(LDFLAGS) +$(BUILDDIR)/test_tick_gate: test_tick_gate.c support.c $(SRCROOT)/cmd_editor.c | $(BUILDDIR) + $(CC) $(CFLAGS) $^ -o $@ $(LDFLAGS) + $(BUILDDIR)/test_page_json: test_page_json.c $(TABLE_ENV) hw_mock.h | $(BUILDDIR) $(CC) $(CFLAGS) $(filter %.c,$^) -o $@ $(LDFLAGS) diff --git a/test/test_tick_gate.c b/test/test_tick_gate.c new file mode 100644 index 00000000..3a9a62b9 --- /dev/null +++ b/test/test_tick_gate.c @@ -0,0 +1,83 @@ +/* + * test_tick_gate.c - the timers of the main loop keep time by the system + * tick, however often the loop passes. The gate is the TICKS_DUE macro from + * rtl837x_common.h; the loop bodies of idle() and handle_tx() are restated + * here, so this checks the arithmetic, not those functions. + */ +#include +#include + +#include "rtl837x_common.h" +#include "rtl837x_stp.h" +#include "support.h" + +volatile uint32_t ticks; + +#define STP_TICK_STEP (SYS_TICK_HZ / STP_HZ) +#define STP_CATCH_UP 8 + +static uint8_t stp_tick_last, tx_tick_last; +static unsigned stp_calls, tx_calls; + +/* One pass of the loop: the STP block of idle() and the gate of handle_tx() */ +static void pass(void) +{ + uint8_t n = STP_CATCH_UP; + while (n-- && TICKS_DUE(stp_tick_last, STP_TICK_STEP)) { + stp_tick_last += STP_TICK_STEP; + stp_calls++; + } + if (TICKS_DUE(tx_tick_last, 1)) { + tx_tick_last = (uint8_t)ticks; + tx_calls++; + } +} + +/* Run `seconds` of wall clock with one pass every `pass_us` microseconds. */ +static void run(double pass_us, unsigned seconds) +{ + double t = 0, next_tick = 0, tick_us = 1e6 / SYS_TICK_HZ, end = seconds * 1e6; + ticks = 0; + stp_tick_last = tx_tick_last = 0; + stp_calls = tx_calls = 0; + while (t < end) { + while (next_tick <= t) { ticks++; next_tick += tick_us; } + pass(); + t += pass_us; + } +} + +static void t_rates(void) +{ + printf("[test] STP time follows the tick whatever the loop does\n"); + run(5000, 10); + CHECK(stp_calls == 10 * STP_HZ, "one pass per tick: 500 STP steps in 10 s"); + CHECK(tx_calls == 10 * SYS_TICK_HZ, "and 2000 TX periodics"); + run(16000, 10); + CHECK(stp_calls >= 10 * STP_HZ - 1 && stp_calls <= 10 * STP_HZ, "a 16 ms pass, as under an IPv4 stream: still 500 (was 156 when the clock counted passes)"); + CHECK(tx_calls == 625, "TX periodic once per pass when passes are slower than the tick"); + run(600, 10); + CHECK(stp_calls == 10 * STP_HZ, "a 0.6 ms pass, the loop no longer sleeping: still 500 (was 4166)"); + CHECK(tx_calls == 10 * SYS_TICK_HZ, "TX periodic capped at the tick rate"); + run(1300000, 13); + CHECK(stp_calls <= 10 * STP_CATCH_UP, "a 1.3 s pass loses time instead of firing 65 steps at once"); +} + +static void t_wrap(void) +{ + printf("[test] the byte arithmetic survives the tick counter wrapping\n"); + ticks = 0xfffffffeUL; + stp_tick_last = (uint8_t)ticks; + stp_calls = 0; + for (int i = 0; i < 8; i++) { ticks++; pass(); } + CHECK(stp_calls == 2, "two STP steps across 0xffffffff -> 0x00000006"); +} + +int main(void) +{ + printf("== tick gate tests ==\n"); + t_rates(); + t_wrap(); + printf("\n%d checks, %d failed\n", tests_run, tests_failed); + return tests_failed ? 1 : 0; +}