From 0c5a7bf9db563684fe4b44cc55e85be0aa66d321 Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Sat, 10 Oct 2026 15:07:44 +0200 Subject: [PATCH] mini-dhcpd: stop reporting ARP as a corrupted frame Swiss got a DHCP lease on hardware (10.0.0.64/24), but the script flagged every ARP from the GC as "frame did NOT parse as DHCP ... this is what a corrupted TX frame looks like". That was wrong and actively misleading: the frames are valid lwIP duplicate-address probes (sender 0.0.0.0, target = the offered address), sent to check the lease is free before accepting it. The parser conflated "not DHCP" with "corrupted". A diagnostic that cries wolf on normal traffic is worse than none, because 'malformed' then means nothing. Now: 'info' from the client but not DHCP -- ARP decoded properly (probe vs request vs reply, with addresses), other ethertypes named 'malformed' CLAIMED IPv4/UDP->67 and then failed to parse, i.e. what a genuinely damaged TX frame would look like So the corruption diagnostic still works -- that was the point of using a raw socket -- but it only fires when it means something. scripts/test-mini-dhcpd.py keeps the self-test, now including the exact 60-byte frame from the console (T6), a damaged DHCP packet that must still be flagged (T7), and a raw ethertype (T8). One note for anyone extending that test: the real frame goes in as ONE string literal. Splitting the hex across two adjacent literals is how the first version of T6 silently lost a byte and blamed the parser for decoding the target as 0.10.0.0 -- the parser was correct. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 28 ++++++++++ scripts/mini-dhcpd.py | 55 ++++++++++++++++-- scripts/test-mini-dhcpd.py | 111 +++++++++++++++++++++++++++++++++++++ 3 files changed, 188 insertions(+), 6 deletions(-) create mode 100755 scripts/test-mini-dhcpd.py diff --git a/CLAUDE.md b/CLAUDE.md index 03e9eb9..77675a3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1353,6 +1353,34 @@ parallel master is the default — see "W5100 vs W5500".) --- +## ✅ SWISS GETS A DHCP LEASE ON HARDWARE (2026-10-10) + +`DISCOVER → OFFER → REQUEST → ACK`, lease `10.0.0.64/24`, served by +`scripts/mini-dhcpd.py` on a direct link. This is the first end-to-end proof +of the two paths our own test suite **structurally cannot** exercise: + +- **The command-level interrupt works.** Receiving the OFFER requires + `bba_event_handler` → `bba_cmd_in8(0x03) & 0x80` → `bba_interrupt()` → + `bba_start_rx()`. That chain was dead until the `cmd_ir[7]` fix; + `gc_bba_test` and `netboot` poll `RWP`/`IR` and would never have noticed. +- **libogc2's TX path works** — no TWD write, `EXI_Dma` payload, NCRA ST1 — + so the GC's documented DMA-write 1-bit-shift bug is NOT biting us. +- **The MACRAW MF filter accepts unicast to `SHAR`**: both replies were sent + unicast to `00:09:bf:00:00:01` and arrived. + +Getting there needed, in order: the packed RX descriptor, both command-space +forms, the counted TX length, the command-level interrupt, removing the +device-ID one-shot, and the MF filter. Any one of them missing stops DHCP. + +⚠️ **ARP from the GC is expected, not a fault.** lwIP sends an ARP probe +(`sender 0.0.0.0`, `target` = the offered address) to check the lease is free +before accepting it. The first version of `mini-dhcpd.py` reported every ARP +as a corrupted TX frame; it now decodes them. Its padding is uninitialised +junk rather than zeros, which is also normal — lwIP pads to the 60-byte +ethernet minimum with whatever is in the pbuf. + +--- + ## ⚠️ Open compatibility gaps — see `docs/bba_compat_todo.md` Known differences between this implementation and what real GC software diff --git a/scripts/mini-dhcpd.py b/scripts/mini-dhcpd.py index 0edb049..8a1d133 100755 --- a/scripts/mini-dhcpd.py +++ b/scripts/mini-dhcpd.py @@ -30,6 +30,7 @@ import time ETH_P_ALL = 0x0003 ETH_P_IP = 0x0800 +ETH_P_ARP = 0x0806 BOOTP_MIN = 236 # BOOTP header before the magic cookie MAGIC = b"\x63\x82\x53\x63" @@ -63,16 +64,50 @@ def build_reply(src_mac, dst_mac, src_ip, dst_ip, payload): return dst_mac + src_mac + struct.pack("!H", ETH_P_IP) + ip + udp +def describe_arp(pl): + """Decode an ARP body far enough to be useful during bring-up.""" + if len(pl) < 28: + return "ARP, truncated" + htype, ptype, hlen, plen, op = struct.unpack("!HHBBH", pl[:8]) + if (htype, ptype, hlen, plen) != (1, ETH_P_IP, 6, 4): + return (f"ARP with unexpected htype/ptype/hlen/plen " + f"{htype}/{ptype:#06x}/{hlen}/{plen}") + sha, spa = pl[8:14], socket.inet_ntoa(pl[14:18]) + tha, tpa = pl[18:24], socket.inet_ntoa(pl[24:28]) + kind = {1: "request", 2: "reply"}.get(op, f"op {op}") + if op == 1 and spa == "0.0.0.0": + return (f"ARP probe for {tpa} from {mac_str(sha)} -- lwIP checking the " + f"DHCP lease is not already taken (normal)") + if op == 1: + return f"ARP request: who has {tpa}? tell {spa} ({mac_str(sha)})" + return f"ARP reply: {spa} is at {mac_str(sha)} -> {tpa} ({mac_str(tha)})" + + def parse_dhcp(frame, want_mac): - """Return (kind, info). kind is 'dhcp', 'other', or 'malformed' -- the - last one is the interesting case, so it carries a reason.""" + """Return (kind, info). + + kind is one of: + 'dhcp' a DHCP message, info is the decoded fields + 'info' a frame from the client that is simply not DHCP (ARP, etc.) + 'malformed' it CLAIMED to be IPv4/UDP->67 and then failed to parse + 'other' not from the client we care about + + ⚠️ 'info' and 'malformed' used to be the same thing, which made this tool + report every ARP as a corrupted frame. A diagnostic that cries wolf on + normal traffic is worse than no diagnostic: the whole point is that + 'malformed' means something. ARP from the GC is expected -- lwIP probes + its DHCP lease before accepting it, and the probe also happens to prove + the TX path works for non-IP ethertypes. + """ if len(frame) < 14: return "other", None dst, src, etype = frame[0:6], frame[6:12], struct.unpack("!H", frame[12:14])[0] if want_mac and src != want_mac: return "other", None + if etype == ETH_P_ARP: + return "info", describe_arp(frame[14:]) if etype != ETH_P_IP: - return "malformed", f"from {mac_str(src)} but ethertype {etype:#06x}, not IPv4" + return "info", f"ethertype {etype:#06x} (not IPv4 or ARP)" ip = frame[14:] if len(ip) < 20: return "malformed", "IPv4 header truncated" @@ -181,10 +216,18 @@ def main(): continue seen_any = True ts = time.strftime("%H:%M:%S") + if kind == "info": + print(f"[{ts}] {info} ({len(frame)} bytes)") + if a.hex: + print(" " + binascii.hexlify(frame[:64]).decode()) + continue if kind == "malformed": - print(f"[{ts}] ⚠️ frame from the client did NOT parse as DHCP: {info}") - print(f" {len(frame)} bytes -- this is what a corrupted") - print(f" TX frame looks like; compare against a known-good one") + # Reached only when the frame said IPv4/UDP->67 and then did not + # hold up -- i.e. a genuinely damaged DHCP packet, which is what + # a corrupted TX path would produce. + print(f"[{ts}] ⚠️ DAMAGED DHCP frame from the client: {info}") + print(f" {len(frame)} bytes -- a well-formed DISCOVER " + f"would have parsed; suspect the TX path") print(" " + binascii.hexlify(frame[:64]).decode()) continue name = MSG_NAMES.get(info["msg_type"], str(info["msg_type"])) diff --git a/scripts/test-mini-dhcpd.py b/scripts/test-mini-dhcpd.py new file mode 100755 index 0000000..d1e38f2 --- /dev/null +++ b/scripts/test-mini-dhcpd.py @@ -0,0 +1,111 @@ +import importlib.util, socket, struct, binascii, sys +spec = importlib.util.spec_from_file_location("d", "/workspace/scripts/mini-dhcpd.py") +d = importlib.util.module_from_spec(spec); spec.loader.exec_module(d) + +GC = bytes.fromhex("0009bf000001") +SRV = bytes.fromhex("aabbccddeeff") +fails = [] + +def discover(magic=d.MAGIC, dport=67, etype=d.ETH_P_IP): + bootp = struct.pack("!BBBB4sHH", 1, 1, 6, 0, b"\xde\xad\xbe\xef", 0, 0x8000) + bootp += socket.inet_aton("0.0.0.0") * 4 + bootp += GC + b"\0" * 10 + b"\0" * 64 + b"\0" * 128 + magic + bootp += bytes([53, 1, 1]) + bytes([55, 3, 1, 3, 6]) + b"\xff" + udp = struct.pack("!HHHH", 68, dport, 8 + len(bootp), 0) + bootp + ip = struct.pack("!BBHHHBBH4s4s", 0x45, 0, 20 + len(udp), 1, 0, 64, 17, 0, + socket.inet_aton("0.0.0.0"), socket.inet_aton("255.255.255.255")) + ip = ip[:10] + struct.pack("!H", d.ip_csum(ip)) + ip[12:] + return GC_frame(etype, ip + udp) + +def GC_frame(etype, pl): + return bytes.fromhex("ffffffffffff") + GC + struct.pack("!H", etype) + pl + +# T1: a well-formed DISCOVER parses +kind, info = d.parse_dhcp(discover(), GC) +if kind != "dhcp" or info["msg_type"] != d.DHCP_DISCOVER: + fails.append(f"T1 DISCOVER parse: {kind} {info and info.get('msg_type')}") +elif info["chaddr"] != GC or info["xid"] != b"\xde\xad\xbe\xef": + fails.append(f"T1 fields: chaddr={info['chaddr'].hex()} xid={info['xid'].hex()}") +else: + print("T1 well-formed DISCOVER parses, chaddr + xid correct") + +# T2: a corrupted magic cookie is reported as MALFORMED, not ignored. +# This is the case that matters: a shifted/corrupt TX frame must be +# visible, not silently dropped the way a UDP socket would drop it. +kind, why = d.parse_dhcp(discover(magic=b"\x00\x00\x00\x00"), GC) +if kind != "malformed": + fails.append(f"T2 corrupt cookie reported as {kind}, want malformed") +else: + print(f"T2 corrupt cookie -> malformed: {why}") + +# T3: frames from other MACs are ignored +kind, _ = d.parse_dhcp(discover(), bytes.fromhex("001122334455")) +if kind != "other": + fails.append(f"T3 foreign MAC: {kind}") +else: + print("T3 foreign MAC ignored") + +# T4: a reply is a valid IPv4/UDP frame with a correct IP checksum +bootp = d.build_bootp(d.DHCP_OFFER, b"\xde\xad\xbe\xef", 0x8000, GC, + "10.0.0.64", "10.0.0.1", "255.255.255.0", + "10.0.0.1", "10.0.0.1", 3600) +out = d.build_reply(SRV, GC, "10.0.0.1", "10.0.0.64", bootp) +if out[0:6] != GC: + fails.append("T4 reply not unicast to the client MAC") +ip = out[14:] +if d.ip_csum(ip[:20]) != 0: + fails.append(f"T4 IP checksum invalid ({d.ip_csum(ip[:20]):#06x}, want 0)") +sp, dp, ulen, _ = struct.unpack("!HHHH", ip[20:28]) +if (sp, dp) != (67, 68): + fails.append(f"T4 ports {sp}->{dp}, want 67->68") +if ulen != 8 + len(bootp): + fails.append(f"T4 UDP length {ulen}, want {8 + len(bootp)}") +if struct.unpack("!H", ip[2:4])[0] != len(ip): + fails.append("T4 IP total length disagrees with the frame") +if len(bootp) < 300: + fails.append(f"T4 BOOTP {len(bootp)} bytes, under the 300 minimum") +if not fails: + print(f"T4 OFFER: unicast to {d.mac_str(out[0:6])}, IP csum ok, " + f"67->68, {len(out)} bytes, BOOTP {len(bootp)}") + +# T5: the OFFER decodes back to the right yiaddr/options +y = ip[28 + 16:28 + 20] +if socket.inet_ntoa(y) != "10.0.0.64": + fails.append(f"T5 yiaddr {socket.inet_ntoa(y)}, want 10.0.0.64") +else: + print("T5 yiaddr = 10.0.0.64") + + +# T6: the REAL frame from the console, which the first version of this script +# reported as a corrupted TX frame. It is a valid lwIP DHCP-lease ARP probe. +# Exact frame from the console paste, as ONE literal: splitting it across +# two string literals is how the first version of this test lost a byte and +# blamed the parser for decoding tpa as 0.10.0.0. +real = bytes.fromhex("ffffffffffff0009bf000001080600010800060400010009bf000001000000000000000000000a000040c0004800000000008031b4b08052fef88031") +assert len(real) == 60, len(real) +kind, info = d.parse_dhcp(real, GC) +if kind != "info": + fails.append(f"T6 real ARP probe classified as {kind!r}, want 'info' " + f"- must NOT be reported as corrupted") +elif "probe" not in info or "10.0.0.64" not in info: + fails.append(f"T6 ARP probe description unhelpful: {info}") +else: + print(f"T6 real console frame -> {info}") + +# T7: a frame that IS IPv4/UDP->67 but damaged must still be 'malformed', +# otherwise fixing T6 would have thrown away the diagnostic. +kind, why = d.parse_dhcp(discover(magic=b"\xde\xad\xbe\xef"), GC) +if kind != "malformed": + fails.append(f"T7 damaged DHCP classified as {kind!r}, want 'malformed'") +else: + print(f"T7 damaged DHCP still flagged: {why}") + +# T8: a non-ARP, non-IP ethertype is informational, not alarming +kind, info = d.parse_dhcp(GC_frame(0x88b5, b"\x00" * 46), GC) +if kind != "info" or "0x88b5" not in info: + fails.append(f"T8 netboot ethertype: {kind!r} {info!r}") +else: + print(f"T8 raw ethertype -> {info}") + +print("\nFAILURES:\n " + "\n ".join(fails) if fails else "\nAll DHCP script tests passed.") +sys.exit(1 if fails else 0)