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

Make Packet Number a standard varint, with no inference #1502

Closed
martinduke opened this issue Jun 29, 2018 · 4 comments
Closed

Make Packet Number a standard varint, with no inference #1502

martinduke opened this issue Jun 29, 2018 · 4 comments
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@martinduke
Copy link
Contributor

Now that packet numbers start at 0, I am not sure there is a ton of value in truncating these numbers. There are some advantages to making it a standard 1 to 8 byte-varint:

  • Reuse existing varint code instead of implementing something special that can be screwed up.
  • Eliminate the need to infer the most significant bits of the PN, which is another thing to screw up that has pretty bad consequences for connection integrity (e.g. acking packets that weren't actually received)
  • Eliminating the inference also makes it easier for debugging tools like Wireshark to correctly display the PN.

I assert that the fraction of connections that will need 64-bit packet numbers will be exceedingly small. There are probably very minor bit-efficiency issues (connections with 255-64K packets sent at a relatively low rate, etc), but these pale in comparison to the bytes we've put into variable length CIDs, etc.

We already encoding the PN this way in the Largest_acked field of the ACK frame, so I believe the same principles should apply.

If people agree with this assessment, I'll drop a PR.

@ianswett
Copy link
Contributor

This will make users of long-lived bandwidth sensitive apps unhappy. WebRTC audio-only connections are a good example of this. Today they can commonly use a single byte packet number, and with this they'd commonly be using 4 byte PNs.

@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Jul 2, 2018
@martinthomson
Copy link
Member

pretty bad consequences for connection integrity (e.g. acking packets that weren't actually received)

If you reconstruct the wrong packet number, all that happens is that you can't decrypt the packet. That's not fatal, it just looks like loss. The damage is limited to a reduction in throughput.

In addition to the concern @ianswett raised, a 2-bit-prefixed varint can use the one octet encoding less often. Audio is probably OK to use a one octet encoding; the one-octet/6-bit encoding can't be used at around a 640ms RTT for the most common 20ms latency, which is a pretty long hop for real-time audio. Add video and higher packet rates and lower latency encodings start to have to use 2 octets on a shorter RTT. The current encoding allows the rate or RTT to double before spilling over.

@siyengar
Copy link

siyengar commented Jul 2, 2018

Size overhead also becomes important in use cases like text messaging where the message size becomes comparable to the protocol overhead. With this change we would rollover to 2 bytes with 256 packets which in effect means 256 messages. Sending a lol to someone where the protocol overhead itself is larger than the message.

One alternative which might be interesting is var length encoding of the truncated packet number so that we don’t need the 4 byte packet number type in the short header

@martinduke
Copy link
Contributor Author

@martinthomson Good point on the drops. I know when I'm licked. Closing.

@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Development

No branches or pull requests

5 participants