Repository navigation
test(cpworker): test checksums, parse_packet bounds, zmq flush and control-socket errors by spec - #314
Merged
Conversation
added 2 commits
October 13, 2026 15:25
…ntrol-socket errors by spec A coverage and mutation run (#243) found tests that pass without checking the behaviour: the TCP/UDP checksum tests verify a checksum with the code that computed it, no parse_packet case ends exactly at the L4 header, the ZMQ batch flush rules of docs/ZMQ-WIRE-FORMAT.md section 8 have no test, and the control socket's error paths are never driven. Tests added, no source change: - packet_split: known-answer TCP/UDP checksums over IPv4 and IPv6, with expected values from an independent RFC 1071 implementation. The lengths reach the scalar and SSE2 loops and end on an odd byte. - packet_split: frames that end where the TCP/UDP header ends (pure ACK, empty datagram; with VLAN, TCP options, an IPv6 extension header) parse with an empty payload, and one byte less does not parse. - output_zmq: a batch is flushed only when the next record would pass the 1 MiB buffer, when a packet comes more than a second after the first one, and from zmq_heartbeat() once an idle batch is over a second old. - control socket: a handshake that is not JSON, names another version or has no string version, and a request that is not JSON or has no string command, close the connection without a reply; an unknown command gets status ERROR and the connection stays usable. Each new test fails against a mutant of the code it covers: dropped or misread pseudo-header words, the odd trailing byte in the wrong half, `<` turned into `<=` at the L4 header checks, `>` turned into `>=` in the flush conditions, an unknown command answered OK, and any version accepted at handshake. Refs #243
test_stats_summary_has_wall_time_and_monotonic_time bracketed the snapshot's CLOCK_REALTIME reading with time(). On x86_64 glibc, time() reads the coarse realtime clock through the vDSO. That clock lags CLOCK_REALTIME by up to a tick, so a snapshot taken in the first milliseconds of a second can be one second later than the time() call made after it. CI failed this way on PR #314: Expected 1791876577 to be less than or equal to 1791876576 The bounds now come from clock_gettime(CLOCK_REALTIME), the clock the snapshot reads.
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.
Problem
A coverage and mutation run (#243) found tests that pass without checking the behaviour they cover. Several of its items were addressed by later PRs (#278, #279, #282, #287, #304). This PR covers the remaining cpworker unit-level gaps:
parse_packetcase ends exactly at the end of the TCP/UDP header. A pure ACK or an empty datagram is such a frame, and changing<to<=at the L4 header checks went undetected.docs/ZMQ-WIRE-FORMAT.md§8 (buffer limit, one-second age, idle flush fromzmq_heartbeat()) have no test.control_socket_test.godrives only the valid handshake andping. Bad handshakes, malformed requests and unknown commands are never sent over the real socket.Change
Tests only. No source change.
unix_control:test_stats_summary_has_wall_time_and_monotonic_time(from fix(cpworker): keep zmq heartbeats out of fwd counters; stamp stats with wall-clock time #309) failed CI on this PR withExpected 1791876577 to be less than or equal to 1791876576. The test bracketed the snapshot'sCLOCK_REALTIMEreading withtime(), which on x86_64 glibc reads the coarse realtime clock through the vDSO. That clock lagsCLOCK_REALTIMEby up to a tick, so a snapshot taken in the first milliseconds of a second could fail the upper bound. The bounds now come fromclock_gettime(CLOCK_REALTIME).packet_split: known-answer TCP/UDP checksums over IPv4 and IPv6. The expected values come from an independent RFC 1071 implementation over the RFC 793 / RFC 768 / RFC 8200 §8.1 pseudo-header. The lengths reach the scalar and SSE2 loops and end on an odd byte.packet_split: frames that end where the TCP/UDP header ends parse with an empty payload, and one byte less does not parse. The cases are IPv4 and IPv6, TCP and UDP, with a VLAN tag, with TCP options and with an IPv6 extension header.output_zmq: a batch that fills the 1 MiB buffer exactly is not flushed; the next record flushes it. A packet one second after the batch's first packet joins the batch; a packet two seconds after flushes it.zmq_heartbeat()flushes an idle batch once it is over a second old.control_socket_test.goadds the subtestrejected_requests:version.command.status: ERRORand the connection stays usable.Not changed, still open in #243:
cpdaemon/pkg/cpmandcpgolib/cpworker(item 6).The
caplen < offset + ip_hdr_lenmutant named in the issue cannot be detected. The same holds for the VLAN and IPv6 extension-header length checks. In each case a frame that ends there is rejected by the following L4 header check anyway.ZMQ_PKTS_FLUSH_MAX_NUMcannot trigger either: the smallest record is 36 bytes, so the 1 MiB buffer fills after about 29k records.Tests
The new tests pass on the current code and fail against these mutants (each applied alone):
protocol, the IPv6 pseudo-header readingd[2]ford[3], and the odd trailing byte added to the high half.parse_packet:<→<=at the IPv4 TCP header length check and at the IPv4 and IPv6 UDP header checks.>→>=at the payload-offset check.<→<=at the IPv4 total-length and IPv6 payload-length checks.>→>=in the buffer-full condition, the packet-age condition and the idle-flush condition, and the idle flush removed entirely.OK, and any version ≥v1accepted at handshake.Suites run on Ubuntu 24.04:
run_test.sh allpasses.Refs #243