Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(s2n-quic-qns): add option to set max mtu #1782

Merged
merged 1 commit into from
Jun 1, 2023
Merged

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented May 31, 2023

Description of changes:

When using the perf server and client it's nice to have a CLI argument to set the max MTU to see how that affects performance. I've also added the ability to swap tokio runtimes via CLI argument as well.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -289,7 +289,7 @@ pub struct Counters {

impl Counters {
pub fn print_header(&self) {
eprintln!(
println!(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing a println instead of a eprintln makes it easier to pipe the output to a tsv file for later use

Comment on lines -35 to -36
S2N_UNSTABLE_CRYPTO_OPT_TX=1 \
S2N_UNSTABLE_CRYPTO_OPT_RX=1 \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably best to be testing using what is in main rather than experimental features

@camshaft camshaft marked this pull request as ready for review May 31, 2023 23:18
@@ -12,7 +12,9 @@ case "$PS" in
../../target/release/s2n-quic-qns \
perf \
server \
--port $SERVER_PORT
--port $SERVER_PORT \
--max-mtu 16000 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this 16000?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's over localhost, which technically supports 64k MTUs. But after playing around with some values it produced the best perf overall.

@camshaft camshaft merged commit d6905e7 into main Jun 1, 2023
@camshaft camshaft deleted the camshaft/qns-perf-mtu branch June 1, 2023 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants