refactor(s2n-quic-platform): use socket ring for tokio #1792
Conversation
camshaft
force-pushed
the
camshaft/io-tokio-upgrade
branch
3 times, most recently
from
2e06837 to
cffd965
Compare
camshaft
force-pushed
the
camshaft/io-tokio-upgrade
branch
from
cffd965 to
5fd3f24
Compare
camshaft
force-pushed
the
camshaft/io-tokio-upgrade
branch
2 times, most recently
from
7f1a3cd to
3b2e56b
Compare
camshaft
force-pushed
the
camshaft/io-tokio-upgrade
branch
from
9f7241d to
cd946a7
Compare
| } | ||
|
|
||
| #[inline] | ||
| unsafe fn set_payload_len(&mut self, len: usize) { | ||
| debug_assert!(len <= core::u32::MAX as usize); | ||
| debug_assert!(len <= core::u16::MAX as usize); |
There was a problem hiding this comment.
why are we going from u32 to u16. was the previous check not correct?
There was a problem hiding this comment.
the msg_len is a u32. but in general we don't allow payloads to be bigger that u16
quic/s2n-quic-platform/Cargo.toml
Outdated
| @@ -17,25 +17,25 @@ testing = ["std", "generator", "futures/std", "io-testing"] # Testing allows to | |||
| io-testing = ["bach"] | |||
| generator = ["bolero-generator", "s2n-quic-core/generator"] | |||
| tokio-runtime = ["futures", "tokio"] | |||
| xdp = ["s2n-quic-xdp"] | |||
| xdp = ["dep:s2n-quic-xdp"] | |||
There was a problem hiding this comment.
https://doc.rust-lang.org/cargo/reference/features.html#optional-dependencies - basically makes it explicit that it's enabling a dependency, not a feature. but i'm going to remove this for now and do a separate PR to fix the --all-feature stuff in CI
| @@ -90,6 +73,7 @@ impl Io { | |||
| }, | |||
| }); | |||
|
|
|||
| // try to use the tokio runtime handle if provided | |||
There was a problem hiding this comment.
can you also add a comment on the else case. is that case used when s2n-quic runs inside another program which already started the tokio runtime.
| @@ -8,13 +8,25 @@ use core::ops::ControlFlow; | |||
| use socket2::{Domain, Protocol, Socket, Type}; | |||
| use std::io; | |||
|
|
|||
| /// Calls the given libc function and wraps the result in an `io::Result`. | |||
| macro_rules! libc { | |||
There was a problem hiding this comment.
doesnt have to be in this PR but all setsockopt calls can use a comment explaining what its doing. Otherwise reader has to lookup the call and or be familiar with it.
camshaft
force-pushed
the
camshaft/io-tokio-upgrade
branch
from
db0dc35 to
0d69d19
Compare
Description of changes:
This change finally wires up the new "socket v2" async ring buffers to the tokio IO provider. It also deletes all of the "socket v1" code.
Testing:
Since this is just a refactor, all of the existing tests should pass, including the QNS tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.