Repository navigation
uci: net_dhcp_acquire rejects empty/short GET_IPADDR replies; net_tcp_send rejects a non-2-byte write reply - #286
Merged
Merged
Conversation
The comment said the U64E has a single interface (index 0). On fw 3a1ff9ff, Ethernet (rmii_interface.cc, init prio 51) always registers as index 0, cable or not, and WiFi (wifi.cc, prio 52) as index 1. The WiFi lease was observed on iface 1 at 10.43.23.83. Comment-only change; the probe loop already covers indices 0..3. PRG sha256, identical on origin/master (249f65d) and this branch: uci b61ba7924b5c5e91077ceeaac8b08f4246bfb2c3515df2c9a80104d658e764bc uci-onchip 45bea88a6c27138009de543394241054e3a31873aa5766c466158b316b18da01 uci-comb 2b3e6c5d7070e43d7a591b4280f519918bd4b02921cf776819ec22e6f6ea3900 uci-m3 b124ea9f25d26bfeefc0ff5bc1585876a9256a2ca7d476ccf2f071d09870f35f Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An interface index at or past getNumberOfInterfaces() is answered by
network_target.cc with an EMPTY, valid reply and the status line
"82,PARAMETER(S) OUT OF RANGE"; the $DF1C ERROR bit stays clear (its one
setter is a PUSH while not idle), so uci_check_err returns C=0. The old
probe then read 12 bytes that were never sent:
* uci_read_resp_bytes spun its 65,536 fenced iterations for the first
missing byte (~7.5 s per out-of-range probe at 48 MHz, ~6 min at
1 MHz), reached whenever no in-range interface has a lease;
* whatever uci_ipaddr_resp already held was copied to net_local_ip and,
if non-zero, returned with C=0.
The second needs an empty or short reply with no full reply before it in
the same call, which firmware does not produce on a box with at least
one interface (in-range indices come first and always return 12 bytes),
so it is a contract fix, not a reproduced field failure.
Now: an empty reply (DATA_AV low once VALID) skips the read; only
uci_resp_count == 12 counts as an answer; net_local_ip is zeroed on entry
so a failed call never reads as the last call's address. Same codes as
before ($83 no lease, $82 rejected push, $89 timeout). NET_CODE -32 B on
uci / uci-onchip / uci-comb; uci-m3 has its own probe and is unchanged.
tools/test_uci_dhcp_probe.py runs the shipped bytes against a modelled
firmware that answers GET_IPADDR from the bytes written to $DF1D.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… redundant Rebased onto 191b783 (fix/uci-resp-bounded-wait), which removed the 65,536-iteration DATA_AV spin from uci_read_resp_bytes. An empty reply now returns from the read at once with uci_resp_count = 0. - net_dhcp_acquire's own empty-reply pre-check is dropped: the DATA_AV test, the uci_resp_count clear and the uci_settle import. The uci_resp_count != 12 check alone rejects an empty reply. Against b8fdf8f6 (191b783 uci-onchip) the no-lease case already passes without it, and the stale-lease cases still fail. - Spin-cost figures are superseded and removed. e602e2b stated as fact "~7.5 s per out-of-range probe at 48 MHz, ~6 min at 1 MHz", and its test docstring added ~5.6 s at 64 MHz. Those were DERIVED (65,536 x ~5,489 cycles), not measured, and described a wait 191b783 no longer has. One hardware point exists for the old wait: on dce1b94 (45bea88a) a forced out-of-range probe took 8.1 s from key to screen at 48 MHz on the U64E. - Test: every failed-acquire case asserts net_local_ip reads 0.0.0.0. This kills the zero-on-entry mutant. The entry clear is kept for a caller that skips net_init (none in tree today), per net_abi.inc's "zero until net_dhcp_acquire succeeds". - The uninitialised-buffer case becomes leftover state on direct entry. uci_ipaddr_resp is file-backed in CRYPTO_HOT and loads as 12 zero bytes, so the case needs earlier code in the same RAM lifetime. - net.s: the CMD_FAILED arm no longer cites an out-of-range index; it only handles a rejected push. - Test docstring: the indexed GET_IPADDR handler dates from b4046f34 (2018), not 915cfbe7. - uci-m3/net.s: no longer claims "same contract". It accepts a reply of 4 bytes or more; uci requires the full 12. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… count net_tcp_send read the "bytes written" reply into uci_write_resp without checking how many bytes arrived. uci_read_resp_bytes stores only what was sent, so an empty or 1-byte reply left the previous chunk's count in place and the send advanced on it. The #230(a) mis-parse shape is exactly that: "21,UNKNOWN COMMAND" with no data. A good 4-byte send followed by an empty reply returned C=0 with net_last_error $00. A reply count other than 2 is now a failed send: $87 UCI_ERR_SHORT_WRITE, C=1. That is the same verdict and code as the $FFFF arm beside it, and as uci-m3's send, which already checks m3_rd_count == 2. No new code. The reply is fully drained and acked first, so the interface is left idle. Tests: two checks added to tools/test_uci_timeout_recovery.py (a stale count after a good send, and 0/1-byte replies). Both fail on 728b95e and pass here. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-opens #281 against master. #281 was stacked on its parent PR's branch and merged into that branch after the parent had already merged, so its commits never reached master. The head branch is unchanged, and the commits are the ones reviewed in #281; its body follows verbatim.
Merge after the spin-fix PR (
fix/uci-resp-bounded-wait, 191b783). This PR is based on that branch so it shows only these four commits.Defects
net_dhcp_acquiretook a lease from a reply that did not contain one. The firmware answers an out-of-range interface index with an empty, valid reply and the status line82,PARAMETER(S) OUT OF RANGE. The$DF1CERROR bit stays clear (its only setter is a PUSH while not idle), so the old code copied whateveruci_ipaddr_respalready held. Now only a full 12-byte reply is a lease, and a failed call leavesnet_local_ipat 0.0.0.0. In production the in-range indices are probed first and always refill the buffer, so the stale lease needs an empty or short reply with no full one before it. This is a contract fix.net_tcp_sendreused the previous chunk's written count. A SOCKET_WRITE reply that is not exactly 2 bytes (the UCI command routine: two handshake races found in c64-test-harness #230(a)21,UNKNOWN COMMANDshape) leftuci_write_respunchanged, and the send succeeded on it. Such a reply now fails as$87 UCI_ERR_SHORT_WRITEwith C=1, the same verdict as the$FFFFarm and as uci-m3's send, which already checks the count. No new error code.Red / green (shipped bytes on the 6502 model)
tools/test_uci_dhcp_probe.py(new, inpytest.initestpaths): 3/6 on the spin tip (uci-onchipb8fdf8f6); the re-init, leftover-state and short-reply cases fail. 6/6 here.tools/test_uci_timeout_recovery.py(+2 checks): 4/6 on 728b95e. A good send followed by an empty reply returned C=0 with err$00. 6/6 here.Mutants
==12,!=0,>=4).>=1, failure routed to the non-fatal path).Size (
measure_margins.py --build)NET_CODE is 24 B smaller on every UCI profile than on the spin tip (uci, comb and demo-store 224→200; onchip 232→208). Nothing else moves. ip65, ip65-onchip, both ip65 wiki builds and uci-m3 (
b124ea9f) are byte-identical.fe0c573e029f4754ddd69dc4475253f28844421542101508aba5e33487bb80cf4e28bdd3e126664556e798d1461a527bcdcfc32f3acdabe0560f476468b8c3af04182f14fa52d31c090e83873997ed2cd84a8a2121d7962fb338b95fe5af27cfHardware
All runs were on a U64E (Ultimate 64 Elite, fw 3.15, FPGA 126, core 1.50) at 48 MHz, under the DeviceLock queue.
net_dhcp_acquireto issue a single GET_IPADDR for index 4 after two good re-inits.45bea88a): "DHCP OK" after 8.1 s with the previous lease (10.43.23.83), err$00.b8fdf8f6): the stale lease again, in 0.1 s.56f100f8, the DHCP fix alone): DHCP FAILED in 0.0 s, 0.0.0.0, err$83.82,PARAMETER(S) …line, which confirms the empty reply with ERROR clear.boot_check.pyon4e28bdd3: PASS.rig_https_local.pyon theHTTPS_SNI=www.foo.invalidbuild3c45c10adf0cbdd53f4c0c5299e4bb8a07bc9f628b4ff7ef1169cee7cdb23271: PASS, HTTP 200, and the listener confirmed the handshake and the decrypted GET.phase2_check.pyPASS, and two 'I' re-inits in one boot took interface 0 (ethernet) in 0.1 s.Superseded figures
e602e2b's message stated "~7.5 s per out-of-range probe at 48 MHz, ~6 min at 1 MHz" as fact (and its test docstring added ~5.6 s at 64 MHz). Those figures were derived from 65,536 × ~5,489 cycles, not measured, and they describe a wait the spin fix removes. The one measurement is the 8.1 s above, taken key-to-screen.
🤖 Generated with Claude Code