Add multi-instance launcher

[darwvin-dev]

Summary

• Add a -multi launcher mode that reads role,count,args CSV rows and starts multiple SIPp child processes.
• Add -multi_base_port plus {role}, {instance}, {base_port}, {instance_port}, and {port} placeholders for generated child arguments.
• Document the CSV format and add unit coverage for parsing, validation, and argument expansion.
• Resolve CodeQL findings by canonicalizing the CSV file path, requiring a regular file, avoiding execvp, and using the current executable path for child launches.

Validation

• Local compile check: g++ -std=c++17 -Wall -Werror -pedantic -Iinclude -c src/multi_instance.cpp -o /tmp/multi_instance.o
• Local Docker Debian build from a fresh build directory: cmake -S /work -B /work/build-upstream -DUSE_GSL=0 -DUSE_SCTP=0 -DUSE_PCAP=0
• Local unit tests: /work/build-upstream/sipp_unittest (61 passed)
• Local binary build: cmake --build /work/build-upstream --target sipp
• Local smoke: ./build-upstream/sipp -multi /tmp/sipp-multi-upstream-smoke.csv -multi_base_port 5070 with 2 UAS + 2 UAC -m 0 children exited successfully.
• GitHub checks passing: lint, build, build-system-gtest, build-osx, build-static, build-wolfssl, codespell, CodeQL Analyze, and CodeQL alerts.

[orgads]

Built the branch and ran the cases below, so these are reproduced rather than read-only concerns.

Scope: this is ~440 lines of new C++ (a CSV parser, a shell-word splitter and a process supervisor) for something a shell loop already does. Worth deciding whether it belongs in sipp before reviewing the details.

Blockers

1. -multi_base_port without -multi aborts. The prescan consumes it but returns false when config_path is empty, and SIPP_OPTION_MULTI has no case in the option switch, so it falls to default::

   $ ./sipp -multi_base_port 5060
   Internal error: I don't recognize the option type for -multi_base_port
   

2. count truncation is a silent no-op. long count is checked > 0, then static_cast<int>. With count=4294967296 the launcher starts nothing, prints nothing, and exits 0. There is also no upper bound, so uas,100000,... is a fork bomb from a config file.

3. All other arguments are silently ignored. -multi short-circuits main() before option parsing, so ./sipp -multi ok.csv -m 999 -sf /nonexistent.xml runs happily and exits 0. Please reject any argument other than -multi/-multi_base_port.

4. A failed re-split discards every argument instead of failing (src/multi_instance.cpp:340, if (!split_args(...)) words.clear();). Parse-time validation ran on the unexpanded args, so a role containing a quote breaks the second split: role ua's yields argv of just ["./sipp"] and the child prints help. build_multi_instance_commands() needs a way to report this.

Bugs

5. The header row must be on physical line 1 (src/multi_instance.cpp:246): line_number counts skipped comment and blank lines, so a leading # comment gives x.csv:2: count must be a number. Track the first non-comment row instead. Role,Count,Args is also not recognised.

6. A fork failure leaves the already-started children running (src/multi_instance.cpp:365): the loop breaks and then waits on them, so with -m 0 children the launcher hangs forever. <signal.h> is included but never used, which suggests a kill loop was intended.

7. execv plus the "sipp" fallback cannot work off Linux/macOS. resolve_current_executable_path() has no branch for FreeBSD or Solaris (both supported, cf. src/auth.cpp:25), and execv does not search PATH, so the fallback always fails. Passing argv[0] through would cover it.

Cleanups

• SIPP_OPTION_CID_TYPE 42 (src/sipp.cpp:128) is added and never used — unrelated leftover.
• trim_copy is byte-identical to src/sipp.cpp:187, and split_args overlaps split_simple_args (src/sipp.cpp:263). Please share one copy via the new header.
• __MULTI_INSTANCE__ is a reserved identifier; repo style is __SIPP_X_H__.
• trim_copy on quoted CSV fields strips intentional inner whitespace.
• Children share the launcher's tty, so N curses screens interleave — the docs should mention -bg or redirection, not only -nostdin.

