From a16d6f04320de7027b5055780ddaa0c1ace22755 Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Fri, 9 Oct 2026 12:20:14 +0200 Subject: [PATCH] netboot: stop the diagnostics slowing the loop they diagnose Hardware dump: "frame 1 (110 B) etype=6c86 at 12: 72 33 33 00 00 00 16 00". 33 33 00 00 00 16 is an IPv6 MLDv2 multicast MAC from the host, but it sits at offset ONE, behind a leading 0x72 -- and 0x72 is 114, the low byte of the W5100's MACRAW 2-byte length prefix. Hence 6c86 at offset 12: the last byte of the source MAC and the first of 86DD, both one byte late. An earlier dump in the SAME build read FF FF FF FF FF FF 00 2B, perfectly aligned. Two frames, one build, different alignment -- that is a lapped read of a partially overwritten page, not a systematic off-by-one. The alignment fix is intact. Two self-inflicted costs removed from the poll loop, whose speed is exactly what decides whether the writer laps us: - The 32-byte hex dump ran on EVERY frame, adding an EXI transaction per iteration. The diagnostic was making the fault it was diagnosing worse. It now runs only when a descriptor is actually rejected. - RRP was re-read from the device every poll, costing another transaction for a value we had just written ourselves. It is now tracked locally and kept in step at the three places that move it. That takes the per-frame cost from six EXI transactions to four. It does not make the race winnable on a busy link -- the ring is 15 pages and has no overflow protection -- but it stops me handing back a loop that is slower than the one that produced the measurement. Co-Authored-By: Claude Opus 5 --- gc_bba_test/source/netboot.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/gc_bba_test/source/netboot.c b/gc_bba_test/source/netboot.c index a4060a6..287ffdc 100644 --- a/gc_bba_test/source/netboot.c +++ b/gc_bba_test/source/netboot.c @@ -45,14 +45,18 @@ static u32 ring_lag(u32 rpage, u32 wpage) return (wpage >= rpage) ? (wpage - rpage) : (wpage + 15u - rpage); } +/* Our own view of RRP. Re-reading it from the device every poll cost an EXI + * transaction for a value we had just written ourselves -- pure overhead in + * the one loop whose speed decides whether the writer laps us. */ +static u32 s_rrp; + static int ring_next(u8 *buf, u32 cap) { - u8 rp[2], wp[2], hdr[4]; + u8 wp[2], hdr[4]; u32 rpage, wpage, dlen, flen, addr; - if (bba_read(REG_RRP_LO, rp, 2) < 0) return -1; if (bba_read(REG_RWP_LO, wp, 2) < 0) return -1; - rpage = ((u32)rp[1] << 8) | rp[0]; + rpage = s_rrp; wpage = ((u32)wp[1] << 8) | wp[0]; if (rpage == wpage) return 0; /* empty */ if (rpage < 1 || rpage > 15) return -2; /* pointer corrupt */ @@ -75,14 +79,15 @@ static int ring_next(u8 *buf, u32 cap) if (bba_stream((u16)addr, hdr, 4) < 0) return -1; memcpy(s_last_desc, hdr, 4); s_last_page = rpage; - /* Keep the head of the page for the error path. Guessing at a 4-byte - * descriptor has already cost several round trips; 32 bytes shows - * whether an ethernet header is present and AT WHAT OFFSET, which - * settles "bad length" versus "reading the wrong place" outright. */ - if (bba_stream((u16)addr, s_last_dump, sizeof(s_last_dump)) < 0) - return -1; dlen = ((u32)hdr[2] << 8) | hdr[3]; - if (dlen <= 4 || dlen > 1518 + 4) return -3; /* descriptor implausible */ + if (dlen <= 4 || dlen > 1518 + 4) { + /* Only now is a hex dump worth an EXI transaction. Doing it for + * EVERY frame (as an earlier version did) added a round trip to the + * poll loop that is already losing the race against the writer -- + * i.e. the diagnostic made the fault it was diagnosing worse. */ + bba_stream((u16)addr, s_last_dump, sizeof(s_last_dump)); + return -3; + } flen = dlen - 4; if (flen > cap) flen = cap; @@ -102,6 +107,7 @@ static int ring_next(u8 *buf, u32 cap) u32 pages = (dlen + 255) >> 8; u32 next = ((rpage - 1 + pages) % 15) + 1; if (bba_w8(REG_RRP_LO, (u8)next) < 0) return -1; + s_rrp = next; } return (int)flen; } @@ -190,6 +196,7 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg)) return -5; } if (bba_w8(REG_RRP_LO, wp[0]) < 0) return -1; + s_rrp = wp[0]; if (status) { snprintf(line, sizeof(line), "ring synced at page %u", wp[0]); status(line); } } @@ -217,8 +224,10 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg)) { u8 wp[2]; if (bba_read(REG_RWP_LO, wp, 2) < 0) return -1; - if (wp[0] >= 1 && wp[0] <= 15) + if (wp[0] >= 1 && wp[0] <= 15) { bba_w8(REG_RRP_LO, wp[0]); + s_rrp = wp[0]; + } } continue; } @@ -257,8 +266,10 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg)) { u8 wp[2]; if (bba_read(REG_RWP_LO, wp, 2) < 0) return -1; - if (wp[0] >= 1 && wp[0] <= 15) + if (wp[0] >= 1 && wp[0] <= 15) { bba_w8(REG_RRP_LO, wp[0]); + s_rrp = wp[0]; + } } continue; }