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

Restructure short header section #4407

Merged
merged 4 commits into from
Dec 7, 2020
Merged

Restructure short header section #4407

merged 4 commits into from
Dec 7, 2020

Conversation

janaiyengar
Copy link
Contributor

@janaiyengar janaiyengar commented Nov 25, 2020

Fixes #4397.

This PR restructures 17.3 to be aligned with 17.2. Note that while the diff seems to show whole paragraphs changed in the Spin Bit section, all I've done is replaced a packet with a short header with a 1-RTT packet.

Note that I've not replaced all uses of short header. I've retained it where it is used alongside long header, for symmetry.

This still touches more text than I had hoped, but it makes the structure of Section 17 clearer.

@janaiyengar janaiyengar changed the title Clarify that short headers and 1-RTT are 1:1 Restructure short header section Nov 25, 2020
Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

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

This looks good. The actual changes are much smaller than the diff appears, and I think the symmetry with 17.2 helps readability despite the extra layer.

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
janaiyengar and others added 2 commits November 24, 2020 17:02
Co-authored-by: Mike Bishop <mbishop@evequefou.be>
Copy link
Member

@kazuho kazuho left a comment

Choose a reason for hiding this comment

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

Thank you @janaiyengar. This LGTM.

Copy link
Member

@kazuho kazuho left a comment

Choose a reason for hiding this comment

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

Update: I think we might want to make corresponding changes to the TLS draft.

Re TLS draft, I think we can replace all the occurrences of "short header packet(s)", "packet(s) with (a) short header" in the TLS draft with "1-RTT packet(s)", as well making necessary changes to table 1.

@janaiyengar
Copy link
Contributor Author

janaiyengar commented Nov 26, 2020

@kazuho: Thanks for the note. I've updated the tls doc, but I've left most uses of short header as they are. I've retained them for symmetry, since in all the ones I've retained, it appears alongside long header.

@janaiyengar janaiyengar merged commit ba59d1d into master Dec 7, 2020
@janaiyengar janaiyengar deleted the short-header branch December 7, 2020 20:13
@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels Dec 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"1-RTT packet" is defined nowhere
5 participants