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

Version field <-> Version is not invariant #3785

Merged
merged 5 commits into from
Jul 29, 2020
Merged

Conversation

martinthomson
Copy link
Member

...it's just a Version field.

This is a subtle one, but it follows from the work that others have been
doing on greasing versions or providing additions to echconfig for QUIC
connection establishment. It basically says that Version is for
endpoints, not middleboxes.

...it's just a Version field.

This is a subtle one, but it follows from the work that others have been
doing on greasing versions or providing additions to echconfig for QUIC
connection establishment.  It basically says that Version is for
endpoints, not middleboxes.
@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -invariants labels Jun 24, 2020
@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. and removed editorial An issue that does not affect the design of the protocol; does not require consensus. labels Jun 30, 2020
@project-bot project-bot bot added this to Design Issues in Late Stage Processing Jun 30, 2020
@martinthomson martinthomson linked an issue Jul 1, 2020 that may be closed by this pull request
@ekr
Copy link
Collaborator

ekr commented Jul 1, 2020

I like the idea here, but it seems like the distinction in the text is too subtle. Why not remove this entirely?

@martinthomson
Copy link
Member Author

By "this", do you mean striking This value can be used by endpoints to identify a QUIC Version.?

@larseggert larseggert removed this from Design Issues in Late Stage Processing Jul 1, 2020
@ekr
Copy link
Collaborator

ekr commented Jul 1, 2020

I was actually suggesting removing it from invariants entirely.

@martinduke
Copy link
Contributor

I would quibble a bit about whether or not version aliasing really violates the existing text, but I still think this change is a good one. For example, if we fail to prevent ossification then we're going to end up with this being the "fake" version while the real version is buried in the TPs.

Copy link
Contributor

@nharper nharper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good change to be making.

@@ -252,8 +252,9 @@ Packets for the same QUIC connection might use different connection ID values.

## Version

QUIC versions are identified with a 32-bit integer, encoded in network byte
order. Version 0 is reserved for version negotiation (see
The Version field contains a 32-bit integer, encoded in network byte order.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(If this is off-topic for this PR, please ignore.)

Is there a reason why we specify that the Version field is an integer, as opposed to a 32-bit value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only so that we can use numbers to talk about values. TLS picked up this crazy {3, 3} notation that annoys the hell out of me. People read that to mean that there are two discrete things with different semantics, when there is just one value and one semantic. You could always write out the hex (as people now do for connection IDs), but then that's just a number.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely agree we don't want something like the TLS notation.

The only operation we need to do on QUIC versions (as encoded on the wire) is to check for equality. We don't need ordering of them (to check if one is less than another), and we certainly don't need to be able to do arithmetic on them, hence we don't need to consider them as numbers. By saying it's a 32-bit value (instead of an integer), we avoid needing to specify that it's network byte order. When specifying a version, you can write it out in hex (e.g. 0xff00001d), but that can be treated as a sequence of bytes instead of an integer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would s/integer, .../identifier/ work for others?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally like 32-bit identifier. 32-bit opaque identifier might be even better, or could be overkill.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with identifier.

QUIC versions are identified with a 32-bit integer, encoded in network byte
order. Version 0 is reserved for version negotiation (see
The Version field contains a 32-bit integer, encoded in network byte order.
This value can be used by endpoints to identify a QUIC Version. A Version field
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up with version ossification and the real version ends up in transport params, is it still the case that an endpoint can use the Version field to identify a QUIC Version? Perhaps "This value can be used by endpoints in identifying a QUIC Version" instead. That way there's room that an endpoint might consider other factors in identifying the QUIC Version used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, this gets into whether it is necessary or sufficient information to identify a QUIC version and whether we need to say anything more. "can" might be read to imply that it is sufficient, though "can" doesn't imply necessity. But "can" also means that it is possible for endpoints to use the field. That is the meaning I intended and I think that's OK.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wiggle room around "can" sounds good to me.

@@ -252,8 +252,9 @@ Packets for the same QUIC connection might use different connection ID values.

## Version

QUIC versions are identified with a 32-bit integer, encoded in network byte
order. Version 0 is reserved for version negotiation (see
The Version field contains a 32-bit integer, encoded in network byte order.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with identifier.

@martinthomson martinthomson merged commit f329f7d into master Jul 29, 2020
@martinthomson martinthomson deleted the version-not branch July 29, 2020 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-invariants design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Versions are for endpoints
7 participants