quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.76k stars 380 forks source link

quinn-udp support more PKTINFO fields? #1380

Closed leshow closed 2 years ago

leshow commented 2 years ago

What is the goal of quinn-udp? Currently, there is nothing QUIC specific about it AFAICT? Correct me if I have missed something.

I ask because I am interested in an async udp socket abstraction that exposes more parameters for sendmsg & recvmsg for non-QUIC related low level networking projects.

I am looking for a pass-through (maybe through a field on Transmit) to instead set ifindex to some non-zero value and ipi_spec_dst to 0. As a practical example, you can see this in dnsmasq src here.

I was going to experiment using quinn-udp as a base, which brings me back to what the goals are for the crate. Would you consider merging changes upstream or is it totally outside the objective of that crate? It seems like there is an opportunity for a general async udp crate that exposes some of these more advanced features.

djc commented 2 years ago

For something that fits well in the current design, I'd be open to merging it.

Ralith commented 2 years ago

Bear in mind that quinn-udp's interface is changing in https://github.com/quinn-rs/quinn/pull/1364 and becoming independent of async, and in general we will not hesitate to break quinn-udp's interface when doing so is useful to quinn.

leshow commented 2 years ago

I went a little further than just adding the index field to Transmit. I suspect you do not want these changes but I will offer them up anyway. Feel free to close the issue if that's the case.

Summary of changes:

Ralith commented 2 years ago

It looks like those changes are based on the pre-#1364 quinn-udp and heavily involve async, so as I warned above, they cannot be merged regardless.

leshow commented 2 years ago

I did take some changes post #1364, but I didn't need async-std support so didn't include other parts... Also, correct me if I'm wrong here, but quinn-udp falls back to regular sendto/recvfrom on platforms that don't support *mmsg right? I was looking for explicit access to those methods.

Anyway, if you're interested in the changes I mentioned, I could put them on top of the current main branch after #1364 and write corresponding methods for async-std. I'm not looking for a guarantee it'll be merged or anything but it is a little extra work so I wanted to check before I sink any time into it.

Ralith commented 2 years ago

I could put them on top of the current main branch after https://github.com/quinn-rs/quinn/pull/1364 and write corresponding methods for async-std

To reiterate, #1364 rendered quinn-udp entirely independent of any form of async, so I'm not sure why you'd have to do anything with async-std.

quinn-udp falls back to regular sendto/recvfrom on platforms that don't support *mmsg right?

Yes, graceful degradation is a key feature.

Anyway, if you're interested in the changes I mentioned

I don't think these do anything for us, so probably best to save the effort, but thanks for offering!

leshow commented 2 years ago

To reiterate, https://github.com/quinn-rs/quinn/pull/1364 rendered quinn-udp entirely independent of any form of async, so I'm not sure why you'd have to do anything with async-std.

In that PR I saw a trait being implemented by TokioRuntime & AsyncStdRuntime, assumed any additional socket methods would have to be added there.

In any case, thanks for following up. I'll close this.

Ralith commented 2 years ago

In that PR I saw a trait being implemented by TokioRuntime & AsyncStdRuntime

Those are not part of quinn-udp.