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

[Michael Tuexen <[e-mail]>]

> On 7. Dec 2019, at 02:03, Martin Duke <[e-mail]> wrote:
> 
> 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).
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"
> 
> Cons:
> - 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.
> - 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). 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?
> 
> 
> *****
> 
> 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.
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
> 
> Martin
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>