Fix ACK_MP preventing bundling a ping when eliciting an ACK #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For now only the "main" quiche CI job tests both features to avoid adding too many new jobs, in the future we might decide to change this. Ref cloudflare#1614.
In some cases we are turning Vec values into slices to pass them to functions that will then turn the slice back into a Vec. Instead just pass `Vec`s around to avoid some unnecessary allocations.
An event was removed in 48aac48 but the enum wasn't renumbered, so the Rust values didn't match the C ones anymore. Fixes cloudflare#1650
The system key is not mandatory, and add BoringSSL to dependencies while at it.
Motivation: socklen_t should be used for passing the length of sockaddr. This was already done in the ffi code but not in the c header file Modifications: Use socklen_t Result: Correct type in c header file
Motivation: ConnectionIdIter is not used, let's just remove it. Modifications: Remove unused code Result: Cleanup
Motivation: Some of the methods used *source_cid* and *destination_cid* in the name while most others uses *scid* and *dcid*. Naming of methods should be consistent to make it easier to discover things. Modifications: Rename methods for consistent naming Result: Cleanup of public API
…ration Motivation: We need to expose some methods via the C api to be able to over the remote peer connection ids that it can be used for migration Modifications: Add missing ffi Result: Be able to handle connection migrations
We would like to modify extension data in an SslRef to support async callbacks. To do so we need access to a Connection's `SslRef`.
Motivation: At the moment its impossible to call retire_dcid(...) via the C API. Modifications: Add quiche_conn_retire_dcid to header file and also add the implementation to ffi. Result: Be able to retire destination connection id
Motivation: At the moment its impossible to get all source ids via the C api. Modifications: Add C / ffi impl to get all source ids Result: Fix gap between rust and C api
Motivation: There was no C api exposed to allow handling PathEvent. Modifications: Expose c functions to handle PathEvent Result: Be able to handle and consume PathEvent
Motivation: The implementation of quiche_conn_new_scid(...) made it impossible to know if the call was a success or not as even on a sucessfull call it might return a negative value. Modifications: Change signature to make it possible to detect if success or not. Result: Fix implementation
Motivation: Quiche provides mutliple methods that allows to send packets on a specific path. These were not exposed in the C api. Modifications: - Add missing functions to the c API Result: Be able to send packets on a specific path
Motivation: Quiche didnt expose all methods via c functions that are required to start connection migration. Modifications: Add missing ffi implementatins and add functions to c header file. Result: Be able to do connection migration on the client side.
Motivation: Connection.is_resumed() is not exposed via FFI / C. Modifications: Add quiche_conn_is_resumed Result: Missing API added
…_conn_is_dgram_recv_queue_full(...) methods Motivation: quiche_conn_is_dgram_send_queue_full(...) and quiche_conn_is_dgram_recv_queue_full(...) is missing in the c API Modifications: Add functions to C header / add FFI impl Result: Missing functions added
Updates the requirements on [ring](https://github.com/briansmith/ring) to permit the latest version. - [Commits](https://github.com/briansmith/ring/commits) --- updated-dependencies: - dependency-name: ring dependency-type: direct:production ... Fixes cloudflare#1678 Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Motivation: set_disable_dcid_reuse(...) and quiche_config_set_ticket_key(...) are not exposed via the c API. Modifications: Add FFI implementation and add functions to c header file. Result: Add missing config methods
Looks like a new warning in nightly.
There were several places where off_front() would be called in rapid succession. off_front() potentially has to iterate a large queue to find the "front" value. In rapid succession, that value is identical, leading to wasted work. This change removes the ready() function (which called off_front) and refactors code to minimise off _front() calls.
RFC 9000 has some low-level requirements for who is allowed to send a NEW_TOKEN frame, and what the token value can be. This change adds stricter NEW_TOKEN frame checks. It leaves any more advanced handling of the Retry mechanism as continuing TODO.
Follow-up to 1684118. This error was added in the previous commit, but we can use a more specific error instead, also to be more consistent with existing code (e.g. see HANDSHAKE_DONE case).
- packet number space map - spaced packet number - `PathEvent::Closed` now includes error code and reason - function refactoring in lib.rs and frame.rs
This commit introduces the following features.
- Leave to the application to choose the multipath extensions or not
- Build on the `PathEvent`s to let the application decide which paths to use
- Provide default reasonable behavior into `quiche` while letting the
application provide optimised behavior
== Requesting the Multipath feature
The application can request multipath through the `Config` structure using a
specific API,`set_multipath()`.
```rust
config.set_multipath(true);
```
The boolean value determines whether the underlying connection negotiates the
multipath extension. Once the handshake succeeded, the application can check
whether the connection has the multipath feature enabled using the
`is_multipath_enabled()` API on the `Connection` structure.
== Path management
The API requires the application to specify on which paths/4-tuples it wants
to send non-probing packets. Paths must first be validated before using them.
This is automatically done for servers, and client must use `probe_path()`.
Once the path is validated, the application decides whether it wants active
usage through the `set_active()` API. It provides a single API entrypoint to
request its usage or not. For active path usage, it can use the following.
```rust
if let Some(PathEvent::Validated(local, peer)) = conn.path_event_next() {
conn.set_active(local, peer, true).unwrap();
}
```
Then, the path will then be considered to use non-probing packets.
On the other hand, if for some reason the application wants to temporarily
stop sending non-probing frames on a given path, it can do the following.
```rust
conn.set_active(local, peer, false).unwrap();
```
Note that in such state, quiche still replies to PATH_CHALLENGEs observed on
that path.
Finally, the multipath design allows a QUIC endpoint to close/abandon a
given path along with an error code and error message, without altering the
connection's operations as long as another path is available.
```rust
conn.abandon_path(local, peer, 42, "Some error message".into()).unwrap();
```
-- Retrocompatibility note
- The `Closed` variant of `PathEvent` is now a 4-tuple that, in addition to
the local and peer `SocketAddr`, also contains an `u64` error code and a
`Vec<u8>` reason message.
- There is a new variant of `PathEvent`: `PeerPathStatus` reports to the
application that the peer advertised some status for a given 4-tuple.
- There are two new `Error` variants: `UnavailablePath` and
`MultiPathViolation`.
-- Note
Currently this API is only available when multipath feature is enabled over
the session (i.e., `conn.is_multipath_enabled()` returns `true`). If the
extension is not enabled, `set_active()` and `abandon_path()` return an
`Error::InvalidState`. Actually, this API might sound "double usage" along
with the `migrate()` API (as there is no real "connection migration" with
multipath). Should we just keep the `set_active()` or similarly named API
and include the `migrate()` functionality in `set_active()`? Actually, an
client application without the multipath feature could just migrate using
`set_active(local, peer, true)`, setting the previous path in unused mode
under the hood.
== Scheduling sent packets
Similarly to the connection migration PR, there are two ways to control how
`quiche` schedules packets on paths.
The first consists in letting `quiche` handles this by itself. The
application simply uses the `send()` method. In the current master code,
`quiche` automatically handles path validation processing thanks to the
internal `get_send_path_id()` `Connection` method. The multipath patch
extends this method and iterates over all active paths following the lowest
latency path having its congestion window open heuristic (a reasonable
default in multipath protocols).
```rust
loop {
let (write, send_info) = match conn.send(&mut out) {
Ok(v) => v,
Err(quiche::Error::Done) => break,
Err(e) => {
conn.close(false, 0x1, b"fail").ok();
break;
},
};
// write to socket bound to `send_info.from`
}
```
The second option is to let the application choose on which path it wants
to send the next packet. The application can iterate over the available paths
and their corresponding statistics using `path_stats()` and schedules packets
using `send_on_path()` method. This can be useful when the use case requires
some specific scheduling strategies. See `apps/src/client.rs` for an example
of such application-driven scheduling.
And fix clippy
Credits to @toshiiw for finding the issue.
Through the addition of the `find_scid_seq()` method on `Connection`.
That was not doing much.
…should be elicited
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.