Subject: [quicwg/base-drafts] Adds an overview section for loss detection and restructures a bit (#397)

From: janaiyengar <[email-masked]>
Date: Mon, 13 Mar 2017 16:05:10 -0700

You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/397

-- Commit Summary --

  * Adds an overview section for loss detection and restructures the section a bit.
  * added changelog note

-- File Changes --

    M draft-ietf-quic-recovery.md (97)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/397.patch
https://github.com/quicwg/base-drafts/pull/397.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/397

---

From: janaiyengar <[email-masked]>
Date: Mon, 13 Mar 2017 16:08:20 -0700

@janaiyengar pushed 1 commit.

244a171  missed one

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/397/files/8961e04c4dcaf34dff9026f2be592d86f7e4c063..244a1714abc62c5c38f71006f6fc94045344812a

---

From: janaiyengar <[email-masked]>
Date: Mon, 13 Mar 2017 16:12:30 -0700

Merged #397.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/397#event-998179232

---

From: Martin Thomson <[email-masked]>
Date: Mon, 13 Mar 2017 17:42:40 -0700

martinthomson commented on this pull request.

> +    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
+    a time thresold. This allows for senders to be tolerant of short periods of
+    significant reordering. In this mechanism, a QUIC sender marks a packet as
+    lost when a packet larger than it is acknowledged and a threshold amount of
+    time has passed since the packet was sent.
+
+  * Handshake packets are special in a number of ways, and a separate alarm
+    period is used for them.
+
+
+## Algorithm Details
+
+### Constants of interest

Try to maintain a consistent use of Title Case or Sentence case for headings.  Ideally we would be consistent with the [style guide](https://tools.ietf.org/html/rfc7322#section-3.4), which recommends the former.  

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/397#pullrequestreview-26702203

---

From: Martin Thomson <[email-masked]>
Date: Mon, 13 Mar 2017 17:39:57 -0700

>  # 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}

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".

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/397#pullrequestreview-26701909

---

From: ianswett <[email-masked]>
Date: Thu, 16 Mar 2017 08:00:28 -0700

ianswett commented on this pull request.

Thanks for the overview Jana.

-## Constants of interest
+  * If all else fails, a Retransmission Timeout (RTO) alarm is always set when
+    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

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

+  * A packet is marked as lost if at least one packet that was sent a threshold
+    number of packets (kReorderingThreshold) after it has been
+    acknowledged. This indicates that the unacknowledged packet is either lost
+    or reordered beyond the specified threshold. This mechanism combines both
+    TCP's FastRetransmit and FACK mechanisms.
+
+  * If a packet is near the tail, where fewer than kReorderingThreshold packets
+    are sent after it, the sender cannot expect to detect loss based on the
+    previous mechanism. In this case, a sender uses both ack information and an
+    alarm to detect loss. Specifically, when the last sent packet is
+    acknowledged, the sender waits a short period of time to allow for
+    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

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

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.

-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}
+
+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

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."

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/397#pullrequestreview-27162230