Subject: [quicwg/base-drafts] Use time_of_last_sent_packet (#923)

From: ianswett <[email-masked]>
Date: Mon, 13 Nov 2017 06:59:07 +0000 (UTC)

time_of_last_sent_packet should be used to set the loss detection timer, instead of using now.

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

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

-- Commit Summary --

  * Use time_of_last_sent_packet

-- File Changes --

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

-- Patch Links --

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

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Nov 2017 01:41:40 +0000 (UTC)

@MikeBishop pushed 1 commit.

aa00e34  Break long line in pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/923/files/8cbaaae4e1a634ebf7869244ed50a5eb78926579..aa00e34549f0193f20570d608563f9253720cc43

---

From: Subodh Iyengar <[email-masked]>
Date: Tue, 14 Nov 2017 01:48:16 +0000 (UTC)

siyengar commented on this pull request.

> @@ -687,7 +687,7 @@ Pseudocode for SetLossDetectionAlarm follows:
       alarm_duration = alarm_duration * (2 ^ handshake_count)
     else if (loss_time != 0):

this is also weird, it's possible that loss_time is greater than tlp timer or RTO, shouldn't we take the min(loss_time, rto_timer, tlp_timer) as the early retransmit timer, or at least min(loss_time, rto)

-- 
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/923#pullrequestreview-76309004

---

From: Subodh Iyengar <[email-masked]>
Date: Tue, 14 Nov 2017 02:02:47 +0000 (UTC)

> @@ -701,7 +701,8 @@ Pseudocode for SetLossDetectionAlarm follows:
       alarm_duration = max(alarm_duration, kMinRTOTimeout)
       alarm_duration = alarm_duration * (2 ^ rto_count)
 
-    loss_detection_alarm.set(now + alarm_duration)
+    loss_detection_alarm.set(time_of_last_sent_packet

for handshake packets should this be the time since the last handshake packet was transmitted?

-- 
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/923#pullrequestreview-76311210

---

From: Subodh Iyengar <[email-masked]>
Date: Sat, 18 Nov 2017 00:58:03 +0000 (UTC)

github won't let me comment on the particular line of code, however 

in detect loss packets the delay until lost is also wrong, it should be 

1/8 * max(srtt, lrtt) vs 9/8 -> or maybe it should be 1 /4 because we should at least wait for a 1/4 rtt
and it should be timereordering vs 1 + timereordering.

-- 
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/923#issuecomment-345404724

---

From: Lars Eggert <[email-masked]>
Date: Sat, 18 Nov 2017 14:19:37 +0000 (UTC)

larseggert approved this pull request.

-- 
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/923#pullrequestreview-77616994

---

From: ianswett <[email-masked]>
Date: Tue, 28 Nov 2017 14:24:48 +0000 (UTC)

ianswett commented on this pull request.

That's a good point, given the other packets are not decryptable, and hence will not generate an acknowledgement until the handshake completes, #962 filed.

-- 
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/923#discussion_r153505505

---

From: janaiyengar <[email-masked]>
Date: Thu, 30 Nov 2017 02:02:35 +0000 (UTC)

janaiyengar approved this pull request.

I dont think this is an issue. This is at the beginning of the connection, and loss_time only applies to early retransmit. It's possible that loss_time is > TLP or RTO period, but that would be very very strange.

-- 
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/923#pullrequestreview-80058852

---

From: janaiyengar <[email-masked]>
Date: Wed, 29 Nov 2017 18:02:43 -0800

Merged #923.

-- 
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/923#event-1364794928