Subject: [quicwg/base-drafts] split fast retransmit modes (#1226)

From: Marten Seemann <[email-masked]>
Date: Sun, 18 Mar 2018 04:03:19 -0700

Fixes #1212.

Requesting a review from @ianswett, @siyengar, @janaiyengar.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * split fast retransmit modes

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/1226.patch
https://github.com/quicwg/base-drafts/pull/1226.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/1226

---

From: ianswett <[email-masked]>
Date: Sun, 18 Mar 2018 05:35:54 -0700

ianswett commented on this pull request.

I like the new text organization at the top describing the modes, but I'm not a fan of the new pseudocode, because it seems more verbose than the old code, and not that much clearer.

> @@ -201,6 +201,21 @@ implemented in QUIC.
 
 ### Fast Retransmit
 
+Fast retransmit can be run two different modes: packet number based and time

run in

> @@ -201,6 +201,21 @@ implemented in QUIC.
 
 ### Fast Retransmit
 
+Fast retransmit can be run two different modes: packet number based and time
+based.
+
+We derive this default from recommendations for TCP loss recovery {{?RFC5681}}
+{{?RFC6675}}. It is possible for networks to exhibit higher degrees of
+reordering, causing a sender to detect spurious losses. Making QUIC deal with

I would phrase it as "spuriously detect losses."

> @@ -201,6 +201,21 @@ implemented in QUIC.
 
 ### Fast Retransmit
 
+Fast retransmit can be run two different modes: packet number based and time
+based.
+
+We derive this default from recommendations for TCP loss recovery {{?RFC5681}}
+{{?RFC6675}}. It is possible for networks to exhibit higher degrees of
+reordering, causing a sender to detect spurious losses. Making QUIC deal with
+such networks is important open research, and implementers are encouraged to
+explore this space. Detecting spurious losses leads to unnecessary

Spuriously detecting losses

> @@ -201,6 +201,21 @@ implemented in QUIC.
 
 ### Fast Retransmit
 
+Fast retransmit can be run two different modes: packet number based and time
+based.
+
+We derive this default from recommendations for TCP loss recovery {{?RFC5681}}
+{{?RFC6675}}. It is possible for networks to exhibit higher degrees of
+reordering, causing a sender to detect spurious losses. Making QUIC deal with
+such networks is important open research, and implementers are encouraged to
+explore this space. Detecting spurious losses leads to unnecessary
+retransmissions and may result in degraded performance due to the actions of
+the congestion controller upon detecting loss. Implementers MAY use algorithms
+developed for TCP, such as TCP-NCR {{?RFC4653}}, to improve QUIC's reordering
+resilience, though care should be taken to map TCP specifics to QUIC correctly.

I think it's worth commenting that both modes could be run simultaneously, but the psuedocode only describes how to run one at once.

>    a packet lost.  In fraction of an RTT.
 
 kUsingTimeLossDetection (default false):
-: Whether time based loss detection is in use.  If false, uses FACK style
+: Whether time-based loss detection is in use.  If false, uses FACK style

This is called FACK style here, but probably should be called "packet number based loss detection" for consistency?  It may be worth referencing FACK above when describing packet number based loss detection.

-- 
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/1226#pullrequestreview-104795366

---

From: Subodh Iyengar <[email-masked]>
Date: Sun, 18 Mar 2018 05:43:39 -0700

siyengar commented on this pull request.

> @@ -209,20 +224,19 @@ reordering of packets in the network.
 
 The RECOMMENDED initial value for kReorderingThreshold is 3.
 
-We derive this default from recommendations for TCP loss recovery {{?RFC5681}}
-{{?RFC6675}}. It is possible for networks to exhibit higher degrees of
-reordering, causing a sender to detect spurious losses. Detecting spurious
-losses leads to unnecessary retransmissions and may result in degraded
-performance due to the actions of the congestion controller upon detecting
-loss. Implementers MAY use algorithms developed for TCP, such as TCP-NCR
-{{?RFC4653}}, to improve QUIC's reordering resilience, though care should be
-taken to map TCP specifics to QUIC correctly. Similarly, using time-based loss
-detection to deal with reordering, such as in PR-TCP, should be more readily
-usable in QUIC. Making QUIC deal with such networks is important open research,
-and implementers are encouraged to explore this space.
+#### Time-based Fast Retransmit
+
+Using time-based loss detection to deal with reordering, such as in PR-TCP,

maybe add a reference to PR-TCP here?

> @@ -846,6 +843,29 @@ DetectLostPackets(largest_acked):
     else if (loss_time == 0 && delay_until_lost != infinite):
       loss_time = now() + delay_until_lost - time_since_sent

i think this might be cleaner to split this up to the end of the loop.

it looks like the intention is to the early retransmit timer to be the loss time of the first packet not lost by packet ordering. It might be cleaner like

```
 last_unacked = largest_acked.packet_number
 foreach (unacked < largest_acked.packet_number):
     ....
     last_unacked = unacked
 if (last_unacked < largest_acked.packet_number) :
    lost_time = now() + delay_until_lost - sent_packets[last_unacked].time_since_sent
```

and similarly in the other one as well

-- 
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/1226#pullrequestreview-104795418

---

From: Marten Seemann <[email-masked]>
Date: Sun, 18 Mar 2018 13:37:02 +0000 (UTC)

marten-seemann commented on this pull request.

I'm not familiar with FACK. Can you suggest something?

-- 
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/1226#discussion_r175289542

---

From: Marten Seemann <[email-masked]>
Date: Sun, 18 Mar 2018 13:40:02 +0000 (UTC)

What exactly would you link here?

-- 
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/1226#discussion_r175289630

---

From: Subodh Iyengar <[email-masked]>
Date: Sun, 18 Mar 2018 13:49:05 +0000 (UTC)

> @@ -725,9 +725,9 @@ sent, and then the RTO timer is set.
 
 #### Early Retransmit Alarm
 
-Early retransmit {{?RFC5827}} is implemented with a 1/4 RTT timer. It is
-part of QUIC's time based loss detection, but is always enabled, even when
-only packet reordering loss detection is enabled.
+Early retransmit {{?RFC5827}} is only used when using Fast Retransmit in packet
+number mode. It is implemented with a 1/4 RTT timer. It is part of QUIC's time

this seems a bit confusing, i think we could say something like

``We expect to receive feedback about the sent packet within 1 RTT of the time_since_sent. Early retransmit is implemented by setting a (1 + 1/4) RTT timer since the time the packet is sent. ```

-- 
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/1226#pullrequestreview-104797922

---

From: Subodh Iyengar <[email-masked]>
Date: Sun, 18 Mar 2018 14:01:19 +0000 (UTC)

sorry i meant first_unacked.

-- 
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/1226#discussion_r175290237