Re: [tsvwg] WGLC for DPLPMTUD - Response to WGLC comments (presumed resolved)

[Maksim Proshin <[e-mail]>]

Hi Gorry,

Thanks! I think we're aligned now. 
Regarding can/MAY, I think "usually improves" is the best phrasing here. It seems I misread it initially so my proposal with MAY wasn't appropriate.

BR, Maxim


-----Original Message-----
From: Gorry Fairhurst [mailto:[e-mail]] 
Sent: Monday, December 23, 2019 13:23
To: Maksim Proshin <[e-mail]>
Cc: [e-mail]
Subject: Re: [tsvwg] WGLC for DPLPMTUD - Response to WGLC comments (presumed resolved)

Great, see below,

Sent from my iPad

> On 23 Dec 2019, at 08:19, Maksim Proshin <[e-mail]> wrote:
> 
> Hi Gorry,
> 
> I'm fine with your answers to these comments but I'd like to check that my understanding of the following response is correct:
> 
> 4.
> Sec 4.5.1: “PTB messages that have been validated MAY be utilized by the DPLPMTUD algorithm, but MUST NOT be used directly to set the PLPMTU.” – what is a purpose if this MUST NOT here? What is a risk to apply it after the validation? If it’s not applied immediately, it might cause data loss. Or did you mean something different here?
>  GF: Done
> NEW:
>     PTB messages that have been validated MAY be utilized by the
>           DPLPMTUD algorithm, but MUST NOT be used directly to set the PLPMTU.
>           A method that utilizes these PTB messages can improve the speed at
>           the which the algorithm detects an appropriate PLPMTU by triggering
>           an immediate probe for the PTB_SIZE, compared to
>           one that relies solely on probing using a timer-based
>           search algorithm.
> [Maxim] So the basic idea of DPLMTUD is to switch to the BASE state and start from BASE_PMTU if PTB_SIZE < PLPMTU. Also the PLPMTU value is changed to BASE_PMTU in that case. Is my understanding correct? 
> 
To be sure: The way I would see this is...
1) you can implement and ignore PTB messages ... this is at least robust, in which case the PLPMTU needs to return to a safe value (base) and probe higher. This takes several RTTs to find a new PLPMTU that is larger than base.

2) you can (and would be encouraged to) implement and use Validated PTB messages. In this case, the usual next step after receiving an ICMP message, is to probe with the PTB size indicated in the PTB ICMP message, and see if that works - which it usually will within one RTT, resulting in an appropriate choice of the PLPMTU.
2a) If that probe did not work, then the sender will revert to the safe PMTU (base) and grow from there.

> Would it better to replace "can" by MAY in the added sentence?
> 
In this case, I thought this described that improvements can happen, rather than permitting the protocol to take an action. Perhaps we ought to change it to “is expected to improve...” or usually improves?

Gorry

