Re: [quicwg/base-drafts] Rewrite Packet Size section (#2036)

[Martin Thomson <[e-mail]>]

martinthomson commented on this pull request.

Editorial comments for the most part.  This looks like an improvement.

@igorlord, @martinduke, this is something you have shown interest in.  Could you review?

>  
-## Path Maximum Transmission Unit
+The PMTU is the maximum size of the entire IP datagram including the IP header,
+UDP header, and UDP payload.  The UDP payload includes the QUIC packet header,
+protected payload, and any authentication fields. This can be depend upon the
+current path characteristics.  Therefore, the current largest UDP payload an
+implementation will send is referred to as QUIC Maximum Packet Size (MPS).

Yes, please use words.  We set the bar high for new definitions and acronyms, because they tend to make the document less readable.  I'm not sure that we even need this concept to be introduced.

>  
-## Path Maximum Transmission Unit
+The PMTU is the maximum size of the entire IP datagram including the IP header,

We've used "IP packet" throughout, consistent with the IP RFCs.

```suggestion
The PMTU is the maximum size of the entire IP packet including the IP header,
```

> -The PADDING frame provides a useful option for PMTU probe packets. PADDING
-frames generate acknowledgements, but they need not be delivered reliably. As a
-result, the loss of PADDING frames in probe packets does not require
-delay-inducing retransmission. However, PADDING frames do consume congestion
-window, which may delay the transmission of subsequent application data.
-
-When implementing the algorithm in Section 7.2 of {{!PLPMTUD}}, the initial
-value of search_low SHOULD be consistent with the IPv6 minimum packet size.
-Paths that do not support this size cannot deliver Initial packets, and
-therefore are not QUIC-compliant.
-
-Section 7.3 of {{!PLPMTUD}} discusses trade-offs between small and large
-increases in the size of probe packets. As QUIC probe packets need not contain
-application data, aggressive increases in probe size carry fewer consequences.
-
+larger than 1280 bytes (assuming the minimum IP header size).  This results in

```suggestion
larger than 1280 bytes. Assuming the minimum IP header size, this results in
```

> +Each pair of local and remote addresses could have a different PMTU.  QUIC
+implementations that implement any kind of PMTU discovery therefore SHOULD
+maintain a MPS for each combination of local and remote IP addresses.
+
+If a QUIC endpoint determines that the PMTU between any pair of local and
+remote IP addresses has fallen below the size needed to support the smallest
+allowed MPS, it MUST immediately cease sending QUIC packets on the
+affected path.  This could result in termination of the connection if an
+alternative path cannot be found.
+
+### Processing ICMP Messages to reduce the PMTU {#icmp-pmtud}
+
+PMTU discovery {{!RFC1191}} {{!RFC8201}} relies on reception of ICMP messages
+(e.g., IPv6 Packet Too Big, PTB, messages) that indicate when a packet is
+dropped because it is larger than the local router MTU. DPLPMTUD can also
+optionally utilise these messages.  This use of ICMP messages is

```suggestion
optionally use these messages.  This use of ICMP messages is
```

Don't overutilize "utilize".

FWIW, I think that for this document, we have chosen to use American English, even though that is the inferior variant.

> +unknown tunnel overheads or IP header options/extensions.
+
+Each pair of local and remote addresses could have a different PMTU.  QUIC
+implementations that implement any kind of PMTU discovery therefore SHOULD
+maintain a MPS for each combination of local and remote IP addresses.
+
+If a QUIC endpoint determines that the PMTU between any pair of local and
+remote IP addresses has fallen below the size needed to support the smallest
+allowed MPS, it MUST immediately cease sending QUIC packets on the
+affected path.  This could result in termination of the connection if an
+alternative path cannot be found.
+
+### Processing ICMP Messages to reduce the PMTU {#icmp-pmtud}
+
+PMTU discovery {{!RFC1191}} {{!RFC8201}} relies on reception of ICMP messages
+(e.g., IPv6 Packet Too Big, PTB, messages) that indicate when a packet is

```suggestion
(e.g., IPv6 Packet Too Big messages) that indicate when a packet is
```

> +potentially vulnerable to off-path attacks that successfully guess the IP
+address 3-tuple and reduce the PMTU to a bandwidth-inefficient value
+{{!RFC8201}}.
+
+QUIC endpoints SHOULD provide validation to protect from off-path injection of
+ICMP messages as specified in {{!RFC8201}} and Section 5.2 of {{!RFC8085}}. This
+uses the quoted packet supplied in the payload of an ICMP message, which, when
+present, can be used to associate the message with a corresponding transport
+connection {{!DPLPMTUD}}.
+
+The IPv4 Router requirements {{!RFC1812} state that the quoted packet should
+contain as much of the original datagram as possible without the length of the
+ICMP datagram exceeding 576 bytes. IPv6 routers include as much of invoking
+packet as possible without the ICMPv6 packet exceeding 1280 bytes {{!RFC4443}}.
+The size of the quoted packet can actually be smaller, or the information
+unintelligible, for various reasons {{!DPLPMTUD}}.

A section reference would be useful here.

> +{{!RFC8201}}.
+
+QUIC endpoints SHOULD provide validation to protect from off-path injection of
+ICMP messages as specified in {{!RFC8201}} and Section 5.2 of {{!RFC8085}}. This
+uses the quoted packet supplied in the payload of an ICMP message, which, when
+present, can be used to associate the message with a corresponding transport
+connection {{!DPLPMTUD}}.
+
+The IPv4 Router requirements {{!RFC1812} state that the quoted packet should
+contain as much of the original datagram as possible without the length of the
+ICMP datagram exceeding 576 bytes. IPv6 routers include as much of invoking
+packet as possible without the ICMPv6 packet exceeding 1280 bytes {{!RFC4443}}.
+The size of the quoted packet can actually be smaller, or the information
+unintelligible, for various reasons {{!DPLPMTUD}}.
+
+When a randomized source port is used, this can provide some protection from

This could be read to imply that PMTUD probes could use a different source port.  That can have the effect of hitting a different path.  This probably needs to be qualified.

> +* An endpoint could store additional information from the IP or UDP headers to
+  use for validation (for example, the IP ID or UDP checksum).
+
+The endpoint SHOULD ignore all ICMP messages that are not validated or do not
+carry sufficient quoted packet payload to perform validation.  Any reduction in
+the QUIC MPS MAY be provisional until QUIC's loss detection algorithm
+determines that the quoted packet has actually been lost.
+
+## Considerations for Datagram Packetization Layer PMTU Discovery
+
+Section 6.4 of {{!DPLPMTUD}} provides considerations for
+implementing Datagram Packetization Layer PMTUD (DPLPMTUD) with QUIC.
+
+When implementing the algorithm in Section 5.3 of
+{{!DPLPMTUD}}, the initial value of BASE_PMTU SHOULD be
+consistent with the minimum QUIC packet size.

Saying what that is (1280, I assume, without reading DPLPMTUD) would be helpful.

> +
+The endpoint SHOULD ignore all ICMP messages that are not validated or do not
+carry sufficient quoted packet payload to perform validation.  Any reduction in
+the QUIC MPS MAY be provisional until QUIC's loss detection algorithm
+determines that the quoted packet has actually been lost.
+
+## Considerations for Datagram Packetization Layer PMTU Discovery
+
+Section 6.4 of {{!DPLPMTUD}} provides considerations for
+implementing Datagram Packetization Layer PMTUD (DPLPMTUD) with QUIC.
+
+When implementing the algorithm in Section 5.3 of
+{{!DPLPMTUD}}, the initial value of BASE_PMTU SHOULD be
+consistent with the minimum QUIC packet size.
+
+A PADDING frame can be used to generate PMTU probe packets. PADDING need not be

PADDING frames are not retransmitted if the packet they are contained in is lost.

> +larger than 1280 bytes (assuming the minimum IP header size).  This results in
+a QUIC MPS of 1232 bytes for IPv6 and 1252 bytes for IPv4. A QUIC
+implementation MAY be more conservative in computing the QUIC MPS to allow for
+unknown tunnel overheads or IP header options/extensions.
+
+Each pair of local and remote addresses could have a different PMTU.  QUIC
+implementations that implement any kind of PMTU discovery therefore SHOULD
+maintain a MPS for each combination of local and remote IP addresses.
+
+If a QUIC endpoint determines that the PMTU between any pair of local and
+remote IP addresses has fallen below the size needed to support the smallest
+allowed MPS, it MUST immediately cease sending QUIC packets on the
+affected path.  This could result in termination of the connection if an
+alternative path cannot be found.
+
+### Processing ICMP Messages to reduce the PMTU {#icmp-pmtud}

```suggestion
### Processing ICMP Packet Too Big Messages {#icmp-pmtud}
```

or just

```suggestion
### ICMP Packet Too Big Messages {#icmp-pmtud}
```

-- 
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/2036#pullrequestreview-177465831