Subject: [quicwg/base-drafts] Server SHOULD send full-sized datagrams until the path is validated (#4188)

From: Kazuho Oku <[email-masked]>
Date: Wed, 07 Oct 2020 21:12:00 -0700

Closes #4183.

The recommendation is written so that it would prevent the two failure modes discussed in the issue:
* client and server doing ping-pong due to only the tail of the server-sent packets reaching the client
* the connection getting stuck after the handshake is complete, when Initial and Handshake packets will be small (particularly when TLS session tickets are used)
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * server sends full-size datagrams until the path is validated

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Wed, 07 Oct 2020 22:00:52 -0700

@martinthomson commented on this pull request.

I think that this is better moved to [Section 14.1](https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#name-initial-packet-size), with a reference back to this section instead.  The purpose of this section is to talk about address validation, not the minimum MTU guarantee, which is what this recommendation safeguards.

> +Until the path is validated, a server SHOULD pad the Initial, Handshake, 1-RTT
+packets it sends so that their UDP payload size will be at least 1200 bytes.

```suggestion
Until the path is validated, a server SHOULD pad all
packets it sends so that their UDP payload size will be at least 1200 bytes.
```

> @@ -1943,6 +1943,11 @@ the client during connection establishment with a Retry packet (see
 {{validate-retry}}) or in a previous connection using the NEW_TOKEN frame (see
 {{validate-future}}).
 
+Until the path is validated, a server SHOULD pad the Initial, Handshake, 1-RTT
+packets it sends so that their UDP payload size will be at least 1200 bytes.
+Doing so guarantees that the handshake would make progress only when the path is
+capable of handling QUIC traffic (see {{packet-size}}).

```suggestion
capable of handling QUIC traffic; see {{packet-size}}.
```

-- 
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/4188#pullrequestreview-504428817

---

From: Martin Thomson <[email-masked]>
Date: Wed, 07 Oct 2020 22:06:03 -0700

Maybe also:

```suggestion
capable of handling QUIC traffic in both directions; see {{packet-size}}.
```

-- 
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/4188#discussion_r501450489

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 07 Oct 2020 22:59:29 -0700

@kazuho commented on this pull request.

The server cannot pad Retry packets.

-- 
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/4188#discussion_r501465912

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 07 Oct 2020 22:59:42 -0700

@kazuho pushed 1 commit.

1214c15bdd7a8658856cca438d8c73a662dc9049  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/6e76817342368a2da09ab0d0dd960d550cb7a78c..1214c15bdd7a8658856cca438d8c73a662dc9049

---

From: Marten Seemann <[email-masked]>
Date: Wed, 07 Oct 2020 23:12:20 -0700

@marten-seemann requested changes on this pull request.

You don't need to pad Handshake packets, if you consistently pad Initials: Without the ServerHello, the client won't be able to decrypt the Handshake packet.

-- 
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/4188#pullrequestreview-504454604

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 07 Oct 2020 23:24:14 -0700

This is necessary to prevent the ping-pong that is being described in [the issue](https://github.com/quicwg/base-drafts/issues/4183#issue-715343219) which is caused by the receipt of a Handshake packet triggering a retransmission.

-- 
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/4188#discussion_r501474525

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 07 Oct 2020 23:27:10 -0700

@martinthomson Thank you for the review.

> I think that this is better moved to Section 14.1, with a reference back to this section instead. The purpose of this section is to talk about address validation, not the minimum MTU guarantee, which is what this recommendation safeguards.

Is the suggestion to move also the client-side padding requirement in section 8.1 (MUST pad Initial packets) to section 14.1? The intent of adding the text to section 8.1 was so that the paragraph goes like:
* client MUST pad all Initial packets
* servers can send a Retry
* servers SHOULD pad packets until the path is being validated.

I'm fine with expanding the last bullet point to cover both endpoints, though I thin we might want to keep the first bullet point and the third point in the same section?

-- 
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/4188#issuecomment-705359466

---

From: Martin Thomson <[email-masked]>
Date: Thu, 08 Oct 2020 00:11:36 -0700

I see your logic about placement and that makes sense.  I would ask that you reference this from 14.1 though, as that is where people might look for packet size requirements.

Thanks for pointing out Retry; all except Retry would be fine.

```suggestion
Until the path is validated, a server SHOULD pad all packets it sends, other
than Retry, so that their UDP payload size will be at least 1200 bytes.
```

-- 
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/4188#pullrequestreview-504485288

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 00:38:09 -0700

@kazuho pushed 1 commit.

78500d16536f14fd24492707d51916395270947a  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/1214c15bdd7a8658856cca438d8c73a662dc9049..78500d16536f14fd24492707d51916395270947a

---

From: Marten Seemann <[email-masked]>
Date: Thu, 08 Oct 2020 02:02:39 -0700

@marten-seemann commented on this pull request.

If you pad all Initial packets (meaning: all datagrams containing Initial), you don't need to pad Handshake packets. An undecryptable packet doesn't reset any timers, so if won't lead to the ping-pong you describe.

This is easier to implement (for me at least), since I can use the same logic for packing packets on the client and server side: Pad all packets that contain an Initial.

-- 
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/4188#discussion_r501560543

---

From: Nick Banks <[email-masked]>
Date: Thu, 08 Oct 2020 08:14:30 -0700

@nibanks commented on this pull request.

I agree with @marten-seemann. If the first flight pads in both directions, ACKs of those packets should be enough to validate the 1200 MTU requirement, and further packets shouldn't be required to pad, right?

-- 
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/4188#discussion_r501801902

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 08 Oct 2020 14:36:33 -0700

@janaiyengar approved this pull request.

Editorial suggestion, but this LGTM, thanks @kazuho for writing it up.

@marten-seemann @nibanks : https://quicwg.org/base-drafts/draft-ietf-quic-recovery.html#section-6.2.3-1 suggests that a client can retransmit on receiving HS packets before it has keys for them, which can cause a ping-pong.

> +Until the path is validated, a server SHOULD pad all packets it sends, other
+than Retry, so that their UDP payload size will be at least 1200 bytes.
+Doing so guarantees that the handshake would make progress only when the path is
+capable of handling QUIC traffic; see {{packet-size}}.

```suggestion
Until the client address is validated, a server SHOULD ensure that all
packets it sends, with the exception of Retry packets, have a UDP payload
size of at least 1200 bytes. Doing so ensures that the handshake progresses
only if the path is capable of handling QUIC traffic; see {{packet-size}}.
```

-- 
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/4188#pullrequestreview-505135394

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 16:47:18 -0700

@marten-seemann @nibanks I'm not sure if I agree.

Let me start by pointing out the basics that I think is true, before I dive into the details of the recovery draft:
* When the path MTU is below 1200 bytes, it is better to make sure that none of the packets reach the client rather than letting some pass through, because client might extend their timeout if they receive some packets from the server.
* Machinery defined in the transport draft should not depend on the specifics of the recovery draft.
* What we are trying to provide is a recommendation (SHOULD), and that should be the best practice rather than the minimum that an endpoint is supposed to do. And because it is merely a recommendation, endpoints can adopt alternative strategies.

Regarding the specifics of the recovery draft, [the second paragraph of the recovery draft](https://quicwg.org/base-drafts/draft-ietf-quic-recovery.html#section-6.2.3-2) states that an server MAY retransmit CRYPTO data before the PTO timer expires, when it receives an Initial packet containing duplicate CRYPTO data.

The question is if this paragraph prohibits servers from retransmitting Handshake packets (or 1-RTT packets) containing CRYPTO data. If such action is prohibited, then we will not have ping-pong. If such action is _not_ prohibited, we might see ping-ping if we only pad datagrams carrying Initial packets. That is because if only the Handshake packets (or the 1-RTT packets) reach the client, the client is explicitly allowed to retransmit Initial packets containing CRYPTO data.

My understanding is that such action is _not_ prohibited. In fact, I would argue that the intent of this section is to allow such an action.

Consider the case where the server sent three datagrams in its first flight: 1) a full-sized datagram carrying an Initial and a Handshake packet, 2) a full-sized datagram carrying a Handshake packet, 3) a half-sized datagram carrying a Handshake packet.

When the server receives an Initial carrying duplicate CRYPTO data, the draft states that it can assume that the Initial that the server sent has been lost. What does that exactly mean? The natural assumption would be that either or both of datagrams _2_ and _3_ reached the client, but _1_ did not. It is far less likely that the client received all the three datagrams, but there was a bit flip in the Initial packet, and that the client is buffering the Handshake packets it was unable to decrypt.

Then, why should a server limit the retransmission to only Initial data?

To summarize, my understanding is that section 6.2.3 of the recovery draft suggests that the server MAY retransmit Handshake packets when receiving an Initial packet that contain duplicate CRYPTO data. Therefore, there's the possibility that we would see a ping-pong even with a server that is following the recommendations in the recovery draft, unless it pads all packets that it sends until the path is validated.

-- 
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/4188#discussion_r502082470

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 16:48:07 -0700

@kazuho pushed 1 commit.

5727b871212a92c82bd6d6563341eaf9ad8e5507  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/78500d16536f14fd24492707d51916395270947a..5727b871212a92c82bd6d6563341eaf9ad8e5507

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 16:58:38 -0700

@kazuho pushed 1 commit.

56569b9dcce3788d24b3dfc6576603f6ffca1da9  wordwrap

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/5727b871212a92c82bd6d6563341eaf9ad8e5507..56569b9dcce3788d24b3dfc6576603f6ffca1da9

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 17:02:22 -0700

@kazuho pushed 1 commit.

8ec73e164c39a30c5ed7f803c34add8af989ad08  update back reference to cover padding

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/56569b9dcce3788d24b3dfc6576603f6ffca1da9..8ec73e164c39a30c5ed7f803c34add8af989ad08

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 17:03:21 -0700

@martinthomson Thank you for the re-review, back reference has been added in 8ec73e1.

-- 
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/4188#issuecomment-705890196

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 18:19:34 -0700

@kazuho pushed 1 commit.

97b8798895b524bf8c8f1f35ae06d8386f938bed  reduce the recommendation of padding to Initial packets only; restrict the number of packets being sent pre-PTO

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/8ec73e164c39a30c5ed7f803c34add8af989ad08..97b8798895b524bf8c8f1f35ae06d8386f938bed

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 08 Oct 2020 18:27:40 -0700

@janaiyengar requested changes on this pull request.

Huh, it looks like you've changed your recommendation to only Initial packets. I think that ought to be all packets.

> @@ -4099,8 +4104,9 @@ bytes.  A server MAY also immediately close the connection by sending a
 CONNECTION_CLOSE frame with an error code of PROTOCOL_VIOLATION; see
 {{immediate-close-hs}}.
 
-The server MUST also limit the number of bytes it sends before validating the
-address of the client; see {{address-validation}}.
+Until the server validates the client address, a server MUST also limit the

```suggestion
Until the server validates the client address, it MUST limit the
```

> +the number of bytes it sends and SHOULD pad the Initial packets; see
+{{address-validation}}.

```suggestion
number of bytes it sends, and it SHOULD ensure that the UDP
payload of all datagrams it sends at least 1200 bytes; see
{{address-validation}}.
```

> @@ -663,9 +663,10 @@ in Initial packets, or the client's estimated RTT is too small. When a
 client receives Handshake or 1-RTT packets prior to obtaining Handshake keys,
 it may assume some or all of the server's Initial packets were lost.
 
-To speed up handshake completion under these conditions, an endpoint MAY send
-a packet containing unacknowledged CRYPTO data earlier than the PTO expiry,
-subject to the address validation limits in Section 8.1 of {{QUIC-TRANSPORT}}.
+To speed up handshake completion under these conditions, an endpoint MAY, for a
+limited number of occasions per each connection, send a packet containing

Why is this necessary?

+Until the client address is validated, servers SHOULD ensure that UDP datagrams
+containing Initial packets have UDP payloads of at least 1200 bytes.  Doing so

I would revert this to what you had earlier... I don't think there's a good reason to not recommend padding all packets, since client behavior is uncertain here. The SHOULD here means that implementations can deviate from this recommendation, as long as they know what they're doing.

-- 
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/4188#pullrequestreview-505289821

---

From: Marten Seemann <[email-masked]>
Date: Thu, 08 Oct 2020 19:52:18 -0700

Retry can validate the client's address, so the ping-pong could still happen in that case.

-- 
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/4188#discussion_r502156280

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 20:22:18 -0700

@kazuho pushed 1 commit.

27ff97756f68c02568320726c69948bfe439e828  Drop "unless the client address is validated", merging the requirements.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/97b8798895b524bf8c8f1f35ae06d8386f938bed..27ff97756f68c02568320726c69948bfe439e828

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 20:23:34 -0700

@marten-seemann Wow. That's true. Addressed in bd089c9.

-- 
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/4188#discussion_r502163473

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 20:28:32 -0700

@kazuho pushed 1 commit.

271010c6d38cbb015aebf41ccf391d64b09e5b1d  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/bd089c925ad5928c5448e45e04b3d40ef848078d..271010c6d38cbb015aebf41ccf391d64b09e5b1d

---

From: Martin Thomson <[email-masked]>
Date: Thu, 08 Oct 2020 20:29:51 -0700

+{{address-validation}}.

I want to keep the Initial piece.

This is the optimization that leads to the infinite regress.  This is attempting to say "A client MUST limit the number of times it sends Initial packets in response to packets it cannot process."

-- 
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/4188#discussion_r502164890

---

From: Martin Thomson <[email-masked]>
Date: Thu, 08 Oct 2020 20:30:33 -0700

> @@ -4099,8 +4101,9 @@ bytes.  A server MAY also immediately close the connection by sending a
 
-The server MUST also limit the number of bytes it sends before validating the
-address of the client; see {{address-validation}}.
+Until the server validates the client address, it MUST limit the
+the number of bytes it sends and SHOULD pad the Initial packets; see

```suggestion
the number of bytes it sends and SHOULD pad Initial packets; see
```

-- 
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/4188#pullrequestreview-505336097

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 20:32:08 -0700

@kazuho pushed 1 commit.

a3351beeb2561209fb2114c117e3dcae6288a755  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/271010c6d38cbb015aebf41ccf391d64b09e5b1d..a3351beeb2561209fb2114c117e3dcae6288a755

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 12 Oct 2020 14:21:28 -0700

> @@ -1920,10 +1920,12 @@ that are uniquely attributed to a single connection. This includes datagrams
 that contain packets that are successfully processed and datagrams that contain
 packets that are all discarded.
 
-Clients MUST ensure that UDP datagrams containing Initial packets have UDP
-payloads of at least 1200 bytes, adding PADDING frames as necessary.
-A client that sends padded datagrams allows the server to
-send more data prior to completing address validation.
+Clients MUST and servers SHOULD ensure that UDP datagrams containing Initial
+packets have UDP payloads of at least 1200 bytes, adding PADDING frames as
+necessary.  A client that sends padded datagrams allows the server to send more

Maybe it would be better to limit the scope and say _servers SHOULD pad Initial packets that carry crypto data_.

The problem of padding all Initial packets is that ACK-only Initial packets consume full MTU, when the send window is still controlled by the amplification limit.

-- 
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/4188#pullrequestreview-506923679

---

From: ianswett <[email-masked]>
Date: Mon, 12 Oct 2020 15:11:07 -0700

@ianswett commented on this pull request.

FWIW, I am happy with this being limited to once.

-- 
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/4188#discussion_r503559444

---

From: ianswett <[email-masked]>
Date: Mon, 12 Oct 2020 15:19:03 -0700

Some comments, but I agree this is a real issue, which for some reason I thought we already dealt with correctly.

@martinduke suggestion:

```suggestion
Any datagram that contains an Initial packet MUST be at least 1200 Bytes unless the packet contains
a CONNECTION_CLOSE frame.  A client that sends padded datagrams allows the server to send more
```

I don't think this change is necessary anymore.

I think making a carve-out for ACK-only packets sounds attractive, but it could result in the client sending an Initial, getting it acknowledged, but not receiving the server's Initial.  Then things get complex to reason about.

-- 
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/4188#pullrequestreview-506948972

---

From: Martin Thomson <[email-masked]>
Date: Mon, 12 Oct 2020 18:03:32 -0700

I implemented this last week, and it isn't quite as good if you only do it once.  It's better than zero, but multiple times is also OK.  The unlimited thing is the main problem.

I don't think that the special rules for ACK-only is enough to justify an exception.  I could tolerate non-ack-eliciting packets being exempt from this padding rule, but it seems unnecessary.  We already have one exception.

This CONNECTION_CLOSE thing is great.  It also means that it avoids us having to grapple with enforcement of the minimum size at the client.  Or at least, we can more directly recommend against enforcement.

I think that this is just a mistake.  As we worked out, validation of the client address is not the signal we want (see Retry).  There are two separate statements here:

> The server MUST also limit the number of bytes it sends before validating the address of the client.

And

> The server MUST pad any datagram containing an Initial packet, unless it contains a CONNECTION_CLOSE frame.

(Or SHOULD for the second, depending on your priors.)

-- 
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/4188#discussion_r503602206

---

From: martinduke <[email-masked]>
Date: Tue, 13 Oct 2020 11:46:33 -0700

@martinduke commented on this pull request.

I agree with @martinthomson. The amplification limit disappears when the server validates the address. The padding requirement/recommendation applies to all Initial packets, because you still have to probe the MTU, and the client is often not sure if the server has validated it.

-- 
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/4188#discussion_r504180494

---

From: martinduke <[email-masked]>
Date: Tue, 13 Oct 2020 11:53:45 -0700

Endpoints that don't coalesce are already trading performance for simplicity. The added sacrifice here isn't that large.

1) Clients: if it only receives an Initial, padding is critical to raising the amplification limit. If it gets a Handshake packet too, it will momentarily send a Handshake ACK that will allow it to free the Initial context and get the cwnd back.

