Subject: [quicwg/base-drafts] Prohibit PADDING in response to ACK (#3104)

From: martinduke <[email-masked]>
Date: Wed, 16 Oct 2019 15:44:13 -0700

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

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

-- Commit Summary --

  * Prohibit PADDING in response to ACK

-- File Changes --

    M draft-ietf-quic-transport.md (17)

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Wed, 16 Oct 2019 19:12:15 -0700

ianswett commented on this pull request.

Some editorial suggestions, including improving the existing text.

> @@ -3061,14 +3061,15 @@ of doing so.
 
 Packets containing only ACK frames are not congestion controlled, so there are
 limits on how frequently they can be sent.  An endpoint MUST NOT send more than
-one ACK-frame-only packet in response to receiving an ack-eliciting packet (one
-containing frames other than ACK and/or PADDING).  An endpoint MUST NOT send a
-packet containing only an ACK frame in response to a non-ack-eliciting packet
-(one containing only ACK and/or PADDING frames), even if there are packet gaps
-which precede the received packet. Limiting ACK frames avoids an infinite
-feedback loop of acknowledgements, which could prevent the connection from ever
-becoming idle. However, the endpoint acknowledges non-ack-eliciting packets when
-it sends an ACK frame.
+one ACK-frame-only packet in response to receiving an ACK-eliciting packet

Maybe we should shorten this to "ACK-only packet" or even "ACK packet" and then define it at the beginning of the document?

> @@ -3061,14 +3061,15 @@ of doing so.
 
 Packets containing only ACK frames are not congestion controlled, so there are
 limits on how frequently they can be sent.  An endpoint MUST NOT send more than
-one ACK-frame-only packet in response to receiving an ack-eliciting packet (one
-containing frames other than ACK and/or PADDING).  An endpoint MUST NOT send a
-packet containing only an ACK frame in response to a non-ack-eliciting packet
-(one containing only ACK and/or PADDING frames), even if there are packet gaps
-which precede the received packet. Limiting ACK frames avoids an infinite
-feedback loop of acknowledgements, which could prevent the connection from ever
-becoming idle. However, the endpoint acknowledges non-ack-eliciting packets when
-it sends an ACK frame.
+one ACK-frame-only packet in response to receiving an ACK-eliciting packet
+(one containing frames other than ACK and/or PADDING).  An endpoint MUST NOT
+send a packet containing only an ACK frame, or only an ACK and PADDING frame,
+in response to a non-ACK-eliciting packet (one containing only ACK and/or

I'd remove the "one containing only ACK and/or PADDING frames)" now that it's well defined.

> @@ -3061,14 +3061,15 @@ of doing so.
 
 Packets containing only ACK frames are not congestion controlled, so there are
 limits on how frequently they can be sent.  An endpoint MUST NOT send more than
-one ACK-frame-only packet in response to receiving an ack-eliciting packet (one
-containing frames other than ACK and/or PADDING).  An endpoint MUST NOT send a
-packet containing only an ACK frame in response to a non-ack-eliciting packet
-(one containing only ACK and/or PADDING frames), even if there are packet gaps
-which precede the received packet. Limiting ACK frames avoids an infinite
-feedback loop of acknowledgements, which could prevent the connection from ever
-becoming idle. However, the endpoint acknowledges non-ack-eliciting packets when
-it sends an ACK frame.
+one ACK-frame-only packet in response to receiving an ACK-eliciting packet
+(one containing frames other than ACK and/or PADDING).  An endpoint MUST NOT

```suggestion
.  An endpoint MUST NOT
```

-- 
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/3104#pullrequestreview-302968984

---

From: martinduke <[email-masked]>
Date: Wed, 16 Oct 2019 23:43:40 -0700

martinduke commented on this pull request.

Actually, non-ack-eliciting is already defined, and would make this flow much better.

-- 
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/3104#discussion_r335831784

---

From: martinduke <[email-masked]>
Date: Wed, 16 Oct 2019 23:53:56 -0700

@martinduke pushed 1 commit.

6f72fe83d554c96c55e1e2485c30287184639b29  Took Ian's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3104/files/1a8baf8a2564bf657a040eb58ca5a50c98204aee..6f72fe83d554c96c55e1e2485c30287184639b29

---

From: martinduke <[email-masked]>
Date: Wed, 16 Oct 2019 23:58:54 -0700

ce83dc4ef954710bddbab03e49e9d1b966106743  addressed comment I missed:

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3104/files/6f72fe83d554c96c55e1e2485c30287184639b29..ce83dc4ef954710bddbab03e49e9d1b966106743

---

From: Nick Banks <[email-masked]>
Date: Thu, 17 Oct 2019 08:14:02 -0700

nibanks approved this pull request.

LGTM. Though all the "non-ACK-eliciting packets" is a bit of a mouth full.

-- 
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/3104#pullrequestreview-303336657

---

From: ianswett <[email-masked]>
Date: Fri, 18 Oct 2019 20:45:13 -0700

ianswett 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/3104#pullrequestreview-304193946

---

From: Martin Thomson <[email-masked]>
Date: Mon, 21 Oct 2019 18:49:57 -0700

martinthomson approved this pull request.

> @@ -3061,14 +3061,13 @@ of doing so.
 
 Packets containing only ACK frames are not congestion controlled, so there are
 limits on how frequently they can be sent.  An endpoint MUST NOT send more than
-one ACK-frame-only packet in response to receiving an ack-eliciting packet (one
-containing frames other than ACK and/or PADDING).  An endpoint MUST NOT send a
-packet containing only an ACK frame in response to a non-ack-eliciting packet
-(one containing only ACK and/or PADDING frames), even if there are packet gaps
-which precede the received packet. Limiting ACK frames avoids an infinite
-feedback loop of acknowledgements, which could prevent the connection from ever
-becoming idle. However, the endpoint acknowledges non-ack-eliciting packets when
-it sends an ACK frame.
+one ACK-frame-only packet in response to receiving an ack-eliciting packet.  An
+endpoint MUST NOT send a non-ack-eliciting packet in response to a non-ack-
+eliciting packet, even if there are packet gaps which precede the received

A nit:  Though this hyphenation looks sane when presented this way, it won't be guaranteed to render nicely.  It might appear as `non-ack- eliciting` (note the extra space).  I think that you should remove the wrapping on the hyphen.

-- 
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/3104#pullrequestreview-304934649

---

From: martinduke <[email-masked]>
Date: Tue, 22 Oct 2019 03:40:51 +0000 (UTC)

40380a041495ef042562f3c3b8bcd224d704c206  MT's nit

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3104/files/ce83dc4ef954710bddbab03e49e9d1b966106743..40380a041495ef042562f3c3b8bcd224d704c206

---

From: Martin Thomson <[email-masked]>
Date: Tue, 22 Oct 2019 15:19:59 -0700

Merged #3104 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/3104#event-2735018128