diff --git a/docs/perfsonar/tools_scripts/CHANGELOG.md b/docs/perfsonar/tools_scripts/CHANGELOG.md index 08ddfeb..a378a0e 100644 --- a/docs/perfsonar/tools_scripts/CHANGELOG.md +++ b/docs/perfsonar/tools_scripts/CHANGELOG.md @@ -3,6 +3,7 @@ ### Fixed - **Critical JSON state save corruption fix**: Fixed invalid JSON generation in `--save-state` that caused `--restore-state` to fail +- **Fix (1.3.6)**: Respect `--dry-run` in packet pacing application and ensure audit logic checks actual qdisc state; prevent `--mode apply --dry-run` from changing system qdisc or sysctl state. - Properly quote non-numeric ring buffer values (e.g., `Mini:`, `push`, `n/a`) - Properly quote non-numeric `nm_mtu` values (e.g., `auto`) - Sanitize ring buffer values to ensure numeric-only or properly quoted strings diff --git a/docs/perfsonar/tools_scripts/fasterdata-tuning.sh b/docs/perfsonar/tools_scripts/fasterdata-tuning.sh index 642aa6c..505c758 100755 --- a/docs/perfsonar/tools_scripts/fasterdata-tuning.sh +++ b/docs/perfsonar/tools_scripts/fasterdata-tuning.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # fasterdata-tuning.sh # -------------------- -# Version: 1.3.5 +# Version: 1.3.6 # Author: Shawn McKee, University of Michigan # Acknowledgements: Supported by IRIS-HEP and OSG-LHC # @@ -1271,36 +1271,39 @@ iface_apply() { iface_packet_pacing_audit() { local iface="$1" local pacing_rate="$2" - + # Only audit packet pacing for DTN nodes if [[ "$TARGET_TYPE" != "dtn" ]]; then return 0 fi - + # Check current qdisc - fq enables Linux TCP pacing; tbf indicates explicit rate cap local current_qdisc - current_qdisc=$(tc qdisc show dev "$iface" 2>/dev/null | head -n1) - - # If we're not applying pacing, just report current state - if [[ $APPLY_PACKET_PACING -eq 0 ]]; then - # Accept either fq (TCP pacing) or tbf (explicit cap) as "pacing applied" - if [[ "$current_qdisc" == *" fq"* || "$current_qdisc" == fq* || "$current_qdisc" == *" tbf"* ]]; then - return 0 - fi - return 1 + current_qdisc=$(tc qdisc show dev "$iface" 2>/dev/null | head -n1 || echo "") + + # Accept either fq (TCP pacing) or tbf (explicit cap) as "pacing applied" + if [[ "$current_qdisc" == *" fq"* || "$current_qdisc" == fq* || "$current_qdisc" == *" tbf"* ]]; then + return 0 fi - - return 0 + + # Not found + return 1 } iface_apply_packet_pacing() { local iface="$1" local pacing_rate="$2" - + + # Respect DRY_RUN and target type / feature flag + if [[ $DRY_RUN -eq 1 ]]; then + log_info "Dry-run: would apply packet pacing to $iface (skipped)" + return + fi + if [[ "$TARGET_TYPE" != "dtn" ]] || [[ $APPLY_PACKET_PACING -eq 0 ]]; then return fi - + # Decide on tbf vs fq: use tbf if explicitly requested or a cap rate is provided; otherwise fq if [[ $USE_TBF_CAP -eq 1 || -n "$TBF_CAP_RATE" ]]; then # Determine effective rate: explicit cap wins; else compute fraction of iface speed diff --git a/tests/test-fasterdata-dryrun.sh b/tests/test-fasterdata-dryrun.sh new file mode 100755 index 0000000..8c75f26 --- /dev/null +++ b/tests/test-fasterdata-dryrun.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Simple regression test: verify --mode apply --dry-run does not actually apply packet pacing +set -euo pipefail +SCRIPT="$(pwd)/docs/perfsonar/tools_scripts/fasterdata-tuning.sh" +if [[ ! -x "$SCRIPT" ]]; then + echo "Script not found or not executable: $SCRIPT" >&2 + exit 1 +fi +# Run dry-run apply (may require sudo for access to some commands); expect messages indicating dry-run +OUT=$(sudo "$SCRIPT" --mode apply --target dtn --apply-packet-pacing --dry-run --yes 2>&1 || true) +# Check for dry-run markers +if ! echo "$OUT" | grep -q "Dry-run: would apply packet pacing"; then + echo "FAIL: dry-run did not report packet pacing skip" >&2 + echo "Output was:\n$OUT" >&2 + exit 2 +fi +# Ensure summary does not claim pacing was enabled +if echo "$OUT" | grep -q "Packet pacing: ENABLED"; then + echo "FAIL: dry-run unexpectedly shows Packet pacing ENABLED" >&2 + exit 3 +fi +# Basic success +echo "PASS: dry-run packet pacing behaved as expected" \ No newline at end of file