From d4a2504f9913b68a65fde2e5c05cde4301d4e351 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Thu, 23 Jul 2026 15:45:37 +0000 Subject: [PATCH] Teach the trailing-log gate to see command-substitution call sites Found while fixing count_log_pattern: the gate reported that function clean even though it ends in echo and both callers consume its status. Its call-site patterns only matched direct forms -- if fn, while fn, fn ||, fn && -- so a status consumed through command substitution was invisible, because the line begins with the variable rather than the function name. That is not an exotic form. It is how a shell function returns a value, and it is precisely the shape of the swallowed-failure family this gate exists to catch, so the gate was blind to a large part of its own stated class. Three patterns added for the assignment, if-guarded and test-expression forms. The extension finds four real instances, all value-returning helpers whose trailing echo made their exit status unconditionally zero, and each now carries an explicit return 0. Also corrects the finding message, which described the trailing command as a log call; for these it is the return mechanism, and the hazard is that it fixes the status either way. Validated by breaking what it guards: a probe reintroducing the defect shape behind a command substitution is reported and exits 1, where before the extension it would have passed. --- .../chaos/scenarios/congestion-stress.yaml | 42 +++++++++++++++---- testing/check-trailing-log.py | 9 ++++ testing/static/scripts/bench-multirun.sh | 1 + testing/static/scripts/generate-configs.sh | 2 + testing/static/scripts/netem.sh | 1 + 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/testing/chaos/scenarios/congestion-stress.yaml b/testing/chaos/scenarios/congestion-stress.yaml index 8dce07e..f5aa4c6 100644 --- a/testing/chaos/scenarios/congestion-stress.yaml +++ b/testing/chaos/scenarios/congestion-stress.yaml @@ -119,15 +119,39 @@ assertions: min_nodes_ce_received: 1 # The fourth criterion, "kernel_drop_events > 0 on at least one node", is -# NOT asserted, because the implementation does not meet it: across all 182 -# archived runs of this scenario, including the six that meet the other -# three, no node has ever reported a non-zero kernel_drop_events. Asserting -# it would red the scenario permanently, and asserting a weakened version -# would assert nothing. It stands here as an unmet criterion: either the -# 4 KB recv_buf_size no longer provokes SO_RXQ_OVFL under this traffic, or -# the counter does not reach the snapshot. Until that is settled it is a -# coverage gap, because the transport drop-detection path this scenario -# names as signal 2 is exercised by nothing. +# NOT asserted, because this scenario has never met it: across all 182 +# archived runs, including the six that meet the other three, no node has +# reported a non-zero kernel_drop_events. Asserting it would red the scenario +# permanently, and asserting a weakened version would assert nothing. +# +# CORRECTED 2026-07-23. An earlier version of this note concluded that the +# transport drop-detection path "is exercised by nothing". That is wrong, and +# the error was one of scope: it looked only inside this scenario. Widening +# the scan to every scenario's congestion snapshots -- 2317 files, 11778 node +# records -- finds 51 records reporting kernel_drop_events > 0 across six +# other scenarios (depth-vs-cost, bottleneck-parent, tcp-mesh, cost-avoidance, +# maelstrom-sparse, mixed-technology), with raw drop counts up to 41165. The +# path is live and plumbed through to the snapshot. What is dead is this +# scenario's ability to provoke it. +# +# The inversion is the lead: every scenario that DOES overflow runs with the +# default 2 MB recv_buf_size, while this one, the only scenario that shrinks +# it to 4 KB, never does. SO_RXQ_OVFL counts datagrams arriving at a full +# queue, and the 1 Mbps cap below sets the arrival rate to 125 kB/s per link. +# Linux doubles a requested SO_RCVBUF, so the queue is 8192 bytes and filling +# it takes ~65 ms of reader stall -- ~22 ms even at the busiest node's 3 Mbps +# aggregate. Traffic volume cannot cause the overflow because the volume is +# capped below the rate the buffer drains. The ingress policer works against +# the same signal, discarding excess in tc before the socket ever sees it. +# +# So signal 3 appears to suppress signal 2, and the two may not belong in one +# scenario. Tracked with the experiment that would settle it (raise or drop +# the bandwidth cap for one run and watch the counter) in ISSUE-2026-0084. +# +# Note also that the header above says "small recv_buf_size (8KB)" while +# fips_overrides sets 4096. The 8 KB figure matches what Linux allocates after +# doubling, so the comment may be describing the effective queue rather than +# the setting; it reads as the setting and should say which it means. logging: rust_log: "info" diff --git a/testing/check-trailing-log.py b/testing/check-trailing-log.py index eb8a5c8..b4f4817 100755 --- a/testing/check-trailing-log.py +++ b/testing/check-trailing-log.py @@ -133,6 +133,15 @@ def status_tested(name: str, all_text: str, defining_file: Path) -> list[str]: (rf"^\s*{re.escape(name)}\s+[^\n|&]*&&", " &&"), (rf"^\s*{re.escape(name)}\s*&&", " &&"), (rf"\bif\s+\S*\s*{re.escape(name)}\b.*;\s*then", "if ... ; then"), + # Command substitution. Found 2026-07-23 while fixing a function this + # check reported clean: `total=$(count_log_pattern "$p") || { ... }` + # consumes the status, but the line begins with the variable, so none + # of the patterns above match it. That is not an exotic form — it is + # how a shell function returns a *value*, and it is the shape of the + # `|| true`-swallowed-failure family this check exists to catch. + (rf"=\$\(\s*{re.escape(name)}\b", "=$()"), + (rf"\bif\s+!?\s*\S*=?\$\(\s*{re.escape(name)}\b", "if =$()"), + (rf"\[\s+\"?\$\(\s*{re.escape(name)}\b", "[ $() ]"), ] hits = [] for pat, label in patterns: diff --git a/testing/static/scripts/bench-multirun.sh b/testing/static/scripts/bench-multirun.sh index d9a8547..d2a138b 100755 --- a/testing/static/scripts/bench-multirun.sh +++ b/testing/static/scripts/bench-multirun.sh @@ -120,6 +120,7 @@ ping_path() { return fi echo "${rtt//\// } ${loss:-N/A}" + return 0 } # ── Path definitions ─────────────────────────────────────────────────────── diff --git a/testing/static/scripts/generate-configs.sh b/testing/static/scripts/generate-configs.sh index 9be398b..a1d1a04 100755 --- a/testing/static/scripts/generate-configs.sh +++ b/testing/static/scripts/generate-configs.sh @@ -74,6 +74,7 @@ docker_host_name() { local host host=$(get_node_attr "$topology_file" "$node_id" "docker_host") echo "${host:-node-$node_id}" + return 0 } # Get peers list from topology @@ -118,6 +119,7 @@ get_default_transport() { local topology_file="$1" local transport=$(grep "^default_transport:" "$topology_file" | head -1 | sed 's/.*: *\([a-z]*\).*/\1/') echo "${transport:-udp}" + return 0 } # Get the port for a given transport type diff --git a/testing/static/scripts/netem.sh b/testing/static/scripts/netem.sh index 866a20f..30d1787 100755 --- a/testing/static/scripts/netem.sh +++ b/testing/static/scripts/netem.sh @@ -159,6 +159,7 @@ build_netem_params() { fi echo "$params" + return 0 } # Check if a container is running