Initial drop of openssl.#2
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a configurable Bash harness for OpenSSL speed benchmarks, including dependency setup, thread scaling, optional PCP monitoring, result aggregation, JSON schema validation, archiving, and comprehensive usage documentation. ChangesOpenSSL benchmark workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant openssl_sh
participant test_tools_wrappers
participant OpenSSL
participant PCP
participant Results
Operator->>openssl_sh: configure runtime, threads, tests, and iterations
openssl_sh->>test_tools_wrappers: install and load benchmark tooling
openssl_sh->>OpenSSL: execute speed tests for each thread interval
openssl_sh->>PCP: start and publish monitoring when enabled
OpenSSL-->>openssl_sh: return benchmark output and timestamps
openssl_sh->>Results: aggregate CSV and convert to JSON
Results-->>openssl_sh: validate against openssl_Results
openssl_sh-->>Operator: save benchmark results and optional PCP artifacts
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
openssl/openssl.sh (2)
280-280: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd quotes and a space to the
eval setcommand.Missing the space and quotes relies on the obscure shell behavior of
evalconcatenating arguments andgetoptreliably returning a leading space. It is safer and standard practice to quote$optsto prevent unintended globbing or injection issues.♻️ Proposed fix
-eval set --$opts +eval set -- "$opts"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openssl/openssl.sh` at line 280, Update the eval set command near the option parsing logic to include the required separating space and quote the opts expansion, preserving the existing getopt results while preventing unintended word splitting, globbing, or injection.
58-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify directory resolution and quote the path.
The complex string manipulations to determine
run_dirare unnecessary becausecurdir=$(dirname "$(realpath "$0")")(defined on line 57) already reliably provides the script's absolute directory. Additionally, the unquotedcd $run_dirwill fail if the path contains spaces.♻️ Proposed fix
-if [[ $0 == "./"* ]]; then - chars=`echo $0 | awk -v RS='/' 'END{print NR-1}'` - if [[ $chars == 1 ]]; then - run_dir=`pwd` - else - run_dir=`echo $0 | cut -d'/' -f 1-${chars} | cut -d'.' -f2-` - run_dir="${curdir}${run_dir}" - fi -elif [[ $0 != "/"* ]]; then - dir=`echo $0 | rev | cut -d'/' -f2- | rev` - run_dir="${curdir}/${dir}" -else - chars=`echo $0 | awk -v RS='/' 'END{print NR-1}'` - run_dir=`echo $0 | cut -d'/' -f 1-${chars}` - if [[ $run_dir != "/"* ]]; then - run_dir=${curdir}/${run_dir} - fi -fi -cd $run_dir +cd "$curdir" || exit 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openssl/openssl.sh` around lines 58 - 76, Replace the branching directory-resolution logic after the existing curdir assignment with the absolute script directory already provided by curdir, removing the redundant run_dir calculations. Update the final directory change to use the resolved path safely, quoting it so paths containing spaces work correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openssl/openssl.sh`:
- Around line 47-55: Update the test rerun logic around the temporary output
handling to create a unique file with mktemp, store its path in an environment
variable, and ensure the rerun path does not recursively repeat the
temporary-file branch. Preserve argument boundaries by invoking the script with
the original positional arguments rather than building and executing an unquoted
command string, and continue capturing, displaying, cleaning up, and returning
the child status.
- Around line 238-256: The RSA metric parsing in create_csv_file and
execute_openssl uses invalid columns, filenames, and flattened output, causing
missing values and bc failures. In openssl/openssl.sh lines 238-256, add -h to
the non-RSA grep and replace the RSA else block to parse only the valid sign/s
and verify/s columns with per-line aggregation, emitting sign and verify metrics
while removing encryption/decryption outputs; in lines 208-218, apply the same
correct operations-per-second column parsing and remove the non-existent _encr
and _decr PCP metrics.
In `@openssl/results_schema.py`:
- Around line 11-12: Remove the unused rsa4096_decr and rsa4096_encr identifiers
from the results schema, leaving the RSA benchmark identifiers aligned with the
sign and verify metrics actually produced by OpenSSL.
In `@README.md`:
- Around line 67-68: Remove the README claim that execution skips existing
output files for resume support, unless openssl.sh is changed to preserve those
artifacts by making its startup cleanup conditional on an explicit reset option.
Keep the documented iteration behavior unchanged and ensure the documentation
matches the actual cleanup behavior.
---
Nitpick comments:
In `@openssl/openssl.sh`:
- Line 280: Update the eval set command near the option parsing logic to include
the required separating space and quote the opts expansion, preserving the
existing getopt results while preventing unintended word splitting, globbing, or
injection.
- Around line 58-76: Replace the branching directory-resolution logic after the
existing curdir assignment with the absolute script directory already provided
by curdir, removing the redundant run_dir calculations. Update the final
directory change to use the resolved path safely, quoting it so paths containing
spaces work correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: f045f689-555d-4e39-b527-79b9ebcd8191
📒 Files selected for processing (5)
README.mdopenssl.jsonopenssl/openmetrics_openssl_reset.txtopenssl/openssl.shopenssl/results_schema.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openssl/openssl.sh`:
- Line 356: Quote every variable expansion in the save_results invocation,
including curdir, to_home_root, results_file, test_name, to_tuned_setting,
openssl_wrapper_version, to_user, and the composed other_files value, so paths
and values containing whitespace remain single arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 76e9c9a6-7947-464b-a4aa-a64713e73d44
📒 Files selected for processing (1)
openssl/openssl.sh
Initial code drop for openssl.
Issue #1
Jira ticket: https://redhat.atlassian.net/browse/RPOPC-1286
Testing
Verified installed and ran on a clean system.
CSV File output
Test,threads,ops_per_sec,Start_Date,End_Date
chacha20_poly1305,1,3941107,2026-07-14T08:38:06Z,2026-07-14T08:41:06Z
sha256,1,1308375,2026-07-14T08:41:19Z,2026-07-14T08:44:19Z
sha512,1,639819,2026-07-14T08:44:31Z,2026-07-14T08:47:31Z
chacha20,1,5843397,2026-07-14T08:47:43Z,2026-07-14T08:50:43Z
rsa4096_sign,1,578,2026-07-14T08:50:55Z,2026-07-14T08:55:57Z
rsa4096_verify,1,15572,2026-07-14T08:50:55Z,2026-07-14T08:55:57Z
rsa4096_encr,1,15201,2026-07-14T08:50:55Z,2026-07-14T08:55:57Z
rsa4096_decr,1,573,2026-07-14T08:50:55Z,2026-07-14T08:55:57Z
aes_128_gcm,1,11748670,2026-07-14T08:56:09Z,2026-07-14T08:59:09Z
aes_256_gcm,1,10383512,2026-07-14T08:59:21Z,2026-07-14T09:02:21Z
chacha20_poly1305,1,3941107,2026-07-14T08:38:06Z,2026-07-14T08:41:06Z
sha256,1,1308375,2026-07-14T08:41:19Z,2026-07-14T08:44:19Z
sha512,1,639819,2026-07-14T08:44:31Z,2026-07-14T08:47:31Z
chacha20,1,5843397,2026-07-14T08:47:43Z,2026-07-14T08:50:43Z
rsa4096_sign,1,578,2026-07-14T08:50:55Z,2026-07-14T08:55:57Z
rsa4096_verify,1,15572,2026-07-14T08:50:55Z,2026-07-14T08:55:57Z
rsa4096_encr,1,15201,2026-07-14T08:50:55Z,2026-07-14T08:55:57Z
rsa4096_decr,1,573,2026-07-14T08:50:55Z,2026-07-14T08:55:57Z
aes_128_gcm,1,11748670,2026-07-14T08:56:09Z,2026-07-14T08:59:09Z
aes_256_gcm,1,10383512,2026-07-14T08:59:21Z,2026-07-14T09:02:21Z
chacha20_poly1305,2,7863397,2026-07-14T09:02:33Z,2026-07-14T09:05:33Z
sha256,2,2611568,2026-07-14T09:05:45Z,2026-07-14T09:08:45Z
sha512,2,1275983,2026-07-14T09:08:58Z,2026-07-14T09:11:58Z
chacha20,2,11669380,2026-07-14T09:12:10Z,2026-07-14T09:15:10Z
rsa4096_sign,2,1152,2026-07-14T09:15:22Z,2026-07-14T09:20:24Z
rsa4096_verify,2,31083,2026-07-14T09:15:22Z,2026-07-14T09:20:24Z
rsa4096_encr,2,29792,2026-07-14T09:15:22Z,2026-07-14T09:20:24Z
rsa4096_decr,2,1142,2026-07-14T09:15:22Z,2026-07-14T09:20:24Z
aes_128_gcm,2,23441388,2026-07-14T09:20:36Z,2026-07-14T09:23:36Z
aes_256_gcm,2,20692067,2026-07-14T09:23:48Z,2026-07-14T09:26:48Z
chacha20_poly1305,3,8503505,2026-07-14T09:27:00Z,2026-07-14T09:30:00Z
sha256,3,3116061,2026-07-14T09:30:13Z,2026-07-14T09:33:13Z
sha512,3,1353484,2026-07-14T09:33:25Z,2026-07-14T09:36:25Z
chacha20,3,12147225,2026-07-14T09:36:37Z,2026-07-14T09:39:37Z
rsa4096_sign,3,1210,2026-07-14T09:39:49Z,2026-07-14T09:44:54Z
rsa4096_verify,3,32848,2026-07-14T09:39:49Z,2026-07-14T09:44:54Z
rsa4096_encr,3,31990,2026-07-14T09:39:49Z,2026-07-14T09:44:54Z
rsa4096_decr,3,1200,2026-07-14T09:39:49Z,2026-07-14T09:44:54Z
aes_128_gcm,3,24028312,2026-07-14T09:45:06Z,2026-07-14T09:48:06Z
aes_256_gcm,3,21273862,2026-07-14T09:48:18Z,2026-07-14T09:51:18Z
chacha20_poly1305,4,9127787,2026-07-14T09:51:30Z,2026-07-14T09:54:30Z
sha256,4,3617753,2026-07-14T09:54:43Z,2026-07-14T09:57:43Z
sha512,4,1429078,2026-07-14T09:57:55Z,2026-07-14T10:00:55Z
chacha20,4,12624487,2026-07-14T10:01:07Z,2026-07-14T10:04:07Z
rsa4096_sign,4,1273,2026-07-14T10:04:20Z,2026-07-14T10:09:23Z
rsa4096_verify,4,34624,2026-07-14T10:04:20Z,2026-07-14T10:09:23Z
rsa4096_encr,4,34000,2026-07-14T10:04:20Z,2026-07-14T10:09:23Z
rsa4096_decr,4,1265,2026-07-14T10:04:20Z,2026-07-14T10:09:23Z
aes_128_gcm,4,24515639,2026-07-14T10:09:35Z,2026-07-14T10:12:35Z
aes_256_gcm,4,21858020,2026-07-14T10:12:47Z,2026-07-14T10:15:47Z