* chore: Guard testing functions properly
We have a bunch of functions that in their doc comments say that they are
for testing purposes only. This commit adds guards to those functions
to make sure they are not used in production code.
* More
* feat: Mix up the Initial crypto data a bit
Look for ranges of `N` or more bytes of graphical ASCII data in `data`.
Create at least one split point for each range, multiple ones each `N`
bytes if the range is long enough. Create data chunks based on those
split points. Shuffle the chunks and return them.
CC @martinthomson @dennisjackson
* Fix tests
* Fixes
* Doc fixes
* More tests and corner-case fixes
* Footgun prevention
* WIP; suggestions from @martinthomson
* Address more code review comments
* Refactor loop
* More from @martinthomson
* Again
* Create roughly five chunks per packet
* Latest suggestion from @martinthomson
* Simpler version from @martinthomson
* Fix
* Update neqo-transport/src/crypto.rs
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>
* Use `Decoder`
* Minimize diff
* Fix comment
* Update neqo-transport/src/crypto.rs
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>
* Final suggestions
---------
Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
* bench: add SentPackets::take_ranges benchmark
* perf(transport/recovery): optimize SentPackets::take_range
`SentPackets` keep track of all inflight packets. On receiving an ACK, the acked
packet ranges are removed via `SentPackets:take_ranges`.
In normal network scenarios one can assume that the amount of packets before a
range is small (e.g. due to reordering) or zero, while the amount of packets
after a range is large, i.e. all remaining in-flight packets.
The previous implementation assumed the opposite, leading to an overhead linear
to the amount of packets after the range. This commit changes the algorithm such
that it is linear to the amount of packets before the range, which again, is
assumed to be smaller than the amount of packets after the acked range.
* Update neqo-transport/benches/sent_packets.rs
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Max Inden <mail@max-inden.de>
* Update neqo-transport/src/recovery/sent.rs
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Max Inden <mail@max-inden.de>
* debug_assert for descending ack ranges
* single debug_assert line
* Document large `packets` in front of range scenario
Co-authored-by: Martin Thomson <mt@lowentropy.net>
* Trigger benchmarks
---------
Signed-off-by: Max Inden <mail@max-inden.de>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
* bench: add SentPackets::take_ranges benchmark
* Fix clippy lints for pub mod packets, recovery and sent with feat bench
In order to benchmark `SentPackets::take_ranges`, we need to make `packets`,
`recovery` and `sent` public modules, feature flagged with `bench`.
Public modules have stricter clippy lints.
This commit addresses the failing clippy lints.
* Trigger benchmark
* Update neqo-transport/src/recovery/mod.rs
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Max Inden <mail@max-inden.de>
* Remove useless is_empty
---------
Signed-off-by: Max Inden <mail@max-inden.de>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
* chore: check-in Cargo.lock
This commit checks the `Cargo.lock` file into git.
Version controlling `Cargo.lock` makes e.g. our CI builds more reproducible,
where two consecutive CI runs on the same commit use the same set of
dependencies, even if a compatible update of a dependency was published in
between the two runs.
This is also helpful when cutting patch releases of old Neqo versions, where
dependencies since shipped a breaking change in a patch version, e.g. a MSRV
update. See for example pinned dependencies in a recent Neqo patch release to
the Neqo v0.6 family.
66e60f360e
While previously the recommendation by the cargo team was for libraries to not
check in their `Cargo.lock`, this recommendation has since been replaced by "do
what is best for the project".
https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html
* Update url to v2.5.3
See corresponding mozilla-central patch https://bugzilla.mozilla.org/show_bug.cgi?id=1932137.
* Update zerovec-derive to v0.10.3
* Update shlex to v1.3.0
* Update to url v2.5.4
mozilla-central depends on `url` `v2.5.1`:
https://searchfox.org/mozilla-central/rev/6050bf4eca89956c9d91bfd89fa59294ae32a689/Cargo.lock#6715-6725
The latest version of `url` is `v2.5.3`, which cargo automatically updates to,
given that `Cargo.lock` is not checked in.
`url` v0.2.5.3` introduced the `std` feature which breaks CI.
https://github.com/mozilla/neqo/pull/2219 enabled the feature, unbreaking CI,
but breaking mozilla-central, given that mozilla-central still uses `url`
`v2.5.1` which does not have the the `std` feature.
This commit pins `url` to `v2.5.1` instead and removes the `std` feature.
* test: More migration testing
This expands the migration test quite a bit. It now tests alll the way
until retirement of the original path, for both port-only
(`rebinding_port`) and address-and-port (`rebinding_address_and_port`)
changes.
`rebinding_address_and_port` is succeeding, but `rebinding_port` is
currently failing. That's because we treat it differently for some
reason. If we replace `Paths::find_path_with_rebinding` with
`Paths::find_path`, i.e., do proper path validation when only the port
changes, the test succeeds.
Leaving this out in case I'm missing something about the intent of the
difference.
* fix: Replace `find_path_with_rebinding` with `find_path`
We need to do a path challenge even if only the remote port changes.
* Remove unused arg from `received_on`
* Fixes
* identity
* More review comments
* Simplify
* Fix merge
* Address review comments
* Migrate to preferred address
* Fix URL splitting
* Restore `find_path_with_rebinding`
* Fix test for not doine path challenge if only port rebinds
* Revert "Fix test for not doine path challenge if only port rebinds"
This reverts commit 97fafa6d2e.
* Revert "Restore `find_path_with_rebinding`"
This reverts commit 302961f361.
* Remove now-unused test names
* fmt
---------
Signed-off-by: Lars Eggert <lars@eggert.org>
* ci: Benchmark against google-quiche
* Client
* Enable sccache for bazel
* -c opt
* =
* =
* Again
* Again
* Again
* Again
* Again
* Again
* Again
* true
* true
* Again
* Again
* Again
* Again
* ls
* Again
* -N
* true
* Again
* pid
* Parallel
* ps
* -ge
* Minimize
* Less echo
* No 65536 MTU
* ja4 workflow
* Revert "ja4 workflow"
This reverts commit 648a380933.
* refactor(transport): reuse `now` in qlog whereever available
Instead of using `QLogStream::add_event_data_now`, which internally calls
`std::time::Instant::now()`, pass `now to
`QLogStream::add_event_data_with_instant`.
* Move regex to workspace dep
* Don't prefix now, before and after with time_
* Document preference for _with_time
---------
Co-authored-by: Lars Eggert <lars@eggert.org>
* feat(bin): don't allocate in server UDP recv path
Previously the `neqo-bin` server would read a set of
datagrams from the socket and allocate them:
``` rust
let dgrams: Vec<Datagram> = dgrams.map(|d| d.to_owned()).collect();
```
This was done out of convenience, as handling `Datagram<&[u8]>`s, each borrowing
from `self.recv_buf`, is hard to get right across multiple `&mut self`
functions, that is here `self.run`, `self.process` and `self.find_socket`.
This commit combines `self.process` and `self.find_socket` and passes a socket
index, instead of the read `Datagram`s from `self.run` to `self.process`, thus
making the Rust borrow checker happy to handle borrowing `Datagram<&[u8]>`s
instead of owning `Datagram`s.
* next().or_else()
* re-introduce find_socket
* Simplify process loops
* Make find_socket and read_and_process associated functions
* Reduce diff
* feat: No zero-len CIDs for client
They break the upcoming QNS migration tests.
* Add command line parameter
* Fix message
* Don't enable for migration tests - not needed
* feat: Enable `SSL_ENABLE_CH_EXTENSION_PERMUTATION`
This enables the NSS `SSL_ENABLE_CH_EXTENSION_PERMUTATION` option
by default.
CC @martinthomson @dennisjackson
* fmt
Currently we use `quinn-udp` `v0.5.4`.
`quinn-udp` `v0.5.5` fixes [`recvmmsg` calls on Android x86](https://github.com/quinn-rs/quinn/pull/1966).
`quinn-udp` `v0.5.6` adds [experimental multi-message support on Apple
platforms](https://github.com/quinn-rs/quinn/pull/1993) and [fixes an
unnecessary `windows-sys` version
restriction](https://github.com/quinn-rs/quinn/pull/2021).
While not strictly necessary, given that our current version specification (i.e.
`version = "0.5.4"`) already allows users to use Neqo with `quinn-udp` `v0.5.6`,
this commit updates to `quinn-udp` `v0.5.6` anyways, thus making sure CI tests
with latest version.
In case https://github.com/mozilla/neqo/pull/2208 lands, future compatible
version updates would touch the `Cargo.lock` file, not `Cargo.toml`.
Co-authored-by: Lars Eggert <lars@eggert.org>
* bench(bin/client): don't allocate upload payload upfront
When POSTing a large request to a server, don't allocate the entire request
upfront, but instead, as is done in `neqo-bin/src/server/mod.rs`, iterate over a
static buffer.
Reuses the same logic from `neqo-bin/src/server/mod.rs`, i.e. `SendData`.
See previous similar change on server side https://github.com/mozilla/neqo/pull/2008.
* Inline done()
Previously `neqo-bin`, `neqo-transport` and `test-fixtures` would each define a
`bench` feature. But enabling e.g. `bench` in `neqo-transport` would not enable
`bench` in `test-fixtures`. Thus a benchmark in e.g. `neqo-transport` with
`--features bench` would still be serializing qlog traces.
This commit makes `neqo-bin` and `neqo-transport` forward their `bench` feature
to `test-fixtures`.
Lars Eggert
Martin Thomson
dependabot[bot]
Max Inden
Kershaw