Sharing the UdpSocket with another protocol

[inetic]

We are planning on using quinn in a peer-to-peer app where peers find each other using the BitTorrent Distributed Hash Table (DHT).

It is essential for us that the transport protocol and the DHT protocol work on the same UdpSocket. The primary advantage being that peers don't need to know their external port numbers, but instead can tell nodes in the DHT that they'll be accepting QUIC connections on the same IP:PORT address as one used for the DHT communication (implied_port in the DHT spec.). We have used this same technique with success in one of our other peer-to-peer projects, although there the transport protocol was uTP.

Looking at the code, we came up with two possible modifications that we could do to the quinn's high level wrapper and would be interested in hearing whether any of those (or some we haven't though of yet) would be of interest to the quinn project.

The first approach would be to create a trait with the poll_send, poll_recv and local_addr functions and pass a generic type with this trait to the quinn::Endpoint's constructor functions. The idea is that we could create our own "proxy" wrapper over quinn::UdpSocket that we could pass to quinn::Endpoint::new and thus have access to the raw std::net::UdpSocket. Then quinn::EndpointInner::socket would probably need to be wrapped in a Box.

The second approach would be to modify the quinn::Endpoint to provide API for sending and receiving datagrams. I imagine there could be a function quinn::Endpoint::create_side_channel that could create a SideChannel object having quinn::EndpointRef as one of its members. The SideChannel object would provide recv_from and send_to functions similar to the ones in tokio::net::UdpSocket. If I'm reading the quinns code correctly, I could "tap" to quinn::EndpointInner::drive_recv (here in particular) to get datagrams not processed by quinn. To send a datagram I could probably just insert data into quinn::EndpointInner::outgoing and call wake on the driver.

Pros and Cons

The ratio of pros/cons is probably non indicative as some of them may be trivial and only listed for completeness.

For the proxy wrapper approach:

• Pro: Seems less intrusive to the existing high level wrapper code.
• Con: quinn::EndpointInner::socket will need to be wrapped in a Box.
• Con: The wrapper will probably have to add one or two queues for reading and writing datagrams between quinn::Connection and itself. I have a feeling this may interfere with quinn's congestion timers.
• Con: Our code will have to attempt to parse all incoming datagrams (as opposed to only those that have not been accepted by quinn).

Or perhaps there is another approach? I noticed there is quinn::Connection::send_datagram but we need API for sending and receiving datagrams even if there is no connection established yet.

[djc]

Option (3): we do nothing and you fork the quinn crate, building on top of quinn-proto and quinn-udp directly.

• Pro: we have to do less work
• Con: you have to do more work
• Pro: you have full flexibility in deciding which packet is routed to what parser
• Pro: you get full control over queues
• Pro: no extra allocations

Honestly I feel like this is niche enough that it's not obvious that we want to support a specific API for it, and quinn-proto already exists to offer this kind of low-level control.

[inetic]

we do nothing and you fork the quinn crate,

Might be that I explained myself poorly: forking the quinn crate and doing all the work was always meant to be done by us. I mainly created this issue to see if there is an interest for such an API in quinn and if so how would you prefer it to be done (there was a little hope we'd get a hint or two along the way, but not expected).

The obvious benefit for us - if this was merged - is that we wouldn't need to continually merge with quinn on each new release, but we can live with that. The benefit for you would be the added API, more on that below.

building on top of quinn-proto and quinn-udp directly.

quinn-proto already exists to offer this kind of low-level control.

We thought about this, but we would end up duplicating all the high level (non trivial and already tested) code, modifying only a tiny fraction. I think that is a is hard sell for anyone already using tokio.

Honestly I feel like this is niche enough that it's not obvious that we want to support a specific API for it,

Maybe. I started looking at quinn and quiche only recently, from what I've read so far I gathered quinn is the more preferred candidate for P2P projects. Knowing your external address is a notoriously difficult thing unless there is an infrastructure in place dedicated for it, but that's something P2P projects try to avoid having.

I tried to avoid this to be brief in my previous post, but there are other advantages for such API, another big one is that prior to establishing a connection, one could use such API to create holes in NATs. Thus I think in the context of P2P applications such API is not as niche as it may appear at first glance.

I'm happy to discuss this further, but I also don't want to be pushy. As I expressed, I think we know how to approach this whether it's in a fork or not.

[djc]

Have you already looked at the abstractions introduced in #1364?

[inetic]

I haven't looked into it prior to you pointing it out, thank you for that.

If I understand it correctly, we'd write a wrapper over (e.g.) the tokio runtime, then create our own socket wrapper for which we'd implement the AsyncUdpSocket trait. I think this will work for us.

[inetic]

Just an update on this if someone is interested: we were able to use the code in the abstract runtime PR #1364 and it works pretty well in that peers that were previously not able to connect to each other due to being behind a NAT now can do so.

@Ralith one small change we had to do was to make the AsyncUdpSocket trait public and change Endpoint::{new, new_with_runtime} to accept our custom AsyncUdpSocket implementation instead of std::net::UdpSocket. We couldn't use the runtime.wrap_udp_socket machinery because that doesn't allow us to retain a clone of the AsyncUdpSocket implementation on our side of the code.

If there is anything we can improve that would increase the chance of this change being merged into your code feel free to let us know and we'll try our best.

[Ralith]

make the AsyncUdpSocket trait public

Whoops, we should definitely be reexporting all of those traits, since allowing downstream reimplementations is a big part of the point.

change Endpoint::{new, new_with_runtime} to accept our custom AsyncUdpSocket implementation instead of std::net::UdpSocket

This seems well motivated to me. I'd be happy to accept a PR to the WIP branch, or I can just reimplement it myself once I get some time.

[Ralith]

I've now folded the proposed changes into the outstanding PR, I went for an additional new_with_abstract_socket constructor because I expect the majority of users who otherwise require custom configuration will still find the existing new more convenient.

[Ralith]

Fixed by #1364.