fix(stream): header-only frame CRC; draw-mode evidence guards

Ruling R14: the frame CRC now covers header bytes 0..11 only.  A
payload-wide CRC cost the terminal ~139 instructions per payload byte
(the parser, not the wire or the blitter, was the frame-rate ceiling),
while every datagram already carries an Ethernet FCS the LANCE checks
and the length bound catches truncation.  frame.py, fp_parse and the
harness's own frame signer follow the new rule; a corrupted payload
byte is now ACCEPTED and a corrupted header byte rejected, stated
explicitly in both test suites, and the C golden vectors were
regenerated from frame.py.

Measured after: RX_READY_A write -> first RX_OK read is 1 emulated
frame for a 20,480-byte frame (84 before), which is the frame hook's
resolution.

Ruling R15: draw mode deletes its dump files before the run (wrapper
and Lua) so a failed run cannot be judged on the previous screen; a
wait that hits its cap reports "timed out waiting for the frame" and
the checker reports it as a timeout, not as pixel mismatches; the
checker asserts the VRAM canary word is 0xA5C3, so a payload that
stopped arming it fails instead of passing quietly.

rx mode now corrupts a header byte in slot B, because a payload flip is
accepted by design under R14.
This commit is contained in:
opencode.energia.arti.ee committed 2026-10-30 23:59:20 +00:00
1 parent 5e9b2e9c57
commit 16148d0d24
10 files changed
+324 -100

No files matched your search