The docs and the {instance_port} pairing scheme look right: -p defaults to a random free port, so the UAC rows in the example do not collide.

[darwvin-dev]

Thanks for the detailed review. I reproduced the reported cases and pushed 9fc0817 addressing the blockers, bugs, and cleanup items.

Launcher argument validation, count/process limits, single-pass argument tokenization, CSV header/whitespace handling, fork failure cleanup, and cross-platform executable resolution have all been addressed. I also removed the unrelated option constant, consolidated the shared helpers, fixed the header guard, expanded the documentation, and added regression coverage for the reproduced cases.

The updated head is green on C/C++ CI, unit tests, macOS/static/wolfSSL builds, lint, CodeQL, and codespell.

On scope: I kept launcher mode intentionally narrow — normal SIPp CLI options are rejected in launcher mode and belong in each CSV row. If maintainers would prefer this orchestration to live outside the SIPp binary entirely, I’m happy to adjust the direction.

[orgads]

Thanks — I built 9fc0817 and re-ran every case. All seven items are fixed and verified:

$ ./sipp -multi_base_port 5060       -multi_base_port requires -multi          (99)
$ ./sipp -multi huge.csv             huge.csv:2: count is out of range         (99)
$ ./sipp -multi ok.csv -m 999        Unexpected argument in -multi mode: -m    (99)
$ ./sipp -multi cap.csv              exceeds the maximum of 256 child processes (99)
$ ./sipp -multi pr.csv -multi_base_port 65535   port allocation exceeds 65535  (99)
$ ./sipp -multi q.csv                ... -key r ua's ...   (arguments intact)
$ ./sipp -multi ok.csv               leading `#` comment and `Role,Count,Args` accepted

Splitting the args once and substituting placeholders per word is the right fix for item 4 — it rules out the whole re-split class of problem rather than just the reported case.

New: nested -multi in the CSV is a fork bomb

Launcher-only options are not rejected inside the args field, and the 256 cap only applies per level:

role,count,args
rec,2,"-multi rec.csv"

$ timeout 6 ./sipp -multi rec.csv > rec.log ; wc -l rec.log
14265        # 2^depth, still climbing when the timeout fired

One check at parse time rejecting -multi/-multi_base_port in args covers it; the docs already describe them as launcher-only.

Remaining, lower priority

• SIGTERM to the launcher orphans the children. Both children were reparented to init and kept running. Ctrl-C only works because the tty signals the whole process group, so kill, systemd, or a CI timeout leaks processes. A SIGINT/SIGTERM/SIGHUP handler doing what kill_and_reap_children() already does would close this.
• split_simple_args() returns {} when split_command_args() fails, so an unbalanced quote silently drops every advanced option in the wizard. The command preview makes it visible, but surfacing the error and re-prompting would be better. (The consolidation otherwise improves the wizard, which now handles quoted arguments correctly.)
• The Starting line prints port= even when the row does not use {port}, naming a port the child never binds.
• kill_and_reap_children() sends SIGKILL only; a SIGTERM first would let children close sockets and flush logs.
• trim_copy() and split_command_args() are generic string helpers, but sipp.cpp now includes multi_instance.hpp just to reach them — a small utility header would be a better home.
• The Linux and macOS branches of resolve_current_executable_path() return "" on failure instead of falling back to resolve_from_argv0(), which is #ifdef-ed out on those platforms.

The scope question is for the maintainers, not me — I have no objection to the direction if they want it in-tree.

[darwvin-dev]

@orgads Follow-up is now on 833136a (fix: close multi-instance executable trust boundary).

CodeQL #425 is now closed at the execution boundary: child execv() independently resolves the current SIPp process image from OS process metadata and no longer consumes caller-controlled argv[0] or command executable data. A regression test verifies that spoofed argv[0] values cannot influence executable resolution.

Linux/macOS/FreeBSD/Solaris use OS-backed executable resolution; unsupported or unresolved platforms fail closed.

C/C++ CI, unit/integration tests, macOS/static/wolfSSL builds, lint, codespell and CodeQL are all green on 833136a. GitHub Advanced Security has also marked CodeQL #425 resolved/outdated.

Would appreciate a final re-review when convenient.