rtlplayground: keep the STP and TCP timers on the system tick

stp_timers() ran every fourth pass of the main loop and stp_clock counted
passes, which only measures time while a pass is one tick long. It never
quite was: a pass is as long as the packets it handles, and #482 now takes
up to four of them, so under an IPv4 stream a pass is about 16 ms and STP
time runs at a third of its rate, with hellos 6 s apart and a root aged out
after a minute. Since #483 the loop also stops sleeping while packets wait,
so under a flood of cheap frames passes come faster than ticks and STP time
runs fast instead, shortening the forward delay in exactly the situation a
loop produces. uip_periodic() had the same problem from the other side: it
ran once per pass, so the retransmit timer sped up with the loop.

Both now go by the tick counter the timer interrupt keeps. The STP block
catches up on ticks a long pass missed, at most eight steps at a time, and
handle_tx() runs its periodics once per tick at most. One byte of xdata
more, 48 bytes of code.

A host test drives the gate with one pass every 5 ms, every 16 ms and
every 0.6 ms for ten seconds each; the STP clock lands on 500 steps in
all three, where counting passes gave 156 and 4166. A pass of 1.3 s loses
time rather than firing 65 steps at once, and the arithmetic survives the
tick counter wrapping.

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'
 

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) {
+		uint8_t n = STP_CATCH_UP;
-			stp_clock = STP_TICK_DIVIDER;
+		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;
 

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)
 

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 <stdint.h>
+#include <stdio.h>
+
+#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;
+}