+8
View File
@@ -57,6 +57,14 @@ if [ -f "$ENVFILE" ]; then
fi
mkdir -p "$LOGDIR"
# Ruling R15: draw-mode evidence must come from THIS run. The Lua removes its
# own dump files at startup; removing them here as well means a run that dies
# before the script starts (or before it dumps) leaves no previous screen
# behind for vidstream_check.py to bless.
if [ "$MODE" = "draw" ]; then
rm -f "$LOGDIR/vram-draw.bin" "$LOGDIR/frame-draw.bin" \
"$LOGDIR/draw-status.txt"
fi
# Absolute: the Lua runs with MAME's cwd, so a relative default only resolves
# when the caller happens to be at the repo root.
VID_PAYLOAD="$ROOT/harness/udpstream/build/vidstream.bin"
+85 -23
View File
@@ -19,7 +19,7 @@
-- of POOL_FREE (before) marks the fallback, i.e. the payload keeps running.
--
-- Mode "rx" (Task 4) is "alloc" plus the receive path: it writes a VALID frame
-- into slot A and the same frame with one payload byte flipped into slot B,
-- into slot A and the same frame with one header byte flipped into slot B,
-- raises each slot's ready flag (the role the LANCE receive interrupt plays on
-- the real transport), and asserts RX_OK == 1 && RX_BAD == 1, that RX_LAST_LEN
-- is the accepted frame's payload_len, that RX_SLOT names slot A, and that the
@@ -55,6 +55,9 @@
-- VID_SENTINEL_CLOBBER=1 draw mode only: overwrite the VRAM canary word
-- before the frame is announced, so the payload must count
-- an OS touch. The negative control for OS_TOUCH.
-- VID_DRAW_WAIT draw mode only: emulated frames to wait for the payload to
-- accept the frame (default 300). Set it to 1 to exercise
-- the timeout path.
-- VID_PAYLOAD raw .text image (default harness/udpstream/build/vidstream.bin)
-- VID_ORIGIN payload load (poke) bit address (default 0x018C0000)
-- VID_ENTRY_PTR _start bit address from vidstream.env (default VID_ORIGIN)
@@ -146,7 +149,8 @@ end
-- CCITT-FALSE (polynomial 0x1021, init 0xFFFF, not reflected, no final xor).
-- frame.py, frame_parse.c and this loop are three independent readings of the
-- same spec; the harness signs its own frames rather than reusing a value the
-- payload computed, so the C parser accepting them is a real check.
-- payload computed, so the C parser accepting them is a real check. Since
-- ruling R14 the CRC input is the 12 header bytes only.
local function crc_add(crc, bytes, first, count)
for i = first, first + count - 1 do
crc = (crc ~ (bytes[i] << 8)) & 0xFFFF
@@ -186,7 +190,9 @@ local function make_frame(kind, frame_no, rows, stretch, plen, fill)
end
end
local crc = crc_add(0xFFFF, b, 0, 12)
crc = crc_add(crc, b, 16, plen)
-- Header bytes 0..11 only (ruling R14): the payload is protected by each
-- datagram's Ethernet FCS, not by this CRC. frame.py, frame_parse.c and
-- this loop are three independent readings of the same rule.
b[12] = crc & 0xFF
b[13] = (crc >> 8) & 0xFF
b[14] = 0
@@ -304,7 +310,11 @@ local function inject()
readback_a = verify(a, fa, RX_FRAME)
local fb = make_frame(1, 0x0022, RX_ROWS, RX_STRETCH, RX_PLEN)
fb[16 + 100] = fb[16 + 100] ~ 0x01 -- payload byte flipped, CRC left alone
-- Corrupt a HEADER byte, inside the 12 bytes the CRC covers. Since ruling
-- R14 a flipped PAYLOAD byte is ACCEPTED by fp_parse (the Ethernet FCS is
-- the chip's job), so a payload flip would no longer exercise the reject
-- path; the payload-flip case is pinned by the host tests instead.
fb[4] = fb[4] ~ 0x01 -- frame_no low byte, CRC left alone
poke(b, fb, RX_FRAME)
readback_b = verify(b, fb, RX_FRAME)
@@ -313,7 +323,7 @@ local function inject()
prog:write_u32(RX_READY_A, 1)
prog:write_u32(RX_READY_B, 1)
print(string.format("VID: frame %d: injected slot A (%s, %d B, valid) and slot B (%s, %d B, one payload byte flipped)",
print(string.format("VID: frame %d: injected slot A (%s, %d B, valid) and slot B (%s, %d B, one header byte flipped)",
frames, hex(a), RX_FRAME, hex(b), RX_FRAME))
return true
end
@@ -325,6 +335,28 @@ end
-- compares against what was actually delivered rather than re-deriving it.
local draw_at = nil
local draw_bytes = nil
local rx_ok_at = nil
-- How long to wait for the payload to accept the frame, in EMULATED FRAMES.
-- This hook only runs once a frame, so that is the resolution of the
-- measurement below as well as the timeout. VID_DRAW_WAIT exists so the
-- timeout path can be exercised on purpose (set it to 1).
local DRAW_WAIT = tonumber(os.getenv("VID_DRAW_WAIT") or "300")
-- Evidence freshness (ruling R15): remove this mode's dump files before the
-- run, so a failed run cannot be checked against the previous run's screen.
local DRAW_FILES = { "vram-draw.bin", "frame-draw.bin", "draw-status.txt" }
local function clear_draw_files()
if MODE ~= "draw" then
return
end
for _, name in ipairs(DRAW_FILES) do
os.remove(DUMP_DIR .. "/" .. name)
end
end
clear_draw_files()
local function draw_row_byte(i)
if (i // RX_ROW_BYTES) % 3 == 2 then
@@ -394,10 +426,15 @@ local function write_file(path, s)
end
-- Dump the screen and the payload's record for vidstream_check.py, then end
-- the run. The pass/fail for pixels belongs to the checker; here we only
-- report what the payload itself says (one accepted frame, no rejects, no
-- repaint by the OS).
local function dump_draw()
-- the run. The pass/fail for pixels belongs to the checker; here we report
-- what the payload itself says (one accepted frame, no rejects, no repaint by
-- the OS) plus the measured parse cost.
--
-- `timed_out` matters for what the checker is allowed to say: if the payload
-- never accepted the frame, the screen shows the previous picture and calling
-- that a pixel mismatch would blame the blit for a transport failure. The
-- status file carries TIMEOUT=1 and the checker stops there.
local function dump_draw(timed_out)
local vram = read_vram()
local chars = {}
for i = 0, DRAW_FRAME - 1 do
@@ -416,23 +453,36 @@ local function dump_draw()
local pcv = cpu.state['PC'].value
local sent = prog:read_u16(VRAM_BASE + (VRAM_ROWS - 1) * VRAM_ROW_BITS
+ 1280)
local parse_frames = -1
local ok = okv and okf
if rx_ok_at ~= nil then
parse_frames = rx_ok_at - draw_at
end
print(string.format("VID: heartbeat %d, PC=%s (%s)", alive, hex(pcv),
in_payload(pcv) and "inside the payload" or "OUTSIDE the payload"))
print(string.format("VID: POOL_PTR=%s RX_SLOT_A=%s (an unchanged pointer means "
.. "the payload did not re-enter main)", hex(ptr), hex(slota)))
write_file(DUMP_DIR .. "/draw-status.txt", string.format(
"RX_OK=%d\nRX_BAD=%d\nOS_TOUCH=%d\nRX_READY_A=%d\nSENTINEL=%d\n",
rx_ok, rx_bad, os_touch, ready, sent))
"RX_OK=%d\nRX_BAD=%d\nOS_TOUCH=%d\nRX_READY_A=%d\nSENTINEL=%d\n"
.. "TIMEOUT=%d\nPARSE_FRAMES=%d\n",
rx_ok, rx_bad, os_touch, ready, sent, timed_out and 1 or 0,
parse_frames))
print(string.format("VID: frame %d: screen dumped to %s/vram-draw.bin (%d B), frame to %s/frame-draw.bin (%d B)",
frames, DUMP_DIR, #vram, DUMP_DIR, DRAW_FRAME))
print(string.format("VID: RX_OK=%d RX_BAD=%d OS_TOUCH=%d RX_READY_A=%d SENTINEL=%04X",
rx_ok, rx_bad, os_touch, ready, sent))
if parse_frames >= 0 then
print(string.format("VID: parse cost: RX_READY_A written at frame %d, RX_OK first read at frame %d = %d emulated frames",
draw_at, rx_ok_at, parse_frames))
else
print("VID: parse cost: no RX_OK observed (the frame was never accepted)")
end
if rx_ok ~= 1 or rx_bad ~= 0 or ready ~= 0 or os_touch ~= 0 then
if timed_out or rx_ok ~= 1 or rx_bad ~= 0 or ready ~= 0 or os_touch ~= 0 then
ok = false
end
print(ok and "VID: RESULT PASS" or "VID: RESULT FAIL")
@@ -617,23 +667,35 @@ emu.register_frame_done(function()
end
end
-- Draw mode: inject once, then wait for the payload to consume the slot
-- (it clears RX_READY_A) and give it a few frames of slack before reading
-- the screen back. The timeout is generous on purpose: fp_parse() runs a
-- BITWISE CRC, and a 20 KB frame costs it ~40 emulated frames (measured),
-- so an unlucky 10-frame wait would read the screen before the blit.
-- A payload that never takes the frame still gets its screen dumped, so
-- the checker reports the truth instead of the run hanging.
-- Draw mode: inject once, then wait for the payload to accept the frame.
-- Two separate observations, because they answer different questions:
--
-- * the first frame at which RX_OK reads >= 1 is the PARSE COST
-- (RX_READY_A write -> first RX_OK read, in emulated frames);
-- * RX_READY_A == 0 is the payload done with the slot, i.e. after
-- paint_frame() returned -- that is the screen to dump.
--
-- R15: the wait is bounded, and hitting the bound is reported as a
-- timeout, never as a pixel mismatch.
if entered and MODE == "draw" and draw_at == nil then
if inject_draw() then
draw_at = frames
end
end
if MODE == "draw" and draw_at ~= nil and frames > draw_at + 1 then
if (prog:read_u32(RX_READY_A) == 0 and frames >= draw_at + 5)
or frames >= draw_at + 300 then
dump_draw()
if MODE == "draw" and draw_at ~= nil then
if rx_ok_at == nil and prog:read_u32(RX_OK) >= 1 then
rx_ok_at = frames
end
if rx_ok_at ~= nil and prog:read_u32(RX_READY_A) == 0
and frames > rx_ok_at then
dump_draw(false)
return
end
if frames >= draw_at + DRAW_WAIT then
print(string.format("VID: timed out waiting for the frame: announced at frame %d, RX_OK=%d after %d emulated frames",
draw_at, prog:read_u32(RX_OK), DRAW_WAIT))
dump_draw(true)
return
end
end
+26 -3
View File
@@ -7,7 +7,13 @@ and blit it, then dumps three files:
vram-draw.bin the screen as memory: 1024 rows of 256 bytes from bit 0x04000000
frame-draw.bin the bytes actually poked into the slot (16-byte header + payload)
draw-status.txt key=value lines (RX_OK, RX_BAD, OS_TOUCH, ...)
draw-status.txt key=value lines (RX_OK, RX_BAD, OS_TOUCH, SENTINEL, TIMEOUT, ...)
A run whose wait for the payload expired writes TIMEOUT=1, and that is reported
as a timeout -- not as a screen full of wrong pixels, which would blame the
blit for a frame the payload never accepted (ruling R15). The canary word is
also asserted (0xA5C3), so a payload that stopped arming it is a failure rather
than a silently missing check.
This script is the independent reading of the geometry, not a copy of the code
under test. The frame is 1280 pixels wide, 1 bpp, 160 bytes a row; the screen
@@ -38,6 +44,7 @@ SCREEN_ROWS = 1024
ZERO_ROW_MOD = 3 # rows with r % 3 == 2 are the all-zero rows
ZERO_ROW_REM = 2
FILL_BYTE = 0xF0 # rows that are not zero are 0xF0F0 words -> 0xF0 bytes
CANARY = 0xA5C3 # SENT_VALUE in vidstream.c, at row 1023 / column 1280
failures = 0
@@ -96,6 +103,14 @@ def main():
print("vidstream_check: vram %s (%d B) frame %s (%d B) status %s"
% (args.vram, len(vram), args.frame, len(frame), args.status))
# --- a timed-out wait is not a pixel verdict (ruling R15) -------------
if status.get("TIMEOUT", 0) != 0:
print("FAIL: timed out waiting for the frame (RX_OK=%r, "
"RX_READY_A=%r: the payload did not finish with the slot)"
% (status.get("RX_OK"), status.get("RX_READY_A")))
print("vidstream_check: FAIL (timeout)")
return 1
# --- the frame that was delivered ------------------------------------
rows, stretch = args.rows, args.stretch
if not check(stretch > 0, "stretch must be positive, got %d" % stretch):
@@ -163,13 +178,21 @@ def main():
rx_ok = status.get("RX_OK")
rx_bad = status.get("RX_BAD")
os_touch = status.get("OS_TOUCH")
canary = status.get("SENTINEL")
check(rx_ok == 1, "RX_OK=%r, expected 1 accepted frame" % (rx_ok,))
check(rx_bad == 0, "RX_BAD=%r, expected 0 rejected frames" % (rx_bad,))
check(os_touch == 0,
"OS_TOUCH=%r: something else wrote to the sentinel corner of VRAM"
% (os_touch,))
print("vidstream_check: RX_OK=%r RX_BAD=%r OS_TOUCH=%r"
% (rx_ok, rx_bad, os_touch))
# A missing canary (a payload that never armed it, or a status file from
# an older payload) must fail loudly: 0 was the firmware's RAM pattern, so
# "not written" and "correct" would otherwise look alike.
check(canary == CANARY,
"canary word is %r, expected 0x%04X (SENT_VALUE in vidstream.c)"
% (canary, CANARY))
print("vidstream_check: RX_OK=%r RX_BAD=%r OS_TOUCH=%r canary=%s"
% (rx_ok, rx_bad, os_touch,
("0x%04X" % canary) if isinstance(canary, int) else repr(canary)))
if failures:
print("vidstream_check: FAIL (%d checks)" % failures)
+49 -31
View File
@@ -18,13 +18,19 @@ byte, so the layout is spelled out here and must be mirrored by the payload:
14 2 padding to HEADER_SIZE, zero
The CRC is CCITT-FALSE -- polynomial 0x1021, initial value 0xFFFF, no input or
output reflection, no final XOR -- and it covers header bytes **0..11 plus the
payload**, never the CRC field itself.
output reflection, no final XOR -- and it covers **header bytes 0..11 only**,
never the payload and never the CRC field itself (ruling R14).
``unpack_header(buf)`` with no payload therefore checks a bare header (the CRC
of an empty payload). ``unpack_header(buf, payload)`` checks a whole frame,
which is what the receiving side does. A payload that is not passed is simply
not covered, so a header sealed over a payload will *not* validate without it.
That is a deliberate narrowing of an earlier contract. A payload-wide CRC cost
the terminal ~139 instructions per payload byte; on the 34010 that made the
parser, not the wire or the blitter, the frame-rate ceiling. Nothing is lost by
dropping it: every datagram already carries an Ethernet FCS that the LANCE
validates before the bytes reach a slot, and ``payload_len`` plus the delivered
length catches truncation. What the header CRC still buys is a check that the
geometry a frame was *sent* with is the geometry the terminal acts on.
So a corrupted **payload** byte does not invalidate a frame here -- that is the
chip's job -- while a corrupted **header** byte does.
"""
import struct
@@ -44,7 +50,7 @@ MAGIC = b"VS"
#: Wire format version. Bump when the layout changes.
VERSION = 0x01
#: The CRC covers header bytes [0, CRC_OFFSET) plus the payload.
#: The CRC covers header bytes [0, CRC_OFFSET) and nothing else.
CRC_OFFSET = 12
#: A strip of packed 1-bpp source rows.
@@ -64,8 +70,9 @@ def _make_crc_table():
Keeping the loop here means the polynomial, the init value and the shift
direction are stated exactly once, in the form ``vidstream.c`` repeats.
The table is only how the host gets through a 12 KB frame in ~2 ms instead
of ~28 ms, which a 30 fps sender cannot afford.
The table is only how the host gets through a long buffer quickly; since
ruling R14 the frame CRC itself runs over 12 bytes, but ``crc16`` is still
a general function and the tests cross-check it on long inputs.
"""
table = []
for byte in range(256):
@@ -106,20 +113,18 @@ def pack_header(kind: int, frame_no: int, payload_len: int, rows: int,
stretch: int, payload: bytes | None = None) -> bytes:
"""Build the 16-byte header for a frame.
The CRC is computed over header bytes 0..11 plus ``payload``. Pass the
payload here to seal a complete frame in one call -- and then it must be
exactly ``payload_len`` bytes, or a frame the CRC covers but the length
field denies would go on the wire. Omit ``payload`` to build the header
before the body exists and seal it later with :func:`seal_header`.
The CRC covers header bytes 0..11 only (ruling R14), so the header can be
built before the body exists. ``payload`` is kept as an **advisory**
consistency check: when it is passed it must be exactly ``payload_len``
bytes, which catches a header that describes a body the caller does not
have. Its *contents* are not covered -- the Ethernet FCS is.
"""
kind = _checked("kind", kind, 0xFF)
frame_no = _checked("frame_no", frame_no, 0xFFFF)
payload_len = _checked("payload_len", payload_len, 0xFFFF)
rows = _checked("rows", rows, 0xFF)
stretch = _checked("stretch", stretch, 0xFF)
if payload is None:
body = b""
else:
if payload is not None:
body = bytes(payload)
if len(body) != payload_len:
raise ValueError("payload is %d bytes but payload_len says %d"
@@ -135,35 +140,45 @@ def pack_header(kind: int, frame_no: int, payload_len: int, rows: int,
0, # reserved
)
assert len(prefix) == CRC_OFFSET
crc = crc16(prefix + body)
crc = crc16(prefix)
return prefix + _TAIL.pack(crc, 0)
def seal_header(header: bytes, payload: bytes = b"") -> bytes:
"""Return ``header`` with its CRC field recomputed over ``payload``.
def seal_header(header: bytes, payload: bytes | None = None) -> bytes:
"""Return ``header`` with its CRC field recomputed over header bytes 0..11.
``header`` must be exactly :data:`HEADER_SIZE` bytes: a longer buffer is
rejected rather than silently truncated, so a caller cannot pass
``header + payload`` here and lose the payload from the CRC.
``header + payload`` here and believe the payload was sealed.
Byte-for-byte identical to ``pack_header(..., payload=payload)`` for a
header that came from :func:`pack_header`; kept separate so a caller that
assembles a frame in pieces (header, then payload) can still seal it.
``payload`` is advisory, exactly as in :func:`pack_header`: when it is
passed it must match the header's ``payload_len``, and its bytes are not
covered by the CRC. Sealing is therefore idempotent and order-independent
-- header first or body first, the bytes are the same.
"""
if len(header) != HEADER_SIZE:
raise ValueError("header must be exactly %d bytes, got %d"
% (HEADER_SIZE, len(header)))
crc = crc16(bytes(header[:CRC_OFFSET]) + bytes(payload))
if payload is not None:
want = struct.unpack_from("<H", header, 6)[0]
if len(payload) != want:
raise ValueError("payload is %d bytes but the header says %d"
% (len(payload), want))
crc = crc16(bytes(header[:CRC_OFFSET]))
return bytes(header[:CRC_OFFSET]) + _TAIL.pack(crc, 0)
def unpack_header(buf: bytes, payload: bytes = b"") -> dict:
def unpack_header(buf: bytes, payload: bytes | None = None) -> dict:
"""Parse a frame header and check its CRC.
``payload`` is the frame body that follows the header; the CRC is computed
over ``header[0:12] + payload``. The returned ``crc_ok`` is purely the CRC
result -- callers also check ``magic`` and ``version`` (the C receiver
checks magic and the length bound before it bothers with the CRC).
``crc_ok`` covers header bytes 0..11 only (ruling R14). ``payload`` is
advisory: when it is passed, its length must equal ``payload_len`` (a
truncated or over-long body is a length error, not a CRC error), but the
bytes themselves are never hashed here.
The returned ``crc_ok`` is purely the CRC result -- callers also check
``magic`` and ``version`` (the C receiver checks magic, the version and the
length bound before it bothers with the CRC).
"""
if len(buf) < HEADER_SIZE:
raise ValueError("buffer shorter than a %d-byte header: %d"
@@ -171,6 +186,9 @@ def unpack_header(buf: bytes, payload: bytes = b"") -> dict:
prefix = _PREFIX.unpack_from(buf, 0)
magic, version, kind, frame_no = prefix[:4]
payload_len, rows, stretch, _reserved = prefix[4:]
if payload is not None and len(payload) != payload_len:
raise ValueError("payload is %d bytes but the header says %d"
% (len(payload), payload_len))
stored_crc, _pad = _TAIL.unpack_from(buf, CRC_OFFSET)
return {
"magic": magic,
@@ -180,5 +198,5 @@ def unpack_header(buf: bytes, payload: bytes = b"") -> dict:
"payload_len": payload_len,
"rows": rows,
"stretch": stretch,
"crc_ok": crc16(bytes(buf[:CRC_OFFSET]) + bytes(payload)) == stored_crc,
"crc_ok": crc16(bytes(buf[:CRC_OFFSET])) == stored_crc,
}
+12 -5
View File
@@ -4,13 +4,18 @@
* fp_parse() answers one question: is the block of bytes the transport just
* delivered a complete, uncorrupted frame of the format frame.py writes? It
* checks, in this order, the length bound, the magic and version, and finally
* the CRC -- the CRC is the expensive part, so a truncated or foreign frame is
* rejected before it is computed.
* the CRC over the 12 header bytes.
*
* The CRC is CCITT-FALSE (polynomial 0x1021, initial value 0xFFFF, no input or
* output reflection, no final xor), the same one frame.py implements with a
* 256-entry table. It covers header bytes 0..11 followed by the payload; the
* stored crc16 at offset 12 is never part of its own input.
* 256-entry table. It covers header bytes 0..11 ONLY (ruling R14): the bytes
* at offset 12 are the CRC itself and the payload is never walked. A
* payload-wide CRC cost ~139 instructions per byte on this CPU and made the
* parser the frame-rate ceiling; every datagram already carries an Ethernet
* FCS the LANCE validates, and the length bound below catches truncation.
*
* So a corrupted PAYLOAD byte does not make fp_parse() reject a frame -- the
* chip's FCS is the check for that -- while a corrupted HEADER byte does.
*
* This file is compiled twice: by the 1987 TI C compiler inside the payload
* build (build-stream.sh stages it as FRMPSR.C) and by the host cc for
@@ -62,6 +67,9 @@ unsigned int len;
* See frame_parse.h for the contract. A NULL buffer or header is a rejection
* rather than a crash: the payload calls this with a pointer it published
* itself, and a zero there means "no slot yet".
*
* The CRC covers header bytes 0..11 only, so the body is never touched: a
* 20 KB frame costs the same as a 1 KB one (ruling R14).
*/
int fp_parse(buf, len, out)
unsigned char *buf;
@@ -96,7 +104,6 @@ fp_header *out;
stored = ((unsigned int) buf[FP_CRC_OFF + 1] << 8)
| (unsigned int) buf[FP_CRC_OFF];
crc = fp_crc_add(0xFFFF, buf, FP_PREFIX_SIZE);
crc = fp_crc_add(crc, buf + FP_HEADER_SIZE, plen);
if (crc != stored) {
return 0;
}
+9 -5
View File
@@ -21,8 +21,12 @@
* 14 2 padding, zero
*
* The CRC is CCITT-FALSE (polynomial 0x1021, initial value 0xFFFF, no input or
* output reflection, no final xor) over header bytes 0..11 followed by the
* payload -- never the CRC field itself.
* output reflection, no final xor) over header bytes 0..11 ONLY (ruling R14);
* the payload is never part of it. A corrupted payload byte is therefore
* accepted -- the Ethernet FCS, checked by the LANCE per datagram, is the
* check for those bytes -- while a corrupted header byte is rejected. What
* catches a truncated frame is the `len >= FP_HEADER_SIZE + payload_len` bound
* in fp_parse()'s contract at the end of this file.
*
* ---------------------------------------------------------------------------
* Marker page: bit 0x01810000, byte 0x00302000. A cell below is one 32-bit
@@ -129,9 +133,9 @@ typedef struct {
* 1 and *out filled when buf[0..len) is a well-formed frame, 0 otherwise.
* A frame is well formed when len is at least FP_HEADER_SIZE, the magic and
* version match, len is at least FP_HEADER_SIZE + payload_len, and the stored
* crc16 equals the CCITT-FALSE CRC over buf[0..11] and the payload. Bytes
* beyond the frame are ignored: payload_len is authoritative (the transport
* hands us exactly what arrived, and a tail is not part of the frame).
* crc16 equals the CCITT-FALSE CRC over buf[0..11]. Bytes beyond the frame
* are ignored: payload_len is authoritative (the transport hands us exactly
* what arrived, and a tail is not part of the frame).
*/
int fp_parse();
+51 -9
View File
@@ -2,8 +2,14 @@
The plan's two tests come first, verbatim. The rest pin down the parts of the
format the plan text is ambiguous about, because both ends must agree on them:
the CCITT-FALSE polynomial, the CRC covering header[0:12] *and* the payload, and
the exact byte offsets the C side will mirror.
the CCITT-FALSE polynomial, the CRC covering header[0:12] **only** (ruling R14),
and the exact byte offsets the C side will mirror.
Ruling R14 changed the CRC's coverage, and the consequence is stated here on
purpose: a corrupted **payload** byte leaves a frame ACCEPTED (the Ethernet FCS
of each datagram is the check for those bytes), while a corrupted **header**
byte is REJECTED, and a truncated frame (``len < 16 + payload_len``) is rejected
by the length bound that ``fp_parse`` applies before the CRC.
"""
import struct
@@ -51,18 +57,48 @@ def test_crc16_matches_an_independent_implementation():
assert crc16(data) == binascii.crc_hqx(data, 0xFFFF), n
def test_crc16_spans_the_payload():
def test_crc_covers_the_header_not_the_payload():
"""Ruling R14: payload bytes are the chip's job, header bytes are ours.
A corrupted payload byte must leave ``crc_ok`` True (the LANCE has already
validated each datagram's Ethernet FCS) and a corrupted header byte must
make it False. The payload argument is advisory: it must match
``payload_len`` in length, and its bytes are not hashed.
"""
payload = bytes(range(256)) * 2
h = pack_header(KIND_STRIP, frame_no=1, payload_len=len(payload), rows=8,
stretch=128, payload=payload)
assert unpack_header(h, payload)["crc_ok"] is True
# The same header checked without its payload must not validate: that is the
# difference between "header covers the payload" and "header only".
assert unpack_header(h)["crc_ok"] is False
# And a payload bit flip is caught.
# Same result without the body at all: the body is not part of the CRC.
assert unpack_header(h)["crc_ok"] is True
# A payload bit flip is ACCEPTED here: the Ethernet FCS is the chip's job.
bad = bytearray(payload)
bad[100] ^= 0x80
assert unpack_header(h, bytes(bad))["crc_ok"] is False
assert unpack_header(h, bytes(bad))["crc_ok"] is True
# A header bit flip (inside bytes 0..11) is REJECTED.
hh = bytearray(h)
hh[8] ^= 0x01
assert unpack_header(bytes(hh))["crc_ok"] is False
# The payload argument still has to be the length the header claims: a
# truncated body is a length error, not a CRC error (fp_parse's job).
with pytest.raises(ValueError):
unpack_header(h, payload[:-1])
with pytest.raises(ValueError):
unpack_header(h, payload + b"\x00")
def test_payload_bytes_do_not_change_the_sealed_header():
"""One header, two different bodies: the header bytes are identical."""
a = b"\x00" * 160
b = b"\xFF" * 160
ha = pack_header(KIND_STRIP, 5, len(a), 1, 8, payload=a)
hb = pack_header(KIND_STRIP, 5, len(b), 1, 8, payload=b)
assert ha == hb
assert unpack_header(ha, a)["crc_ok"] is True
assert unpack_header(hb, b)["crc_ok"] is True
def test_seal_header_after_the_payload_is_known():
@@ -94,7 +130,7 @@ def test_seal_header_rejects_over_length_input():
def test_pack_header_rejects_payload_length_mismatch():
"""A CRC sealed over N bytes must not be sent under a different payload_len."""
"""The payload argument is advisory: it must match payload_len when given."""
payload = b"\xAA" * 10
pack_header(KIND_STRIP, 1, 10, 8, 128, payload=payload) # exact: fine
with pytest.raises(ValueError):
@@ -105,6 +141,12 @@ def test_pack_header_rejects_payload_length_mismatch():
pack_header(KIND_STRIP, 1, 10, 8, 128, payload=b"") # explicit empty
# Omitting the payload is the header-only path: no length to disagree with.
assert unpack_header(pack_header(KIND_STRIP, 1, 10, 8, 128))["crc_ok"] is True
# seal_header applies the same advisory rule.
h = pack_header(KIND_STRIP, 1, 10, 8, 128)
assert seal_header(h, b"\x00" * 10) == h
assert seal_header(h) == h # not given: no check
with pytest.raises(ValueError):
seal_header(h, b"\x00" * 9)
# --- the byte layout the C guest will mirror -----------------------------
+72 -15
View File
@@ -4,22 +4,37 @@
* cc -std=c89 -Wall -o /tmp/tfp harness/udpstream/tests/test_frame_parse.c \
* harness/udpstream/frame_parse.c && /tmp/tfp
*
* The frame under test is a GOLDEN VECTOR, not something this test or
* frame_parse.c computed: the bytes in `golden` came from frame.py, the
* sender's implementation of the same spec. That is what makes the test a
* cross-check of the C parser rather than a tautology: if the C CRC, the
* field offsets or the coverage (header bytes 0..11 plus the payload, never
* the CRC field) were wrong, the golden frame would be rejected.
* The frames under test are GOLDEN VECTORS, not something this test or
* frame_parse.c computed: the bytes in `golden` and `golden_short` came from
* frame.py, the sender's implementation of the same spec. That is what makes
* the test a cross-check of the C parser rather than a tautology: if the C CRC,
* the field offsets, the CRC's coverage or the length rule were wrong, these
* frames would not be judged as frame.py judged them.
*
* Regenerate the vector with:
* THE CRC COVERS HEADER BYTES 0..11 ONLY (ruling R14). That has a consequence
* this test states explicitly rather than leaving implicit:
*
* * a corrupted PAYLOAD byte is ACCEPTED -- the bytes between the header and
* the end of the frame are not hashed here; each datagram's Ethernet FCS,
* validated by the LANCE, is what protects them;
* * a corrupted HEADER byte (inside bytes 0..11) is REJECTED;
* * a TRUNCATED frame (len < 16 + payload_len) is REJECTED by the length
* bound, which is the check that has to carry truncation now.
*
* Regenerate both vectors with:
*
* cd harness/udpstream && python3 -c '
* import frame
* pl = bytes((i*7+3) & 0xFF for i in range(80))
* d = frame.pack_header(1, 0x1234, len(pl), 1, 8, payload=pl) + pl
* print(len(d)); print(", ".join("0x%02X" % b for b in d))'
* print("golden:", len(d)); print("\t" + "".join("0x%02X, " % b for b in d))
* pl = bytes((i*7+3) & 0xFF for i in range(81))
* d = frame.pack_header(1, 0x1235, len(pl), 1, 8, payload=pl) + pl
* print("golden_short:", len(d)); print("\t" + "".join("0x%02X, " % b for b in d))'
*
* (kind 1 = KIND_STRIP, frame_no 0x1234, rows 1, stretch 8, payload_len 80.)
* `golden_short` claims 81 payload bytes and is delivered with only 80 of them,
* so its header (and its header CRC) is valid while its length is one short --
* only the length bound can reject it. (kind 1 = KIND_STRIP.)
*/
#include <stdio.h>
@@ -41,7 +56,7 @@ static void check(int cond, const char *what)
static const unsigned char golden[] = {
0x56, 0x53, 0x01, 0x01, 0x34, 0x12, 0x50, 0x00,
0x01, 0x08, 0x00, 0x00, 0xB3, 0x3C, 0x00, 0x00,
0x01, 0x08, 0x00, 0x00, 0x0B, 0x39, 0x00, 0x00,
0x03, 0x0A, 0x11, 0x18, 0x1F, 0x26, 0x2D, 0x34,
0x3B, 0x42, 0x49, 0x50, 0x57, 0x5E, 0x65, 0x6C,
0x73, 0x7A, 0x81, 0x88, 0x8F, 0x96, 0x9D, 0xA4,
@@ -54,6 +69,23 @@ static const unsigned char golden[] = {
0xFB, 0x02, 0x09, 0x10, 0x17, 0x1E, 0x25, 0x2C
};
/* The same shape, one payload byte longer than what is delivered. */
static const unsigned char golden_short[] = {
0x56, 0x53, 0x01, 0x01, 0x35, 0x12, 0x51, 0x00,
0x01, 0x08, 0x00, 0x00, 0x78, 0x3B, 0x00, 0x00,
0x03, 0x0A, 0x11, 0x18, 0x1F, 0x26, 0x2D, 0x34,
0x3B, 0x42, 0x49, 0x50, 0x57, 0x5E, 0x65, 0x6C,
0x73, 0x7A, 0x81, 0x88, 0x8F, 0x96, 0x9D, 0xA4,
0xAB, 0xB2, 0xB9, 0xC0, 0xC7, 0xCE, 0xD5, 0xDC,
0xE3, 0xEA, 0xF1, 0xF8, 0xFF, 0x06, 0x0D, 0x14,
0x1B, 0x22, 0x29, 0x30, 0x37, 0x3E, 0x45, 0x4C,
0x53, 0x5A, 0x61, 0x68, 0x6F, 0x76, 0x7D, 0x84,
0x8B, 0x92, 0x99, 0xA0, 0xA7, 0xAE, 0xB5, 0xBC,
0xC3, 0xCA, 0xD1, 0xD8, 0xDF, 0xE6, 0xED, 0xF4,
0xFB, 0x02, 0x09, 0x10, 0x17, 0x1E, 0x25, 0x2C,
0x33
};
#define GOLDEN_LEN ((unsigned int) sizeof golden)
#define GOLDEN_PLEN 80
@@ -84,15 +116,40 @@ int main(void)
check(h.payload_len == GOLDEN_PLEN, "payload_len == 80");
check(h.payload_off == FP_HEADER_SIZE, "payload_off == 16");
/* ---- a flipped payload byte fails the CRC ---- */
/* ---- a flipped PAYLOAD byte is ACCEPTED (ruling R14) ----
*
* The CRC is over the header only, so the body is not hashed here. The
* Ethernet FCS of each datagram, validated by the LANCE, is what
* protects those bytes; this parser must not pretend otherwise. */
len = load();
buf[FP_HEADER_SIZE + 40] = (unsigned char) (buf[FP_HEADER_SIZE + 40] ^ 0x01);
check(fp_parse(buf, len, &h) == 0, "flipped payload byte rejected");
check(fp_parse(buf, len, &h) == 1,
"flipped payload byte ACCEPTED (header-only CRC; FCS is the chip's job)");
/* ---- a flipped header byte inside the CRC prefix fails too ---- */
/* ---- a flipped header byte inside the CRC prefix is rejected ---- */
len = load();
buf[8] = (unsigned char) (buf[8] + 1);
check(fp_parse(buf, len, &h) == 0, "flipped header byte rejected");
len = load();
buf[3] = (unsigned char) (buf[3] ^ 0x80); /* kind, inside the prefix */
check(fp_parse(buf, len, &h) == 0, "flipped kind byte rejected");
/* ---- a valid header whose payload_len exceeds what arrived ----
*
* `golden_short` claims 81 payload bytes. Delivered whole it is a valid
* frame (so the vector is not simply broken); one byte short it must be
* rejected by the LENGTH BOUND alone, because its header CRC is right. */
{
unsigned int n = (unsigned int) sizeof golden_short;
memcpy(buf, golden_short, sizeof golden_short);
check(fp_parse(buf, n, &h) == 1,
"golden_short delivered whole is accepted");
check(h.payload_len == 81, "golden_short payload_len == 81");
memcpy(buf, golden_short, sizeof golden_short);
check(fp_parse(buf, n - 1, &h) == 0,
"length bound rejects len < 16 + payload_len (header CRC valid)");
}
/* ---- short buffers: one byte short of the frame, and below the header ---- */
len = load();
@@ -126,11 +183,11 @@ int main(void)
buf[FP_CRC_OFF + 1] = (unsigned char) (buf[FP_CRC_OFF + 1] + 1);
check(fp_parse(buf, len, &h) == 0, "corrupt crc high byte rejected");
/* ---- a header that understates payload_len cannot hide bad bytes ---- */
/* ---- editing payload_len is a header edit: the header CRC catches it ---- */
len = load();
buf[6] = 0x40; /* payload_len 0x40 instead of 0x50 */
check(fp_parse(buf, len, &h) == 0,
"payload_len shorter than the sealed payload rejected");
"header payload_len edit (CRC not recomputed) rejected");
/* ---- payload_len is authoritative: a tail in the slot is not the frame ---- */
len = load();
+7 -5
View File
@@ -5,9 +5,10 @@ text leaves implicit but the terminal depends on: the frame is split at 1450
bytes with only the last packet short, and a payload whose length disagrees with
``rows`` is rejected here instead of on the wire.
Note the third plan test checks ``crc_ok`` *with the payload*: the header CRC
covers header[0:12] + payload, so a header-only seal would fail every frame on
the receiving side.
Note the third plan test checks ``crc_ok`` *with the payload*: since ruling R14
the header CRC covers header[0:12] only, so the same result must hold with or
without the body -- the payload argument is an advisory length check, not part
of the CRC.
"""
import pytest
@@ -105,8 +106,9 @@ def test_every_packet_carries_the_same_valid_header():
assert head["payload_len"] == 8 * 160
assert head["rows"] == 8 and head["stretch"] == 128
assert unpack_header(pkts[0][:HEADER_SIZE], b"".join(pkts)[HEADER_SIZE:])["crc_ok"] is True
# A header-only check must not validate: that would mean the CRC missed the body.
assert head["crc_ok"] is False
# The header-only check gives the same answer (ruling R14): the body is
# covered by the Ethernet FCS, not by this CRC.
assert head["crc_ok"] is True
def test_encode_rejects_a_payload_that_is_not_rows_times_160():
+5 -4
View File
@@ -15,10 +15,11 @@ header against the whole payload, exactly as the C receiver does:
[ header | body ... ][ body ... ][ body ]
The header CRC covers header[0:12] **plus the payload**, so ``pack_header`` is
called with ``payload=rows_1bpp``. A header-only CRC would satisfy every local
test that only unpacks the header and would still be rejected by the terminal
for every single frame.
The header CRC covers header[0:12] **only** (ruling R14): the body bytes are
covered by each datagram's Ethernet FCS, and a payload-wide CRC is far too slow
on the terminal's CPU. ``pack_header`` is still called with
``payload=rows_1bpp`` as an advisory check that the body is exactly
``payload_len`` bytes -- it does not change the CRC.
Pure standard library; no dependency on any host-side capture stack.
"""