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

libp2p-yamux: Allow to configure substreams as client or server #1690

Closed
BlackYoup opened this issue Aug 5, 2020 · 8 comments
Closed

libp2p-yamux: Allow to configure substreams as client or server #1690

BlackYoup opened this issue Aug 5, 2020 · 8 comments

Comments

@BlackYoup
Copy link
Contributor

Hello,

This is a follow-up of this issue I opened in yamux: libp2p/rust-yamux#89

As described in libp2p/rust-yamux#89 (comment), my use case requires to implement the "who is the server / client" logic in case of simultaneous connect. It doesn't seem to be something that can be done currently.

From libp2p/rust-yamux#89 (comment)

libp2p-yamux needs to provide a way to override this. Ideally the transport builder also allows access to the connection info.

Would it make sense? If so, I can help and make a PR about it, I just need some pointers as to how I should do it if possible :)

@twittner
Copy link
Contributor

twittner commented Aug 5, 2020

Please have a look at #1691. Other parts of rust-libp2p may run into problems with simultaneous outbound upgrades, but hopefully this PR overcomes the Yamux hurdle.

@BlackYoup
Copy link
Contributor Author

@twittner thanks!

I've only checked the code so far but from what I understand, it's when you create the transport that you decide if your node is a Server node or a Client node.

In my scenario, I don't only have 3 nodes, I have an infinite number of nodes. And those nodes can be both Client and Server nodes depending of if they provide a certain resource or need one. Am I missing something from your code?

For context, here's how I currently create my transport which I then pass to Swarm::listen_on():

let tcp = libp2p::tcp::TcpConfig::new();
let plaintext = PlainText2Config{ local_public_key: keypair.public() };
let mut yamux = libp2p::yamux::Config::default();
yamux.set_max_num_streams(100000);
yamux.set_max_buffer_size(1024 * 1024 * 10);
yamux.set_receive_window(1024 * 1024);
yamux.set_window_update_mode(libp2p::yamux::WindowUpdateMode::OnRead);

let transport = tcp
  .upgrade(Version::V1)
  .authenticate(plaintext)
  .multiplex(yamux);

From this, I fail to see how I could make my use case work. Sorry if it wasn't obvious and thanks for your time!

@twittner
Copy link
Contributor

twittner commented Aug 6, 2020

[...] I have an infinite number of nodes. And those nodes can be both Client and Server nodes depending of if they provide a certain resource or need one.

The original use-case we were discussing in libp2p/rust-yamux#89 was hole punching with simultaneous TCP connects. I do not know about other application specific requirements you have, but for the use case at hand connections are established pairwise and — as discussed in the ticket — yamux needs one part to be server and the other to be client. In order to achieve this, the method Builder::multiplex_ext in this PR gives you access to connection information like the PeerId. If, by convention, the connecting peer with the smaller ID assumes the client role and the other one the server role, Builder::multiplex_ext can be used to assign those roles as follows (modifying your example code slightly):

let tcp = libp2p::tcp::TcpConfig::new();
let plaintext = PlainText2Config { local_public_key: keypair.public() };

let my_id = PeerId::from_public_key(keypair.public());

let mut yamux = libp2p::yamux::Config::default();
yamux.set_max_num_streams(100000);
yamux.set_max_buffer_size(1024 * 1024 * 10);
yamux.set_receive_window(1024 * 1024);
yamux.set_window_update_mode(libp2p::yamux::WindowUpdateMode::OnRead);

let transport = tcp
    .upgrade(Version::V1)
    .authenticate(plaintext)
    .multiplex_ext(move |info, endpoint| {
        if endpoint.is_dialer() && my_id > info.peer_id() {
            yamux.override_mode(yamux::Mode::Server)
        }
        yamux
    });

Of course this is not a general transport because outside of hole punching one does not want to override the mode of the dialer.

@BlackYoup
Copy link
Contributor Author

Thanks, I think I just missed the yamux.override_mode() part :)

I tested it on my setup and it solves my problems, my nodes seem to be able to connect to each other. I just adapted the if endpoint.is_dialer() && my_id > info.peer_id() { to if my_id > info.peer_id() { and added an else statement to set in client mode otherwise.

@romanb
Copy link
Contributor

romanb commented Aug 6, 2020

I tested it on my setup and it solves my problems, [..]

That is good to hear, because it means at least with the plaintext protocol and this control over the multiplexer roles, TCP hole punching is possible with rust-libp2p as long as both sides propose the same protocol(s) in multistream-select (i.e. there can be no actual protocol negotiation without one node assuming the role of "server"). In general, there are unfortunately many protocols besides yamux that require a clear separation of roles (dialer/listener, initiator/responder, ...), like TLS, noise, ... and in general also multistream-select. For the current state of affairs in libp2p w.r.t. TCP simultaneous open and hole punching, you may also be interested in libp2p/specs#196, libp2p/go-tcp-transport#21.

@BlackYoup
Copy link
Contributor Author

Oh ok, I didn't realize the issue was deeper than that. Indeed, I tested with noise and it doesn't seem to tcp hole punch properly, I guess I was lucky to pick up plaintext.

I currently don't need an encryption layer but I may at some point. Just as an information, would it be possible to add the same behaviour with TLS / noise to override who's the dialer / listener? Or is it something out of scope until those specs get approved?

@romanb
Copy link
Contributor

romanb commented Aug 6, 2020

Just as an information, would it be possible to add the same behaviour with TLS / noise to override who's the dialer / listener? Or is it something out of scope until those specs get approved?

I think as long as we can find opt-in solutions that are not very intrusive, like for the multiplexing, these will certainly be considered. There is no problem in general for rust-libp2p to have additional features and options beyond what the current state of the libp2p-specs offer, as long as we can maintain spec-compliance and interoperability at the same time.

@romanb
Copy link
Contributor

romanb commented Aug 17, 2020

The yamux part has been done in #1691. Feel free to open new issues for other constraints you run into.

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

3 participants