2) Servers: If the client has a valid Retry or NEW_TOKEN token, there is no amplification limit. So the performance penalty only applies to a non-coalescing server when:
(a) There is no token, AND the Initial padding causes the handshake flight to go over the amplification limit.
(b) The initial padding causes the handshake flight to go over the initial cwnd.
I find it hard to believe a server deeply concerned about these losses would not coalesce packets for other reasons.

-- 
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/4188#discussion_r504184280

---

From: ianswett <[email-masked]>
Date: Tue, 13 Oct 2020 13:49:56 -0700

I really thought when we discussed this optimization, we said to only do this once, but either I'm misremembering or it never made it into the issue(#3080).

@martinthomson Would the case it's not as good would be repeated loss of the server's Initial packet?

-- 
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/4188#discussion_r504249869

---

From: Martin Thomson <[email-masked]>
Date: Tue, 13 Oct 2020 16:49:37 -0700

Yes, repeated loss is what I'm thinking about.  This is likely rare, but early losses on a flow are more common.  I don't have a firm opinion on this point. One is certainly better than none.  But two doesn't seem to be a real problem either.  For what it is worth, I send up to 4.

-- 
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/4188#discussion_r504321226

---

From: ianswett <[email-masked]>
Date: Tue, 13 Oct 2020 17:07:36 -0700