> BR, Maxim
> 
> -----Original Message-----
> From: G Fairhurst <[e-mail]> 
> Sent: Friday, December 20, 2019 6:19 PM
> To: Maksim Proshin <[e-mail]>
> Cc: [e-mail]
> Subject: Re: [tsvwg] WGLC for DPLPMTUD - Response to WGLC comments (presumed resolved)
> 
> Maxim: I’ve reviewed this draft and below you can find my response to your comments, marked as GF. If you see anything more I should do, please say.
> 
> Best wishes,
> 
> Gorry
> 
> Major comments:
> 
> 
> 2.
> Sec 5.1.1: “This value MUST NOT be smaller than 1 second, and SHOULD be larger than 15 seconds.” and “PROBE_TIMER can be derived from the PL RTT estimate.” – I assume it means that if the RTT estimate is used, it still must not be less than 1 second. Is it true? If so, I think it would be good to understand where 1 second is taken from. Is it due to RTO.Min recommendations? If so, can it be better to say that the value must not be less than RTO.Min of that particular PL?
>  GF:DONE -  Removed this. To meet the CC rules, the requirements are in terms of 1/sec.
> 
> 4.
> Sec 4.5.1: “PTB messages that have been validated MAY be utilized by the DPLPMTUD algorithm, but MUST NOT be used directly to set the PLPMTU.” – what is a purpose if this MUST NOT here? What is a risk to apply it after the validation? If it’s not applied immediately, it might cause data loss. Or did you mean something different here?
>  GF: Done
> NEW:
>     PTB messages that have been validated MAY be utilized by the
>           DPLPMTUD algorithm, but MUST NOT be used directly to set the PLPMTU.
>           A method that utilizes these PTB messages can improve the speed at
>           the which the algorithm detects an appropriate PLPMTU by triggering
>           an immediate probe for the PTB_SIZE, compared to
>           one that relies solely on probing using a timer-based
>           search algorithm.
> 
> 
> 7.
> Im: Sec 1.2: “This has become the recommended approach for implementing PMTU discovery.” – recommended by IETF? can it be referenced/clarified?
> GF: Done - cite RFC8085.
> 
> 8.
> Mi: Sec 1.2: “see Section 4.5” - my understanding was that the section is reference here because it provides more details about “all PTB processing can be disabled and PLPMTUD can completely replace Classical PMTUD” but I failed to find it in that section. Either the reference is wrong or I misunderstood the purpose of having it there.
> GF: DONE - removed citation.
> 
> 
> 10.
> Im/Q: Sec 3: “A DPLPMTUD sender is RECOMMENDED to provide information about the maximum size of packet” – provide to whom? Propose to clarify or rephrase it.
> GF: DONE - Corrected by Magnus /provide/utilise/
> 
> 11.
> Im/Q: Sec 3: “The mechanism needs to be robust to the possibility that packets could be significantly delayed along a network path.” - Isn’t it MUST?
> GF: DONE
> NEW:
> mechanism is REQUIRED to be robust
> 
> The local PL endpoint => The destination PL endpoint?
> GF: DONE Actually it’s covered by the first requirement, removed.
> 
> 12.
> Im: Sec 3, Req 6: I think it would be also good to mention that if data is used in a probe the sender must be able to retransmit it in a smaller
> packet(s) because otherwise it would be lost again and again.
> GF: DONE, added
> NEW:
> possibly using a smaller PLPMTU.
> 
> 13.
> Im: Sec 3, Req 7: Can it be improved saying MUST NOT when data is not used in a probe? And SHOULD when “isolated” data probe is lost.
> HG:
> GF: DONE - addressed in an update for Magnus. The method still works, even if with unfortunate side effects so MUST ius rather harsh - instead we explain why this is important./
> 
> 14.
> Mi: Sec 4.2: “These PLs need to either rely on an application protocol to detect this loss.” – “either” should be removed.
> GF: Done
> 
> 15.
> Mi: Sec 4.3: “There are two alternative mechanism:” –> “mechanisms”
> GF:  DONE (removed the sentence)
> 
> 16.
> Mi: Something wrong with references to RFCs when 2 or more of them are used. E.g. [RFC1191][RFC8201]
> GF: RFC-Ed can resolve!
> 
> 18.
> Mi/Q: Sec 4.5.2: “larger than minimum size accepted” – I assume “larger than or equal to”. If so, all further formulas should also be corrected to consider it.
> GF: DONE - changed to
> NEW:
> and at least
> 
> 19.
> Im: Sec 5.1.2: “expected to work for most paths” – it’s better to say “all paths” due to “expected”.
> GF: DONE rephrased.
> 
> 20.
> Im/Q: Sec 5.1.3: “PROBED_SIZE” – I wonder why it’s not called “PROBE_SIZE”.
> GF: DONE
> 
> 21.
> Im: Sec 5.1.4: I assume that Base phase confirms the connectivity for BASE_PMTU and not in general as the connectivity can be achieved by a smaller PLPMTU value.
> GF: DONE.
> 
> 22.
> Im/Q: Sec 5.1.4: Is it assumed that Base is the first phase? If so, it would be good to clearly state it.
> GF: DONE No longer in latest rev.
> 
> 23.
> Mi: Sec 5.2: “Note: Not all changes are not shown to simplify the diagram.” – the second “not” should be removed.
> GF: DONE Text no longer in latest rev.
> 
> 24.
> Im: There are “may”, “should”, etc in a couple of places. They either should be capital or rephrased.
> GF: DONE - all reworded.
> 
> 26.
> Im/Q: Sec 6.2: Should or should not probe packets influence assoc and path error counters? I think it’s “SHOULD” when a probe is acknowledged and “SHOULD NOT” when it’s lost. In other words probes should only clear the error counters in my view.
> GF: Is this correct:
> NEW:
>  A successful probe updates the association and path
>             counters, but an unsuccessful probe is discounted (assumed
>             to be a result of choosing too large a PLPMTU).
> 
> 27.
> Im/Q: Sec 6.2.1.2: “The HEARTBEAT chunk carries a Heartbeat Information parameter which should include, besides the information suggested in [RFC4960], the probe size ...” – Why is it required? If the sender of HB is able to uniquely associate the HB with the HB ACK, it only needs to remember the current PROBED_SIZE value.
> GF: GF: DONE Text no longer in latest rev. This was intending to be padding to the probed size.
> 
> 28.
> Im: Sec 9: “A node supporting DPLPMTUD MUST therefore appropriately validate the payload of PTB messages” – in addition it must not be used to directly set PTB value as prescribed in section 4.5.1. I would add it.
> GF: DONE
> NEW:
>       The successful processing of this message can trigger a probe and does not
>       directly update the PLPMTU for the path.</t>
> 
> 29.
> Im: Sec 9: A potential attack when PTB increases PLPMTU is not described here. It would be good to add it.
> GF: DONE.