fix(perf): TRex ASTF mode + SSM TimeoutSeconds>=30 + kill stale TCP DUTs

scripts/perf-tests/test/test_run_perf_tests.sh

@@ -0,0 +1,100 @@
+#!/usr/bin/env bash
+#
+# Regression unit tests for scripts/run-perf-tests.sh harness logic.
+#
+# These guard the three harness bugs surfaced by perf run 28509354005 — which
+# cost a full ~30-min EC2 deploy to find. They run in <1s with no AWS and no
+# TRex by sourcing the harness (its `main` is source-guarded) and stubbing the
+# `aws` CLI.
+#
+#   1. SSM TimeoutSeconds < 30  -> AWS ParamValidation rejection.
+#   2. pkill pattern missed the TCP DUT binaries -> stale process held the DPDK
+#      primary-process lock -> next config's EAL init failed.
+#   3. TRex launched in STL mode while the TCP benchmark speaks ASTF -> RPC
+#      configuration mismatch.
+#
+# Usage:  bash scripts/perf-tests/test/test_run_perf_tests.sh
+set -uo pipefail
+
+TEST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+HARNESS="$TEST_DIR/../../run-perf-tests.sh"
+
+PASS=0
+FAIL=0
+ok()  { PASS=$((PASS + 1)); echo "  ok   - $1"; }
+bad() { FAIL=$((FAIL + 1)); echo "  FAIL - $1"; }
+
+# ── Stub the `aws` CLI: record send-command args, report Success on poll ──────
+STUB_DIR="$(mktemp -d)"
+export AWS_LOG="$STUB_DIR/aws.log"
+cat > "$STUB_DIR/aws" <<'STUB'
+#!/usr/bin/env bash
+echo "aws $*" >> "$AWS_LOG"
+case "$*" in
+    *send-command*)                   echo "cmd-stub-0000" ;;  # Command.CommandId
+    *get-command-invocation*Status*)  echo "Success" ;;
+    *)                                echo "" ;;
+esac
+STUB
+chmod +x "$STUB_DIR/aws"
+export PATH="$STUB_DIR:$PATH"
+
+# Source the harness (source-guard keeps main() from running), then relax the
+# strict flags it sets so the tests can drive failure paths deliberately.
+# shellcheck disable=SC1090
+source "$HARNESS"
+set +e +u +o pipefail
+trap - EXIT ERR INT TERM   # drop the harness's safety-net teardown trap
+
+ere_matches() { printf '%s' "$1" | grep -Eq "$DUT_APP_ERE"; }
+
+# ── 1. SSM TimeoutSeconds clamp (>= 30) ──────────────────────────────────────
+echo "== SSM TimeoutSeconds clamp =="
+: > "$AWS_LOG"
+ssm_run_command "i-test" 15 "echo hi" >/dev/null 2>&1
+if grep -q -- '--timeout-seconds 30' "$AWS_LOG"; then ok "15s call is clamped to --timeout-seconds 30"; else bad "15s call is clamped to --timeout-seconds 30"; fi
+if grep -q -- '--timeout-seconds 15' "$AWS_LOG"; then bad "no send-command uses the invalid 15"; else ok "no send-command uses the invalid 15"; fi
+: > "$AWS_LOG"
+ssm_run_command "i-test" 90 "echo hi" >/dev/null 2>&1
+if grep -q -- '--timeout-seconds 90' "$AWS_LOG"; then ok "valid 90s is passed through unchanged"; else bad "valid 90s is passed through unchanged"; fi
+
+# ── 2. DUT_APP_ERE matches every DUT binary (incl. TCP) but not TRex/sshd ─────
+echo "== DUT_APP_ERE composed from the source-of-truth lists =="
+for b in "${DUT_RUST_BINS[@]}"; do
+    if ere_matches "./target/release/$b --ip 10.0.1.10 --port 9000"; then ok "matches $b"; else bad "matches $b (would leak a stale DUT process)"; fi
+done
+for b in "${DUT_OTHER_BINS[@]}"; do
+    if ere_matches "/usr/local/bin/$b -l 0-1 -- --forward-mode=5tswap"; then ok "matches $b"; else bad "matches $b"; fi
+done
+if ere_matches "/opt/trex/_t-rex-64 -i --cfg /etc/trex_cfg.yaml"; then bad "must NOT match t-rex-64 (would kill the generator)"; else ok "does not match t-rex-64"; fi
+if ere_matches "/usr/sbin/sshd -D"; then bad "must NOT match sshd"; else ok "does not match sshd"; fi
+
+echo "== dut_kill_snippet composition (grace + lock cleanup) =="
+SNIP="$(dut_kill_snippet)"
+if [[ "$SNIP" == *"for i in 1 2 3 4 5 6 7 8 9 10"* ]]; then ok "grace list expands to DUT_KILL_GRACE_SECS ($DUT_KILL_GRACE_SECS)"; else bad "grace list expands to $DUT_KILL_GRACE_SECS"; fi
+if [[ "$SNIP" == *"rm -rf /var/run/dpdk/"* ]]; then ok "snippet clears the DPDK lock dir"; else bad "snippet clears the DPDK lock dir"; fi
+
+# ── 3. TRex mode selection: stl for UDP, astf for any *-tcp ───────────────────
+echo "== TRex STL/ASTF mode selection =="
+for c in "rust-dpdk-tcp,tokio-dpdk-tcp,plain-rust-tcp" "rust-dpdk-tcp"; do
+    if [[ "$(trex_mode_for_configs "$c")" == astf ]]; then ok "astf for '$c'"; else bad "astf for '$c'"; fi
+done
+for c in "plain-rust,rust-dpdk,native-dpdk" "native-dpdk-v6,rust-dpdk-v6"; do
+    if [[ "$(trex_mode_for_configs "$c")" == stl ]]; then ok "stl for '$c'"; else bad "stl for '$c'"; fi
+done
+
+# ── ensure_trex_mode restarts only when the mode actually changes ────────────
+echo "== ensure_trex_mode restart-on-change =="
+START_LOG="$STUB_DIR/start.log"; : > "$START_LOG"
+start_trex_server() { echo "$1" >> "$START_LOG"; CURRENT_TREX_MODE="$1"; return 0; }  # stub
+CURRENT_TREX_MODE=stl
+ensure_trex_mode stl >/dev/null 2>&1
+if [[ -s "$START_LOG" ]]; then bad "no restart when mode is unchanged"; else ok "no restart when mode is unchanged"; fi
+ensure_trex_mode astf >/dev/null 2>&1
+if grep -qx astf "$START_LOG"; then ok "restarts to astf when switching from stl"; else bad "restarts to astf when switching from stl"; fi
+
+# ── Summary ──────────────────────────────────────────────────────────────────
+echo ""
+echo "== $PASS passed, $FAIL failed =="
+rm -rf "$STUB_DIR"
+[[ $FAIL -eq 0 ]]

