add initial RTT configurability #2041
Conversation
Failed Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
neqo-bin/src/lib.rs
Outdated
| @@ -111,6 +111,10 @@ pub struct QuicParameters { | |||
| /// The idle timeout for connections, in seconds. | |||
| pub idle_timeout: u64, | |||
|
|
|||
| #[arg(long = "init_rtt", default_value = "100")] | |||
There was a problem hiding this comment.
| #[arg(long = "init_rtt", default_value = "100")] | |
| #[arg(long = "init_rtt", default_value = DEFAULT_INITIAL_RTT)] |
There was a problem hiding this comment.
Can the macro take a Duration like that? I thought that it had to be a string. The other way to spell this is to use Option<u64> and use map_or to get the default.
There was a problem hiding this comment.
Yes, not sure that it can take a Duration. Was trying to suggest to avoid violating POLA and somehow use the same default here that the code is using.
There was a problem hiding this comment.
I'd use Option and move the application of the default to the getter. That works, even if it means that the --help doesn't report the default.
neqo-transport/src/rtt.rs
Outdated
| pub const INITIAL_RTT: Duration = Duration::from_millis(100); | ||
| pub const DEFAULT_INITIAL_RTT: Duration = Duration::from_millis(100); |
There was a problem hiding this comment.
I don't think you have to rename this.
robamu
force-pushed
the
initial-rtt-configurability
branch
from
ee6f44c
to
5445216
Compare
|
@robamu it would be helpful it you could compile the PR and run the test suite locally, just to make sure that CI doesn't immediately fail. |
|
Ah, I can do that. |
|
I have problems running the tests currently.. neqo on initial-rtt-configurability is 📦 v0.8.2 via 🦀 v1.80.1
❯ cargo test
Compiling neqo-bin v0.8.2 (/home/rmueller/Rust/neqo/neqo-bin)
Finished `test` profile [unoptimized + debuginfo] target(s) in 2.57s
Running unittests src/lib.rs (target/debug/deps/neqo_bin-33111872ccada738)
running 1 test
0s 0ms ERROR Minimum NSS version of 3.103
not supported
test tests::write_qlog_file ... FAILED
failures:
---- tests::write_qlog_file stdout ----
thread 'tests::write_qlog_file' panicked at neqo-bin/src/lib.rs:309:80:
called `Result::unwrap()` on an `Err` value: UnsupportedVersion
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
tests::write_qlog_file
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p neqo-bin --lib`I can check and build the project though and I have installed NSS v3.103 according to https://www.linuxfromscratch.org/blfs/view/svn/postlfs/nss.html .. is there something special I need to take care of when installing NSS from the sources for the neqo tests? I can not use the Ubuntu package provides NSS tools/libraries because the version is too old. |
|
Are you sure you are linking against the LFS version of NSS you installed and not some other version? Check with I suggest you follow what's in the readme, i.e., check out the sources and set the environment variables so that neqo will do the right thing. |
|
Running ldd yields the following: neqo on main [$] is 📦 v0.8.2 via 🦀 v1.80.1
❯ ldd /usr/lib/libnss3.so
/usr/lib/libnss3.so: /lib/x86_64-linux-gnu/libnssutil3.so: version `NSSUTIL_3.101' not found (required by /usr/lib/libnss3.so)
linux-vdso.so.1 (0x00007ffce9f5d000)
libnssutil3.so => /lib/x86_64-linux-gnu/libnssutil3.so (0x000072ffb3e2f000)
libplc4.so => /lib/x86_64-linux-gnu/libplc4.so (0x000072ffb3e28000)
libplds4.so => /lib/x86_64-linux-gnu/libplds4.so (0x000072ffb3e23000)
libnspr4.so => /lib/x86_64-linux-gnu/libnspr4.so (0x000072ffb3de3000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x000072ffb3a00000)
/lib64/ld-linux-x86-64.so.2 (0x000072ffb3fbd000)
so.. some problem with libnssutil3 oder some other missing library? I also tried following the readme by setting Compiling neqo-crypto v0.8.2 (/home/rmueller/Rust/neqo/neqo-crypto)
error: failed to run custom build command for `neqo-crypto v0.8.2 (/home/rmueller/Rust/neqo/neqo-crypto)`
Caused by:
process didn't exit successfully: `/home/rmueller/Rust/neqo/target/debug/build/neqo-crypto-5811bbeef5d53e68/build-script-build` (exit status: 101)
--- stdout
cargo:rustc-check-cfg=cfg(nss_nodb)
cargo:rerun-if-env-changed=NSS_DIR
NSPR [1/5] configure ...
--- stderr
./build.sh: line 36: ../configure: No such file or directory
thread 'main' panicked at neqo-crypto/build.rs:135:5:
NSS build failed
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...I have gyp and ninja installed, so I am not sure what is going on here |
...
This looks like at least some of your installation is NSS 3.101 and not 3.103, which would explain the error when you try and run it. If you want to use the LFS-installed NSS, you need to make sure it's installed correctly in the system paths, and that no other (older) versions are lingering.
...
This looks like the environment is not set up correctly. You need to check out NSS and NSPR into a directory hierarchy like this: Your environment then should be: export NSS_DIR=/your/absolute/path/nss/nss
export NSS_TARGET=Release
export LD_LIBRARY_PATH $NSS_DIR/../dist/$NSS_TARGET/lib |
|
I have managed to build and run the tests now with --- STDOUT: neqo-common hrtime::test::update_multi ---
running 1 test
sleep(1ms) → 1.061399ms Δ61.399µs
sleep(1ms) → 1.058584ms Δ58.584µs
sleep(2ms) → 2.048389ms Δ48.389µs
sleep(2ms) → 2.066653ms Δ66.653µs
sleep(3ms) → 3.074642ms Δ74.642µs
sleep(3ms) → 3.073069ms Δ73.069µs
sleep(5ms) → 5.066455ms Δ66.455µs
sleep(5ms) → 5.072656ms Δ72.656µs
sleep(8ms) → 8.064904ms Δ64.904µs
sleep(8ms) → 8.067559ms Δ67.559µs
sleep(10ms) → 10.076534ms Δ76.534µs
sleep(10ms) → 10.071084ms Δ71.084µs
sleep(12ms) → 12.064259ms Δ64.259µs
sleep(12ms) → 12.063176ms Δ63.176µs
sleep(15ms) → 15.066245ms Δ66.245µs
sleep(15ms) → 15.062899ms Δ62.899µs
sleep(20ms) → 20.067497ms Δ67.497µs
sleep(20ms) → 20.06357ms Δ63.57µs
sleep(25ms) → 25.062007ms Δ62.007µs
sleep(25ms) → 25.068479ms Δ68.479µs
sleep(30ms) → 30.326814ms Δ326.814µs
sleep(1ms) → 1.077138ms Δ77.138µs
sleep(30ms) → 31.640385ms Δ1.640385ms
sleep(1ms) → 1.551488ms Δ551.488µs
sleep(2ms) → 2.430786ms Δ430.786µs
sleep(2ms) → 3.138001ms Δ1.138001ms
sleep(3ms) → 3.478609ms Δ478.609µs
sleep(3ms) → 5.995656ms Δ2.995656ms
sleep(5ms) → 5.087443ms Δ87.443µs
sleep(8ms) → 8.070364ms Δ70.364µs
sleep(10ms) → 10.086934ms Δ86.934µs
sleep(12ms) → 12.115404ms Δ115.404µs
sleep(15ms) → 15.082845ms Δ82.845µs
sleep(1ms) → 1.072259ms Δ72.259µs
sleep(2ms) → 2.108822ms Δ108.822µs
sleep(3ms) → 3.101262ms Δ101.262µs
sleep(5ms) → 6.947169ms Δ1.947169ms
sleep(20ms) → 20.086693ms Δ86.693µs
sleep(25ms) → 25.072748ms Δ72.748µs
sleep(30ms) → 30.073519ms Δ73.519µs
test hrtime::test::update_multi ... FAILED
failures:
failures:
hrtime::test::update_multi
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 64 filtered out; finished in 0.27s
--- STDERR: neqo-common hrtime::test::update_multi ---
thread '<unnamed>' panicked at neqo-common/src/hrtime.rs:416:38:
called `Result::unwrap()` on an `Err` value: ()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'hrtime::test::update_multi' panicked at neqo-common/src/hrtime.rs:475:20:
called `Result::unwrap()` on an `Err` value: Any { .. }I see that there is a client & server test and the interoperability tests, should I try to make those work locally as well? |
|
Are you running this in a VM or cloud instance? Because those have bad timer accuracy. IIRC there is either an environment variable or rust feature you can use to skip those timer tests. |
|
Also, if |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2041 +/- ##
==========================================
- Coverage 95.36% 95.32% -0.04%
==========================================
Files 112 112
Lines 36505 36279 -226
==========================================
- Hits 34813 34584 -229
- Misses 1692 1695 +3 ☔ View full report in Codecov by Sentry. |
|
I actually ran this on my (decent) machine.. I can try to have another look to continue this on the weekend.. Maybe some more unittests would be good as well? |
|
Yes, in order for us to land this, CI needs to pass and test coverage needs to be there. (And we need some review approvals.) |
This adds configurability for the initial round-trip time, which has a use-case for something like space-applications where a higher round-trip time might be expected. This is a first draft PR, an end-to-end test with other QUIC stacks still needs to be done.