Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request for Guidance: Implementing an Abstract Transport Layer to Support Both TCP/UDP and QUIC #277

Open
anchalshivank opened this issue Nov 12, 2024 · 23 comments

Comments

@anchalshivank
Copy link

Overview

I am interested in extending rqbit to support both TCP/UDP and QUIC as potential transport protocols. Currently, rqbit operates over TCP/UDP, which aligns with the standard BitTorrent protocol, but adding QUIC support could bring several benefits:

  • Improved Performance: QUIC offers faster connection setup and built-in encryption.
  • Modern Transport Capabilities: QUIC provides multiplexed streams and better handling of network congestion.

Goal

The goal is to introduce an abstract transport layer within rqbit that allows interchangeable use of TCP/UDP or QUIC. This would enable rqbit to operate over traditional BitTorrent networks while also providing the option for a QUIC-based setup.

Request for Guidance

Since I’m new to working with rqbit's codebase, I’d appreciate any guidance on how to approach this. Specifically:

  1. Key Components to Modify:

    • Which modules or files in rqbit are responsible for TCP/UDP handling?
    • Are there specific functions or abstractions already in place that could help with creating a transport layer?
  2. Recommended Approach for Abstraction:

    • Should I create a trait (e.g., Transport) to define basic operations like connect, send, receive, and close?
    • Is there an existing pattern within rqbit for handling multiple protocols that could be extended to support both TCP/UDP and QUIC?
  3. Potential Challenges:

    • Are there known issues or challenges with adapting rqbit for non-standard transports like QUIC?
    • Would supporting QUIC potentially interfere with the BitTorrent protocol in a way that might impact compatibility?
  4. Implementation of QUIC:

    • For implementing the QUIC transport, would you recommend using a library like quinn, or are there other options that might be better suited for integration with rqbit?

Additional Context

I believe this enhancement could make rqbit more adaptable to modern network environments, where QUIC is becoming increasingly common. However, I’d like to ensure I follow the best approach to keep rqbit's structure clean and maintainable.

@ikatson
Copy link
Owner

ikatson commented Nov 14, 2024

BitTorrent doesn't work over QUIC, why would you want to add support for it?

As for abstracting away transport, it already is for stream protocols (TCP, TCP over SOCKS5). It was done just for socks, but extending to support smth like QUIC should be very easy - all you need is implement connect(), AsyncRead and AsyncWrite.

But the real benefit for rqbit would be to add support for uTP, but the current abstraction isn't enough for it - you need to implement congestion control and other parts of uTP.

@radumarias
Copy link

I think there are advantages in using QUIC

μTP Does not natively include encryption. Typically relies on the application layer or external protocols like TLS for security.

Congestion control algorithms are more advanced in QUIC.

μTP Optimized for low latency in congested networks.
May experience higher overhead compared to UDP when prioritizing fairness.

Other p2p solutions are using QUIC like Iroh and lubp2p.

@radumarias
Copy link

We might try to support μTP too and have some benchmarks.

@radumarias
Copy link

you need to implement congestion control and other parts of uTP.

@ikatson why is that? as μTP already supports that.

@ikatson
Copy link
Owner

ikatson commented Nov 15, 2024

I don't understand what rqbit (a torrent client!) has to do with QUIC.

Sure you can add QUIC but what's the point if the bit torrent network doesn't support it?

If it supported QUIC there would be a design doc (BEP) that would explain how does it integrate into existing network.

@radumarias
Copy link

@ikatson I think the original question was not formulated ok.
We want to add this extension for a cluster that is used in our case only internally and we don't require compatibility with default BitTorrent external clients.

We are working on https://github.com/radumarias/rfs and we want to use BitTorrent to sync files between our internal nodes. So our plan is to have a BitTorrent custom client + protocol to make the transfer over UDP, again, just between our internal nodes, this will not communicate with other BitTorrent clients.

You mentioned above that QUIC would be easy to integrate, this is good for us then.
But you also mentioned that μTP would be harder to add because the abstractizarion is not yet prepared. Given these 2 are very similar, they are both over UDP, handle congestion control and retries, can you explain please why is that the case?

@radumarias
Copy link

radumarias commented Nov 15, 2024

Given uTP is in the BitTorrent protocol we would like to add it to rqbit because it will benefit to be able to communicate with other standard implementation clients which have this. And it will fallback to TCP with clients that don't support it. Nowadays most of the BitTorrent clients supports it.

But can you tell us why it would be harder to add compared to QUIC and can you give us some guidance on how to add it?

The support for QUIC we will add just in out internal fork just for internal use. We olan to extend that a bit more and use Iroh actually, not even QUIC directly, because Iroh has an internal DHT which we need because we need internal one in our case, we can't use public DHT and trackers as files need to remain private in our cluster.

