Subject: [quicwg/base-drafts] Martin Duke's nits (#4449)

From: Martin Thomson <[email-masked]>
Date: Tue, 22 Dec 2020 00:01:07 -0800

This should address all of them at once; one per commit for those that I think need action.

This is probably OK:

&gt; 13.3 &quot;it is not forbidden to retransmit copies of frames from lost packets&quot; Is this true for PATH_CHALLENGE? I thought this quite explicitly shouldn&#39;t be copied.

This is not expressly forbidden.  We do say [this](https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#section-8.2.1-4):

&gt; The endpoint MUST use unpredictable data in every PATH_CHALLENGE frame so that it can associate the peer&#39;s response with the corresponding PATH_CHALLENGE.

Which doesn&#39;t really contradict the other.  FWIW, I&#39;m not a fan of allowing frames to be copied, implementations really should do that, but it&#39;s a consequence of having frame idempotency.

This is one I&#39;m not sure about:

&gt; 14 &quot;Thus, modern IPv4 and all IPv6 network paths will be able to support QUIC.&quot; Generally true, but should be qualified for the presence of arbitrary numbers of tunnels.

What is the prevailing advice regarding MTU and tunnels?  Clearly when you tunnel you get a smaller MTU, but I haven&#39;t seen what happens when IPv6 is tunneled over a network that has a minimum MTU.  Most paths don&#39;t have that small an MTU, so it might be that things just fail.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * STREAM_DATA_BLOCKED is just for stream limits
  * Only the Retry token is invalid here
  * Stream flow control can stop in Reset Recvd too
  * Connection ID length isn&#39;t a varint either
  * Cite QUIC-TLS for HRR definition
  * A token might cause an Initial to grow beyond the MTU
  * Remove redundant text

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Tue, 22 Dec 2020 08:36:42 -0800

@ianswett approved this pull request.

> @@ -4968,11 +4967,13 @@ Other than updating the Destination Connection ID and Token fields, the Initial
 packet sent by the client is subject to the same restrictions as the first
 Initial packet.  A client MUST use the same cryptographic handshake message it
 included in this packet.  A server MAY treat a packet that contains a different
-cryptographic handshake message as a connection error or discard it.
+cryptographic handshake message as a connection error or discard it.  Note that
+including a Token field might reduce the available space for the cryptographic

```suggestion
including a Token field reduces the available space for the cryptographic
```

-- 
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/4449#pullrequestreview-557210783

---

From: martinduke <[email-masked]>
Date: Tue, 22 Dec 2020 13:31:07 -0800

@martinduke 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/4449#pullrequestreview-557378020

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 22 Dec 2020 13:45:38 -0800

@janaiyengar approved this pull request.

This looks good to me, with @ianswett 's nit.

-- 
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/4449#pullrequestreview-557384394

---

From: Martin Thomson <[email-masked]>
Date: Tue, 22 Dec 2020 13:59:18 -0800

@martinthomson pushed 1 commit.

9f852472eb6326cee63561b23f48583b43effea4  it does reduce space

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4449/files/5e865ff9eff98e4a24f20005ed5f03a8005a2c00..9f852472eb6326cee63561b23f48583b43effea4

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 22 Dec 2020 14:37:14 -0800

Merged #4449 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/4449#event-4141997699

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 22 Dec 2020 14:38:21 -0800

I realized that there are some answers here in the description, but probably best to move them to the relevant issues anyways.

-- 
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/4449#issuecomment-749813479