Yamux v2.1.0 broke libp2p compat

[Stebalien]

It introduces a new InitialStreamWindowSize config entry to set the initial stream window size. Unfortunately:

1. This (and MaxStreamWindowSize) affect both our receive windows and our initial send window.
2. This isn't something that we negotiate anywhere.
3. If a peer has an initial send window that's too high, the peer will disconnect due to protocol violation.

This affects us here because this library sets the MaxStreamWindowSize to 16MiB. That means, before we updated to Yamux v2.1.0, the initial send & receive window was 16MiB. After v2.1.0, this is the max window and the initial one is set to InitialStreamWindowSize (16KiB). That means an old libp2p node will try to send up to 16MiB of data, while a new node will reset the stream after 16KiB.

1. It sounds like we need to set the InitialStreamWindowSize to 16MiB. This is... unfortunate, but not terrible.
2. We might as well increase the maximum window size...

In the future, it would be nice to have a protocol extension where both sides could try to raise the initial stream window size.

Also note: I attempted to fix this issue by setting InitialStreamWindowSize to 16MiB but something still didn't work... I'm not sure what, but there needs to be more investigation (cc @dirkmc)

Testing: This should have been caught by tests. Why not? Well:

1. We're adjusting defaults here instead of setting them down in go-yamux. That means our go-yamux tests won't catch issues here.
2. I have no idea why this isn't showing up in our other tests, but the likely answer is that we're just sending around really small files/blocks. We need better stress tests.
3. We're not performing interop tests here. We do have interop tests, but they're all mplex based and only work go<->js.

[Stebalien]

• @marten-seemann could you handle this?
• @mxinden let's confirm rust & go agree here on window sizes... I don't think we ever considered these important in the past.
• @vasco-santos could you confirm the same for js-libp2p?

[marten-seemann]

@Stebalien Where do the 16 MB come from? Our spec says it's 256 kB, and I think this is the commit that change the default: libp2p/go-yamux@f6e177c.

The fix is easy: Just revert that change to the constant, and add a big fat warning that this is a value taken from the spec, not an implementation choice.

Testing: This should have been caught by tests.

I fully agree. We should have something similar to the quic-transport interop tests here.

[Stebalien]

I bumped it to 1MiB in 2018, Vyzo bumped it to 16MiB in 2019... I'm a bit concerned that 256KiB will break it in practice.

[Stebalien]

We could pull a "liberal on the inputs" kind of thing. I.e., set the initial send window size to 256KiB (seems like the safest) but set the initial receive window to 1MiB (or 16...). After some time, we can lower the initial receive window.

Basically, the send window is the only part that really matters.

[marten-seemann]

I bumped it to 1MiB in 2018, Vyzo bumped it to 16MiB in 2019...

Are you referring to the config.MaxStreamWindowSize? That's the size we'd use for the first window update.

We initialized the window to 256 kB in v2.0.0: https://github.com/libp2p/go-yamux/blob/b6118a4b2a9b3038a0da9f29d4670b718f6850e2/stream.go#L51-L66

[Stebalien]

Oh, oh that makes me feel so much better. Yeah, then we just need to make this non-configurable and set it to 256KiB.

Note: I would like to make it somewhat configurable. If it's not too hard, it would be nice to send an immediate update on opening a stream that sets it to some higher minimum. But that's not critical.

[Stebalien]

That would explain:

Also note: I attempted to fix this issue by setting InitialStreamWindowSize to 16MiB but something still didn't work... I'm not sure what, but there needs to be more investigation...

Streams going the other way likely had problems.

[vasco-santos]

could you confirm the same for js-libp2p?

we do not have yamux support in js, but thanks for pinging

[mxinden]

@mxinden let's confirm rust & go agree here on window sizes...

• Initial window size: rust-yamux follows the spec value of 256KB.

• Maximum window size: rust-yamux does not enforce a maximum window size as a sender. Thus, if a receiver chooses to send a large window update (e.g. 16MB) rust-yamux happily adds the additional credit on top of the existing credit. As a receiver rust-yamux uses the initial window size (256KB) and sends window update messages once the sender depleted half or more of the window. See Send WindowUpdate message early rust-yamux#100 for an in-depth analysis of sending the window update early as a receiver.

• Maximum data frame size: rust-yamux by default splits data frames larger than 16KB before sending to allow interleaving of control frames such as window updates and thus improve throughput. See benchmarks and corresponding pull request. rust-yamux does not restrict the size of individual data frames when receiving (other than the usual window size enforcement).

---

I don't think we ever considered these important in the past.

Mostly agree. Only thing I remember is back in 2020 where Toralf discovered that rust-yamux was incompatible with go-yamux in regards to window updates. The corresponding fixes have been rolled out.

This crate understands the initial window update as the total receive window size whereas go-yamux adds the window update to the implicit default of 256 KiB.

libp2p/rust-yamux#92

---

As far as I know rust-yamux is fully compatible with go-yamux < v2.1.0. With libp2p/go-yamux#57 the two (rust-yamux and go-yamux) should be fully compatible again.

@Stebalien @marten-seemann please correct me in case I am missing something.

[Stebalien]

Maximum window size: rust-yamux does not enforce a maximum window size as a sender. Thus, if a receiver chooses to send a large window update (e.g. 16MB) rust-yamux happily adds the additional credit on top of the existing credit. As a receiver rust-yamux uses the initial window size (256KB) and sends window update messages once the sender depleted half or more of the window. See libp2p/rust-yamux#100 for an in-depth analysis of sending the window update early as a receiver.

This matches what go does now (after the bug was fixed) and did before the bug was introduced.

Maximum data frame size: rust-yamux by default splits data frames larger than 16KB before sending to allow interleaving of control frames such as window updates and thus improve throughput. See benchmarks and corresponding pull request. rust-yamux does not restrict the size of individual data frames when receiving (other than the usual window size enforcement).

Interesting. Go splits at 64KiB (configurable). Although we never really benchmarked it.