Another approach for us, which makes more sense now, is to first add support for uTP, as our initial goal was to have BitTorrent over UDP, we didn't know that uTP is actually in the protocol, if we did this would gave been out first approach, and we just use Iroh for DHT.

@ikatson
Copy link
Owner

ikatson commented Nov 16, 2024

I was wrong - it should be similar in effort. I glossed over BEP 29 (uTP) to double-check - in my memory it changed the operation of the BitTorrent protocol itself, so leaked from transport to application layer. But it doesn't seem to be the case.

So as long as this transport can be made AsyncRead + AsyncWrite - the changes should be minimal. You just change this "connect() -> Box" to return smth different than it does, and none of the internals (which operate over bittorrent application layer) will need to be changed.

@radumarias
Copy link

thanks @ikatson . can we crate an issue here do add the support for uTP?

@ikatson
Copy link
Owner

ikatson commented Nov 17, 2024

of course @radumarias , feel free to create an issue and mention that you're planning to do it

@radumarias
Copy link

@anchalshivank please do that and work on it, thanks

@KolbyML
Copy link

KolbyML commented Jan 22, 2025

Hi guys we are using uTP as well for our protocol https://github.com/ethereum/utp

@ikatson are you planning to build your own implementation? I think there would be value in building 1 very good implementation we could all use

@ikatson
Copy link
Owner

ikatson commented Jan 22, 2025

@KolbyML I wasn't planning to, unless it's simple. Which it isn't :D

Your library says "This library is currently unstable, with known issues. Use at your own discretion."
What does that mean, and what happens if we use it in librqbit?

@KolbyML
Copy link

KolbyML commented Jan 22, 2025

@KolbyML I wasn't planning to, unless it's simple. Which it isn't :D

Your library says "This library is currently unstable, with known issues. Use at your own discretion." What does that mean, and what happens if we use it in librqbit?

We are about to bring https://github.com/ethereum/trin into production this year so I would say our uTP implementation must become production ready as well. Currently my focus on ethereum/trin is improving performance as we will need it going into production.

The only known issue is a connection can only send a maximum of ~50 MB (conservatively speaking), which would be fixed if we implemented this todo https://github.com/ethereum/utp/blob/d894487d80a650bffd9e511fd9071b05c1ddb40d/src/sent.rs#L361

or in other words the library can only send a maximum of u16/2^16 packets a connection due to us not properly handling the ack window rolling over

@ikatson
Copy link
Owner

ikatson commented Jan 22, 2025

@KolbyML I was just trying to hack together a PoC of using your lib in librqbit, faced a couple challenges:

  1. architectural challenge: tokio's TcpStream allows both reading and writing at the same time. It implements AsyncRead and AsyncWrite. That's the one we use in librqbit today. However the only interface utp-rs::UtpStream provides is 2 async functions - "read_to_eof" and "write" both of which take &mut self, making it impossible to both read and write.
  2. would be cool to have tokio::io::AsyncRead + tokio::io::AsyncWrite be implemented for UtpStream out of the box so that we don't need a custom inefficient wrapper.

#1 is by far the more important one which makes it seemingly impossible to use in librqbit in its current state.
For reference, here's how it's used today for tokio::net::TcpStream. We read and write to the peer's socket concurrently and react on messages.

But if UtpStream is blocked on reading, we can't write anything to it.

Is there something I'm missing?

@radumarias
Copy link

@ikatson @KolbyML we have this implementation made by @anchalshivank https://github.com/anchalshivank/rfs-utp which we plan to integrate in the client and have it as a separate crate too.

  1. would be cool to have tokio::io::AsyncRead + tokio::io::AsyncWrite be implemented for UtpStream out of the box so that we don't need a custom inefficient wrapper.

We have this kind of impl https://github.com/anchalshivank/rfs-utp/blob/main/src/udp_stream.rs#L247 I guess we can address 1. too with our UtpStream, @anchalshivank could you please add your thoughts on this?

The impl is functional and it's fully async, just we need to do some more tests and small changes, but @anchalshivank please comment more.

Do you gyus think we should collaborate maybe in merging these two into a better impl and use that?

@KolbyML
Copy link

KolbyML commented Jan 22, 2025

@KolbyML I was just trying to hack together a PoC of using your lib in librqbit, faced a couple challenges:

  1. architectural challenge: tokio's TcpStream allows both reading and writing at the same time. It implements AsyncRead and AsyncWrite. That's the one we use in librqbit today. However the only interface utp-rs::UtpStream provides is 2 async functions - "read_to_eof" and "write" both of which take &mut self, making it impossible to both read and write.
  2. would be cool to have tokio::io::AsyncRead + tokio::io::AsyncWrite be implemented for UtpStream out of the box so that we don't need a custom inefficient wrapper.