In my experience looking at traces, losing two server Initials in a row is quite rare, and there's still PTO as a backup, so I'm still leaning towards limiting this to once.  Maybe we can require limiting it and specify a RECOMMENDED value of 1 or 2?

-- 
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/4188#discussion_r504326519

---

From: Martin Thomson <[email-masked]>
Date: Tue, 13 Oct 2020 17:22:20 -0700

But now you are telling me what to do without cause :p  But I can live with a recommendation.  My point was that the only hard requirement is that this is limited.

-- 
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/4188#discussion_r504330499

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 13 Oct 2020 18:40:35 -0700

@janaiyengar commented on this pull request.

I'm fine with requiring limiting it. To be useful to implementers, we should have some value here, but I would not RECOMMEND it. We can simply say that experience has shown 1 early retransmission to be adequate to accommodate common loss scenarios.

-- 
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/4188#discussion_r504350824

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 13 Oct 2020 19:41:39 -0700

@kazuho pushed 1 commit.

0410b6055abae4e98292593fc0fd1ba04bfc0bb7  when to pad is independent from path validation

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/a3351beeb2561209fb2114c117e3dcae6288a755..0410b6055abae4e98292593fc0fd1ba04bfc0bb7

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 13 Oct 2020 19:42:23 -0700

Thank you folks. I think there's consensus that padding should better not be tied to path validation. Addressed the text in 0410b60. PTAL.

