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

Consider dropping multistream-select sim open #2330

Closed
mxinden opened this issue Jun 4, 2023 · 8 comments · Fixed by #2557
Closed

Consider dropping multistream-select sim open #2330

mxinden opened this issue Jun 4, 2023 · 8 comments · Fixed by #2557
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up

Comments

@mxinden
Copy link
Member

mxinden commented Jun 4, 2023

By default go-libp2p supports the multistream-select simultaneous open extension:

In order to support direct connections through NATs with hole punching, we need to account for simultaneous open. In such cases, there is no single initiator and responder, but instead both peers act as initiators. This breaks protocol negotiation in multistream-select, which assumes a single initator.

This draft proposes a simple extension to the multistream protocol negotiation in order to select a single initator when both peers are acting as such.

https://github.com/libp2p/specs/blob/master/connections/simopen.md

See also go-multistream/client.go as the entrypoint.

The simultaneous open extension is needed for uncoordinated simultaneous connection attempts on TCP. Note that coordinated simultaneous connection attempts in libp2p's hole punching does not require the extension. In that case the role (initiator, responder) is determined through the DCUtR protocol. See DCUtR specification.

There is a downside to supporting the multistream-select simultaneous open extension, namely when negotiating a single security protocol on a new normal TCP connection. "Normal" as in not a simultaneous connect, i.e. initiated only from one side.

Theoretically, when negotiating a single protocol only, one can use multistream-select's optimistic protocol negotiation. More specifically the dialer can propose the single protocol and then directly proceed, without waiting for confirmation from the remote, saving one round-trip in the happy path.

Though unfortunately, one can not combine the simultaneous open extension with optimistic protocol negotiation, as the former needs to wait for the response from the remote to determine whether to enter the initiator selection phase.

multistream-select with simultaneous open extension on "normal" TCP connection:

sequenceDiagram
    A ->> B: /libp2p/simultaneous-connect
    Note left of A: A can not proceed optimistically and negotiate the security protocol.<br/>B might enter _initiator selection phase_.<br/>Need to wait for B's response.
    B ->> A: `/libp2p/simultaneous-connect` or `na`

Dropping multistream-select sim open would allow go-libp2p to compete with tcp+tls+http in connection-establishment latency when supporting a single security protocol only. More specifically it would allow go-libp2p to establish a TCP+TLS connection in two round trips. At the expense of failed connection establishment on uncoordinated TCP simultaneous open.

Worth noting:

@marten-seemann
Copy link
Contributor

As @vyzo pointed out, this breaks NAT type detection. This also breaks general address detection. Commented on the issue.

I can't find any measurements. Also surprised that they would on this PR, a coordinated test setup can't measure what's happening in the wild. Wrong link?

  • One could argue that go-libp2p's QUIC is the better TCP+XXX in the first place. Thus there is not much value in investing time here.

Agreed. We should have some measurements, and if they show that it's a rare event, we should drop it. @vyzo, you had some numbers, didn't you?

@vyzo
Copy link
Contributor

vyzo commented Jun 4, 2023

There is a scenario where it happens a lot: unit tests!

@marten-seemann
Copy link
Contributor

That's fine, we have smart dialing logic now, which will prefer QUIC ;)

Any scenario in the wild where it's critical?

@vyzo
Copy link
Contributor

vyzo commented Jun 4, 2023

Critical? No.
But it would be nice to support this dark corner of the TCP spec. Can we at least get some measurements before we yank?

@marten-seemann
Copy link
Contributor

That’s what I’d like to do as well. The measurement should include a reconnect even triggered by a sudden network failure (e.g. killing all TCP connections using iptables for 30 (?) seconds or so).

@mxinden Could you instrument a Kubo node and run this experiment? Reporting can be as simple as putting a print statement in the sim open code path.

@marten-seemann marten-seemann added kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up exp/intermediate Prior experience is likely helpful effort/hours Estimated to take one or several hours labels Jun 15, 2023
@marten-seemann
Copy link
Contributor

@mxinden Any progress?

@mxinden
Copy link
Member Author

mxinden commented Jun 19, 2023

@mxinden Any progress?

No progress.

Currently prioritizing this below everything else on my todo list most notably libp2p/test-plans#63, libp2p/rust-libp2p#4053 and libp2p/rust-libp2p#2883. I still think QUIC is the better TCP+XXX. With the majority of IPFS traffic using QUIC already today, fixing this does not have a high impact. Am I missing something?

@marten-seemann
Copy link
Contributor

With that kind of argument, we'll never get rid of multistream-select sim-open. However, this seems like a very easy win. It will cost us no more than a few hours (at most), and remove a fair bit of complexity in multistream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants