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

Adds an overview section for loss detection and restructures a bit #397

Merged
merged 3 commits into from
Mar 13, 2017

Conversation

janaiyengar
Copy link
Contributor

No description provided.

@janaiyengar janaiyengar merged commit 2951cff into master Mar 13, 2017
@martinthomson martinthomson deleted the recovery branch March 13, 2017 23:39
# Loss Detection

We now describe QUIC's loss detection as functions that should be called on
packet transmission, when a packet is acked, and timer expiration events.
## Overview {#overview}
Copy link
Member

Choose a reason for hiding this comment

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

You don't need a specific overview section here. The overview can go in the top-level section. My technical writing teacher say that it is bad form to include empty sections, even if they have child sections, so you can avoid two problems. Also, I assume that you need an overview for congestion control as well, so you avoid creating two sections with the label "overview".


## Algorithm Details

### Constants of interest
Copy link
Member

Choose a reason for hiding this comment

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

Try to maintain a consistent use of Title Case or Sentence case for headings. Ideally we would be consistent with the style guide, which recommends the former.

Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

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

Thanks for the overview Jana.

any retransmittable packet is outstanding. When this alarm fires, all
unacknowledged packets are marked as lost.

* Instead of a packet threshold to tolerate reordering, a QUIC sender may use
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to either place this as bullet 2 or merge it into the first bullet.

reordering and then marks any unacknowledged packets as lost. This mechanism
is based on the Linux implementation of TCP Early Retransmit.

* If a packet is sent at the tail, there are no packets sent after it, and the
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't mark any packets lost in this case until an ack is received.


## Constants of interest
* If all else fails, a Retransmission Timeout (RTO) alarm is always set when
Copy link
Contributor

Choose a reason for hiding this comment

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

That's not really true. We only arm the RTO if we've exceeded max TLP and early retransmit timer and time loss detection don't apply.

QUIC uses a combination of ack information and alarms to detect lost packets.
An unacknowledged QUIC packet is marked as lost in one of the following ways:

* A packet is marked as lost if at least one packet that was sent a threshold
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this sentence a bit hard to read. Is it possible to change these bullets to be structured in parallel and all begin with When?

ie: "When a packet sent more than a threshold number of packets(kReorderingThreshold) after an unackowledged packet is acknowledged."

@martinthomson martinthomson added the editorial An issue that does not affect the design of the protocol; does not require consensus. label May 22, 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 this pull request may close these issues.

3 participants