search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
120.09k stars 17.24k forks source link

x/net/quic: Signal `unsupported` for key-update interop test #67138

Open larseggert opened 2 weeks ago

larseggert commented 2 weeks ago

Proposal Details

Based on https://interop.seemann.io/, go-x-net doesn't seem to support key updates. Please signal this to the runner by returning unsupported for this interop test case.

ianlancetaylor commented 2 weeks ago

CC @neild

neild commented 2 weeks ago

x/net/quic does support key updates. I think the problem may be that the client initiates the first update later in the connection than the interop runner wants. (After 1000 packets, and the interop test wants an update in the first megabyte of data transferred.)

larseggert commented 2 weeks ago

OK, so as far as the interop runner is concerned, that is then lack of support for the test. I'd suggest you either do the key update earlier during the test, or mark it as unsupported?

gopherbot commented 2 weeks ago

Change https://go.dev/cl/582855 mentions this issue: quic: initiate key rotation earlier in connections

neild commented 2 weeks ago

I believe this used to pass, but probably broke after https://go.dev/cl/564476, which reduces the total number of packets sent by the client in this test configuration under the 1000 packet threshold.

It's a bit weird to set the key rotation policy specifically to make this test pass, but 1000 packets is a completely arbitrary number anyway, so just always rotating earlier might be simplest.

larseggert commented 2 weeks ago

Most other clients pass the test name in via a command line option, and then configure their default behavior as needed. Might want to do that too? (Or do a special client just for interop, but might be overkill.)

neild commented 2 weeks ago

We intentionally have few configuration parameters, and the interop client uses the public package API. So the question is how we'd configure this.

Probably through a GODEBUG setting, but we don't have any GODEBUGs that adjust behavior at the moment so adding one would be a reasonably significant change.