Subject: [tsvwg] Review: draft-ietf-datagram-plpmtud-12

From: Martin Duke <[email-masked]>
Date: Fri, 6 Dec 2019 17:03:39 -0800

As promised, here is my review:

I see no showstoppers in moving this to WGLC. However, there are a couple
of minor content additions I would like to see and a series of nits. I
believe this draft is important to the future success of datagram protocols
like QUIC.

Substantive Issues:

1. Unless I missed it, there is no guidance as to whether probe packets
should count against flightsize for congestion control.  I would suggest
that this standard exempt MTU probes from CC if not using application data
(possibly preempting other standards).

Cons:
- This is conflicts with existing protocol designs (like QUIC)

Pros:
- Item 7 in Section 3 already preempts existing protocols by telling us to
ignore the loss signal for CC purposes.
- This prevents PLPMTUD from incurring any sort of performance penalty
- The timer requirements probably prevent any measurable impact on network
congestion.

But either way some language would be helpful (e.g. section 4.1 could say
"the probe packet is subject to congestion control in accordance with the
contents of the packet and the relevant congestion control specification".)

2. I am not fond of the way that important considerations relating to the
loss of non-probe packets are buried in a reference to RFC 4821 in Sec 4.1.
Good PROBE_COUNT incrementing logic is quite subtle (see "Probe
Inconclusive" in Sec 7.6.4 of RFC 4821
<https://www.rfc-editor.org/rfc/rfc4821.html#section-7.6.4>). I suspect
implementers will spend most of their time in Section 5, and I would like
more discussion of these important issues in that section, probably
directly under the definition of PROBE_COUNT.

Personally, I find the 4821 language to be quite vague and I would like an
example safe algorithm (for example, off the top of my head, " decrement
PROBE_COUNT for each non-probe packet lost between when the probe packet is
sent and it is declared lost").

*****

Editorial issues and nits:

Abstract: s/functionally/functionality

Section 1.1: In the first paragraph the clause that begins "and a method"
should probably be reworded and become its own sentence.

Section 2:
- I am unclear of the difference between "Packet Black Hole" and "ICMP
Black Hole". By a literal reading of the definition, I think ICMP black
holes are a superset of packet black holes? In any case, these subtypes
appear only once in the draft (Sec 1.2), so perhaps simply eliminating this
distinction is easiest. I don't know that Packet Black Holes are a useful
concept for this draft.

- Reading the definition for PL a few times, I was unclear if for SCTP/QUIC
over UDP, UDP was the PL or the upper protocol. From later context I
*think* I inferred it to be the latter. I recognize that defining the upper
protocol is difficult but I would like another pass to this. I'm happy to
discuss further in a separate thread.

Section 3:
- Item 1. I am unclear who the DPLPMTUD sender is to provide the
information to. Is this to the peer, like a TCP MSS option, or is this an
internal interface? Please clarify. If internal, please rephrase to "A
DLPMTUD sender is RECOMMENDED to consider local link limitations..."

- Can we order this section with REQUIRED up front and MAYs down below?

- The blanket adoption of RFC 4821 requirements is not well written. If the
intention is to have all the 4821 requirements PLUS the 8 listed here, then
#7 is listed twice. If that is not the intent that this is worded
incorrectly.

Section 4.2: "When supported, this mechanism [reporting specific datagrams]
SHOULD also be used by DPLPMTUD to acknowledge reception of a probe
packet." I think this sits somewhat uneasily with QUIC. The closest
analogue to SCTP HEARTBEAT/HEARTBEAT_ACK (IRRC) is QUIC
PATH_CHALLENGE/PATH_RESPONSE. We would certainly use PING instead of this.
In practice, QUIC's total unambiguity with ACKs and packet numbers renders
this irrelevant, but I don't love how this is worded.

Sec 4.5.1. Do we want to say anything about how NATs can complicate PTB
validation, or are most NATs smart enough to handle this?

Sec 6.2.3.4 I think it is too strong to say that DTLS makes SCTP Common
Header validation "not possible". There are certainly somewhat expensive
options like storing the beginning of each encrypted packet or
re-encrypting a stored copy of the plaintext packet to compare.

Martin

---

From: Michael Tuexen <[email-masked]>
Date: Sat, 7 Dec 2019 17:04:05 +0100

> On 7. Dec 2019, at 02:03, Martin Duke <[email-masked]> wrote:
> 
> I see no showstoppers in moving this to WGLC. However, there are a couple of minor content additions I would like to see and a series of nits. I believe this draft is important to the future success of datagram protocols like QUIC.
> 
> Substantive Issues:
> 
> 1. Unless I missed it, there is no guidance as to whether probe packets should count against flightsize for congestion control.  I would suggest that this standard exempt MTU probes from CC if not using application data (possibly preempting other standards).
At least this is what is done for SCTP:

6.2.  DPLPMTUD for SCTP
   
Section 10.2 of [RFC4821] specifies a recommended PLPMTUD probing
method for SCTP.  It recommends the use of the PAD chunk, defined in
[RFC4820] to be attached to a minimum length HEARTBEAT chunk to build
a probe packet.  This enables probing without affecting the transfer
of user messages and without interfering with congestion control.
This is preferred to using DATA chunks (with padding as required) as
path probes.

Maybe we should replace "without interfering with congestion control"
by "without interfering with congestion control and flow control"
> - This is conflicts with existing protocol designs (like QUIC)
I thought in QUIC you also have the possibility to probe with non-user data. Am I wrong?
> 
> Pros:
> - Item 7 in Section 3 already preempts existing protocols by telling us to ignore the loss signal for CC purposes.
> - The timer requirements probably prevent any measurable impact on network congestion.
> 
> But either way some language would be helpful (e.g. section 4.1 could say "the probe packet is subject to congestion control in accordance with the contents of the packet and the relevant congestion control specification"..)
> 
> 2. I am not fond of the way that important considerations relating to the loss of non-probe packets are buried in a reference to RFC 4821 in Sec 4.1. Good PROBE_COUNT incrementing logic is quite subtle (see "Probe Inconclusive" in Sec 7.6.4 of RFC 4821). I suspect implementers will spend most of their time in Section 5, and I would like more discussion of these important issues in that section, probably directly under the definition of PROBE_COUNT.
> 
> Personally, I find the 4821 language to be quite vague and I would like an example safe algorithm (for example, off the top of my head, " decrement PROBE_COUNT for each non-probe packet lost between when the probe packet is sent and it is declared lost").
Why would the handling of non-probe packets affect PROBE_COUNT?
> Section 1.1: In the first paragraph the clause that begins "and a method" should probably be reworded and become its own sentence.
> 
> Section 2: 
> - I am unclear of the difference between "Packet Black Hole" and "ICMP Black Hole". By a literal reading of the definition, I think ICMP black holes are a superset of packet black holes? In any case, these subtypes appear only once in the draft (Sec 1.2), so perhaps simply eliminating this distinction is easiest. I don't know that Packet Black Holes are a useful concept for this draft.
> 
> - Reading the definition for PL a few times, I was unclear if for SCTP/QUIC over UDP, UDP was the PL or the upper protocol. >From later context I *think* I inferred it to be the latter. I recognize that defining the upper protocol is difficult but I would like another pass to this. I'm happy to discuss further in a separate thread.
> 
> Section 3:
> - Item 1. I am unclear who the DPLPMTUD sender is to provide the information to. Is this to the peer, like a TCP MSS option, or is this an internal interface? Please clarify. If internal, please rephrase to "A DLPMTUD sender is RECOMMENDED to consider local link limitations..."
> 
> - The blanket adoption of RFC 4821 requirements is not well written. If the intention is to have all the 4821 requirements PLUS the 8 listed here, then #7 is listed twice. If that is not the intent that this is worded incorrectly.
> 
> Section 4.2: "When supported, this mechanism [reporting specific datagrams] SHOULD also be used by DPLPMTUD to acknowledge reception of a probe packet." I think this sits somewhat uneasily with QUIC. The closest analogue to SCTP HEARTBEAT/HEARTBEAT_ACK (IRRC) is QUIC PATH_CHALLENGE/PATH_RESPONSE. We would certainly use PING instead of this. In practice, QUIC's total unambiguity with ACKs and packet numbers renders this irrelevant, but I don't love how this is worded.
> 
> Sec 4.5.1. Do we want to say anything about how NATs can complicate PTB validation, or are most NATs smart enough to handle this?
> 
> Sec 6.2.3.4 I think it is too strong to say that DTLS makes SCTP Common Header validation "not possible". There are certainly somewhat expensive options like storing the beginning of each encrypted packet or re-encrypting a stored copy of the plaintext packet to compare.
So you advocating that the DTLS layer
(a) stores the last N messages sent out
(b) does the ICMP handling and packet verification.
Are you aware of a DTLS implementation doing this?
Is the necessary verification step specified?

Best regards
Michael

---

From: Martin Duke <[email-masked]>
Date: Sat, 7 Dec 2019 11:27:11 -0800

Responses inline

> by "without interfering with congestion control and flow control"

I agree that the SCTP section addresses SCTP specifically, but I would hope
for more general guidance.

> I thought in QUIC you also have the possibility to probe with non-user

QUIC has plenty of non-user data. However, PADDING frames count against
flightsize according to the quic-recovery draft. Probe packets are already
an exception to congestion control in their loss response, and I'm arguing
for a further exception for flightsize.

> >
> > Personally, I find the 4821 language to be quite vague and I would like
> an example safe algorithm (for example, off the top of my head, " decrement

IMO a naive reading of Section 5 is that I should increment PROBE_COUNT for
each loss of a probe regardless of the overall loss context. 4821
recommends simply ignoring the probe loss (not incrementing PROBE_COUNT) if
there are other losses. I would like something of this nature explicitly
discussed in Sec 5.

> I am certainly not advocating for anything like this. I am merely
nitpicking that the statement that this is "not possible" is too strong.

Thanks,
Martin

---

From: Gorry Fairhurst <[email-masked]>
Date: Sun, 08 Dec 2019 08:38:04 +0000

See below on losses.

On 07/12/2019, 19:27, Martin Duke wrote:
> Responses inline
>
> On Sat, Dec 7, 2019 at 8:04 AM Michael Tuexen 
> <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>     > On 7. Dec 2019, at 02:03, Martin Duke <[email-masked]
>     <mailto:[email-masked]>> wrote:
>     >
>     >
>     > 1. Unless I missed it, there is no guidance as to whether probe
>     packets should count against flightsize for congestion control.  I
>     would suggest that this standard exempt MTU probes from CC if not
>     using application data (possibly preempting other standards).
> I agree that the SCTP section addresses SCTP specifically, but I would 
> hope for more general guidance.
>
>     >
>     > Cons:
>     I thought in QUIC you also have the possibility to probe with
>     non-user data.. Am I wrong?
>
>
> QUIC has plenty of non-user data. However, PADDING frames count 
> against flightsize according to the quic-recovery draft. Probe packets 
> are already an exception to congestion control in their loss response, 
> and I'm arguing for a further exception for flightsize.
>
>     >
>     > Personally, I find the 4821 language to be quite vague and I
>     would like an example safe algorithm (for example, off the top of
>     my head, " decrement PROBE_COUNT for each non-probe packet lost
>     between when the probe packet is sent and it is declared lost").
> IMO a naive reading of Section 5 is that I should increment 
> PROBE_COUNT for each loss of a probe regardless of the overall loss 
> context. 4821 recommends simply ignoring the probe loss (not 
> incrementing PROBE_COUNT) if there are other losses. I would like 
> something of this nature explicitly discussed in Sec 5.
True RFC4821 allows a sender to have a heueristic that can differentiate 
isolated probe loss, and loss as a part of a wider loss event, and then 
to count a loss during a wide loss event as "congestion" loss (i.e., 
reneging on the probe not being considered congestion-controlled and 
making the approriate adjustments retrospectively to cwnd, even though 
the probes were not originally counted as congetsion-controlled.) That 
makes the congestion control reaction slightly stronger, and would 
enable a probe count to not be adjusted (because the probe was no longer 
treated as a special non-CC packet), and would then allow the probe to 
be resent any number of times following some heuristic determines it is 
safe to resend.

This is complicated  and as  RFC4281 notes "

    Most of the difficulty in implementing PLPMTUD arises because it
    needs to be implemented in several different places within a single
    node."

and  I am also unaware of running code that actually implemented these 
more complex interactions that RFC4821 permits.

In contrast DPLPMTUD takes the approach that this complexity can be 
avoided by simply limiting the probing - such that failed probes are not 
counted against the cwnd, but then only a certain number (max probes) of 
unproductive probes can be sent, before the sender concludes that PLPMTU 
is not raised.

If that's acceptable, do we need to ad to Requirement 7 in Section 3 , 
which currently says this:

Loss of a probe packet SHOULD NOT be treated as an indication of congestion.
- That can be improved if you have suggestions.

>     >
>     > Sec 6.2.3.4 I think it is too strong to say that DTLS makes SCTP
>     Common Header validation "not possible". There are certainly
>     somewhat expensive options like storing the beginning of each
>     encrypted packet or re-encrypting a stored copy of the plaintext
>     packet to compare.
Gorry

---

From: Martin Duke <[email-masked]>
Date: Mon, 9 Dec 2019 10:16:56 -0800

gorry, response below:

> unproductive probes can be sent, before the sender concludes that PLPMTU

I think this reply conflates three questions that need not be unified:
1) Are Probe packets counted against flightsize?
2) Are Probe packet losses treated as congestion signals?
3) Under what circumstances do probe losses indicate that the packet is too
big?

IMO these design decisions can be completely independent -- they certainly
are in our QUIC code, which uses a bastardized version of PLPMTUD.

For #1, as I replied to Michael, the SCTP section is doing the right thing
but I would like general advice for other congestion controls to not count
probes at all. At the moment the QUIC spec suggests that probes would count
against flightsize.

The draft is fine with respect to #2.

If #3 is an intentional change from the "probe inconclusive" text in 4821,
then the draft text is fine. However, Requirement 7 of Section 3 says
"*isolated* loss of a probe packet" is an indication the path does not
support the packet size. Either strike "isolated," or my original comment
stands.

> - That can be improved if you have suggestions.

The requirement is fine except for the comment in #3 above.

Martin

---

From: Gorry Fairhurst <[email-masked]>
Date: Tue, 10 Dec 2019 12:42:37 +0000

Thanks Martin - I think I am starting to understand you issue, so please 
check below:

On 09/12/2019 18:16, Martin Duke wrote:
> gorry, response below:
>
> On Sun, Dec 8, 2019 at 12:38 AM Gorry Fairhurst <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>
>     >

So, do you mean that (1) a sender COULD use cwnd to determine whether to 
probe (i.e. only

probe if flight size allows), actually I am OK discounting them from 
this limit

providing they are as infrequent as specified in DPLPMTUD - but I would 
mind if a design

wanted to be a little more conservative. If I understand, this is about 
when to probe.

but (2) but the sender needs to still should realise that attempts to 
raise the PMTU

are likely to fail, so you shouldn't count loss of a probe as 
congestion, which is what

section 3, requirement 7 was about.

> 3) Under what circumstances do probe losses indicate that the packet 
> is too big?
>
That can be clarified - PLPMTU is reduced only when PROBE_COUNT>MAX_PROBES,

so never for an isolated loss.

> IMO these design decisions can be completely independent -- they 
> certainly are in our QUIC code, which uses a bastardized version of 
> PLPMTUD.
>
> For #1, as I replied to Michael, the SCTP section is doing the right 
> thing but I would like general advice for other congestion controls to 
> not count probes at all. At the moment the QUIC spec suggests that 
> probes would count against flightsize.
>
See if I understood above?
> If #3 is an intentional change from the "probe inconclusive" text in 
> 4821, then the draft text is fine. However, Requirement 7 of Section 3 
> says "*isolated* loss of a probe packet" is an indication the path 
> does not support the packet size. Either strike "isolated," or my 
> original comment stands.
>
Oh, I think I may see...

<<The DPLPMTUD sender treats

        isolated loss of a probe packet (with or without a corresponding
        PTB message) as a potential indication of a PMTU limit for the
        path.  >>

the word "potential" here was expected to read, "not yet confirmed, so 
try again to check...:"

- I see that isn't really clear, and we will rewrite the sentence.

I also now have a note that this could finally prove problematic, and 
we'll check how to best resolve:

<< All of the requirements in [RFC4821  <https://tools.ietf.org/html/rfc4821>] also apply to the use of the
    technique with a datagram PL. >>

Gorry

---

From: Martin Duke <[email-masked]>
Date: Tue, 10 Dec 2019 08:20:44 -0800

> On 09/12/2019 18:16, Martin Duke wrote:
<snip>

> section 3, requirement 7 was about.

I *think* we are in agreement that it is permissible but not required to
ignore cwnd when sending probles. It would be nice for this to be in the
general guidelines of the draft rather than just the SCTP-specific section.

> so never for an isolated loss.

I've read 4821 a little more carefully and withdraw this objection. I see
now that 4821 takes a single probe loss, if not accompanied by non-probe
loss, as an MTU signal, whereas DPLPMTUD requires *multiple* probe losses
without reference to non-probe losses. Now that I understand the status
quo, this seems reasonably robust. Sorry for the misdirection.

> See if I understood above?

Yes

To summarize, other than general guidance about ignoring CWND I now have no
design issues -- just the nits in my original email.

---

From: Gorry Fairhurst <[email-masked]>
Date: Tue, 10 Dec 2019 17:30:03 +0000

For me, your questions were really helpful at finding parts with 
ambiguity. - I think understand what I should update, and we'll include 
that in the next revision.

Gorry

On 10/12/2019, 16:20, Martin Duke wrote:
>
>
> On Tue, Dec 10, 2019 at 4:42 AM Gorry Fairhurst <[email-masked] 
> I *think* we are in agreement that it is permissible but not required 
> to ignore cwnd when sending probles. It would be nice for this to be 
> in the general guidelines of the draft rather than just the 
> SCTP-specific section.
>
>
> I've read 4821 a little more carefully and withdraw this objection. I 
> see now that 4821 takes a single probe loss, if not accompanied by 
> non-probe loss, as an MTU signal, whereas DPLPMTUD requires *multiple* 
> probe losses without reference to non-probe losses. Now that I 
> understand the status quo, this seems reasonably robust. Sorry for the 
> misdirection.
>
>
> To summarize, other than general guidance about ignoring CWND I now 
> have no design issues -- just the nits in my original email.