-- 
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/4188#discussion_r504366588

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 13 Oct 2020 19:48:54 -0700

@kazuho pushed 1 commit.

6e0fe53d45021108f4f562225fab0647bf5456de  at most once is likely enough

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/0410b6055abae4e98292593fc0fd1ba04bfc0bb7..6e0fe53d45021108f4f562225fab0647bf5456de

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 13 Oct 2020 19:49:47 -0700

Sounds like we are converging; took a stub in commit 6e0fe53. PTAL.

-- 
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/4188#discussion_r504368528

---

From: Martin Thomson <[email-masked]>
Date: Tue, 13 Oct 2020 22:11:16 -0700

@martinthomson approved this pull request.

I could tolerate a change to MUST for both client and server, if we added the two tweaks discussed:

* ack-eliciting packets are padded (this is safe for clients too, but less obviously good)
* CONNECTION_CLOSE is not ack-eliciting

I can definitely still live with this.

+data prior to completing address validation.  Endpoints using UDP datagrams of

```suggestion
data prior to completing address validation.  Endpoints sending UDP datagrams of
```

-- 
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/4188#pullrequestreview-507998276

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 13 Oct 2020 22:15:11 -0700

@kazuho pushed 1 commit.

681c02ed7bb3bb14a1c0fc3f4ae6ea91be466b70  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/6e0fe53d45021108f4f562225fab0647bf5456de..681c02ed7bb3bb14a1c0fc3f4ae6ea91be466b70

---

From: Martin Thomson <[email-masked]>
Date: Tue, 13 Oct 2020 22:29:15 -0700

FWIW, CONNECTION_CLOSE is already non-ack-eliciting.  I just forgot.

-- 
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/4188#issuecomment-708165931

---

From: martinduke <[email-masked]>
Date: Wed, 14 Oct 2020 10:23:51 -0700

@martinduke approved this pull request.

I can live with these changes, but I would like the CONNECTION_CLOSE text to go in and would prefer a MUST to a SHOULD for servers.

-- 
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/4188#pullrequestreview-508569523

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 13:16:28 -0700

I think nobody is opposing to having a MUST, but the debate is the specifics.

I was hoping if we could converge on the specifics of that MUST, but maybe I should update this PR in one way to narrow down the discussion.

-- 
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/4188#issuecomment-708636242

---

From: martinduke <[email-masked]>
Date: Wed, 14 Oct 2020 13:29:28 -0700

Thank you for clarifying that we agree on a MUST. I understand the concern about validation if we exempt certain packets; however, while regrettable this is no net increase in decrypt operations vs the status quo.

-- 
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/4188#issuecomment-708642559

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 13:39:50 -0700

@kazuho pushed 1 commit.

34ad87496a13ddc545917346096ed24f420c99a6  servers MUST pad Initial packets carrying CRYPTO frames

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/681c02ed7bb3bb14a1c0fc3f4ae6ea91be466b70..34ad87496a13ddc545917346096ed24f420c99a6

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 13:43:02 -0700

@kazuho pushed 1 commit.

c30347b1103d89f116f48f7b8d3b04aeae7828d0  wordwrap

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/34ad87496a13ddc545917346096ed24f420c99a6..c30347b1103d89f116f48f7b8d3b04aeae7828d0

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 13:46:58 -0700

I've updated the pull request so that it describes one specific way of having a MUST. Admittedly, it reflects my preference (the rationale explained in https://github.com/quicwg/base-drafts/issues/4183#issuecomment-708061598), but the hope is that having something specific being written down helps us continue the discussion.

-- 
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/4188#issuecomment-708650856

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 16:11:52 -0700

@kazuho pushed 1 commit.

67e49f85b8403ab8cd78f6cd985b78aa08bd9a9f  pad Initial with CRYPTO frames -> pad ack-eliciting Initial

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/c30347b1103d89f116f48f7b8d3b04aeae7828d0..67e49f85b8403ab8cd78f6cd985b78aa08bd9a9f

---

From: David Schinazi <[email-masked]>
Date: Wed, 14 Oct 2020 16:21:13 -0700

@DavidSchinazi approved this pull request.

Thanks for the discussion, this PR looks great!

-- 
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/4188#pullrequestreview-508836729

---

From: ianswett <[email-masked]>
Date: Wed, 14 Oct 2020 16:25:55 -0700

@ianswett approved this pull request.

One suggestion on wording, but overall this WFM.

> @@ -663,9 +663,11 @@ in Initial packets, or the client's estimated RTT is too small. When a
 
-To speed up handshake completion under these conditions, an endpoint MAY send
-a packet containing unacknowledged CRYPTO data earlier than the PTO expiry,
-subject to the address validation limits in Section 8.1 of {{QUIC-TRANSPORT}}.
+To speed up handshake completion under these conditions, an endpoint MAY, for a
+limited number of occasions per each connection, send a packet containing
+unacknowledged CRYPTO data earlier than the PTO expiry, subject to the address
+validation limits in Section 8.1 of {{QUIC-TRANSPORT}}. Doing so at most once
+for each connection will be adequate to accommodate common loss scenarios.

I think it might be worth highlighting the 'ping-pong' risk, so I tried to do that without using those words.

```suggestion
for each connection is adequate to quickly recover from a single packet loss
without incurring a risk of repeated packet reflection.
```

-- 
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/4188#pullrequestreview-508718152

---

From: Martin Thomson <[email-masked]>
Date: Wed, 14 Oct 2020 16:41:19 -0700

I don't think that the packet reflection this is well-enough explained to use it 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/4188#discussion_r505073753

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 16:45:58 -0700

I am in favor of @ianswett's suggestion.

The principles we have for preventing ping-pong are:
* do not repeatedly send an ACK-eliciting ACK
* when not receiving an ACK, rely on PTO backoff

This paragraph is talking when an endpoint MAY violate those rules. Therefore, IMO it is good to point out that risk.

-- 
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/4188#discussion_r505076997

---

From: Martin Thomson <[email-masked]>
Date: Wed, 14 Oct 2020 17:34:03 -0700

I would rather add:

> A client that does not limit the number of times it sends unacknowledged CRYPTO data in response to unauthenticated data risks creating an infinite exchange of packets.

There are other reasons why this might happen (hitting an endpoint that thinks it closed, for example.)

-- 
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/4188#discussion_r505097144

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 14 Oct 2020 17:57:43 -0700

Thanks, @kazuho. I can live with this. A few comments, editorial.

> +payloads of at least 1200 bytes, adding PADDING frames as necessary.  Servers
+MUST ensure that UDP datagrams containing ack-eliciting Initial packets have UDP
+payloads of at least 1200 bytes.  A client that sends padded datagrams allows
+the server to send more data prior to completing address validation.  Endpoints
+sending UDP datagrams of at least 1200 bytes ensures that the handshake
+progresses only if the path is capable of handling QUIC traffic; see
+{{packet-size}}.

```suggestion
payloads of at least 1200 bytes, adding PADDING frames as necessary.  Servers
MUST ensure that UDP datagrams containing ack-eliciting Initial packets have UDP
payloads of at least 1200 bytes. Sending UDP datagrams of at least 1200 bytes
ensures that the handshake progresses only if the path is capable of handling
QUIC traffic; see {{packet-size}}. Additionally, a client that sends padded
datagrams allows the server to send more data prior to completing address
validation.
```

> +{{packet-coalesce}}.  Similarly, a server MUST expand the payload of all UDP
+datagrams carrying ack-eliciting Initial packets.  Sending UDP datagrams of this
+size ensures that the network path supports a reasonable Path Maximum
+Transmission Unit (PMTU), in both directions.  Additionally, a client padding
+Initial packets helps reduce the amplitude of amplification attacks caused by
+server responses toward an unverified client address; see

Can you spell it out fully for the server? "A server MUST similarly expand the payload of all UDP datagrams carrying ack-eliciting Initial packets to at least the smallest allowed maximum packet size (1200 bytes)."

Also, "a client padding Initial packets" --> "a client that expands Initial packets"

>  {{address-validation}}.
 
-Datagrams containing Initial packets MAY exceed 1200 bytes if the client
+Datagrams containing Initial packets MAY exceed 1200 bytes if the endpoint

```suggestion
Datagrams containing Initial packets MAY exceed 1200 bytes if the sender
```

> +address of the client, and MUST pad ack-eliciting Initial packets that it sends;
+see {{address-validation}}.

```suggestion
address of the client; see {{address-validation}}.
```

I don't think you need this here, since this is described earlier in this very section.

> +payloads of at least 1200 bytes, adding PADDING frames as necessary.  Servers
+MUST ensure that UDP datagrams containing ack-eliciting Initial packets have UDP
+payloads of at least 1200 bytes.  A client that sends padded datagrams allows
+the server to send more data prior to completing address validation.  Endpoints
+sending UDP datagrams of at least 1200 bytes ensures that the handshake
+progresses only if the path is capable of handling QUIC traffic; see
+{{packet-size}}.

In addition, let's remove the server text from here, since that is unnecessary for address validation. The server datagram padding needs to only be mentioned below, in the Initial Packet Size section.

-- 
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/4188#pullrequestreview-508861314

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:26:35 -0700

@kazuho pushed 2 commits.

3518d540808164672fef304f85585488c5e176a0  editorial
a51ad561b1d5c08274a4e8a1ac14304845c74748  @martinthomson's suggestion with tweaks

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/67e49f85b8403ab8cd78f6cd985b78aa08bd9a9f..a51ad561b1d5c08274a4e8a1ac14304845c74748

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:29:28 -0700

In 3518d54, applied changes suggested by @ianswett to the existing sentence.
In a51ad56, added the rationale suggested by @martinthomson with tweaks. Specifically, I changed the subject to "endpoints" so that it would be generic, and used "back off", as we suggest using that for Stateless Resets, etc.

PTAL.

-- 
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/4188#discussion_r505111499

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:31:03 -0700

@kazuho pushed 1 commit.

1bb33b1e11f25c783ab23c945a277968df4ecd99  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/a51ad561b1d5c08274a4e8a1ac14304845c74748..1bb33b1e11f25c783ab23c945a277968df4ecd99

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:31:36 -0700

@kazuho pushed 1 commit.

3f875caf7239cb071601ea46939f545b6687a5a8  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/1bb33b1e11f25c783ab23c945a277968df4ecd99..3f875caf7239cb071601ea46939f545b6687a5a8

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:36:12 -0700

@kazuho pushed 1 commit.

e3a596f2eb224e0fa1cbcd5865f4a4a70b7dbe3d  revert changes to the address validation section, as server-side requirements are unrelated

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/3f875caf7239cb071601ea46939f545b6687a5a8..e3a596f2eb224e0fa1cbcd5865f4a4a70b7dbe3d

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:36:39 -0700

@janaiyengar Following your last comment, I've reverted the changes to this section (i.e. address validation). That means that the suggested change right above is not applied. Please see if you want anything to be resurrected.

-- 
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/4188#discussion_r505113525

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:40:15 -0700

@kazuho pushed 1 commit.

c550cb4399b41241f7394d8a2d5728fb87b9f72b  editorial

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/e3a596f2eb224e0fa1cbcd5865f4a4a70b7dbe3d..c550cb4399b41241f7394d8a2d5728fb87b9f72b

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:40:30 -0700

Thank you for the suggestions. Done in c550cb4.

-- 
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/4188#discussion_r505114581

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 18:41:23 -0700

@kazuho pushed 1 commit.

adbe600fae62a5aaac76876a634baf2c00575c04  wordwrap

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4188/files/c550cb4399b41241f7394d8a2d5728fb87b9f72b..adbe600fae62a5aaac76876a634baf2c00575c04

---

From: Martin Thomson <[email-masked]>
Date: Wed, 14 Oct 2020 18:52:21 -0700

Tweaks.

> @@ -663,9 +663,13 @@ in Initial packets, or the client's estimated RTT is too small. When a
 
-To speed up handshake completion under these conditions, an endpoint MAY send
-a packet containing unacknowledged CRYPTO data earlier than the PTO expiry,
-subject to the address validation limits in Section 8.1 of {{QUIC-TRANSPORT}}.
+To speed up handshake completion under these conditions, an endpoint MAY, for a
+limited number of occasions per each connection, send a packet containing
+unacknowledged CRYPTO data earlier than the PTO expiry, subject to the address
+validation limits in Section 8.1 of {{QUIC-TRANSPORT}}. Doing so at most once
+for each connection is adequate to quickly recover from a single packet loss.
+Endpoints that do not back off from retransmitting packets in response to

```suggestion
Endpoints that do not cease retransmitting packets in response to
```

> @@ -663,9 +663,13 @@ in Initial packets, or the client's estimated RTT is too small. When a
 
-To speed up handshake completion under these conditions, an endpoint MAY send
-a packet containing unacknowledged CRYPTO data earlier than the PTO expiry,
-subject to the address validation limits in Section 8.1 of {{QUIC-TRANSPORT}}.
+To speed up handshake completion under these conditions, an endpoint MAY, for a
+limited number of occasions per each connection, send a packet containing
+unacknowledged CRYPTO data earlier than the PTO expiry, subject to the address
+validation limits in Section 8.1 of {{QUIC-TRANSPORT}}. Doing so at most once
+for each connection is adequate to quickly recover from a single packet loss.
+Endpoints that do not back off from retransmitting packets in response to
+unauthenticated data risks creating an infinite exchange of packets.

```suggestion
unauthenticated data risk creating an infinite exchange of packets.
```

-- 
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/4188#pullrequestreview-508883020

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 19:15:33 -0700

@kazuho pushed 1 commit.

8f5dff7dddbf7936c4ef3f6934d997fb8f1a216a  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/4188/files/adbe600fae62a5aaac76876a634baf2c00575c04..8f5dff7dddbf7936c4ef3f6934d997fb8f1a216a

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 14 Oct 2020 19:20:01 -0700

@kazuho pushed 1 commit.

b1ab5f8a6ccd1aecb0ac44e38b4a660c4ef7fd11  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/4188/files/8f5dff7dddbf7936c4ef3f6934d997fb8f1a216a..b1ab5f8a6ccd1aecb0ac44e38b4a660c4ef7fd11

---

From: Martin Thomson <[email-masked]>
Date: Thu, 15 Oct 2020 16:34:25 -0700

Merged #4188 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/4188#event-3883965663

---

From: Martin Thomson <[email-masked]>
Date: Thu, 15 Oct 2020 16:35:14 -0700

Thanks for patiently working through this @kazuho.  This was difficult, but the fix was, in the end, fairly tidy.

-- 
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/4188#issuecomment-709643748

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 15 Oct 2020 16:48:03 -0700

Apologies for leaving the "Requires Changes" flag on. This PR LGTM.

-- 
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/4188#issuecomment-709647015

---

From: Marten Seemann <[email-masked]>
Date: Thu, 15 Oct 2020 20:24:27 -0700

Why was this already merged? I don't believe that this PR is a good solution for the issue, as I've detailed in https://github.com/quicwg/base-drafts/issues/4183#issuecomment-708876280. 

I still think we should include ACK-only packets in the padding requirement. This would allow us to get rid of the change made in this PR that limits the number of retransmissions on undecryptable packets, and it would prevent a ping-pong (even if that ping-pong comes with an exponential backoff).

-- 
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/4188#issuecomment-709708138