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

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

Open
larseggert opened this issue May 2, 2024 · 7 comments
Open

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

larseggert opened this issue May 2, 2024 · 7 comments
Labels
Milestone

Comments

@larseggert
Copy link

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.

@gopherbot gopherbot added this to the Unreleased milestone May 2, 2024
@ianlancetaylor
Copy link
Contributor

CC @neild

@neild
Copy link
Contributor

neild commented May 2, 2024

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
Copy link
Author

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
Copy link

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

@neild
Copy link
Contributor

neild commented May 2, 2024

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
Copy link
Author

larseggert commented May 2, 2024

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
Copy link
Contributor

neild commented May 2, 2024

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.

gopherbot pushed a commit to golang/net that referenced this issue May 3, 2024
The QUIC interop runner "keyrotate" test requires that the client
initiate a key rotation early in the connection. With our current
ack frequency, it seems that we need to rotate within the first
300-400 packets for the test to pass.

Reduce the initial key rotation from 1000 to 100 packets.
Rotating earlier shouldn't have any real downsides
(rotation is cheap and generally done once per connection,
except for very long-lived connections), and this is simpler
than providing a way to tune the rotation interval in one
specific test.

For golang/go#67138

Change-Id: I33d47ea35ed39f0a13c171adb2b0698f8c93050e
Reviewed-on: https://go-review.googlesource.com/c/net/+/582855
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

4 participants