* it's the weekend, this is useful in my head
so let's check in totally broken code.
* more
* wip
* wip
* wip
* try more stuff
* moar
* more wip
* can't send anything when there's nothing to send
This stopped us from going to the next space to coalesce the Initial
with the Handshake packet on the server-side. And probably lots of
other stuff
* wip
* and check for loss probes that need to be sent
* some doc updates
* remove SendableFrames::validation, it was redundant
Turns out this was exactly the same case as
SendableFrames::space_id_only.
* Add a test that checks we send on an available path
* simplify the packet scheduling logic a little
* Simplify, immediate close during handshake is broken though
* wip
* Guess we do need a new field for this
Slightly sad to have to make an explicit exception for this, at least
currently.
* clippy
* may_send_data has been made smarter, don't need this
* wip
* tweak logging
* proptests should maybe log errors?
* drive to idle for a 1000 iterations, 100 is a bit small
* turns out we don't need this and i find it a bit confusing
* remove stuff split off into #494
* Remove changes split off into 495
* typo
* style
* ww
* remove this old thing
* tweak
* needs_loss_probe is a variable above, it's not that bad
* collapse these two cases. it's a hard call though
* well doh, the doc comment says why
* wordsmithing, because 90% of packet scheduling is about that
* put this back, split off into another pr
* fix doc comment
* Typos from code review
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com>
* Do not allocate and move scheduling info to separate function
Two bits of PR review:
- Move the scheduling to a separate function.
- Avoid an allocation, but this has some tradeoffs.
- I now have two loops that look for the PathId by doing `next_path_id
= self.path.keys().find(|i| **i > path_id).copied();`. It might be
possible to fold the MTU discovery in the main poll loop, MTU packets
would get a slightly higher priority but probably not really harmful
overall.
- I now need to do the computation for
`have_validated_status_available_space many more times.
I'm not sure how much the compiler manages to remove all of that. Is
it smart enough to figure out that
`have_validate_status_available_space` won't change between the calls
and does it move it out? Does it make the iteration as fast as the
previous version?
On the other hand, we now have some situations where we don't have to
compute the scheduling information, and no longer need to compute it
for all paths if we don't send on the last path.
What do you think, which version is better (though I also adopted
@matheus23's feedback about splitting it off to a function, but that
doesn't affect this really. It does make the diff a little bit more
though)?
As an aside, in working out of how scheduling should work it was
really helpful to have to extremely explicit as a bunch of data that's
computed up-front. But it's fair that now we know this is how it
should work that we can implement it in the most optimal way.
* Small attempt at making this clearer
* fix docs
* refactor(proto): ordering of REACH_OUT and OBSERVED_ADDR building
The ordering of these frames was a bit too eager.
REACH_OUT is important timing-wise. But it is not more important than
HANDSHAKE_DONE, PING, IMMEDIATE_ACK, ACK, ACK_FREQUENCY and should
anyway not be sent on a path that also needs
PATH_CHALLENGE. PATH_RESPONSE could be the one exception but it is
also small.
If one of those frames do end up in the same packet as REACH_OUT there
will still be place for the REACH_OUT frame. The CRYPTO frame is left
after it, because after the handshake that is only carrying auxiliarry
non-time-sensitive information, and REACH_OUT is also only possible in
the data space kind.
OBSERVED_ADDRESS is definitely not that high priority, it might need
to move back even further.
---------
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com>
* it's the weekend, this is useful in my head
so let's check in totally broken code.
* more
* wip
* wip
* wip
* try more stuff
* moar
* more wip
* can't send anything when there's nothing to send
This stopped us from going to the next space to coalesce the Initial
with the Handshake packet on the server-side. And probably lots of
other stuff
* wip
* and check for loss probes that need to be sent
* some doc updates
* remove SendableFrames::validation, it was redundant
Turns out this was exactly the same case as
SendableFrames::space_id_only.
* Add a test that checks we send on an available path
* simplify the packet scheduling logic a little
* Simplify, immediate close during handshake is broken though
* wip
* Guess we do need a new field for this
Slightly sad to have to make an explicit exception for this, at least
currently.
* clippy
* may_send_data has been made smarter, don't need this
* wip
* tweak logging
* proptests should maybe log errors?
* drive to idle for a 1000 iterations, 100 is a bit small
* turns out we don't need this and i find it a bit confusing
* remove stuff split off into #494
* Remove changes split off into 495
* typo
* style
* ww
* remove this old thing
* tweak
* needs_loss_probe is a variable above, it's not that bad
* collapse these two cases. it's a hard call though
* well doh, the doc comment says why
* wordsmithing, because 90% of packet scheduling is about that
* put this back, split off into another pr
* fix doc comment
* Typos from code review
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com>
* Do not allocate and move scheduling info to separate function
Two bits of PR review:
- Move the scheduling to a separate function.
- Avoid an allocation, but this has some tradeoffs.
- I now have two loops that look for the PathId by doing `next_path_id
= self.path.keys().find(|i| **i > path_id).copied();`. It might be
possible to fold the MTU discovery in the main poll loop, MTU packets
would get a slightly higher priority but probably not really harmful
overall.
- I now need to do the computation for
`have_validated_status_available_space many more times.
I'm not sure how much the compiler manages to remove all of that. Is
it smart enough to figure out that
`have_validate_status_available_space` won't change between the calls
and does it move it out? Does it make the iteration as fast as the
previous version?
On the other hand, we now have some situations where we don't have to
compute the scheduling information, and no longer need to compute it
for all paths if we don't send on the last path.
What do you think, which version is better (though I also adopted
@matheus23's feedback about splitting it off to a function, but that
doesn't affect this really. It does make the diff a little bit more
though)?
As an aside, in working out of how scheduling should work it was
really helpful to have to extremely explicit as a bunch of data that's
computed up-front. But it's fair that now we know this is how it
should work that we can implement it in the most optimal way.
* Small attempt at making this clearer
* fix docs
* Rename field based on more feedback
* fix docs
---------
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com>
* Allow configuring the runtime type in the bulk bench
Also switch the default to the multithreaded runtime. I think that is more
representative of actual use.
* fmt
* clippy
---------
Co-authored-by: Floris Bruynooghe <flub@n0.computer>
* feat(proto): issue CIDs in order of ascending path ID
We used to issue CIDs in sequence order, but not in reverse order of
path ID. This is not the order in which you need to have CIDs. This
fixes this to always issue them in order of ascending path ID and then
ascending order of sequence ID.
By introducing the newtype to do this, we also ensure that this order
is respected even when retransmits come into play. The newtype keeps
its sorting invariance when retransmits are merged back in.
Another benefit of the newtype is that this order is now enforced in a
specific place. Before it was implicit on the reliance between how the
endpoint ID generated the CIDs, how it then sent them to the
connection and how the connection stored and consumed them. It was
very implicit.
WRT to the cost of doing all the ordered inserts: CIDs are issued
relatively infrequently and usually not in huge numbers. Even
considering we want to increase those numbers in the future I think
using a simple Vec as storage is a decent choice for numbers of a few
100 CIDs that can be expected at most.
Fixes#137.
* fix aws-lc-rs tests, hopefully
* lol
* fix test
* Use cmp::Reverse instead of manual cmp
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com>
* Sort less for the sorting gods
This optimises how we sort:
- Sort is still not manually implemented, as tempting it is to
implement insertion sort. I'm a believer of not manually
implementing algorithms.
- The major downside is that all functions need to be aware of the
invariants. Everyone needs to manipulate the fields.
---------
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com>
* ci(docs): Check internal docs as well
We also want to check that the internal docs are all correct, so we do
not get broken links etc. We have a lot of internal docs, internal
docs are great!
* turns out that syntax is not supported
* fixup all the doc errors
* fix format
* naming nitpicking, fewer changes
We also have a `connection::send_buffer:proptests` module, which did
not match the nextest naming for the proptest profile. So those tests
were being run in a normal test run.
Because switching that module name to be "proptest" is problematic
since the crate is also called "proptest" and that creates naming
conflicts with the `user super::*` I opted to change the
tests/proptest module to become proptests instead.
* ci: Add ESP32-C3 build check for noq, noq-proto, and noq-udp
Cross-compile check using riscv32imc-esp-espidf target with
nightly + build-std. No ESP-IDF SDK needed for cargo check.
* test: Deliberately break ESP32 build to verify CI catches it
Use unix.rs instead of posix_minimal.rs on espidf to confirm the
esp32_check CI job fails as expected. Revert after verifying.
* Revert "test: Deliberately break ESP32 build to verify CI catches it"
This reverts commit 45f096edc8.
* Correct ci step name
* Use sccache for new ci checks.
* Rename fallback.rs to posix_minimal.rs
Use it for platforms that are unix, but don't support advanced stuff like
CMSG, GRO, GSO.
Also fix some compile errors in the former fallback.rs
* Test noq-udp with the new posix_minimal configuration
* Eliminate warnings for posix_minimal config.
* shut up clippy
* chore: Attempt at drafting a readme
* spelling
* Fix crypto wording
* typo
* remove support for version 2
seems that needs a little more work
* add modern stuff folks demand
* qlog typo
* Try and be slightly less confusing
* WIP
* Prettify parsing code
* Fix error in docs
* clippy
* Some optimizations:
- use a lazy iterator over the ALPNs
- fast path for when the ALPNs are in the first crypto frame
Also some tests for the rarely used assembly stuff.
* Fix doc comments regarding allocations
* Add a newtype wrapper for the decrypted initial packet.
We can then add all the various convenience methods to it, not to the main Incoming.
So even if we have a lot of stuff on it we don't pollute the incoming API.
Also we cache the decryption.
* Extend comments about decrypt
* Switch to `aes-gcm` crate for retry token logic when using rustls
* Split `rustls-ring` and `rustls-aws-lc-rs` features into `rustls`, `ring` and `aws-lc-rs` features.
* Better document the features.
* `cargo make format`
* Fix outdated feature references
* Avoid double-defining `configured_provider`
* File an issue about suddenly working PQC handshakes now breaking a test
* Fix rebase
* Only enable `aes-gcm` dependency when needed
* Add features for backwards compatibility
* Add some links to the spec's retry packet integrity section
Drop the connection state lock before cloning ConnectionRef in the existing-path branch of open_path_ensure, and add a multipath regression test that verifies ensuring an existing path resolves quickly and returns consistent path details.
At some point in the past this probably worked around some borrowing
issues. Now it is distracting (just kidding, it was always
distracting but sadly necessary).
Some refactors:
- Move code around so that helpers structs come after the Connection,
kind of more the style and keeps connection code more together.
- Use some defensive coding in the spaces impls. Fix a few bugs
detected by it
* fix path challenge issues
* the tests
* keep two set of challenges
* fmt
* remove proptest, there's no need
* tone down descriptions
* passive migration sets ip
* adjust test
* spelling
* take into account MSRV
* add more info to the ignored variant
* allow the case when the challenge was sent without knowing our src ip
Frando