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

Why is packet_number an argument for OnPacketSent? #107

Closed
martinduke opened this issue Dec 30, 2016 · 2 comments · Fixed by #149
Closed

Why is packet_number an argument for OnPacketSent? #107

martinduke opened this issue Dec 30, 2016 · 2 comments · Fixed by #149
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@martinduke
Copy link
Contributor

In draft-quic-ietf-recovery, packet_number is listed as a parameter. But it is not used in the pseudocode, is not sent on to SetLossDetectionAlarm(), and there is no obvious place that SetLossDetectionAlarm() would use it.

@ianswett
Copy link
Contributor

It's there so I can store the time when the packet was sent, and then use that later to calculate the RTT. I'll send out a PR that does that and clarifies some other details and you can see what you think.

@Aron-Schats
Copy link

I wonder whether the loss detection and recovery mechanisms can be explained without the use of the pseudo-code, which seems like a cop-out. The pseudo-code does not match what occurs in proto-quic code. (This is probably OK, as the draft may be describing a future version of QUIC.)

In addition, alarm mode is mentioned, but the different modes are not listed.

ianswett added a commit that referenced this issue Jan 13, 2017
Defines how variables such as smoothed_rtt and rttvar are calculated, as well as replacing many numbers with constants.

Intends to fix #107
ianswett added a commit that referenced this issue Jan 13, 2017
* Replace magic numbers with constants and define RTT calculations

Defines how variables such as smoothed_rtt and rttvar are calculated, as well as replacing many numbers with constants.

Intends to fix #107

* Update draft-ietf-quic-recovery.md

* Remove trailing whitespace

* More trailing whitespace

* Update draft-ietf-quic-recovery.md

* Whitespace
@mnot mnot added the editorial An issue that does not affect the design of the protocol; does not require consensus. label Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants