Tokio v1 #14
Tokio v1 #14
Conversation
| libc = { version = "~0.2", optional = true } | ||
| mio = { version = "~0.6", optional = true } | ||
| tokio-core = { version = "~0.1", optional = true } | ||
| tokio = {version = "~1.29", features = ["io-util", "net"], optional = true } |
There was a problem hiding this comment.
Is that 1.29 minimal version needed? What's the feature that is not in the lower ones?
| @@ -0,0 +1,33 @@ | |||
| // For format details, see https://aka.ms/devcontainer.json. For config options, see the | |||
There was a problem hiding this comment.
I'm not sure I like this thing in here… this seems to be one of the things that every developer likes to have different.
| @@ -37,6 +37,9 @@ sh tests/clean.sh | |||
|
|
|||
| Sudo permission is required. | |||
|
|
|||
| ## MSRV | |||
| The current MSRV(minimum supported rust version) is 1.60. | |||
There was a problem hiding this comment.
I don't think a 0.1 crate needs to specify/hold itself to some specific MSRV or even specify it 😇. Specifically, I'm not sure if anyone in future changes will take the care to update it.
|
|
||
| use futures::{Future, Stream}; | ||
| use tokio_core::reactor::{Core, Interval}; | ||
| use tun_tap::r#async::Async; |
There was a problem hiding this comment.
Style nitpick. It looks a bit strange to intermix blocks of "std" imports with blocks of "foreign". My own personal preference is to have 3 blocks (std, foreign, internal), but I'm flexible in the order of these… and I try to keep each separate block sorted.
There was a problem hiding this comment.
Also, as async is now a keyword, and you seem to have modified the API anyway, it would be a good time to think of a better name.
| }, | ||
| Err(ref e) if e.kind() == ErrorKind::WouldBlock => Ok(FAsync::NotReady), | ||
| Err(e) => Err(e), | ||
| impl AsyncRead for Async { |
There was a problem hiding this comment.
I'm afraid implementing AsyncRead/AsyncWrite for a pcap interface is conceptually wrong. These trains are for things that streams of bytes without being split into messages.
The pcap interface is more message oriented ‒ more like a UdpSocket, where message boundaries have a meaning. Exposing interface like this could lead to weird hard to reproduce bugs where sometimes some kind of buffer is accidentally split into two messages or two messages joined into one buffer.
Issue #13
This is pretty big changes to async part of API and I'm noob in tokio infrastructure, so it's definitely needs a picky review and further testing.
I tested it only in devcontainer on aarch64 so long.
VPN example turned out to be tricky to adapt, so I'll do it later.
MSRV changed to 1.60 because of 'version-sink' crate.
TODO: