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.