From 2e79e7efeb26f06eb59a1d4f3444ea63fc3e20c3 Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Thu, 12 Dec 2024 10:00:24 -0800 Subject: [PATCH 1/3] refactor(bench): remove historical benchmarks (#4940) --- bindings/rust/bench/Cargo.toml | 35 +- bindings/rust/bench/README.md | 48 +- bindings/rust/bench/benches/handshake.rs | 30 +- bindings/rust/bench/benches/throughput.rs | 29 +- .../images/historical-perf-handshake.svg | 947 ------------------ .../images/historical-perf-throughput.svg | 383 ------- bindings/rust/bench/scripts/bench-memory.sh | 30 - bindings/rust/bench/scripts/bench-past.sh | 80 -- bindings/rust/bench/src/bin/graph_memory.rs | 134 --- bindings/rust/bench/src/bin/graph_perf.rs | 306 ------ bindings/rust/bench/src/bin/memory.rs | 191 ---- bindings/rust/bench/src/harness.rs | 8 +- bindings/rust/bench/src/lib.rs | 8 +- bindings/rust/bench/src/openssl.rs | 5 +- 14 files changed, 29 insertions(+), 2205 deletions(-) delete mode 100644 bindings/rust/bench/images/historical-perf-handshake.svg delete mode 100644 bindings/rust/bench/images/historical-perf-throughput.svg delete mode 100755 bindings/rust/bench/scripts/bench-memory.sh delete mode 100755 bindings/rust/bench/scripts/bench-past.sh delete mode 100644 bindings/rust/bench/src/bin/graph_memory.rs delete mode 100644 bindings/rust/bench/src/bin/graph_perf.rs delete mode 100644 bindings/rust/bench/src/bin/memory.rs diff --git a/bindings/rust/bench/Cargo.toml b/bindings/rust/bench/Cargo.toml index c061bedbeef..ab4da6cb9d4 100644 --- a/bindings/rust/bench/Cargo.toml +++ b/bindings/rust/bench/Cargo.toml @@ -3,31 +3,14 @@ name = "bench" version = "0.1.0" edition = "2021" -[features] -default = ["rustls", "openssl"] -rustls = ["dep:rustls", "rustls-pemfile"] -openssl = ["dep:openssl"] -memory = ["plotters", "crabgrind", "structopt"] -historical-perf = ["plotters", "serde_json", "semver"] - [dependencies] s2n-tls = { path = "../s2n-tls" } errno = "0.3" libc = "0.2" strum = { version = "0.25", features = ["derive"] } -rustls = { version = "0.23", optional = true } -rustls-pemfile = { version = "2", optional = true } -openssl = { version = "0.10", features = ["vendored"], optional = true } -crabgrind = { version = "0.1", optional = true } -structopt = { version = "0.3", optional = true } -serde_json = { version = "1.0", optional = true } -semver = { version = "1.0", optional = true } - -[dependencies.plotters] -version = "0.3" -optional = true -default-features = false -features = ["all_series", "all_elements", "full_palette", "svg_backend"] +rustls = { version = "0.23" } +rustls-pemfile = { version = "2" } +openssl = { version = "0.10", features = ["vendored"] } [dev-dependencies] criterion = "0.5" @@ -37,18 +20,6 @@ pprof = { version = "0.12", features = ["criterion", "flamegraph"] } env_logger = "0.10" log = "0.4" -[[bin]] -name = "memory" -required-features = ["memory"] - -[[bin]] -name = "graph_memory" -required-features = ["memory"] - -[[bin]] -name = "graph_perf" -required-features = ["historical-perf"] - [[bench]] name = "handshake" harness = false diff --git a/bindings/rust/bench/README.md b/bindings/rust/bench/README.md index e444f1e765b..b0cdb34cd3d 100644 --- a/bindings/rust/bench/README.md +++ b/bindings/rust/bench/README.md @@ -8,7 +8,7 @@ All benchmarks are run in an idealized environment, using only a single thread a ``` # generate rust bindings -../generate.sh +../generate.sh --skip-tests # run all benchmarks cargo bench @@ -36,37 +36,6 @@ Throughput benchmarks measure round-trip throughput with the client and server c To generate flamegraphs, run `cargo bench --bench handshake --bench throughput -- --profile-time 5`, which profiles each benchmark for 5 seconds and stores the resulting flamegraph in `target/criterion/[bench-name]/[lib-name]/profile/flamegraph.svg`. -## Memory benchmarks - -To run all memory benchmarks, run `scripts/bench-memory.sh`. Graphs of memory usage will be generated in `images/`. - -Memory benchmark data is generated using the `memory` binary. Command line arguments can be given to `cargo run` or to the built executable located at `target/release/memory`. The usage is as follows: - -``` -memory [(pair|client|server)] [(s2n-tls|rustls|openssl)] [--reuse-config (true|false)] [--shrink-buffers (true|false)] -``` - -- `(pair|client|server)`: target to memory bench, defaults to `server` -- `(s2n-tls|rustls|openssl)`: library to be benched, if unset benches all libraries -- `--reuse-config`: if `true` (default), reuse configs between connections -- `--shrink-buffers`: if `true` (default), shrink buffers owned by connections - -To view a callgraph of memory usage, use [KCachegrind](https://github.com/KDE/kcachegrind) on `xtree.out` generated from memory benching: - -``` -kcachegrind target/memory////xtree.out -``` - -To view a flamegraph of memory usage, use [heaptrack](https://github.com/KDE/heaptrack) with `heaptrack_gui` also installed. Run heaptrack with the `memory` executable and target/library options: - -``` -heaptrack target/release/memory (pair|client|server) (s2n-tls|rustls|openssl) -``` - -## Historical benchmarks - -To do historical benchmarks, run `scripts/bench-past.sh`. This will checkout old versions of s2n-tls back to v1.3.16 in `target/` and run benchmarks on those with the `historical-perf` feature, disabling Rustls and OpenSSL benches. - ## PKI Structure ``` ┌────root──────┐ @@ -88,21 +57,6 @@ The last version benched is v1.3.16, since before that, the s2n-tls Rust binding v1.3.30-1.3.37 are not benched because of dependency issues when generating the Rust bindings. However, versions before and after are benched, so the overall trend in performance can still be seen without the data from these versions. -### Sample output - -Because these benches take a longer time to generate (>30 min), we include the results from historical benching (as of v1.3.47) here. - -Notes: -- Two sets of parameters for the handshake couldn't be benched before 1.3.40, since security policies that negotiated those policies as their top choice did not exist before then. -- There is no data from 1.3.30 to 1.3.37 because those versions have a dependency issue that cause the Rust bindings not to build. However, there is data before and after that period, so the performance for those versions can be inferred via interpolation. -- The improvement in throughput in 1.3.28 was most likely caused by the addition of LTO to the default Rust bindings build. -- Since the benches are run over a long time, noise on the machine can cause variability, and background processes can cause spikes. -- The variability can be seen with throughput especially because it is calculated as the inverse of time taken. - -![historical-perf-handshake](images/historical-perf-handshake.svg) - -![historical-perf-throughput](images/historical-perf-throughput.svg) - ## Implementation details We use Rust bindings for s2n-tls and OpenSSL. All of our benchmarks are run in Rust on a single thread for consistency. diff --git a/bindings/rust/bench/benches/handshake.rs b/bindings/rust/bench/benches/handshake.rs index 6fc7c5b2a20..b86d0a81b0d 100644 --- a/bindings/rust/bench/benches/handshake.rs +++ b/bindings/rust/bench/benches/handshake.rs @@ -1,13 +1,10 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -#[cfg(feature = "openssl")] -use bench::OpenSslConnection; -#[cfg(feature = "rustls")] -use bench::RustlsConnection; use bench::{ harness::TlsBenchConfig, CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, - Mode, S2NConnection, SigType, TlsConnPair, TlsConnection, PROFILER_FREQUENCY, + Mode, OpenSslConnection, RustlsConnection, S2NConnection, SigType, TlsConnPair, TlsConnection, + PROFILER_FREQUENCY, }; use criterion::{ criterion_group, criterion_main, measurement::WallTime, BatchSize, BenchmarkGroup, Criterion, @@ -27,25 +24,20 @@ fn bench_handshake_for_library( { // make configs before benching to reuse let crypto_config = CryptoConfig::new(CipherSuite::default(), kx_group, sig_type); - let client_config = T::Config::make_config(Mode::Client, crypto_config, handshake_type); - let server_config = T::Config::make_config(Mode::Server, crypto_config, handshake_type); + let client_config = + T::Config::make_config(Mode::Client, crypto_config, handshake_type).unwrap(); + let server_config = + T::Config::make_config(Mode::Server, crypto_config, handshake_type).unwrap(); // generate all harnesses (TlsConnPair structs) beforehand so that benchmarks // only include negotiation and not config/connection initialization bench_group.bench_function(T::name(), |b| { b.iter_batched_ref( || -> Result, Box> { - if let (Ok(client_config), Ok(server_config)) = - (client_config.as_ref(), server_config.as_ref()) - { - let connected_buffer = ConnectedBuffer::default(); - let client = - T::new_from_config(client_config, connected_buffer.clone_inverse())?; - let server = T::new_from_config(server_config, connected_buffer)?; - Ok(TlsConnPair::wrap(client, server)) - } else { - Err("invalid configs".into()) - } + let connected_buffer = ConnectedBuffer::default(); + let client = T::new_from_config(&client_config, connected_buffer.clone_inverse())?; + let server = T::new_from_config(&server_config, connected_buffer)?; + Ok(TlsConnPair::wrap(client, server)) }, |conn_pair| { // harnesses with certain parameters fail to initialize for @@ -67,14 +59,12 @@ fn bench_handshake_with_params( sig_type: SigType, ) { bench_handshake_for_library::(bench_group, handshake_type, kx_group, sig_type); - #[cfg(feature = "rustls")] bench_handshake_for_library::( bench_group, handshake_type, kx_group, sig_type, ); - #[cfg(feature = "openssl")] bench_handshake_for_library::( bench_group, handshake_type, diff --git a/bindings/rust/bench/benches/throughput.rs b/bindings/rust/bench/benches/throughput.rs index 7ad675202f4..b6785552320 100644 --- a/bindings/rust/bench/benches/throughput.rs +++ b/bindings/rust/bench/benches/throughput.rs @@ -1,9 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -#[cfg(feature = "openssl")] use bench::OpenSslConnection; -#[cfg(feature = "rustls")] use bench::RustlsConnection; use bench::{ harness::TlsBenchConfig, CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, @@ -26,25 +24,20 @@ fn bench_throughput_for_library( T::Config: TlsBenchConfig, { let crypto_config = CryptoConfig::new(cipher_suite, KXGroup::default(), SigType::default()); - let client_config = T::Config::make_config(Mode::Client, crypto_config, HandshakeType::default()); - let server_config = T::Config::make_config(Mode::Server, crypto_config, HandshakeType::default()); + let client_config = + T::Config::make_config(Mode::Client, crypto_config, HandshakeType::default()).unwrap(); + let server_config = + T::Config::make_config(Mode::Server, crypto_config, HandshakeType::default()).unwrap(); bench_group.bench_function(T::name(), |b| { b.iter_batched_ref( || -> Result, Box> { - if let (Ok(client_config), Ok(server_config)) = - (client_config.as_ref(), server_config.as_ref()) - { - let connected_buffer = ConnectedBuffer::default(); - let client = - T::new_from_config(client_config, connected_buffer.clone_inverse())?; - let server = T::new_from_config(server_config, connected_buffer)?; - let mut conn_pair = TlsConnPair::wrap(client, server); - conn_pair.handshake()?; - Ok(conn_pair) - } else { - Err("invalid configs".into()) - } + let connected_buffer = ConnectedBuffer::default(); + let client = T::new_from_config(&client_config, connected_buffer.clone_inverse())?; + let server = T::new_from_config(&server_config, connected_buffer)?; + let mut conn_pair = TlsConnPair::wrap(client, server); + conn_pair.handshake()?; + Ok(conn_pair) }, |conn_pair| { if let Ok(conn_pair) = conn_pair { @@ -68,13 +61,11 @@ pub fn bench_throughput_cipher_suites(c: &mut Criterion) { &mut shared_buf, cipher_suite, ); - #[cfg(feature = "rustls")] bench_throughput_for_library::( &mut bench_group, &mut shared_buf, cipher_suite, ); - #[cfg(feature = "openssl")] bench_throughput_for_library::( &mut bench_group, &mut shared_buf, diff --git a/bindings/rust/bench/images/historical-perf-handshake.svg b/bindings/rust/bench/images/historical-perf-handshake.svg deleted file mode 100644 index 06f65b30056..00000000000 --- a/bindings/rust/bench/images/historical-perf-handshake.svg +++ /dev/null @@ -1,947 +0,0 @@ - - - -Performance of handshake by version since Jun 2022 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -Time - - -Version - - - - - - - - - - - - - - - - - - - - - - - - - -0 ms - - - -2 ms - - - -4 ms - - - -6 ms - - - -8 ms - - - - -1.3.16 - - - -1.3.18 - - - -1.3.20 - - - -1.3.22 - - - -1.3.24 - - - -1.3.26 - - - -1.3.28 - - - -1.3.30 - - - -1.3.32 - - - -1.3.34 - - - -1.3.36 - - - -1.3.38 - - - -1.3.40 - - - -1.3.42 - - - -1.3.44 - - - -1.3.46 - - - -1.3.48 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -handshake-x25519 - - -handshake-no-mTLS - - -handshake-rsa2048 - - -handshake-rsa4096 - - -handshake-rsa3072 - - -handshake-secp256r1 - - -handshake-mTLS - - -handshake-ecdsa384 - - - - - - - - - - diff --git a/bindings/rust/bench/images/historical-perf-throughput.svg b/bindings/rust/bench/images/historical-perf-throughput.svg deleted file mode 100644 index a27ad4e054c..00000000000 --- a/bindings/rust/bench/images/historical-perf-throughput.svg +++ /dev/null @@ -1,383 +0,0 @@ - - - -Performance of round trip throughput by version since Jun 2022 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -Throughput - - -Version - - - - - - - - - - - - - - - - - - - - - - - - - -0 GB/s - - - -0.2 GB/s - - - -0.4 GB/s - - - -0.6 GB/s - - - -0.8 GB/s - - - - -1.3.16 - - - -1.3.18 - - - -1.3.20 - - - -1.3.22 - - - -1.3.24 - - - -1.3.26 - - - -1.3.28 - - - -1.3.30 - - - -1.3.32 - - - -1.3.34 - - - -1.3.36 - - - -1.3.38 - - - -1.3.40 - - - -1.3.42 - - - -1.3.44 - - - -1.3.46 - - - -1.3.48 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -throughput-AES_128_GCM_SHA256 - - -throughput-AES_256_GCM_SHA384 - - - - diff --git a/bindings/rust/bench/scripts/bench-memory.sh b/bindings/rust/bench/scripts/bench-memory.sh deleted file mode 100755 index fcc7e4e3840..00000000000 --- a/bindings/rust/bench/scripts/bench-memory.sh +++ /dev/null @@ -1,30 +0,0 @@ -#!/usr/bin/env bash - -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 - -# Benches memory usage for all possible configurations and generate plots in images/ -# All given arguments (ex. `--config aws-lc-config/s2n.toml` to use AWS-LC) are passed to Cargo - -set -e - -pushd "$(dirname "$0")"/.. > /dev/null - -cargo build --release --features memory --bin memory --bin graph_memory "$@" - -# iterate through all possible options -for reuse_config in false true -do - for shrink_buffers in false true - do - for bench_target in client server pair - do - valgrind --tool=massif --depth=1 --massif-out-file="target/memory/massif.out" --time-unit=ms target/release/memory $bench_target --reuse-config $reuse_config --shrink-buffers $shrink_buffers - rm target/memory/massif.out - done - done -done - -cargo run --release --features memory --bin graph_memory "$@" - -popd > /dev/null diff --git a/bindings/rust/bench/scripts/bench-past.sh b/bindings/rust/bench/scripts/bench-past.sh deleted file mode 100755 index 57eaf04b738..00000000000 --- a/bindings/rust/bench/scripts/bench-past.sh +++ /dev/null @@ -1,80 +0,0 @@ -#!/usr/bin/env bash - -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 - -# Run historical benchmarking by checking out old version of s2n-tls into target/ -# Criterion JSON results get cached to target/historical-perf/[bench-group-name]/[version].json -# Results are then plotted, saved to images/historical-perf-[bench-name].svg -# All given arguments (ex. `--config aws-lc-config/s2n.toml` to use AWS-LC) are passed to Cargo - -# immediately bail if any command fails -set -e - -# suppress stdout and most cargo warnings -exec >/dev/null -export CARGO_TERM_QUIET=true -export RUSTFLAGS=-Awarnings - -# go to bench directory -pushd "$(dirname "$0")"/../ -bench_path="$(pwd)" - -# delete past runs -rm -rf target/historical-perf - -# make Cargo.toml point s2n-tls to the cloned old version -sed -i "s|s2n-tls = .*|s2n-tls = { path = \"target/s2n-tls/bindings/rust/s2n-tls\" }|" Cargo.toml - -# ensure Cargo.toml gets changed back on exit; retains original exit status -trap "{ status=$?; sed -i 's|s2n-tls = .*|s2n-tls = { path = \"../s2n-tls\" }|' $bench_path/Cargo.toml; exit $status; }" EXIT - -# clone copy of repo to target/s2n-tls -echo "cloning repo" >&2 -mkdir -p target -cd target -rm -rf s2n-tls -git clone --quiet https://github.com/aws/s2n-tls -cd s2n-tls/bindings/rust/ -copied_bindings_path="$(pwd)" - -# get list of tags sorted newest to oldest -sorted_tags="$(git tag -l | sort -rV)" - -# last tag we want is v1.3.16, get line number of v1.3.16 in sorted_tags -line_num_last_tag=$(echo "$sorted_tags" | grep "v1.3.16" --line-number | head -n 1 | cut -d":" -f1) - -# loop through all tags in order up to v1.3.16 -for tag in $(echo "$sorted_tags" | head -$line_num_last_tag) -do - ( - # go to s2n-tls/bindings/rust/ inside copied repo - cd $copied_bindings_path - - echo "checkout tag $tag" >&2 - git checkout $tag --quiet - - echo "generating rust bindings" >&2 - # if generate.sh fails, exit out of block - ./generate.sh || exit 1 - - echo "running cargo bench and saving results" >&2 - cd $bench_path - rm -rf target/criterion - cargo bench --no-default-features --no-fail-fast - - # cache criterion outputs from this bench into target/historical-perf - for bench_group in $(ls target/criterion | grep -v "report") - do - mkdir -p target/historical-perf/$bench_group/ - cp target/criterion/$bench_group/s2n-tls/new/estimates.json target/historical-perf/$bench_group/$tag.json - done - ) || echo "failed, trying next tag" - echo -done - -# graph results -cd $bench_path -cargo run --release --no-default-features --features historical-perf --bin graph_perf - -popd diff --git a/bindings/rust/bench/src/bin/graph_memory.rs b/bindings/rust/bench/src/bin/graph_memory.rs deleted file mode 100644 index c633e69d7b9..00000000000 --- a/bindings/rust/bench/src/bin/graph_memory.rs +++ /dev/null @@ -1,134 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -use plotters::{ - prelude::{ - ChartBuilder, IntoDrawingArea, IntoSegmentedCoord, LabelAreaPosition, Rectangle, - SVGBackend, SegmentValue, - }, - style::{AsRelative, Color, IntoFont, Palette, Palette99, RGBAColor, WHITE}, -}; -use std::{ - collections::BTreeMap, - error::Error, - fs::{read_dir, read_to_string}, - path::Path, -}; - -struct Stats { - mean: f64, - stderr: f64, -} - -fn get_bytes_from_snapshot(path: &Path, i: i32) -> i32 { - // number of bytes in snapshot starts on 8th line, 12th character - read_to_string(format!("{}/{i}.snapshot", path.display())) - .unwrap() - .lines() - .nth(7) - .unwrap()[11..] - .parse() - .unwrap() -} - -/// Get the difference in bytes between two snapshots, which is memory of the -/// `i`th TlsConnPair (client and server) -fn get_bytes_diff(path: &Path, i: i32) -> i32 { - get_bytes_from_snapshot(path, i + 1) - get_bytes_from_snapshot(path, i) -} - -fn get_memory_data(path: &Path) -> Stats { - let data: Vec = (0..100).map(|i| get_bytes_diff(path, i) as f64).collect(); - let mean = data.iter().sum::() / (data.len() as f64); - let variance: f64 = - data.iter().map(|x| (x - mean) * (x - mean)).sum::() / ((data.len() - 1) as f64); - let stdev = variance.sqrt(); - let stderr = stdev / (data.len() as f64).sqrt(); - - Stats { mean, stderr } -} - -/// Gets data from memory benching and plots it -fn plot_memory_data(param_name: &str, target_name: &str) -> Result<(), Box> { - // go through each library name directory (ex. "s2n-tls") and calculate stats - let mut stats: BTreeMap = Default::default(); // btree to sort by name - for dir_entry in read_dir(format!("target/memory/{param_name}/{target_name}"))? { - let dir_path = dir_entry?.path(); - let dir_name = dir_path.file_name().unwrap().to_str().unwrap().to_string(); - stats.insert(dir_name.clone(), get_memory_data(&dir_path)); - } - - // calculate things for plotting - let num_bars = stats.len(); - let x_labels: Vec = stats.iter().map(|kv| kv.0.clone()).collect(); - let max_mem = 120_000.0; // constant to keep scale same for all graphs - - // setup plotting - let chart_path = format!("images/memory-{target_name}-{param_name}.svg"); - let drawing_area = SVGBackend::new(&chart_path, (600, 500)).into_drawing_area(); - drawing_area.fill(&WHITE)?; - - let mut ctx = ChartBuilder::on(&drawing_area) - .caption( - format!("Memory of {target_name} with {param_name}"), - ("sans-serif", 30).into_font(), - ) - .set_label_area_size(LabelAreaPosition::Left, (15).percent()) // axes padding - .set_label_area_size(LabelAreaPosition::Bottom, (6).percent()) - .build_cartesian_2d( - (0..num_bars - 1).into_segmented(), - 0.0..(1.1 * max_mem), // upper y bound on plot is 1.1 * y_max - )?; - - let axis_label_style = ("sans-serif", 18).into_font(); - - ctx.configure_mesh() - .light_line_style(RGBAColor(235, 235, 235, 1.0)) // change gridline color - .bold_line_style(RGBAColor(225, 225, 225, 1.0)) - .x_labels(num_bars) - .x_label_formatter(&|x| { - // change axis labels to name of bar - let x = match *x { - SegmentValue::CenterOf(x) => x, - _ => 0, - }; - x_labels.get(x).unwrap().to_string() - }) - .x_label_style(axis_label_style.clone()) - .y_desc("Memory (kB)") - .y_labels(10) // max number of labels on y axis - .y_label_formatter(&|y| format!("{} kB", y / 1000.0)) - .y_label_style(axis_label_style) - .draw()?; - - // draw bars - // x coord is index of bench name in x_labels - ctx.draw_series(stats.iter().enumerate().map(|(i, (_name, stats))| { - // define each bar as a Rectangle - let x0 = SegmentValue::Exact(i); - let x1 = SegmentValue::Exact(i + 1); - let color = Palette99::pick(i).filled(); - let mut bar = Rectangle::new([(x0, 0.0), (x1, stats.mean)], color); - bar.set_margin(0, 0, 30, 30); // spacing between bars - bar - }))?; - - Ok(()) -} - -/// Plots all available data in target/memory and stores graphs in images -fn main() -> Result<(), Box> { - // iterate through param options ex. shrink-buffers or reuse-config - for param_dir_entry in read_dir("target/memory")? { - let param_dir_path = param_dir_entry?.path(); - let param_name = param_dir_path.file_name().unwrap().to_str().unwrap(); - - // iterate through targets, ex. client or server - for target_dir_entry in read_dir(¶m_dir_path)? { - let target_name = target_dir_entry?.file_name().to_string_lossy().to_string(); - plot_memory_data(param_name, &target_name)?; - } - } - - Ok(()) -} diff --git a/bindings/rust/bench/src/bin/graph_perf.rs b/bindings/rust/bench/src/bin/graph_perf.rs deleted file mode 100644 index f3a238e01ba..00000000000 --- a/bindings/rust/bench/src/bin/graph_perf.rs +++ /dev/null @@ -1,306 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -use plotters::{ - prelude::{ - BindKeyPoints, ChartBuilder, ErrorBar, IntoDrawingArea, LabelAreaPosition, Rectangle, - SVGBackend, SeriesLabelPosition, - }, - series::LineSeries, - style::{AsRelative, Color, IntoFont, Palette, Palette99, RGBAColor, BLACK, WHITE}, -}; -use semver::Version; -use serde_json::Value; -use std::{ - collections::{BTreeSet, HashMap}, - fs::{read_dir, read_to_string}, - path::Path, -}; - -struct Stats { - mean: f64, - stderr: f64, -} - -struct VersionDataPoint { - version: Version, // x coordinate - mean: f64, // y coordinate - stderr: f64, // y error bar -} - -struct VersionDataSeries { - name: String, // ex. throughput-AES_128_GCM_SHA256 - data: Vec, -} - -struct DataPoint { - x: i32, - y: f64, - y_bar: f64, -} - -struct DataSeries { - name: String, - data: Vec, -} - -/// Get the relevant stats in a given JSON bench output -fn process_single_json(path: &Path) -> Stats { - let json_str = read_to_string(path).unwrap(); - let json_value: Value = serde_json::from_str(json_str.as_str()).unwrap(); - let stats = json_value.get("mean").unwrap(); - Stats { - mean: stats.get("point_estimate").unwrap().as_f64().unwrap(), - stderr: stats.get("standard_error").unwrap().as_f64().unwrap(), - } -} - -/// Get data from directory of Criterion json outputs, given directory path -/// Outputs a Vec of (version, mean, stderr) sorted by version -fn parse_bench_group_data(path: &Path) -> Vec { - let mut data: Vec = read_dir(path) - .unwrap() - .map(|dir_entry| { - let path = dir_entry.unwrap().path(); - let stats = process_single_json(&path); - let tag = path.file_stem().unwrap().to_str().unwrap(); - let version = Version::parse(&tag[1..]).unwrap(); - VersionDataPoint { - version, - mean: stats.mean, - stderr: stats.stderr, - } - }) - .collect(); - data.sort_by(|data_point_1, data_point_2| data_point_1.version.cmp(&data_point_2.version)); - data -} - -/// Gets data from all bench groups given a prefix (ex. "handshake") for the bench group names -fn get_all_data(prefix: &str) -> Vec { - read_dir("target/historical-perf") - .unwrap() - .map(|dir_entry| dir_entry.unwrap().path()) - .filter(|path| { - // get all paths starting with prefix - path.file_name() - .unwrap() - .to_str() - .unwrap() - .starts_with(prefix) - }) - .map(|path| { - // get data in each directory - VersionDataSeries { - name: path.file_name().unwrap().to_string_lossy().into_owned(), - data: parse_bench_group_data(&path), - } - }) - .collect() -} - -fn get_unique_versions(data: &[VersionDataSeries]) -> BTreeSet { - data.iter() - .flat_map(|data_series| { - data_series - .data - .iter() - .map(|version_data_point| version_data_point.version.clone()) - }) - .collect() -} - -/// Converts all VersionDataSeries in version_data to DataSeries -fn convert_to_data_series( - version_data: Vec, - version_to_x: &HashMap<&Version, i32>, -) -> Vec { - version_data - .into_iter() - .map(|version_data_series| DataSeries { - name: version_data_series.name, - data: version_data_series - .data - .into_iter() - .map(|version_data_point| DataPoint { - // map VersionDataPoints to DataPoints - x: version_to_x[&&version_data_point.version], - y: version_data_point.mean, - y_bar: version_data_point.stderr * 1.96, // 95% confidence interval - }) - .collect(), - }) - .collect() -} - -/// Plots given DataSeries with given chart parameters -fn plot_data String, G: Fn(&f64) -> String>( - data: &[DataSeries], - image_name: &str, - bench_name: &str, - x_label_formatter: &F, - y_label: &str, - y_label_formatter: &G, -) { - // get x_max and y_max for plotting range - let x_max = data - .iter() - .flat_map(|data_series| data_series.data.iter().map(|data_point| data_point.x)) - .max_by(|a, b| a.partial_cmp(b).unwrap()) - .unwrap(); - let y_max = data - .iter() - .flat_map(|data_series| data_series.data.iter().map(|data_point| data_point.y)) - .max_by(|a, b| a.partial_cmp(b).unwrap()) - .unwrap(); - - // setup plotting - let path = format!("images/historical-perf-{image_name}.svg"); - let drawing_area = SVGBackend::new(&path, (1000, 500)).into_drawing_area(); - drawing_area.fill(&WHITE).unwrap(); - - let mut ctx = ChartBuilder::on(&drawing_area) - .caption( - format!("Performance of {bench_name} by version since Jun 2022"), - ("sans-serif", 30).into_font(), - ) - .set_label_area_size(LabelAreaPosition::Left, (17).percent()) // axes padding - .set_label_area_size(LabelAreaPosition::Bottom, (11).percent()) - .build_cartesian_2d( - // bounds for plot - // plot every other x coord starting from 1 (not 0 which is default) - (0..(x_max + 1)).with_key_points((1..(x_max + 1)).step_by(2).collect()), - 0.0..(1.2 * y_max), - ) - .unwrap(); - - let axis_label_style = ("sans-serif", 18).into_font(); - - ctx.configure_mesh() - .light_line_style(RGBAColor(235, 235, 235, 1.0)) // gridline color - .bold_line_style(RGBAColor(225, 225, 225, 1.0)) - .x_desc("Version") // axis labels - .x_labels(20) // max number of labels - .x_label_style(axis_label_style.clone()) - .x_label_formatter(x_label_formatter) - .y_desc(y_label) - .y_labels(5) - .y_label_formatter(y_label_formatter) - .y_label_style(axis_label_style) - .draw() - .unwrap(); - - // go through each DataSeries and plot them - for (i, data_series) in data.iter().enumerate() { - // remove data that returned error while benching - // heuristic: times < 1% of y_max are invalid/had error - let filtered_data = data_series - .data - .iter() - .filter(|data_point| data_point.y > 0.01 * y_max) - .collect::>(); - - let color = Palette99::pick(i); - - // draw error bars - ctx.draw_series(filtered_data.iter().map(|data_point| { - ErrorBar::new_vertical( - data_point.x, - data_point.y - data_point.y_bar, - data_point.y, - data_point.y + data_point.y_bar, - &color, - 3, - ) - })) - .unwrap(); - - // draw lines with legend entry - ctx.draw_series(LineSeries::new( - filtered_data - .iter() - .map(|data_point| (data_point.x, data_point.y)), - color.stroke_width(2), - )) - .unwrap() - .label(&data_series.name) - .legend(move |(x, y)| Rectangle::new([(x, y - 5), (x + 10, y + 5)], color.filled())); - } - - // enable legend - ctx.configure_series_labels() - .position(SeriesLabelPosition::LowerRight) - .margin(10) - .border_style(BLACK) - .background_style(WHITE) - .draw() - .unwrap(); -} - -fn main() { - let handshake_data = get_all_data("handshake"); - let throughput_data = get_all_data("throughput"); - - // combine all versions present in handshake and throughput data - // also fill in missing version v1.3.15 and v1.3.30-v1.3.37 - let mut versions = get_unique_versions(&handshake_data); - versions.extend(get_unique_versions(&throughput_data).into_iter()); - versions.extend((15..16).chain(30..38).map(|p| Version::new(1, 3, p))); - let versions = versions.into_iter().collect::>(); - - // map versions to x coordinates - let version_to_x = versions - .iter() - .enumerate() - .map(|(i, version)| (version, i as i32)) - .collect::>(); - - // convert from Vec to Vec for plotting - let handshake_data: Vec = convert_to_data_series(handshake_data, &version_to_x); - let mut throughput_data = convert_to_data_series(throughput_data, &version_to_x); - - // convert data from ns to transfer of 100KB of data -> bytes/s throughput - throughput_data = throughput_data - .into_iter() - .map(|data_series| { - const TRANSFER_SIZE: f64 = 1e5; - const NANO_SIZE: f64 = 1e-9; - DataSeries { - name: data_series.name, - data: data_series - .data - .into_iter() - .map(|data_point| { - let mean_throughput = TRANSFER_SIZE / (data_point.y * NANO_SIZE); - let stderr_throughput = mean_throughput - - TRANSFER_SIZE / ((data_point.y + data_point.y_bar) * NANO_SIZE); - DataPoint { - x: data_point.x, - y: mean_throughput, - y_bar: stderr_throughput, - } - }) - .collect(), - } - }) - .collect(); - - let x_label_formatter = |x: &i32| format!("{}", versions[*x as usize]); - - plot_data( - &handshake_data, - "handshake", - "handshake", - &x_label_formatter, - "Time", - &|y| format!("{} ms", y / 1e6), - ); - plot_data( - &throughput_data, - "throughput", - "round trip throughput", - &x_label_formatter, - "Throughput", - &|y| format!("{} GB/s", y / 1e9), - ); -} diff --git a/bindings/rust/bench/src/bin/memory.rs b/bindings/rust/bench/src/bin/memory.rs deleted file mode 100644 index aeee51338fd..00000000000 --- a/bindings/rust/bench/src/bin/memory.rs +++ /dev/null @@ -1,191 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -#[cfg(feature = "openssl")] -use bench::OpenSslConnection; -#[cfg(feature = "rustls")] -use bench::RustlsConnection; -use bench::{ - ConnectedBuffer, CryptoConfig, HandshakeType, Mode, S2NConnection, TlsConnPair, TlsConnection, -}; -use std::{error::Error, fs::create_dir_all}; -use structopt::{clap::arg_enum, StructOpt}; - -arg_enum! { - enum MemoryBenchTarget { - Client, - Server, - Pair, - } -} - -impl std::fmt::Debug for MemoryBenchTarget { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{}", - match self { - MemoryBenchTarget::Client => "client", - MemoryBenchTarget::Server => "server", - MemoryBenchTarget::Pair => "pair", - } - ) - } -} - -/// Bench the memory taken by either a client, server, or pair of connections -fn memory_bench(opt: &Opt) -> Result<(), Box> { - let reuse_config: bool = opt.reuse_config.parse()?; - let shrink_buffers: bool = opt.shrink_buffers.parse()?; - - // store data in directory based on params, target, and library name - let params_string = match (reuse_config, shrink_buffers) { - (false, false) => "no-optimizations", - (true, false) => "reuse-config", - (false, true) => "shrink-buffers", - (true, true) => "reuse-config-shrink-buffers", - }; - let dir_name = &format!( - "target/memory/{params_string}/{:?}/{}", - opt.target, - T::name() - ); - - println!("benching {:?} {} {}", opt.target, T::name(), params_string); - - // create the directory that will hold memory snapshots and xtree - create_dir_all(dir_name).unwrap(); - - // create space to store TlsConnections - const BENCH_SIZE: usize = 100; - let mut connections = Vec::new(); - match opt.target { - MemoryBenchTarget::Client | MemoryBenchTarget::Server => { - connections.reserve_exact(BENCH_SIZE) - } - // for each connection pair, need to save two connections - MemoryBenchTarget::Pair => connections.reserve_exact(BENCH_SIZE * 2), - }; - - // reserve space for buffers before benching - // shrink buffers before and after handshake to keep memory net zero - let mut buffers: Vec = (0..BENCH_SIZE) - .map(|_| { - let mut buffer = ConnectedBuffer::new(); - buffer.shrink(); - buffer - }) - .collect(); - - // handshake one harness to initalize libraries - let mut conn_pair = TlsConnPair::::default(); - conn_pair.handshake().unwrap(); - - // make configs - let client_config = T::make_config( - Mode::Client, - CryptoConfig::default(), - HandshakeType::default(), - )?; - let server_config = T::make_config( - Mode::Server, - CryptoConfig::default(), - HandshakeType::default(), - )?; - - // tell valgrind/massif to take initial memory snapshot - crabgrind::monitor_command(format!("snapshot {dir_name}/0.snapshot")).unwrap(); - - // make and handshake conn pairs - for i in 1..BENCH_SIZE + 1 { - // make conn pair - let mut conn_pair; - if reuse_config { - let client_conn = T::new_from_config(&client_config, buffers.pop().unwrap())?; - let server_conn = T::new_from_config( - &server_config, - client_conn.connected_buffer().clone_inverse(), - )?; - conn_pair = TlsConnPair::wrap(client_conn, server_conn); - } else { - conn_pair = TlsConnPair::::new( - CryptoConfig::default(), - HandshakeType::default(), - buffers.pop().unwrap(), - )?; - } - - // handshake conn pair - conn_pair.handshake()?; - if shrink_buffers { - conn_pair.shrink_connection_buffers(); - } - conn_pair.shrink_connected_buffers(); - - // store bench target(s) - let (client, server) = conn_pair.split(); - match opt.target { - MemoryBenchTarget::Client => connections.push(client), - MemoryBenchTarget::Server => connections.push(server), - MemoryBenchTarget::Pair => { - connections.push(client); - connections.push(server); - } - }; - - // take memory snapshot - crabgrind::monitor_command(format!("snapshot {dir_name}/{i}.snapshot"))?; - } - - // take xtree snapshot - crabgrind::monitor_command(format!("xtmemory {dir_name}/xtree.out"))?; - - Ok(()) -} - -#[derive(StructOpt)] -/// Generate TLS connections and record memory used after each connection. -/// Snapshots are stored in target/memory/[params]/[target] -struct Opt { - /// Which connection(s) to memory bench - #[structopt(possible_values = &MemoryBenchTarget::variants(), case_insensitive = true, default_value = "pair")] - target: MemoryBenchTarget, - - /// If set, run benches with only a specific library - #[structopt()] - lib_name: Option, - - /// Reuse configs when making connections - #[structopt(long, default_value = "true")] - reuse_config: String, - - /// Shrink connection buffers after handshake to simulate idle connection - #[structopt(long, default_value = "true")] - shrink_buffers: String, -} - -fn main() -> Result<(), Box> { - assert!(!cfg!(debug_assertions), "need to run in release mode"); - - let opt = Opt::from_args(); - - match &opt.lib_name { - Some(lib_name) => match lib_name.as_str() { - "s2n-tls" => memory_bench::(&opt)?, - #[cfg(feature = "rustls")] - "rustls" => memory_bench::(&opt)?, - #[cfg(feature = "openssl")] - "openssl" => memory_bench::(&opt)?, - _ => panic!("invalid library"), - }, - None => { - memory_bench::(&opt)?; - #[cfg(feature = "rustls")] - memory_bench::(&opt)?; - #[cfg(feature = "openssl")] - memory_bench::(&opt)?; - } - } - - Ok(()) -} diff --git a/bindings/rust/bench/src/harness.rs b/bindings/rust/bench/src/harness.rs index c535afaf528..eff9bc629e3 100644 --- a/bindings/rust/bench/src/harness.rs +++ b/bindings/rust/bench/src/harness.rs @@ -417,11 +417,7 @@ impl Default for ConnectedBuffer { #[cfg(test)] mod tests { use super::*; - #[cfg(feature = "openssl")] - use crate::OpenSslConnection; - #[cfg(feature = "rustls")] - use crate::RustlsConnection; - use crate::{S2NConnection, TlsConnPair}; + use crate::{OpenSslConnection, RustlsConnection, S2NConnection, TlsConnPair}; use std::path::Path; use strum::IntoEnumIterator; @@ -440,9 +436,7 @@ mod tests { #[test] fn test_all() { test_type::(); - #[cfg(feature = "rustls")] test_type::(); - #[cfg(feature = "openssl")] test_type::(); } diff --git a/bindings/rust/bench/src/lib.rs b/bindings/rust/bench/src/lib.rs index e6ef9ec4333..95d92b42ab4 100644 --- a/bindings/rust/bench/src/lib.rs +++ b/bindings/rust/bench/src/lib.rs @@ -2,21 +2,17 @@ // SPDX-License-Identifier: Apache-2.0 pub mod harness; -#[cfg(feature = "openssl")] pub mod openssl; -#[cfg(feature = "rustls")] pub mod rustls; pub mod s2n_tls; -#[cfg(feature = "openssl")] -pub use crate::openssl::OpenSslConnection; -#[cfg(feature = "rustls")] -pub use crate::rustls::RustlsConnection; pub use crate::{ harness::{ get_cert_path, CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, Mode, PemType, SigType, TlsConnPair, TlsConnection, }, + openssl::OpenSslConnection, + rustls::RustlsConnection, s2n_tls::S2NConnection, }; diff --git a/bindings/rust/bench/src/openssl.rs b/bindings/rust/bench/src/openssl.rs index 012f2aac0b1..24ec9e9e125 100644 --- a/bindings/rust/bench/src/openssl.rs +++ b/bindings/rust/bench/src/openssl.rs @@ -4,7 +4,8 @@ use crate::{ get_cert_path, harness::{ - CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, Mode, TlsConnection, TlsBenchConfig, + CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, Mode, TlsBenchConfig, + TlsConnection, }, PemType::*, }; @@ -43,7 +44,6 @@ pub struct OpenSslConfig { } impl TlsBenchConfig for OpenSslConfig { - fn make_config( mode: Mode, crypto_config: CryptoConfig, @@ -148,7 +148,6 @@ impl TlsConnection for OpenSslConnection { ) } - fn new_from_config( config: &Self::Config, connected_buffer: ConnectedBuffer, From a70b2b083613a406598165f1af56727ffa238506 Mon Sep 17 00:00:00 2001 From: Sam Clark <3758302+goatgoose@users.noreply.github.com> Date: Mon, 16 Dec 2024 11:04:29 -0500 Subject: [PATCH 2/3] refactor(s2n-tls-hyper): Add HttpsConnector builder (#4976) --- bindings/rust/s2n-tls-hyper/src/connector.rs | 102 ++++++++++++++----- 1 file changed, 78 insertions(+), 24 deletions(-) diff --git a/bindings/rust/s2n-tls-hyper/src/connector.rs b/bindings/rust/s2n-tls-hyper/src/connector.rs index ea5670d62f0..8721bff70ee 100644 --- a/bindings/rust/s2n-tls-hyper/src/connector.rs +++ b/bindings/rust/s2n-tls-hyper/src/connector.rs @@ -24,49 +24,64 @@ use tower_service::Service; /// which sends and receives requests over TCP. The `HttpsConnector` struct wraps an HTTP connector, /// and uses it to negotiate TLS when the HTTPS scheme is in use. #[derive(Clone)] -pub struct HttpsConnector { +pub struct HttpsConnector { http: Http, - conn_builder: Builder, + conn_builder: ConnBuilder, } -impl HttpsConnector +impl HttpsConnector where - Builder: connection::Builder, - ::Output: Unpin, + ConnBuilder: connection::Builder, + ::Output: Unpin, { - /// Creates a new `HttpsConnector`. + /// Creates a new `HttpsConnector` with the default settings. + /// + /// Use `HttpsConnector::builder()` instead to configure an `HttpsConnector`. + /// + /// `conn_builder` will be used to produce the s2n-tls Connections used for negotiating HTTPS, + /// which can be an `s2n_tls::config::Config` or other `s2n_tls::connection::Builder`. + /// + /// Note that s2n-tls-hyper will override the ALPN extension to negotiate HTTP. Any ALPN values + /// configured on `conn_builder` with APIs like + /// `s2n_tls::config::Builder::set_application_protocol_preference()` will be ignored. + pub fn new(conn_builder: ConnBuilder) -> HttpsConnector { + HttpsConnector::builder(conn_builder).build() + } + + /// Creates a `Builder` to configure a new `HttpsConnector`. /// /// `conn_builder` will be used to produce the s2n-tls Connections used for negotiating HTTPS, /// which can be an `s2n_tls::config::Config` or other `s2n_tls::connection::Builder`. /// - /// This API creates an `HttpsConnector` using the default hyper `HttpConnector`. To use an - /// existing HTTP connector, use `HttpsConnector::new_with_http()`. + /// This API creates a `Builder` with the default hyper `HttpConnector`. To use an existing HTTP + /// connector, use `HttpsConnector::builder_with_http()`. /// /// Note that s2n-tls-hyper will override the ALPN extension to negotiate HTTP. Any ALPN values /// configured on `conn_builder` with APIs like /// `s2n_tls::config::Builder::set_application_protocol_preference()` will be ignored. - pub fn new(conn_builder: Builder) -> HttpsConnector { + pub fn builder(conn_builder: ConnBuilder) -> Builder { let mut http = HttpConnector::new(); // By default, the `HttpConnector` only allows the HTTP URI scheme to be used. To negotiate // HTTP over TLS via the HTTPS scheme, `enforce_http` must be disabled. http.enforce_http(false); - Self { http, conn_builder } + HttpsConnector::builder_with_http(http, conn_builder) } } -impl HttpsConnector +impl HttpsConnector where - Builder: connection::Builder, - ::Output: Unpin, + ConnBuilder: connection::Builder, + ::Output: Unpin, { - /// Creates a new `HttpsConnector`. + /// Creates a `Builder` to configure a new `HttpsConnector`. /// /// `conn_builder` will be used to produce the s2n-tls Connections used for negotiating HTTPS, /// which can be an `s2n_tls::config::Config` or other `s2n_tls::connection::Builder`. /// - /// This API allows an `HttpsConnector` to be constructed with an existing HTTP connector, as follows: + /// This API allows an `HttpsConnector` to be constructed with an existing HTTP connector, as + /// follows: /// ``` /// use s2n_tls_hyper::connector::HttpsConnector; /// use s2n_tls::config::Config; @@ -77,16 +92,33 @@ where /// // Ensure that the HTTP connector permits the HTTPS scheme. /// http.enforce_http(false); /// - /// let connector = HttpsConnector::new_with_http(http, Config::default()); + /// let connector = HttpsConnector::builder_with_http(http, Config::default()).build(); /// ``` /// - /// `HttpsConnector::new()` can be used to create the HTTP connector automatically. + /// `HttpsConnector::builder()` can be used to create the HTTP connector automatically. /// /// Note that s2n-tls-hyper will override the ALPN extension to negotiate HTTP. Any ALPN values /// configured on `conn_builder` with APIs like /// `s2n_tls::config::Builder::set_application_protocol_preference()` will be ignored. - pub fn new_with_http(http: Http, conn_builder: Builder) -> HttpsConnector { - Self { http, conn_builder } + pub fn builder_with_http(http: Http, conn_builder: ConnBuilder) -> Builder { + Builder { http, conn_builder } + } +} + +/// Builder used to configure an `HttpsConnector`. Create a new Builder with +/// `HttpsConnector::builder`. +pub struct Builder { + http: Http, + conn_builder: ConnBuilder, +} + +impl Builder { + /// Builds a new `HttpsConnector`. + pub fn build(self) -> HttpsConnector { + HttpsConnector { + http: self.http, + conn_builder: self.conn_builder, + } } } @@ -96,19 +128,22 @@ where // https://docs.rs/hyper-util/latest/hyper_util/client/legacy/connect/trait.Connect.html // // The hyper compatibility traits for `Service::Response` are implemented in `MaybeHttpsStream`. -impl Service for HttpsConnector +impl Service for HttpsConnector where Http: Service, Http::Response: Read + Write + Connection + Unpin + Send + 'static, Http::Future: Send + 'static, Http::Error: Into>, - Builder: connection::Builder + Send + Sync + 'static, - ::Output: Unpin + Send, + ConnBuilder: connection::Builder + Send + Sync + 'static, + ::Output: Unpin + Send, { - type Response = MaybeHttpsStream; + type Response = MaybeHttpsStream; type Error = Error; type Future = Pin< - Box, Error>> + Send>, + Box< + dyn Future, Error>> + + Send, + >, >; fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { @@ -180,6 +215,25 @@ mod tests { use hyper_util::{client::legacy::Client, rt::TokioExecutor}; use std::{error::Error as StdError, str::FromStr}; + #[tokio::test] + async fn connector_creation() { + let config = Config::default(); + let connector_from_new = HttpsConnector::new(config.clone()); + let _assert_type: HttpsConnector = connector_from_new; + + let connector_from_builder = HttpsConnector::builder(config.clone()).build(); + let _assert_type: HttpsConnector = connector_from_builder; + + let http: u32 = 10; + let connector_from_builder_with_http = + HttpsConnector::builder_with_http(http, config.clone()).build(); + let _assert_type: HttpsConnector = connector_from_builder_with_http; + + let builder = HttpsConnector::builder(config.clone()); + let connector_from_builder = builder.build(); + let _assert_type: HttpsConnector = connector_from_builder; + } + #[tokio::test] async fn test_unsecure_http() -> Result<(), Box> { let connector = HttpsConnector::new(Config::default()); From e50f31984236bb79879e2a9437017a5663665cd5 Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Mon, 16 Dec 2024 14:59:17 -0800 Subject: [PATCH 3/3] refactor(bindings/bench): make harness own IO (#4847) --- bindings/rust/bench/benches/handshake.rs | 19 +-- bindings/rust/bench/benches/throughput.rs | 20 +-- bindings/rust/bench/src/harness/io.rs | 67 ++++++++ .../bench/src/{harness.rs => harness/mod.rs} | 154 +++--------------- bindings/rust/bench/src/lib.rs | 4 +- bindings/rust/bench/src/openssl.rs | 33 +--- bindings/rust/bench/src/rustls.rs | 38 ++--- bindings/rust/bench/src/s2n_tls.rs | 64 +++----- 8 files changed, 148 insertions(+), 251 deletions(-) create mode 100644 bindings/rust/bench/src/harness/io.rs rename bindings/rust/bench/src/{harness.rs => harness/mod.rs} (73%) diff --git a/bindings/rust/bench/benches/handshake.rs b/bindings/rust/bench/benches/handshake.rs index b86d0a81b0d..6b011278568 100644 --- a/bindings/rust/bench/benches/handshake.rs +++ b/bindings/rust/bench/benches/handshake.rs @@ -2,15 +2,14 @@ // SPDX-License-Identifier: Apache-2.0 use bench::{ - harness::TlsBenchConfig, CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, - Mode, OpenSslConnection, RustlsConnection, S2NConnection, SigType, TlsConnPair, TlsConnection, + harness::TlsBenchConfig, CipherSuite, CryptoConfig, HandshakeType, KXGroup, Mode, + OpenSslConnection, RustlsConnection, S2NConnection, SigType, TlsConnPair, TlsConnection, PROFILER_FREQUENCY, }; use criterion::{ criterion_group, criterion_main, measurement::WallTime, BatchSize, BenchmarkGroup, Criterion, }; use pprof::criterion::{Output, PProfProfiler}; -use std::error::Error; use strum::IntoEnumIterator; fn bench_handshake_for_library( @@ -33,19 +32,9 @@ fn bench_handshake_for_library( // only include negotiation and not config/connection initialization bench_group.bench_function(T::name(), |b| { b.iter_batched_ref( - || -> Result, Box> { - let connected_buffer = ConnectedBuffer::default(); - let client = T::new_from_config(&client_config, connected_buffer.clone_inverse())?; - let server = T::new_from_config(&server_config, connected_buffer)?; - Ok(TlsConnPair::wrap(client, server)) - }, + || -> TlsConnPair { TlsConnPair::from_configs(&client_config, &server_config) }, |conn_pair| { - // harnesses with certain parameters fail to initialize for - // some past versions of s2n-tls, but missing data can be - // visually interpolated in the historical performance graph - if let Ok(conn_pair) = conn_pair { - let _ = conn_pair.handshake(); - } + conn_pair.handshake().unwrap(); }, BatchSize::SmallInput, ) diff --git a/bindings/rust/bench/benches/throughput.rs b/bindings/rust/bench/benches/throughput.rs index b6785552320..ae43cf955b6 100644 --- a/bindings/rust/bench/benches/throughput.rs +++ b/bindings/rust/bench/benches/throughput.rs @@ -4,15 +4,14 @@ use bench::OpenSslConnection; use bench::RustlsConnection; use bench::{ - harness::TlsBenchConfig, CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, - Mode, S2NConnection, SigType, TlsConnPair, TlsConnection, PROFILER_FREQUENCY, + harness::TlsBenchConfig, CipherSuite, CryptoConfig, HandshakeType, KXGroup, Mode, + S2NConnection, SigType, TlsConnPair, TlsConnection, PROFILER_FREQUENCY, }; use criterion::{ criterion_group, criterion_main, measurement::WallTime, BatchSize, BenchmarkGroup, Criterion, Throughput, }; use pprof::criterion::{Output, PProfProfiler}; -use std::error::Error; use strum::IntoEnumIterator; fn bench_throughput_for_library( @@ -31,18 +30,13 @@ fn bench_throughput_for_library( bench_group.bench_function(T::name(), |b| { b.iter_batched_ref( - || -> Result, Box> { - let connected_buffer = ConnectedBuffer::default(); - let client = T::new_from_config(&client_config, connected_buffer.clone_inverse())?; - let server = T::new_from_config(&server_config, connected_buffer)?; - let mut conn_pair = TlsConnPair::wrap(client, server); - conn_pair.handshake()?; - Ok(conn_pair) + || -> TlsConnPair { + let mut conn_pair = TlsConnPair::from_configs(&client_config, &server_config); + conn_pair.handshake().unwrap(); + conn_pair }, |conn_pair| { - if let Ok(conn_pair) = conn_pair { - let _ = conn_pair.round_trip_transfer(shared_buf); - } + let _ = conn_pair.round_trip_transfer(shared_buf); }, BatchSize::SmallInput, ) diff --git a/bindings/rust/bench/src/harness/io.rs b/bindings/rust/bench/src/harness/io.rs new file mode 100644 index 00000000000..732d41deaf3 --- /dev/null +++ b/bindings/rust/bench/src/harness/io.rs @@ -0,0 +1,67 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::{cell::RefCell, collections::VecDeque, io::ErrorKind, pin::Pin, rc::Rc}; + +pub type LocalDataBuffer = RefCell>; + +#[derive(Debug)] +pub struct TestPairIO { + /// a data buffer that the server writes to and the client reads from + pub server_tx_stream: Pin>, + /// a data buffer that the client writes to and the server reads from + pub client_tx_stream: Pin>, +} + +impl TestPairIO { + pub fn client_view(&self) -> ViewIO { + ViewIO { + send_ctx: self.client_tx_stream.clone(), + recv_ctx: self.server_tx_stream.clone(), + } + } + + pub fn server_view(&self) -> ViewIO { + ViewIO { + send_ctx: self.server_tx_stream.clone(), + recv_ctx: self.client_tx_stream.clone(), + } + } +} + +/// A "view" of the IO. +/// +/// This view is client/server specific, and notably implements the read and write +/// traits. +/// +// This struct is used by Openssl and Rustls which both rely on a "stream" abstraction +// which implements read and write. This is not used by s2n-tls, which relies on +// lower level callbacks. +pub struct ViewIO { + pub send_ctx: Pin>, + pub recv_ctx: Pin>, +} + +impl std::io::Read for ViewIO { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + let res = self.recv_ctx.borrow_mut().read(buf); + if let Ok(0) = res { + // We are "faking" a TcpStream, where a read of length 0 indicates + // EoF. That is incorrect for this scenario. Instead we return WouldBlock + // to indicate that there is simply no more data to be read. + Err(std::io::Error::new(ErrorKind::WouldBlock, "blocking")) + } else { + res + } + } +} + +impl std::io::Write for ViewIO { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.send_ctx.borrow_mut().write(buf) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} diff --git a/bindings/rust/bench/src/harness.rs b/bindings/rust/bench/src/harness/mod.rs similarity index 73% rename from bindings/rust/bench/src/harness.rs rename to bindings/rust/bench/src/harness/mod.rs index eff9bc629e3..33420cc606c 100644 --- a/bindings/rust/bench/src/harness.rs +++ b/bindings/rust/bench/src/harness/mod.rs @@ -1,15 +1,11 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -use std::{ - cell::RefCell, - collections::VecDeque, - error::Error, - fmt::Debug, - fs::read_to_string, - io::{ErrorKind, Read, Write}, - rc::Rc, -}; +mod io; +pub use io::{LocalDataBuffer, ViewIO}; + +use io::TestPairIO; +use std::{error::Error, fmt::Debug, fs::read_to_string, rc::Rc}; use strum::EnumIter; #[derive(Clone, Copy, EnumIter)] @@ -161,10 +157,7 @@ pub trait TlsConnection: Sized { fn name() -> String; /// Make connection from existing config and buffer - fn new_from_config( - config: &Self::Config, - connected_buffer: ConnectedBuffer, - ) -> Result>; + fn new_from_config(config: &Self::Config, io: ViewIO) -> Result>; /// Run one handshake step: receive msgs from other connection, process, and send new msgs fn handshake(&mut self) -> Result<(), Box>; @@ -184,29 +177,13 @@ pub trait TlsConnection: Sized { /// Read application data from ConnectedBuffer fn recv(&mut self, data: &mut [u8]) -> Result<(), Box>; - - /// Shrink buffers owned by the connection - fn shrink_connection_buffers(&mut self); - - /// Clear and shrink buffers used for IO with another connection - fn shrink_connected_buffer(&mut self); - - /// Get reference to internal connected buffer - fn connected_buffer(&self) -> &ConnectedBuffer; } +/// A TlsConnPair owns the client and server tls connections along with the IO buffers. pub struct TlsConnPair { - client: C, - server: S, -} - -impl TlsConnPair { - pub fn new(client_config: &C::Config, server_config: &S::Config) -> TlsConnPair { - let connected_buffer = ConnectedBuffer::default(); - let client = C::new_from_config(&client_config, connected_buffer.clone_inverse()).unwrap(); - let server = S::new_from_config(&server_config, connected_buffer).unwrap(); - Self { client, server } - } + pub client: C, + pub server: S, + pub io: TestPairIO, } impl Default for TlsConnPair @@ -242,7 +219,7 @@ where // handshake the client and server connections. This will result in // session ticket getting stored in client_config - let mut pair = TlsConnPair::::new(&client_config, &server_config); + let mut pair = TlsConnPair::::from_configs(&client_config, &server_config); pair.handshake()?; // NewSessionTicket messages are part of the application data and sent // after the handshake is complete, so we must trigger an additional @@ -255,10 +232,13 @@ where // on the connection. This results in the session ticket in // client_config (from the previous handshake) getting set on the // client connection. - return Ok(TlsConnPair::::new(&client_config, &server_config)); + return Ok(TlsConnPair::::from_configs( + &client_config, + &server_config, + )); } - Ok(TlsConnPair::::new( + Ok(TlsConnPair::::from_configs( &C::Config::make_config(Mode::Client, crypto_config, handshake_type).unwrap(), &S::Config::make_config(Mode::Server, crypto_config, handshake_type).unwrap(), )) @@ -270,13 +250,14 @@ where C: TlsConnection, S: TlsConnection, { - /// Wrap two TlsConnections into a TlsConnPair - pub fn wrap(client: C, server: S) -> Self { - assert!( - client.connected_buffer() == &server.connected_buffer().clone_inverse(), - "connected buffers don't match" - ); - Self { client, server } + pub fn from_configs(client_config: &C::Config, server_config: &S::Config) -> Self { + let io = TestPairIO { + server_tx_stream: Rc::pin(Default::default()), + client_tx_stream: Rc::pin(Default::default()), + }; + let client = C::new_from_config(client_config, io.client_view()).unwrap(); + let server = S::new_from_config(server_config, io.server_view()).unwrap(); + Self { client, server, io } } /// Take back ownership of individual connections in the TlsConnPair @@ -325,93 +306,6 @@ where Ok(()) } - - /// Shrink buffers owned by the connections - pub fn shrink_connection_buffers(&mut self) { - self.client.shrink_connection_buffers(); - self.server.shrink_connection_buffers(); - } - - /// Clear and shrink buffers used for IO between the connections - pub fn shrink_connected_buffers(&mut self) { - self.client.shrink_connected_buffer(); - self.server.shrink_connected_buffer(); - } -} - -/// Wrapper of two shared buffers to pass as stream -/// This wrapper `read()`s into one buffer and `write()`s to another -/// `Rc>>` allows sharing of references to the buffers for two connections -#[derive(Clone, Eq)] -pub struct ConnectedBuffer { - recv: Rc>>, - send: Rc>>, -} - -impl PartialEq for ConnectedBuffer { - /// ConnectedBuffers are equal if and only if they point to the same VecDeques - fn eq(&self, other: &ConnectedBuffer) -> bool { - Rc::ptr_eq(&self.recv, &other.recv) && Rc::ptr_eq(&self.send, &other.send) - } -} - -impl ConnectedBuffer { - /// Make a new struct with new internal buffers - pub fn new() -> Self { - let recv = Rc::new(RefCell::new(VecDeque::new())); - let send = Rc::new(RefCell::new(VecDeque::new())); - - // prevent (potentially slow) resizing of buffers for small data transfers, - // like with handshake - recv.borrow_mut().reserve(10000); - send.borrow_mut().reserve(10000); - - Self { recv, send } - } - - /// Makes a new ConnectedBuffer that shares internal buffers but swapped, - /// ex. `write()` writes to the buffer that the inverse `read()`s from - pub fn clone_inverse(&self) -> Self { - Self { - recv: self.send.clone(), - send: self.recv.clone(), - } - } - - /// Clears and shrinks buffers - pub fn shrink(&mut self) { - self.recv.borrow_mut().clear(); - self.recv.borrow_mut().shrink_to_fit(); - self.send.borrow_mut().clear(); - self.send.borrow_mut().shrink_to_fit(); - } -} - -impl Read for ConnectedBuffer { - fn read(&mut self, dest: &mut [u8]) -> Result { - let res = self.recv.borrow_mut().read(dest); - match res { - // rustls expects WouldBlock on read of length 0 - Ok(0) => Err(std::io::Error::new(ErrorKind::WouldBlock, "blocking")), - Ok(len) => Ok(len), - Err(err) => Err(err), - } - } -} - -impl Write for ConnectedBuffer { - fn write(&mut self, src: &[u8]) -> Result { - self.send.borrow_mut().write(src) - } - fn flush(&mut self) -> Result<(), std::io::Error> { - Ok(()) // data already available to destination - } -} - -impl Default for ConnectedBuffer { - fn default() -> Self { - Self::new() - } } #[cfg(test)] diff --git a/bindings/rust/bench/src/lib.rs b/bindings/rust/bench/src/lib.rs index 95d92b42ab4..b7b79bfa228 100644 --- a/bindings/rust/bench/src/lib.rs +++ b/bindings/rust/bench/src/lib.rs @@ -8,8 +8,8 @@ pub mod s2n_tls; pub use crate::{ harness::{ - get_cert_path, CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, Mode, - PemType, SigType, TlsConnPair, TlsConnection, + get_cert_path, CipherSuite, CryptoConfig, HandshakeType, KXGroup, Mode, PemType, SigType, + TlsConnPair, TlsConnection, }, openssl::OpenSslConnection, rustls::RustlsConnection, diff --git a/bindings/rust/bench/src/openssl.rs b/bindings/rust/bench/src/openssl.rs index 24ec9e9e125..86c2538eaed 100644 --- a/bindings/rust/bench/src/openssl.rs +++ b/bindings/rust/bench/src/openssl.rs @@ -4,8 +4,8 @@ use crate::{ get_cert_path, harness::{ - CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, Mode, TlsBenchConfig, - TlsConnection, + CipherSuite, CryptoConfig, HandshakeType, KXGroup, Mode, TlsBenchConfig, TlsConnection, + ViewIO, }, PemType::*, }; @@ -26,8 +26,7 @@ pub struct SessionTicketStorage { } pub struct OpenSslConnection { - connected_buffer: ConnectedBuffer, - connection: SslStream, + connection: SslStream, } impl Drop for OpenSslConnection { @@ -148,10 +147,7 @@ impl TlsConnection for OpenSslConnection { ) } - fn new_from_config( - config: &Self::Config, - connected_buffer: ConnectedBuffer, - ) -> Result> { + fn new_from_config(config: &Self::Config, io: ViewIO) -> Result> { // check if there is a session ticket available // a session ticket will only be available if the Config was created // with session resumption enabled @@ -170,11 +166,8 @@ impl TlsConnection for OpenSslConnection { unsafe { connection.set_session(ticket)? }; } - let connection = SslStream::new(connection, connected_buffer.clone())?; - Ok(Self { - connected_buffer, - connection, - }) + let connection = SslStream::new(connection, io)?; + Ok(Self { connection }) } fn handshake(&mut self) -> Result<(), Box> { @@ -241,20 +234,6 @@ impl TlsConnection for OpenSslConnection { Ok(()) } - /// With OpenSSL's API, not possible after connection initialization: - /// In order to shrink buffers owned by the connection, config has to built - /// with `builder.set_mode(SslMode::RELEASE_BUFFERS);`, which tells the - /// connection to release buffers only when it's idle - fn shrink_connection_buffers(&mut self) {} - - fn shrink_connected_buffer(&mut self) { - self.connected_buffer.shrink(); - } - - fn connected_buffer(&self) -> &ConnectedBuffer { - &self.connected_buffer - } - fn resumed_connection(&self) -> bool { self.connection.ssl().session_reused() } diff --git a/bindings/rust/bench/src/rustls.rs b/bindings/rust/bench/src/rustls.rs index 39da2777da9..91c0a665094 100644 --- a/bindings/rust/bench/src/rustls.rs +++ b/bindings/rust/bench/src/rustls.rs @@ -3,8 +3,8 @@ use crate::{ harness::{ - read_to_bytes, CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, Mode, - TlsBenchConfig, TlsConnection, + read_to_bytes, CipherSuite, CryptoConfig, HandshakeType, KXGroup, Mode, TlsBenchConfig, + TlsConnection, ViewIO, }, PemType::{self, *}, SigType, @@ -32,7 +32,9 @@ use std::{ }; pub struct RustlsConnection { - connected_buffer: ConnectedBuffer, + // the rustls connection has to own the io view, because it is passed as an + // argument to read/write rather than being part of the connection configuration + io: ViewIO, connection: Connection, } @@ -74,7 +76,7 @@ impl RustlsConfig { rustls_pemfile::read_one(&mut BufReader::new(&*read_to_bytes(pem_type, sig_type))) .unwrap(); if let Some(rustls_pemfile::Item::Pkcs8Key(pkcs_8_key)) = key { - return pkcs_8_key.into(); + pkcs_8_key.into() } else { // https://docs.rs/rustls-pemfile/latest/rustls_pemfile/enum.Item.html panic!("unexpected key type: {:?}", key); @@ -168,10 +170,7 @@ impl TlsConnection for RustlsConnection { "rustls".to_string() } - fn new_from_config( - config: &Self::Config, - connected_buffer: ConnectedBuffer, - ) -> Result> { + fn new_from_config(config: &Self::Config, io: ViewIO) -> Result> { let connection = match config { RustlsConfig::Client(config) => Connection::Client(ClientConnection::new( config.clone(), @@ -182,14 +181,11 @@ impl TlsConnection for RustlsConnection { } }; - Ok(Self { - connected_buffer, - connection, - }) + Ok(Self { io, connection }) } fn handshake(&mut self) -> Result<(), Box> { - Self::ignore_block(self.connection.complete_io(&mut self.connected_buffer))?; + Self::ignore_block(self.connection.complete_io(&mut self.io))?; Ok(()) } @@ -220,7 +216,7 @@ impl TlsConnection for RustlsConnection { .writer() .write(&data[write_offset..data.len()])?; self.connection.writer().flush()?; - self.connection.complete_io(&mut self.connected_buffer)?; + self.connection.complete_io(&mut self.io)?; } Ok(()) } @@ -229,7 +225,7 @@ impl TlsConnection for RustlsConnection { let data_len = data.len(); let mut read_offset = 0; while read_offset < data.len() { - self.connection.complete_io(&mut self.connected_buffer)?; + self.connection.complete_io(&mut self.io)?; read_offset += Self::ignore_block( self.connection .reader() @@ -239,18 +235,6 @@ impl TlsConnection for RustlsConnection { Ok(()) } - fn shrink_connection_buffers(&mut self) { - self.connection.set_buffer_limit(Some(1)); - } - - fn shrink_connected_buffer(&mut self) { - self.connected_buffer.shrink(); - } - - fn connected_buffer(&self) -> &ConnectedBuffer { - &self.connected_buffer - } - fn resumed_connection(&self) -> bool { if let rustls::Connection::Server(s) = &self.connection { s.received_resumption_data().is_some() diff --git a/bindings/rust/bench/src/s2n_tls.rs b/bindings/rust/bench/src/s2n_tls.rs index 63fa42d70d4..5a0cdc2d489 100644 --- a/bindings/rust/bench/src/s2n_tls.rs +++ b/bindings/rust/bench/src/s2n_tls.rs @@ -3,8 +3,8 @@ use crate::{ harness::{ - read_to_bytes, CipherSuite, ConnectedBuffer, CryptoConfig, HandshakeType, KXGroup, Mode, - TlsConnection, + read_to_bytes, CipherSuite, CryptoConfig, HandshakeType, KXGroup, LocalDataBuffer, Mode, + TlsConnection, ViewIO, }, PemType::*, }; @@ -19,9 +19,8 @@ use std::{ borrow::BorrowMut, error::Error, ffi::c_void, - io::{ErrorKind, Read, Write}, + io::{Read, Write}, os::raw::c_int, - pin::Pin, sync::{Arc, Mutex}, task::Poll, time::SystemTime, @@ -153,8 +152,6 @@ impl crate::harness::TlsBenchConfig for S2NConfig { } pub struct S2NConnection { - // Pin> is to ensure long-term *mut to IO buffers remains valid - connected_buffer: Pin>, connection: Connection, handshake_completed: bool, } @@ -165,27 +162,34 @@ impl S2NConnection { /// s2n-tls IO is usually used with file descriptors to a TCP socket, but we /// reduce overhead and outside noise with a local buffer for benchmarking unsafe extern "C" fn send_cb(context: *mut c_void, data: *const u8, len: u32) -> c_int { - let context = &mut *(context as *mut ConnectedBuffer); + let context = &*(context as *const LocalDataBuffer); let data = core::slice::from_raw_parts(data, len as _); - context.write(data).unwrap() as _ + let bytes_written = context.borrow_mut().write(data).unwrap(); + bytes_written as c_int } - /// Unsafe callback for custom IO C API + // Note: this callback will be invoked multiple times in the event that + // the byte-slices of the VecDeque are not contiguous (wrap around). unsafe extern "C" fn recv_cb(context: *mut c_void, data: *mut u8, len: u32) -> c_int { - let context = &mut *(context as *mut ConnectedBuffer); + let context = &*(context as *const LocalDataBuffer); let data = core::slice::from_raw_parts_mut(data, len as _); - context.flush().unwrap(); - match context.read(data) { - Err(err) => { - // s2n-tls requires the callback to set errno if blocking happens - if let ErrorKind::WouldBlock = err.kind() { + match context.borrow_mut().read(data) { + Ok(len) => { + if len == 0 { + // returning a length of 0 indicates a channel close (e.g. a + // TCP Close) which would not be correct. To just communicate + // "no more data", we instead set the errno to WouldBlock and + // return -1. errno::set_errno(errno::Errno(libc::EWOULDBLOCK)); -1 } else { - panic!("{err:?}"); + len as c_int } } - Ok(len) => len as _, + Err(err) => { + // VecDeque IO Operations should never fail + panic!("{err:?}"); + } } } @@ -201,16 +205,13 @@ impl TlsConnection for S2NConnection { "s2n-tls".to_string() } - fn new_from_config( - config: &Self::Config, - connected_buffer: ConnectedBuffer, - ) -> Result> { + fn new_from_config(config: &Self::Config, io: ViewIO) -> Result> { let mode = match config.mode { Mode::Client => s2n_tls::enums::Mode::Client, Mode::Server => s2n_tls::enums::Mode::Server, }; - let mut connected_buffer = Box::pin(connected_buffer); + let io = Box::pin(io); let mut connection = Connection::new(mode); connection @@ -220,9 +221,11 @@ impl TlsConnection for S2NConnection { .set_receive_callback(Some(Self::recv_cb))?; unsafe { connection - .set_send_context(&mut *connected_buffer as *mut ConnectedBuffer as *mut c_void)? + .set_send_context( + &io.send_ctx as &LocalDataBuffer as *const LocalDataBuffer as *mut c_void, + )? .set_receive_context( - &mut *connected_buffer as *mut ConnectedBuffer as *mut c_void, + &io.recv_ctx as &LocalDataBuffer as *const LocalDataBuffer as *mut c_void, )?; } @@ -231,7 +234,6 @@ impl TlsConnection for S2NConnection { } Ok(Self { - connected_buffer, connection, handshake_completed: false, }) @@ -293,16 +295,4 @@ impl TlsConnection for S2NConnection { } Ok(()) } - - fn shrink_connection_buffers(&mut self) { - self.connection.release_buffers().unwrap(); - } - - fn shrink_connected_buffer(&mut self) { - self.connected_buffer.shrink(); - } - - fn connected_buffer(&self) -> &ConnectedBuffer { - &self.connected_buffer - } }