Subject: Re: [quicwg/base-drafts] Ack Delay and TLP (#1796)

From: Rui Paulo <[email-masked]>
Date: Mon, 01 Oct 2018 10:07:08 -0700

Ian, what did you have in mind?  I wasn't suggesting to count full sized packets, just to allow a constant to be variable.

-- 
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/1796#issuecomment-425985847

---

From: ianswett <[email-masked]>
Date: Mon, 01 Oct 2018 12:21:25 -0700

I realize kMaxDatagramSize is in the constants section, but I'm not sure it should be a constant throughout a connection, given the existence of PMTU, so it's inclusion there may be incorrect or at least misleading.

I was thinking of tracking the number of in flight packets that are >= 1200 bytes on the sender side in addition to the number of bytes in flight and then on the receiver side, only send an ack immediately if two packets >1200 bytes are received.  That would keep the concept of full-sized packets while also allow changing the MTU during a connection without any other issues.

As I mention it, I realize this has the unintended side effect that if the sender wants to delay acks, it can send a 1199 byte packet instead of 1200 byte one.  I'm not sure if that's a feature or a bug.

-- 
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/1796#issuecomment-426030508

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:12:33 -0700

@ianswett pushed 1 commit.

4fc23d7  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/0c4826f48f682fd856cd6581a9bafdf6216fd736..4fc23d782e77d7b1cb2263dc67d22e3930973738

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:14:15 -0700

037bb0e  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/4fc23d782e77d7b1cb2263dc67d22e3930973738..037bb0e5bc518a59d08eaedf53de123b86dc402a

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 15:16:06 +0000 (UTC)

01018b9  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/037bb0e5bc518a59d08eaedf53de123b86dc402a..01018b9ebeedb9946019f5b0748cb734bfdb4c6f

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:16:32 -0700

b0d513d  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/01018b9ebeedb9946019f5b0748cb734bfdb4c6f..b0d513dfa2297bd06a86fe674d891f8e1d59203a

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:16:55 -0700

4ce2b80  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/b0d513dfa2297bd06a86fe674d891f8e1d59203a..4ce2b80ed6d8de5b6e70a1fdef11e023f197d370

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:18:40 -0700

8cf0183  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/4ce2b80ed6d8de5b6e70a1fdef11e023f197d370..8cf01838d92c3d13e06585e58fd70291103ce952

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:20:56 -0700

2202c69  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/8cf01838d92c3d13e06585e58fd70291103ce952..2202c699fa69585f9632d03ea27c225080d7d24a

---

From: MikkelFJ <[email-masked]>
Date: Tue, 02 Oct 2018 08:21:37 -0700

mikkelfj commented on this pull request.

> -or ACN_ECN.  Specifically, implementations MUST attempt to enforce a maximum
-ack delay to avoid causing the peer spurious timeouts.  The RECOMMENDED maximum
-ack delay in QUIC is 25ms.
-
-An acknowledgement MAY be sent for every second full-sized packet, as TCP does
-{{?RFC5681}}, or may be sent less frequently, as long as the delay does not
-exceed the maximum ack delay.  QUIC recovery algorithms do not assume the peer
+excessively delay acknowledgements of packets containing frames other than ACK.
+Specifically, implementations MUST attempt to enforce a maximum
+ack delay to avoid causing the peer spurious timeouts.  The maximum ack delay
+is communicated in the `max_ack_delay` transport parameter and the default
+value is 25ms.
+
+An acknowledgement SHOULD be sent immediately upon receiving two or more
+retransmitable full-sized packets, as TCP does {{?RFC5681}}.  QUIC recovery
+algorithms, including tail loss probe{{tail-loss-probe}}, assume the peer

How do you define full-sized packets when this depends on PMTU discovery?
The text also isn't very clear about receiving drips of full sized packets: can you ack after only one full packet if it exceeds max ACK delay? I'd say obviously yes, but I'm sure the text is very clear (before or ofter this change).

-- 
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/1796#pullrequestreview-160778419

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:22:12 -0700

6117a2d  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/2202c699fa69585f9632d03ea27c225080d7d24a..6117a2de2c7788940578c0519c70bbc999085752

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:24:08 -0700

f199e21  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/6117a2de2c7788940578c0519c70bbc999085752..f199e2163f9e64648fbd2ed3d618f889208db87d

---

From: ianswett <[email-masked]>
Date: Tue, 02 Oct 2018 08:32:03 -0700

058c0f2  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/f199e2163f9e64648fbd2ed3d618f889208db87d..058c0f222a67fa527b2429658e4499f0ecf94337

---

From: janaiyengar <[email-masked]>
Date: Tue, 02 Oct 2018 08:53:12 -0700

janaiyengar commented on this pull request.

> @@ -446,16 +433,16 @@ flight, since this packet adds network load without establishing packet loss.
 ## Generating Acknowledgements
 
 QUIC SHOULD delay sending acknowledgements in response to packets, but MUST NOT
-excessively delay acknowledgements of packets containing frames other than ACK
-or ACN_ECN.  Specifically, implementations MUST attempt to enforce a maximum
-ack delay to avoid causing the peer spurious timeouts.  The RECOMMENDED maximum
-ack delay in QUIC is 25ms.
-
-An acknowledgement MAY be sent for every second full-sized packet, as TCP does
-{{?RFC5681}}, or may be sent less frequently, as long as the delay does not
-exceed the maximum ack delay.  QUIC recovery algorithms do not assume the peer
-generates an acknowledgement immediately when receiving a second full-sized
-packet.
+excessively delay acknowledgements of packets containing frames other than ACK.
+Specifically, implementations MUST attempt to enforce a maximum
+ack delay to avoid causing the peer spurious timeouts.  The maximum ack delay
+is communicated in the `max_ack_delay` transport parameter and the default

s/default/RECOMMENDED

-- 
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/1796#pullrequestreview-160794798

---

From: janaiyengar <[email-masked]>
Date: Tue, 02 Oct 2018 09:02:23 -0700

@janaiyengar pushed 1 commit.

6e3f1e8  Nits

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1796/files/058c0f222a67fa527b2429658e4499f0ecf94337..6e3f1e80b8ad599f23ef717d0d2739c1adf0058f

---

From: janaiyengar <[email-masked]>
Date: Tue, 02 Oct 2018 09:04:07 -0700

janaiyengar 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/1796#pullrequestreview-160800132

---

From: janaiyengar <[email-masked]>
Date: Tue, 02 Oct 2018 09:04:24 -0700

Merged #1796 into master.

-- 
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/1796#event-1879752752