scripts/run-perf-tests.sh

@@ -113,9 +113,35 @@ post_pr_comment() {
 
 # ── SSM Helpers ───────────────────────────────────────────────────────────────
 
+# Single source of truth for the DUT app binaries the perf harness may launch.
+# Rust binaries live under target/release/; testpmd is a bare process name.
+# MUST include the TCP binaries — otherwise a stale process keeps the DPDK
+# primary-process lock and the next config's EAL init fails ("Cannot create
+# lock ... another primary process running"). Add a binary here and everything
+# below (regex, cleanup) composes from it automatically.
+DUT_RUST_BINS=(echo tokio-echo plain-echo tcp-echo tokio-tcp-echo plain-tcp-echo quic-echo-server quic-perf-client)
+DUT_OTHER_BINS=(testpmd dpdk-testpmd)
+DUT_KILL_GRACE_SECS=10   # seconds to wait for a graceful SIGTERM exit before SIGKILL
+
+# ERE matching any DUT process cmdline, composed from the lists above.
+DUT_APP_ERE="target/release/($(IFS='|'; echo "${DUT_RUST_BINS[*]}"))|$(IFS='|'; echo "${DUT_OTHER_BINS[*]}")"
+
+# Remote (POSIX-sh) cleanup snippet: SIGTERM all DUT apps, wait up to
+# DUT_KILL_GRACE_SECS for a graceful exit, then SIGKILL, then clear the DPDK
+# primary-process lock. The wait list is expanded LOCALLY via seq so the string
+# sent over SSM stays POSIX-portable (the remote shell may be dash, not bash).
+dut_kill_snippet() {
+    local ere="$DUT_APP_ERE" grace
+    grace="$(seq "$DUT_KILL_GRACE_SECS" | tr '\n' ' ')"
+    echo "set +e; pkill -TERM -f '$ere' 2>/dev/null; for i in $grace; do pgrep -f '$ere' >/dev/null 2>&1 || break; sleep 1; done; if pgrep -f '$ere' >/dev/null 2>&1; then pkill -9 -f '$ere' 2>/dev/null; sleep 3; fi; rm -rf /var/run/dpdk/ 2>/dev/null"
+}
+
 ssm_run_command() {
     local instance_id="$1"
     local timeout_sec="$2"
+    # AWS SSM send-command rejects TimeoutSeconds < 30 (ParamValidation);
+    # clamp so short cleanup calls (e.g. 15s) don't hard-fail the step.
+    if [[ "$timeout_sec" -lt 30 ]]; then timeout_sec=30; fi
     shift 2
     local command="$*"
 
@@ -459,7 +485,7 @@ dut_bind_dpdk() {
     # Gracefully stop any running DPDK/echo apps first — SIGTERM lets DPDK run
     # rte_eal_cleanup() so vfio-pci devices are properly released.
     ssm_run_command "$DUT_INSTANCE_ID" 30 \
-        "set +e; pkill -TERM -f 'target/release/echo' 2>/dev/null; pkill -TERM -f 'target/release/plain-echo' 2>/dev/null; pkill -TERM -f testpmd 2>/dev/null; pkill -TERM -f dpdk-testpmd 2>/dev/null; for i in 1 2 3 4 5 6 7 8 9 10; do pgrep -f 'target/release/echo|target/release/plain-echo|testpmd' >/dev/null 2>&1 || break; sleep 1; done; if pgrep -f 'target/release/echo|target/release/plain-echo|testpmd' >/dev/null 2>&1; then pkill -9 -f 'target/release/echo' 2>/dev/null; pkill -9 -f 'target/release/plain-echo' 2>/dev/null; pkill -9 -f testpmd 2>/dev/null; pkill -9 -f dpdk-testpmd 2>/dev/null; sleep 3; fi; echo CLEANUP_DONE" || true
+        "$(dut_kill_snippet); echo CLEANUP_DONE" || true
 
     # Use sysfs driver_override — same method that works for TRex binding.
     # This avoids dpdk-devbind.py which can fail in edge cases.
@@ -478,7 +504,7 @@ dut_bind_kernel() {
     log_info "Binding DUT secondary ENI to kernel driver (kernel mode)..."
     # Gracefully stop any running DPDK/echo apps — SIGTERM lets DPDK cleanup run.
     ssm_run_command "$DUT_INSTANCE_ID" 30 \
-        "set +e; pkill -TERM -f 'target/release/echo' 2>/dev/null; pkill -TERM -f 'target/release/plain-echo' 2>/dev/null; pkill -TERM -f testpmd 2>/dev/null; pkill -TERM -f dpdk-testpmd 2>/dev/null; for i in 1 2 3 4 5 6 7 8 9 10; do pgrep -f 'target/release/echo|target/release/plain-echo|testpmd' >/dev/null 2>&1 || break; sleep 1; done; if pgrep -f 'target/release/echo|target/release/plain-echo|testpmd' >/dev/null 2>&1; then pkill -9 -f 'target/release/echo' 2>/dev/null; pkill -9 -f 'target/release/plain-echo' 2>/dev/null; pkill -9 -f testpmd 2>/dev/null; pkill -9 -f dpdk-testpmd 2>/dev/null; sleep 3; fi; rm -rf /var/run/dpdk/ 2>/dev/null; echo CLEANUP_DONE" || true
+        "$(dut_kill_snippet); echo CLEANUP_DONE" || true
 
     local bind_out
     bind_out=$(ssm_run_command "$DUT_INSTANCE_ID" 60 \
@@ -498,7 +524,7 @@ dut_stop_all_apps() {
     # cleanup (rte_eal_cleanup via Drop) so vfio-pci devices are properly released.
     # Only escalate to SIGKILL after 10s if the process won't die.
     stop_result=$(ssm_run_command "$DUT_INSTANCE_ID" 30 \
-        "set +e; pkill -TERM -f 'target/release/echo' 2>/dev/null; pkill -TERM -f 'target/release/tokio-echo' 2>/dev/null; pkill -TERM -f 'target/release/plain-echo' 2>/dev/null; pkill -TERM -f testpmd 2>/dev/null; pkill -TERM -f dpdk-testpmd 2>/dev/null; for i in 1 2 3 4 5 6 7 8 9 10; do pgrep -f 'target/release/echo|target/release/tokio-echo|target/release/plain-echo|testpmd' >/dev/null 2>&1 || break; sleep 1; done; if pgrep -f 'target/release/echo|target/release/tokio-echo|target/release/plain-echo|testpmd' >/dev/null 2>&1; then echo 'SIGTERM did not work, escalating to SIGKILL'; pkill -9 -f 'target/release/echo' 2>/dev/null; pkill -9 -f 'target/release/tokio-echo' 2>/dev/null; pkill -9 -f 'target/release/plain-echo' 2>/dev/null; pkill -9 -f testpmd 2>/dev/null; pkill -9 -f dpdk-testpmd 2>/dev/null; sleep 3; fi; echo 'All apps stopped'") || true
+        "$(dut_kill_snippet); echo 'All apps stopped'") || true
     log_info "Stop result: $stop_result"
     # Clean up stale DPDK shared memory — if the previous app was SIGKILL'd,
     # the shared memory files persist and can cause the next DPDK app to fail
@@ -661,7 +687,11 @@ YAMLEOF
 }
 
 start_trex_server() {
-    log_info "Starting TRex server..."
+    local trex_mode="${1:-stl}"
+    local astf_flag=""
+    [[ "$trex_mode" == "astf" ]] && astf_flag="--astf"
+    CURRENT_TREX_MODE="$trex_mode"
+    log_info "Starting TRex server (mode: $trex_mode)..."
     local TX_PCI="0000:00:06.0"
     local RX_PCI="0000:00:07.0"
 
@@ -696,7 +726,7 @@ start_trex_server() {
     log_info "Starting TRex server..."
     local start_cmd_id
     start_cmd_id=$(ssm_run_command_fire_and_forget "$TREX_INSTANCE_ID" 120 \
-        "pkill -f t-rex-64 2>/dev/null || true; sleep 1; rm -f /var/run/dpdk/ 2>/dev/null || true; cd /opt/trex && nohup /opt/trex/t-rex-64 -i --cfg /etc/trex_cfg.yaml -c 2 </dev/null >/var/log/trex-server.log 2>&1 & disown")
+        "pkill -f t-rex-64 2>/dev/null || true; sleep 2; rm -rf /var/run/dpdk/ 2>/dev/null || true; cd /opt/trex && nohup /opt/trex/t-rex-64 -i $astf_flag --cfg /etc/trex_cfg.yaml -c 2 </dev/null >/var/log/trex-server.log 2>&1 & disown")
     log_info "TRex start command sent (cmd_id: ${start_cmd_id:-none})"
 
     # Wait for TRex to initialize DPDK and start its API server.
@@ -731,6 +761,26 @@ start_trex_server() {
     return 1
 }
 
+# Ensure the TRex server is running in the requested mode (stl | astf),
+# restarting it if the current mode differs. STL drives stateless (UDP)
+# benchmarks; ASTF drives stateful (TCP) benchmarks. A single `t-rex-64 -i`
+# process serves only ONE mode, so mixing UDP and TCP configs in one run
+# restarts TRex between them.
+ensure_trex_mode() {
+    local want="$1"
+    if [[ "${CURRENT_TREX_MODE:-}" == "$want" ]]; then
+        return 0
+    fi
+    log_info "Switching TRex to '$want' mode (was: '${CURRENT_TREX_MODE:-none}')"
+    start_trex_server "$want"
+}
+
+# Derive the TRex mode (stl|astf) for a comma-separated config list: astf if any
+# token ends in -tcp (TCP -> ASTF), else stl (UDP -> STL). Unit-tested.
+trex_mode_for_configs() {
+    [[ ",$1," == *"-tcp,"* ]] && echo astf || echo stl
+}
+
 stop_trex_server() {
     log_info "Stopping TRex server..."
     ssm_run_command "$TREX_INSTANCE_ID" 30 \
@@ -749,6 +799,9 @@ run_benchmark_for_config() {
         return $?
     fi
 
+    # UDP configs need TRex in STL (stateless) mode.
+    ensure_trex_mode stl || { log_error "Failed to start TRex in STL mode"; return 1; }
+
     log_info "Running TRex benchmark for config: $config_name"
 
     # Copy benchmark script to TRex instance using base64 encoding.
@@ -913,14 +966,19 @@ $(head -5 "$RESULTS_DIR/${config_name}.json")
 # Run a TCP (ASTF/stateful) benchmark for a *-tcp config. Mirrors
 # run_benchmark_for_config but deploys run_tcp_benchmark.py and drives it with
 # --payload-sizes / --cps-rates (ASTFClient) instead of run_benchmark.py
-# (STLClient). TRex `t-rex-64 -i` serves both STL and ASTF, so no server-side
-# change is needed — only the python client class differs.
+# (STLClient). TRex must run in ASTF mode for this: ensure_trex_mode astf
+# (re)starts t-rex-64 with --astf before the ASTF client connects.
 run_tcp_benchmark_for_config() {
     local config_name="$1"
     local dst_port="${2:-9000}"
 
     log_info "Running TRex ASTF TCP benchmark for config: $config_name"
 
+    # TCP uses ASTF (stateful); (re)start TRex in ASTF mode if needed. A TRex
+    # server started in STL mode rejects ASTF clients ("RPC configuration
+    # mismatch - server 'STL', client 'ASTF'").
+    ensure_trex_mode astf || { log_error "Failed to start TRex in ASTF mode"; return 1; }
+
     local benchmark_b64
     benchmark_b64=$(base64 -w0 "$SCRIPT_DIR/perf-tests/trex/run_tcp_benchmark.py")
     ssm_run_command "$TREX_INSTANCE_ID" 30 \
@@ -2411,7 +2469,11 @@ Starting TRex configuration (MAC discovery + NIC binding)..."
 - Gateway MAC: \`$TREX_GATEWAY_MAC\`
 Starting TRex server..."
 
-    if ! start_trex_server; then
+    # Start TRex in the mode of the first config type (astf if any *-tcp token,
+    # else stl) so a coherent all-TCP or all-UDP run needs no mid-run restart.
+    local initial_trex_mode
+    initial_trex_mode=$(trex_mode_for_configs "$CONFIGS")
+    if ! start_trex_server "$initial_trex_mode"; then
         # Grab TRex log and NIC state for diagnostics
         local trex_log
         local diag_pci="${TREX_PCI_ADDR:-0000:00:06.0}"
@@ -2694,4 +2756,7 @@ $summary
     log_info "=== All performance tests completed successfully ==="
 }
 
-main "$@"
+# Only run main when executed directly (not when sourced, e.g. by unit tests).
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+    main "$@"
+fi