Benchmark F407 handshake wall-clock at 168 MHz: server-cert vs mTLS - #117
Benchmark F407 handshake wall-clock at 168 MHz: server-cert vs mTLS#117kaidokert wants to merge 7 commits into
Conversation
Raise the J-Trace F407 profile from 30 to 168 MHz (the part's rated max); `frequency_hz` follows `hclk()`, so krabi-caliper's EM_MEASUREMENT ns conversion is real wall-clock. HSI-sourced, so absolute time carries the internal oscillator's ~±1%, but the server-only-vs-mTLS delta is clock-exact since both run on the same clock. Add an mTLS (client-cert) handshake facade + example alongside the server-cert-only one. It replays the seed-0 packets_mtls fixtures with an Ed25519 client-auth signer, so it performs the Certificate + CertificateVerify (client Ed25519 sign) the server-only path never does — the delta is the cost of client-certificate auth. Wire both as cases in the F407 campaign, and add the mTLS row to the QEMU run_suite so CI validates the replay ACCEPTs before the hardware run.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_404857cd-a251-458a-a7d4-dfe5f603bbbf) |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a feature-gated AES-128-GCM-SHA256 and Ed25519 mutual-TLS handshake facade, a Cortex-M example with baseline support, and JTrace F407 measurement configuration using a 168 MHz clock profile. ChangesAES-Ed25519 mTLS handshake
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CortexMExample
participant MtlSFacade
participant DefaultStream
participant CapturedTranscript
CortexMExample->>MtlSFacade: run selected mTLS handshake
MtlSFacade->>DefaultStream: connect with client authentication
DefaultStream->>CapturedTranscript: capture client handshake bytes
CapturedTranscript-->>MtlSFacade: return transmitted transcript
MtlSFacade->>MtlSFacade: validate expected client flights
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new mTLS facade code repeats the same
#[cfg(all(feature = …))]predicate on several items; consider pulling this into a module-levelcfgor a shared cfg alias to reduce duplication and chances of the guards drifting out of sync. - In
run_aes_ed25519_mtls_facade_with_scratch,CannedTransport::<2048>uses a hard-coded capacity; it would be safer to derive this from the fixture lengths (or assert at compile time) so future changes to the fixtures cannot silently overflow or truncate the stream. - The
jtrace-f407path intest_fixturenow hardcodessysclk(168.MHz()); if this feature is ever reused for other frequencies/profiles, it might be worth routing the frequency through a profile/config value so thekrabi-caliperprofile and runtime clock cannot diverge.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new mTLS facade code repeats the same `#[cfg(all(feature = …))]` predicate on several items; consider pulling this into a module-level `cfg` or a shared cfg alias to reduce duplication and chances of the guards drifting out of sync.
- In `run_aes_ed25519_mtls_facade_with_scratch`, `CannedTransport::<2048>` uses a hard-coded capacity; it would be safer to derive this from the fixture lengths (or assert at compile time) so future changes to the fixtures cannot silently overflow or truncate the stream.
- The `jtrace-f407` path in `test_fixture` now hardcodes `sysclk(168.MHz())`; if this feature is ever reused for other frequencies/profiles, it might be worth routing the frequency through a profile/config value so the `krabi-caliper` profile and runtime clock cannot diverge.
## Individual Comments
### Comment 1
<location path="footprint/handshakes/src/lib.rs" line_range="680-683" />
<code_context>
+ black_box(&fixture_aes_ed25519_mtls::SERVER_HELLO);
+ black_box(&fixture_aes_ed25519_mtls::SERVER_FLIGHT);
+ black_box(&fixture_aes_ed25519_mtls::CLIENT_SECOND_FLIGHT);
+ black_box(&fixture_aes_ed25519_mtls::CLIENT_LEAF_DER);
+ true
+}
</code_context>
<issue_to_address>
**suggestion (performance):** The mTLS baseline facade does not reference CLIENT_SEED, so its rodata may be optimized out.
Because `CLIENT_SEED` is only used in `run_aes_ed25519_mtls_facade_with_scratch` and not in `baseline_aes_ed25519_mtls_facade`, the optimizer can drop it from the baseline binary, unlike the other fixtures that are passed through `black_box`. This breaks the “same rodata footprint” guarantee and skews the comparison with the full mTLS facade. To keep the footprints comparable, also call `black_box(&fixture_aes_ed25519_mtls::CLIENT_SEED);` in the baseline facade.
```suggestion
black_box(&fixture_aes_ed25519_mtls::CLIENT_HELLO);
black_box(&fixture_aes_ed25519_mtls::SERVER_HELLO);
black_box(&fixture_aes_ed25519_mtls::SERVER_FLIGHT);
black_box(&fixture_aes_ed25519_mtls::CLIENT_SECOND_FLIGHT);
black_box(&fixture_aes_ed25519_mtls::CLIENT_SEED);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| black_box(&fixture_aes_ed25519_mtls::CLIENT_HELLO); | ||
| black_box(&fixture_aes_ed25519_mtls::SERVER_HELLO); | ||
| black_box(&fixture_aes_ed25519_mtls::SERVER_FLIGHT); | ||
| black_box(&fixture_aes_ed25519_mtls::CLIENT_SECOND_FLIGHT); |
There was a problem hiding this comment.
suggestion (performance): The mTLS baseline facade does not reference CLIENT_SEED, so its rodata may be optimized out.
Because CLIENT_SEED is only used in run_aes_ed25519_mtls_facade_with_scratch and not in baseline_aes_ed25519_mtls_facade, the optimizer can drop it from the baseline binary, unlike the other fixtures that are passed through black_box. This breaks the “same rodata footprint” guarantee and skews the comparison with the full mTLS facade. To keep the footprints comparable, also call black_box(&fixture_aes_ed25519_mtls::CLIENT_SEED); in the baseline facade.
| black_box(&fixture_aes_ed25519_mtls::CLIENT_HELLO); | |
| black_box(&fixture_aes_ed25519_mtls::SERVER_HELLO); | |
| black_box(&fixture_aes_ed25519_mtls::SERVER_FLIGHT); | |
| black_box(&fixture_aes_ed25519_mtls::CLIENT_SECOND_FLIGHT); | |
| black_box(&fixture_aes_ed25519_mtls::CLIENT_HELLO); | |
| black_box(&fixture_aes_ed25519_mtls::SERVER_HELLO); | |
| black_box(&fixture_aes_ed25519_mtls::SERVER_FLIGHT); | |
| black_box(&fixture_aes_ed25519_mtls::CLIENT_SECOND_FLIGHT); | |
| black_box(&fixture_aes_ed25519_mtls::CLIENT_SEED); |
Coverage Report for CI Build 30422226337Coverage remained the same at 85.074%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@footprint/run_suite.py`:
- Line 27: Update the report summary text associated with the suite rows in
run_suite.py so it describes both signature verification and
client-authentication signing, rather than only a “sig-verify path.” Preserve
the new “AES-128-GCM mTLS” row and ensure its rendered description accurately
reflects the CertificateVerify client-signing cost.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7dce8ba-65e1-4c9a-8066-c191bc1a6909
📒 Files selected for processing (6)
footprint/cortex-m/Cargo.tomlfootprint/cortex-m/examples/krabitls_mtls.rsfootprint/cortex-m/krabi-caliper.tomlfootprint/cortex-m/src/lib.rsfootprint/handshakes/src/lib.rsfootprint/run_suite.py
| ROWS = [ | ||
| ("ChaCha20-Poly1305", "Ed25519", "krabitls_chacha", True, ["chacha20", "canned-replay"]), | ||
| ("AES-128-GCM", "Ed25519", "krabitls", True, ["cipher-aes", "canned-replay"]), | ||
| ("AES-128-GCM mTLS", "Ed25519", "krabitls_mtls", True, ["cipher-aes", "canned-replay"]), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe the client-signing cost in the report.
This new mTLS row includes client CertificateVerify signing, but the rendered summary describes all rows as a “sig-verify path.” Update it to cover verification and client-auth signing so the mTLS measurement is accurately described.
Proposed fix
- print("cost of krabitls + the AEAD + the sig-verify path over the harness floor.")
+ print("cost of krabitls + the AEAD + signature verification/client-auth signing")
+ print("paths over the harness floor.")🤖 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 `@footprint/run_suite.py` at line 27, Update the report summary text associated
with the suite rows in run_suite.py so it describes both signature verification
and client-authentication signing, rather than only a “sig-verify path.”
Preserve the new “AES-128-GCM mTLS” row and ensure its rendered description
accurately reflects the CertificateVerify client-signing cost.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51ba1dbc62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ROWS = [ | ||
| ("ChaCha20-Poly1305", "Ed25519", "krabitls_chacha", True, ["chacha20", "canned-replay"]), | ||
| ("AES-128-GCM", "Ed25519", "krabitls", True, ["cipher-aes", "canned-replay"]), | ||
| ("AES-128-GCM mTLS", "Ed25519", "krabitls_mtls", True, ["cipher-aes", "canned-replay"]), |
There was a problem hiding this comment.
This row is executed for every entry in TARGETS, including footprint/risc-v, but that crate has neither a krabitls_mtls example nor a corresponding manifest target (a repo-wide search finds the example only under footprint/cortex-m). Consequently both RISC-V measurements invoke Cargo with a nonexistent example, main() records an incomplete measurement, and the Footprint suite workflow exits with status 1 on every run; either add the RISC-V example or scope this row to Cortex-M.
Useful? React with 👍 / 👎.
The mTLS handshake is an F407-hardware campaign case, not a cross-target footprint row: risc-v has no krabitls_mtls example, and its larger working set doesn't complete on the RAM-constrained mps2-an385 QEMU M3. The F407 hardware run replays it for real and is the validation + timing source.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4ce3fbb9-ccea-4ee6-8728-d70ce94ac702) |
The footprint campaign reports cycles (EM_MEASUREMENT). Add a hardware-only timing example that drives both handshakes through krabi-caliper's PairedSuite at 168 MHz — it emits EM_SUMMARY with frequency_hz, which the runner renders as wall-clock seconds on the job summary. Reuses the existing on-stack facades; a handshake has no CT A/B pair, so each is timed as both sides with the spread gate opened wide. New timing campaign runs alongside the footprint one (which keeps flash/stack).
TRIALS=3 tripped RunError::OddSampleCapacity (the paired runner splits the sample capacity into A/B pairs, so it must be even) — the suite panicked before emitting EM_SUMMARY and the campaign timed out. Use 4.
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 `@footprint/cortex-m/examples/krabitls_timing.rs`:
- Line 24: Replace the hard-coded HCLK_HZ constant with the value derived from
clocks.hclk(), and use that frozen clock value for DWT conversion and emitted
timing metadata. Update the surrounding initialization flow so the derived
hclk_hz is available wherever HCLK_HZ is currently referenced.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a0e4ef1-8c0f-48f8-9c59-11309783cd69
📒 Files selected for processing (4)
.github/workflows/hw-footprint.ymlfootprint/cortex-m/Cargo.tomlfootprint/cortex-m/examples/krabitls_timing.rsfootprint/cortex-m/krabi-caliper.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- footprint/cortex-m/krabi-caliper.toml
|
|
||
| // Must be even — the paired runner splits it into A/B pairs (odd → OddSampleCapacity). | ||
| const TRIALS: usize = 4; | ||
| const HCLK_HZ: u64 = 168_000_000; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For stm32f4xx-hal version 0.22.0, what is the supported API for reading the frozen HCLK frequency from the Clocksvalue returned byfreeze()?
💡 Result:
In stm32f4xx-hal version 0.22.0, the Clocks struct, which is returned by the freeze() method on the RCC configuration, provides the HCLK frequency directly through its hclk() method [1]. Although the Clocks struct contains a private hclk field [2], the API exposes it via a public method named hclk() that returns the frequency (typically as a Hertz type) [1]. You can access this value after calling freeze() as follows: let clocks = rcc.cfgr.freeze; let hclk_freq = clocks.hclk; The Clocks struct encapsulates the "frozen" clock frequencies established after the RCC has been configured [1][2]. Other common clock frequencies, such as sysclk(), pclk1(), and pclk2(), are similarly accessed via their respective methods on the Clocks instance [1][3].
Citations:
- 1: https://docs.rs/stm32f4xx-hal/latest/stm32f4xx_hal/rcc/struct.Clocks.html
- 2: https://github.com/stm32-rs/stm32f4xx-hal/blob/d228ebea/src/rcc/f4/mod.rs
- 3: https://docs.rs/stm32f4xx-hal/latest/stm32f4xx_hal/rcc/index.html
Derive hclk_hz from clocks.hclk() instead of hard-coding 168 MHz.
HCLK_HZ duplicates the RCC configuration; using the frozen clock value for DWT conversion and emitted metadata keeps timing output aligned with the actual clock setup.
🤖 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 `@footprint/cortex-m/examples/krabitls_timing.rs` at line 24, Replace the
hard-coded HCLK_HZ constant with the value derived from clocks.hclk(), and use
that frozen clock value for DWT conversion and emitted timing metadata. Update
the surrounding initialization flow so the derived hclk_hz is available wherever
HCLK_HZ is currently referenced.
The 0.1.4 host renderer's per-op Duration (seconds) column is fed only by EM_MEASUREMENT records (render.rs "## Measurements" iterates benchmark.measurements). PairedSuite emits a PairedResult (Welch CT verdict) and no EM_MEASUREMENT, so it never populates that section — cycles/seconds never appear, on 0.1.3 or 0.1.4. run_footprint already emits EM_MEASUREMENT, so the footprint campaign alone yields the Duration-in-seconds view once the runner renders with caliper 0.1.4. Drop the timing example/profile/campaign.
0.1.4's host renderer adds the per-op Duration column that formats the EM_MEASUREMENT ticks as wall-clock seconds. Opt-in via the immutable image tag; the fleet default stays 0.1.3.
2 participants
Real-time connection-establishment benchmarks on the STM32F407 at its rated
168 MHz, for server-certificate-only vs mutual-TLS (client certificate).
Uses krabi-caliper's existing
EM_MEASUREMENTtiming — no custom measurement.What it measures
Both cases replay the seed-0 canned AES-128-GCM / X25519 / Ed25519 handshake on
the board:
krabitls): X25519 DH + Ed25519 server-cert verify.krabitls_mtls, new): the same plus the client's Certificate +CertificateVerify — an Ed25519 sign the server-only path never does.
The server-only-vs-mTLS delta is the cost of client-certificate authentication.
How the timing is real
sysclk(168.MHz()), HSI PLL,HAL sets flash wait states).
FootprintConfig.frequency_hzfollowshclk(),so krabi-caliper emits
EM_MEASUREMENT … ticks:T frequency_hz:168000000andwall-clock =
T / 168e6.absolute time carries the internal oscillator's ~±1%. The delta between
the two handshakes is clock-exact (same clock). Switchable to HSE for tighter
absolute numbers given the board's crystal frequency.
Pieces
footprint/handshakes:run_aes_ed25519_mtls_facade{,_on_stack}replaying thepackets_mtlsfixtures with anEd25519ClientAuthsigner (mirrors the passingcanned_handshake_mtlstest; on-stack scratch keeps SysTick live like Add Ed25519 handshake hardware measurement #116).footprint/cortex-m/examples/krabitls_mtls.rs+ Cargo registration.krabi-caliper.toml: 168 MHz profile, two campaign cases.run_suite.py: mTLS row so the QEMU footprint job validates the replay ACCEPTsbefore the hardware run.
Numbers
Produced by the
hardwareCI job (the F407 campaign) —EM_MEASUREMENTticks at168 MHz for
krabitlsandkrabitls-mtls. Verified locally: both examples buildfor
thumbv7em-none-eabihfat 168 MHz and the QEMU-target replay compiles clean.🤖 Generated with Claude Code
Summary by Sourcery
Add a mutual-TLS AES-128-GCM/Ed25519 handshake benchmark on STM32F407 at 168 MHz and align the Cortex-M footprint harness to measure real wall-clock time for server-cert-only vs mTLS handshakes.
New Features:
krabitls_mtlswired into the footprint suite and run harness for mutual-TLS benchmarks.Enhancements:
EM_MEASUREMENTreports real wall-clock timing for handshake benchmarks.Summary by CodeRabbit
New Features
Improvements