#1 is by far the more important one which makes it seemingly impossible to use in librqbit in its current state. For reference, here's how it's used today for tokio::net::TcpStream. We read and write to the peer's socket concurrently and react on messages.

But if UtpStream is blocked on reading, we can't write anything to it.

Is there something I'm missing?

We use uTP in a non-standard way as we don't use it directly on UDP, but instead tunnel it in another protocol which negotiates connections which gives us a connection_id to use.

I don't think you are missing anything and we should resolve the issues you outlined. We use a different interface due to our different usecase, I am of the belief we should support the standard interface you need. We just haven't had someone use our library in a standard way yet so the api isn't fleshed out properly.

We would love your feedback

@ikatson
Copy link
Owner

ikatson commented Jan 22, 2025

@radumarias that's cool, thanks for the update. I own no uTP libs, so I can't recommend solutions on merging. But from what it looks like the impl in "rfs-utp" suits librqbit better at least from the API surface.

Inside however looks like it's using tokio::spawn and channels which might limit perf in theory. Time will tell. @anchalshivank @radumarias If your lib is ready to go, we can test that and add as an experimental optional feature. As Streams are core to (lib)rqbit, I wouldn't make it enabled by default until time shows it's stable and performant. And even then, I'm sure TCP will be faster, so I'd use uTP only as fallback if we can't connect at all over TCP to a peer.

@KolbyML
Copy link

KolbyML commented Jan 22, 2025

@ikatson @KolbyML we have this implementation made by @anchalshivank https://github.com/anchalshivank/rfs-utp which we plan to integrate in the client and have it as a separate crate too.

  1. would be cool to have tokio::io::AsyncRead + tokio::io::AsyncWrite be implemented for UtpStream out of the box so that we don't need a custom inefficient wrapper.

We have this kind of impl https://github.com/anchalshivank/rfs-utp/blob/main/src/udp_stream.rs#L247 I guess we can address 1. too with our UtpStream, @anchalshivank could you please add your thoughts on this?

The impl is functional and it's fully async, just we need to do some more tests and small changes, but @anchalshivank please comment more.

Do you gyus think we should collaborate maybe in merging these two into a better impl and use that?

Our implementation is async as well, supporting tokio::io::AsyncRead + tokio::io::AsyncWrite should be a relatively minor change in the grand scheme of things.

I would be very interested in collaborating to build the best uTP implementation possible. The way I see it is 2 people working on 1 implementation is likely to be better then 2 people working on 2 separate implementations in terms of testing, performance, etc.

@ikatson
Copy link
Owner

ikatson commented Jan 22, 2025

If you guys (or all of us) are going to collab on making a standard uTP impl, I'd take inspiration from smth like smol-tcp.
That library is awesome, doesn't depend on any high-level stuff like tokio / channels / spawning etc. A state machine that reacts on incoming and outgoing messages. Low-level zero-alloc code that can be plugged into anything with the most performant manner possible for the use-case at hand.

I'd also look at how the existing C++ impl in libtorrent (or whever it is) works to take some inspiration.

But as you already have 2 working implementations with different tradeoffs, none of it might be relevant or applicable, so all up to you.

@radumarias
Copy link

If you guys (or all of us) are going to collab on making a standard uTP impl, I'd take inspiration from smth like smol-tcp.
That library is awesome, doesn't depend on any high-level stuff like tokio / channels / spawning etc.

@ikatson I I gree, we will investigate that, we also like smol crate, not sure about the relation to smol-tcp. @anchalshivank please see if we could leverage something.

And even then, I'm sure TCP will be faster,

@ikatson why do you expect TPC to be faster? because of tokio?

I would be very interested in collaborating to build the best uTP implementation possible. The way I see it is 2 people working on 1 implementation is likely to be better then 2 people working on 2 separate implementations in terms of testing, performance, etc.

I fully agree, maybe it would be wise to have a call all of us, analyze both the impl and architect one better and then work together on that? What do you think?

@KolbyML
Copy link

KolbyML commented Jan 22, 2025

I would be very interested in collaborating to build the best uTP implementation possible. The way I see it is 2 people working on 1 implementation is likely to be better then 2 people working on 2 separate implementations in terms of testing, performance, etc.

I fully agree, maybe it would be wise to have a call all of us, analyze both the impl and architect one better and then work together on that? What do you think?

This sounds great, what messaging platform would work best to coordinate this call?

@ikatson
Copy link
Owner

ikatson commented Jan 22, 2025

This sounds great, what messaging platform would work best to coordinate this call?

created a telegram chat https://t.me/+IFCaztp_ImtkMWE0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants