Subject: Re: [quicwg/base-drafts] DPLPMTU merge tweaks (#3702)

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 01 Jun 2020 01:00:40 -0700

> Thank you for your changes! I think it might be useful to distinguish the "IP packet" from how we commonly use "packet", so I propose (i) simply use "PMTU probe", and (ii) saying explicitly what that refers to. I've made some suggestions through the text to this end; see what you think.

OK - There are already "IP packets" and "QUIC packets" - so some other term works fro me.

-- 
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/3702#issuecomment-636680044

---

From: ianswett <[email-masked]>
Date: Mon, 01 Jun 2020 16:51:55 -0700

@ianswett approved this pull request.

 A client MUST expand the payload of all UDP datagrams carrying Initial packets
-to at least 1200 bytes, by adding PADDING frames to the Initial packet or by
-coalescing the Initial packet; see {{packet-coalesce}}.  Sending a UDP datagram
-of this size ensures that the network path from the client to the server
-supports a reasonable Maximum Transmission Unit (MTU).  Padding datagrams also
-helps reduce the amplitude of amplification attacks caused by server responses
-toward an unverified client address; see {{address-validation}}.
+to at least the smallest allowed maximum packet size (1200 bytes) by adding
+PADDING frames to the Initial packet or by coalescing the Initial packet; see
+{{packet-coalesce}}.  Sending a UDP datagram of this size ensures that the
+network path from the client to the server supports a reasonable Path Maximum
+Transmission Unit (PMTU).  This also helps reduce the amplitude of amplification
+attacks caused by server responses toward an unverified client address; see
+{{address-validation}}.
 
 Enforcement of the max_udp_payload_size transport parameter

I'd move this paragraph above the one above, since they both talk about maximum packet size and in particular this paragraph uses "additional limit" which reads better one paragraph up.

 Datagrams containing Initial packets MAY exceed 1200 bytes if the client
 believes that the network path and peer both support the size that it chooses.
 
 UDP datagrams MUST NOT be fragmented at the IP layer.  In IPv4
 {{!IPv4=RFC0791}}, the DF bit MUST be set to prevent fragmentation on the path.
 
-A server MUST discard an Initial packet that is carried in a UDP datagram with
-a payload that is less than 1200 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}}.
+A server MUST discard an Initial packet that is carried in a UDP datagram with a
+payload that is less than the smallest allowed maximum packet size  of 1200

```suggestion
payload that is less than the smallest allowed maximum packet size of 1200
```

> -{{?RFC8085}}.
-
-An endpoint SHOULD use Datagram Packetization Layer PMTU Discovery
-({{!DPLPMTUD=I-D.ietf-tsvwg-datagram-plpmtud}}) or implement Path MTU Discovery
-(PMTUD) {{!RFC1191}} {{!RFC8201}} to determine whether the path to a destination
-will support a desired message size without fragmentation.
-
-In the absence of these mechanisms, QUIC endpoints SHOULD NOT send IP packets
-larger than 1280 bytes. Assuming the minimum IP header size, this results in a
-QUIC maximum packet size of 1232 bytes for IPv6 and 1252 bytes for IPv4. A QUIC
-implementation MAY be more conservative in computing the QUIC maximum packet
-size to allow for unknown tunnel overheads or IP header options/extensions.
+An endpoint SHOULD use DPLPMTUD ({{dplpmtud}}) or PMTUD ({{pmtud}}) to determine
+whether the path to a destination will support a desired message size without
+fragmentation.  In the absence of these mechanisms, QUIC endpoints SHOULD NOT
+send IP packets larger than the minimum QUIC packet size.

```suggestion
send IP packets larger than the smallest allowed maximum packet size.
```

> -implementation MAY be more conservative in computing the QUIC maximum packet
-size to allow for unknown tunnel overheads or IP header options/extensions.
+An endpoint SHOULD use DPLPMTUD ({{dplpmtud}}) or PMTUD ({{pmtud}}) to determine
+whether the path to a destination will support a desired message size without
+fragmentation.  In the absence of these mechanisms, QUIC endpoints SHOULD NOT
+send IP packets larger than the minimum QUIC packet size.
+
+Both DPLPMTUD and PMTUD send IP packets that are larger than the current maximum
+packet size.  We refer to these as PMTU probes.  All QUIC packets that are not
+sent in a PMTU probe SHOULD be sized to fit within the maximum packet size to
+avoid the packet being fragmented or dropped {{?RFC8085}}.
+
+If a QUIC endpoint determines that the PMTU between any pair of local and remote
+IP addresses has fallen below the smallest allowed maximum packet size of 1200
+bytes, it MUST immediately cease sending QUIC packets, except for those in PMTU
+probes, on the affected path.  An endpoint MAY terminate the connection if an

This isn't new text, but tt seems like these two statements may be contradictory?  Should this be "except for those in PMTU probes or containing connection close frames, on the affected path."?

-Section 6.3 of {{!DPLPMTUD}} provides considerations for implementing Datagram
-Packetization Layer PMTUD (DPLPMTUD) with QUIC.
+If a QUIC endpoint determines that the PLPMTU between any pair of local and
+remote IP addresses has fallen below the size needed to support the minimum QUIC
+packet size (BASE_PLPMTU), it MUST immediately cease sending QUIC packets on the
+affected path. An endpoint MAY terminate the connection if an alternative path

This seems a bit repetitive of 3862-3866.  It's also still confusing or potentially contradictory to me.

-- 
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/3702#pullrequestreview-422237549

---

From: Martin Thomson <[email-masked]>
Date: Mon, 01 Jun 2020 17:59:40 -0700

@martinthomson pushed 1 commit.

1cd8875b1540d45a7d9d3b6d834115fe97f49c87  Editorial tweaks

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

---

From: Martin Thomson <[email-masked]>
Date: Thu, 04 Jun 2020 16:20:11 -0700

567b253ce4c6121ad9a76e97573d045f9984b430  Use maximum packet size rather than message size

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3702/files/1cd8875b1540d45a7d9d3b6d834115fe97f49c87..567b253ce4c6121ad9a76e97573d045f9984b430

---

From: Gorry Fairhurst <[email-masked]>
Date: Fri, 05 Jun 2020 05:57:27 -0700

A top-level comment on the term "probing":
In Section:  Probing a New Path: A packet containing only probing frames is a "probing packet", and a packet containing any other frame is a "non-probing packet". Please be sure that "probe" can not be confused with "probing" in PLPMTUD.

-- 
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/3702#issuecomment-639463727

---

From: Gorry Fairhurst <[email-masked]>
Date: Fri, 05 Jun 2020 05:59:30 -0700

In the subsection on “Packet Size {#packet-size}”, I think the blocks of text describe two things. There is some general guidance on size, and there are specific instructions for the size of the initial packet. I’d suggest we do try to separate these two, it would make it much easier for someone to understand the size constraints for forming the 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/3702#issuecomment-639464698

---

From: Gorry Fairhurst <[email-masked]>
Date: Fri, 05 Jun 2020 06:05:56 -0700

@gorryfair commented on this pull request.

> -However, prior to learning the value of the transport parameter, endpoints risk
-datagrams being lost if they send packets larger than 1200 bytes.
+({{transport-parameter-definitions}}) might act as an additional limit on the
+maximum packet size. A sender can avoid exceeding this limit, once the value is
+known.  However, prior to learning the value of the transport parameter,
+endpoints risk datagrams being lost if they send packets larger than the
+smallest allowed maximum packet size of 1200 bytes.
+
+A client MUST expand the payload of all UDP datagrams carrying Initial packets
+to at least the smallest allowed maximum packet size (1200 bytes) by adding
+PADDING frames to the Initial packet or by coalescing the Initial packet; see
+{{packet-coalesce}}.  Sending a UDP datagram of this size ensures that the
+network path from the client to the server supports a reasonable Path Maximum
+Transmission Unit (PMTU).  This also helps reduce the amplitude of amplification
+attacks caused by server responses toward an unverified client address; see
+{{address-validation}}.

Needs to be 1200B of payload? (We need to be careful about which layer these numbers are describing, since the datagram itself will be larger.

-- 
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/3702#pullrequestreview-425280196

---

From: Martin Thomson <[email-masked]>
Date: Tue, 09 Jun 2020 00:10:57 -0700

@martinthomson commented on this pull request.

I think we caught this in #3742.

-- 
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/3702#discussion_r437184003

---

From: Martin Thomson <[email-masked]>
Date: Tue, 09 Jun 2020 00:17:30 -0700

Regarding the sizing of Initial packets, see #3746.

I checked the text for probing and we have "PMTUD probe" in one case and "probing packet" elsewhere.  It's not ideal, but I didn't see any cases that were clearly busted.  I don't have a good idea for alternative labels, but would welcome suggestions.

-- 
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/3702#issuecomment-641084004

---

From: Martin Thomson <[email-masked]>
Date: Tue, 09 Jun 2020 00:20:31 -0700

Merged #3702 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/3702#event-3423403053