Issues related to API implementation

[vnt-dev]

How can I contribute code. Is it possible to implement an API similar to 'https://crates.io/crates/tun-rs'?

[nathaniel-bennett]

Hey, thanks for taking interest in this project! Are there any particular features you're looking to contribute or see implemented? If you don't have a specific focus in mind and are just interested in helping out in some way, I've added a list of features that would be good to add to this crate in the near future.

As for API design, I'm not sure that changing this crate to mirror the API of tun-rs would be feasible, but I can certainly try to implement a few more features here that are available in that crate. In general, I'm trying to make Tun and Tap types in this crate have a similar API feel to that of core rust networking building blocks (such as TcpStream/UdpStream, or Ipv4Addr). This means I'm avoiding things like Builder patterns (which is what tun-rs appears to use to instantiate Tun devices) and instead focusing on functions that mutate the state of the Tun device in straightforward, atomic steps. If you'd like to migrate some code from tun-rs or similar to tappers, there are ways you could use a helper function wrapping various configuration steps to achieve substantially the same intended effect that the tun-rs builder pattern has.

[vnt-dev]

Using stream may be inconvenient. recv/send is easier to use

[nathaniel-bennett]

I was actually referring to the TcpStream::send() and TcpStream::recv() functions in that sense, yes

[vnt-dev]

Is recv and send with immutable references feasible

[nathaniel-bennett]

Not if Windows is to be supported, unfortunately. *nix systems use file descriptors, so they could all be implemented using an immutable reference, but the wintun API requires a mutable reference to a WintunSession when allocating a packet that will subsequently be sent over the tunnel:

    fn WintunAllocateSendPacket(session: *mut WintunSession, packet_size: u32) -> *mut u8;

This means that any cross-platform send() implementation that supports wintun will need a mutable reference; otherwise, some cast from immutable to mutable would be needed, which would be UB. The same issue is present for recv():

    fn WintunReceivePacket(session: *mut WintunSession, packet_size: *mut u32) -> *mut u8;

Again, passing a mutable pointer when using an immutable reference to self would result in UB.

[nathaniel-bennett]

I'm curious--is there some key benefit within async code to having immutable send()/recv() functions? Or is it just a matter of convenience? I've not implemented async features for tappers yet, so I don't have much of an idea as to what would be needed there.

[vnt-dev]

Not if Windows is to be supported, unfortunately. *nix systems use file descriptors, so they could all be implemented using an immutable reference, but the wintun API requires a mutable reference to a WintunSession when allocating a packet that will subsequently be sent over the tunnel:

    fn WintunAllocateSendPacket(session: *mut WintunSession, packet_size: u32) -> *mut u8;

This means that any cross-platform send() implementation that supports wintun will need a mutable reference; otherwise, some cast from immutable to mutable would be needed, which would be UB. The same issue is present for recv():

    fn WintunReceivePacket(session: *mut WintunSession, packet_size: *mut u32) -> *mut u8;

Again, passing a mutable pointer when using an immutable reference to self would result in UB.

Wintun session is thread-safe, would there be any issues with converting it to an immutable reference?

[vnt-dev]

The main requirement is to implement concurrent read and write operations. Since 'tappers' does not implement 'split', I'm not sure how to achieve read-write separation.

[nathaniel-bennett]

I went ahead and converted it to an immutable reference based on Wintun being thread-safe; this should hopefully make concurrent read/write easier

[nathaniel-bennett]

I've now implemented AsyncTun/AsyncTap interfaces that work with various async backends (tokio, async-io, smol, mio). These use the usual recv()/send() APIs

[vnt-dev]

tappers/src/mio/tun.rs

Line 46 in a501303

let io = unsafe { UdpSocket::from_raw_socket(tun.read_handle() as RawSocket) };

Converting a Wintun handle into a Socket will fail.

I tested the library async-io = "2.3.4". Using Waitable::new(tun.read_handle()) from async-io allows for asynchronous listening to readable events. However, it’s extremely unstable, and I'm not sure why.

[vnt-dev]

https://github.com/smol-rs/async-io/blob/6aa416c8165a8579c80f5a35b5c80e009c363f9c/src/reactor/windows.rs#L28

    /// Waitable handle for Windows.
    ///
    /// # Invariant
    ///
    /// This describes a valid waitable handle that has not been `close`d. It will not be
    /// closed while this object is alive.
    Handle(RawHandle),

tappers/src/async_io/tun.rs

Line 81 in a501303

unsafe { BorrowedSocket::borrow_raw(self.0.read_handle() as RawSocket) }

async-io supports using RawHandle

[vnt-dev]

tappers/src/mio/tun.rs

Line 46 in a501303

let io = unsafe { UdpSocket::from_raw_socket(tun.read_handle() as RawSocket) };

Converting a Wintun handle into a Socket will fail.

I tested the library async-io = "2.3.4". Using Waitable::new(tun.read_handle()) from async-io allows for asynchronous listening to readable events. However, it’s extremely unstable, and I'm not sure why.

Using async-io to manage wintun-read-event-handle results in some unusual behaviors, such as high latency and event loss.

[nathaniel-bennett]

I was worried about that--some of the workarounds I used to make wintuns read-only handle fit with the various async backends felt like potentially buggy hacks.

Wait, so does using Waitable lead to high latency/event loss too?

[nathaniel-bennett]

tappers/src/mio/tun.rs

Line 46 in a501303

let io = unsafe { UdpSocket::from_raw_socket(tun.read_handle() as RawSocket) };

Converting a Wintun handle into a Socket will fail.

I tested the library async-io = "2.3.4". Using Waitable::new(tun.read_handle()) from async-io allows for asynchronous listening to readable events. However, it’s extremely unstable, and I'm not sure why.

It looks like mio doesn't expose anything that could be used to specifically poll the read handle. However, it does expose the Windows-specific NamedPipe, which implements FromRawHandle. I might try using that for read events and see how it goes--it feels like a hack (again), but I've read from issues/PRs in other projects that this is generally how people have gotten around the API limitations of mio.

[vnt-dev]

Yes, converting to Socket, Async, or NamedPipe all results in errors. Using Waitable can capture read events, but it’s unstable.

[nathaniel-bennett]

It looks like async backends use a global threadpool (albeit managed by Win32) to wait on events anyways--this is what the async-io Waitable docs say:

The current implementation waits on the handle by registering it in the application-global Win32 threadpool. However, in the futur it may be possible to migrate to an implementation on Windows 10 that uses a mechanism similar to MsgWaitForMultipleObjectsEx.

Given this, and that HANDLE polling APIs are all unstable/result in errors for wintun, I think I'll go ahead and re-implement the windows-specific recv() function as a spawned task.

[vnt-dev]

Using a sub-thread works better. The internal implementation of async-io is also based on sub-threads.

[nathaniel-bennett]

Alright, I've re-implemented it so that a blocking sub-thread is used instead of waiting on a HANDLE--hopefully this resolves all of the above problems. I don't have a good test flow for checking sync/async read() and write() functions yet, but once I finish implementing functions that enable adding/removing routes from the interface that should change 👍