Subject: Re: [tsvwg] Review of draft-ietf-tsvwg-datagram-plpmtud-01.txt

From: Tom Jones <[email-masked]>
Date: Wed, 6 Jun 2018 15:43:19 +0100

> Hi,
> 
> I have reviewed  draft-ietf-tsvwg-datagram-plpmtud-01 and have some 
> comments.
> 
> First of all, I like the work, but we all need to help improve this into 
> a really solid specification.

Hi Magnus,

Thanks for the detailed comments, remarks inline:

> 1. Front page headers should have an Updates: 4821.

Done

> 2. Section 1.3:
> 
> It does not require
>    cooperation from the lower layers (except that they are consistent
>    about which packet sizes are acceptable).
> 
> I wonder about the text in the parenthesis. What is the intention with 
> this, and which lower layers are referenced here? From an end-point's 
> perspective, it has no knowledge about if some of the flows packet will 
> get an lower layer extension header added, or if they will for a while 
> go though some tunnel reducing the effective MTU, and later move away. 
> Even within an endpoint it can be hard to guarantee that the 
> implementation doesn't add or change things.

Agree, removed the text in parens

> 3. Section 3:
> 
>    1.  PMTU parameters: A PLPMTUD sender is REQUIRED to provide
>        information about the maximum size of packet that can be
>        transmitted by the sender on the local link (the link MTU and MAY
>        utilize similar information about the receiver when this is
>        supplied (note this could be less than EMTU_R).  Some
>        applications also have a maximum transport protocol data unit
>        (PDU) size, in which case there is no benefit from probing for a
>        size larger than this (unless a transport allows multiplexing
>        multiple applications PDUs into the same datagram).
> 
> I think this is good to have, but not required to know. The lack of the 
> actual maximum local link information will only make the probing more 
> inefficient. So I suggest that you change from REQUIRED to RECOMMENDED.

Changed to RECOMMENED

> 4. Section 3:
> 
>    4.  Processing PTB messages: A PLPMTUD sender MAY optionally utilize
>        PTB messages received from the network layer to help identify
>        when a path does not support the current size of packet probe.
>        Any received PTB message SHOULD/MUST be verified before it is
>        used to update the PMTU discovery information [RFC8201].
> 
> Is the SHOULD/MUST a question of what we should write, or an uncertainty 
> inherited from RFC 8201? If it is a question, I would suggest to 
> reformulate this into something a bit more actionable and matching what 
> is written later.
>
> PTBs indicating reduction of effective MTU SHALL NOT be used unless 
> succesfully verified. And even then are not mandatory to use.
>

Changed to MUST

> 5. Section 3:
> 
>    9.  Shared effective PMTU state: The PMTU value could also be stored
>        with the corresponding entry in the destination cache and used by
>        other PL instances.  The specification of PLPMTUD [RFC4821]
>        states: "If PLPMTUD updates the MTU for a particular path, all
>        Packetization Layer sessions that share the path representation
>        (as described in Section 5.2 of [RFC4821]) SHOULD be notified to
>        make use of the new MTU and make the required congestion control
>        adjustments".  Such methods need to robust to the wide variety of
>        underlying network forwarding behaviours.  Section 5.2 of
>        [RFC8201] provides guidance on the caching of PMTU information
>        and also the relation to IPv6 flow labels.
> 
> This is more a question, than a comment. Can we actually have shared 
> state for MTU information based on source and destination IP addresses? 
> What is a path representation here?
> 
> The reasons I am worried, is that we can clearly have unidirectional MTU.
> 
> Secondly, Carrier Grade NATs can easily create an issue here. Consider a 
> case where you have two different hosts behind the same CGN, lets call 
> them A and B. As the access path between CGN and A and B respectively 
> are different they can have different MTU on those legs. However from a 
> server (S) perspective, it looks like a shared path for traffic going 
> from S -> A and S->B, however in reality they are different. This case 
> would cause an interesting case where if A has the larger value, it will 
> update the table for the path, and immediately B will reduce it again. 
> And both may get very consistent path MTUs. How do one deal with this case?
> 

I think the search algorithm has to take the shared PLPMTU value in as a
signal, but should only use it as an interesting point to try. It should
be left to implementations to trigger a new search phase.

> 6. Section 3.1:
> 
>      This results in three possible ways that a sender can create a probe
>    packet:
> 
>    Probing using appication data:  A probe packet that contains a data
>       block supplied by an application that matches the size required
>       for the probe.  This method requests the application to issue a
>       data block of the desired probe size.  If the application/
>       transport needs protection from the loss of an unsuccessful probe
>       packet, the application/transport needs then to perform transport-
>       layer retransmission/repair of the data block (e.g., by
>       retransmission after loss is detected or by duplicating the data
>       block in a datagram without the padding).
> 
>    Probing using appication data and padding data:  A probe packet that
>       contains a data block supplied by an application that is combined
>       with padding to inflate the length of the datagram to the size
>       required for the probe.  If the application/transport needs
>       protection from the loss of this probe packet, the application/
>       transport may perform transport-layer retransmission/repair of the
>       data block (e.g., by retransmission after loss is detected or by
>       duplicating the data block in a datagram without the padding
>       data).
> 
>    Probing using padding data:  A probe packet that contains only
>       control information together with any padding needed to inflate
>       the packet to the size required for the probe.  Since these probe
>       packets do not carry an application-supplied data block,they do
>       not typically require retransmission, although they do still
>       consume network capacity and incur endpoint processing.
> 
> 
> First of all, I would reverse the order of these bullets. And then add 
> that they are listed in decreasing order of preference. The reason I say 
> so is that I think using all padding is way simplest to implement and 
> have least impact on the failure of the probes if the higher layer 
> supports it. Secondly, using Padding to increase from current effective 
> MTU to probe size makes retransmission and recovery using the effective 
> one efficient. Thus, probing with actually larger application data 
> requires additional handling when retransmitting it.
> 

Reordered

> In addition for the first bullet: "by duplicating the data block in a 
> datagram without the padding)." As there is no padding in this case, 
> this needs to be deleted, and likely expanded with new sentences saying 
> that there might need to use fragmentation at a suitable layer to be 
> able to resend that application data used in the probe.
> 

Trimmed "without padding"

> 7. Section 3.3:
> 
>    When the current effective PMTU is no longer supported by the network
>    path, the transport needs to detect this and reduce the effective
>    PMTU.
> 
> I am missing something about how one detects that it is not any longer 
> supported. Persistent or elevated packet loss rates is a trigger here, 
> as well as verified PTB messages. But something needs to be said about 
> this also in this overview state.
> 

Will add text to make this clear

> 8. Section 3.3:
>       A
>       fragmented datagram MUST NOT be used as a probe packet (see
>       [RFC8201]).
> 
> Isn't anything else than IPvX fragmentation that MUST NOT be used here. 
> From a packetization layer perspective one way of creating a probe 
> packet that is larger than current effective MTU would be to fragment a 
> higher layer unit in the PL to reach that size.

Clarified that IPv4 packet must be sent with DF

> 9. Section 4.1:
> 
> Between the first and second paragraph I am missing the paragraph about 
> raising the Probe Size after a successful probe round (First paragraph) 
> and also ensure that the highest successful probe is put into a 
> provisional PMTU variable.
>

Added a paragraph to clarify

> 10. Section 4.1:
> 
> Once probing is completed, the sender continues to use the effective
>    PMTU until either a PTB message is received or the PMTU_RAISE_TIMER
>    expires.
> 
> I get confused by the use of "continues to use" here. Is the effective 
> PMTU the prior to probing one, or the the value of PMTU established by 
> the previous paragraph? If the later, then maybe be say:
> 
> Once probing is completed, the sender will use the effective
>    PMTU established until either a PTB message is received or the 
> PMTU_RAISE_TIMER
>    expires.
> 

Clarified

> 11. Section 4.2:
> 
> What is already written is good and deals with off-path attackers as 
> well as spurious wrong values that indicate the wrong value that is larger.
> 
> However, I wonder if this can be strengthened to deal with on-path 
> attackers as well as with spurious values that are to low.
> 
> So responding to a PTB message has the potential advantage that it 
> minimizes the amount of persistent loss the flow is subject to. For the 
> issue of wrongly stated PMTU values, such as byte flipped one, that 
> someone commented on in the TSVWG discussion, I think one may need to 
> probe the stated size to verify the suggested size, or at least lower 
> the effective MTU and verify that it works (and if one is not generating 
> packets at PMTU side, then one needs to send probes). Then after having 
> verified the switch down is at a working place, initiate a probe to 
> verify that the provided value was the largest supported one.
> 

We have some ideas for validating PTB and handling invalid values, we
will try and incorporate these in a future rev.

> In addition on-path attackers could forge PTB messages to drive down the 
> PMTU. A possible solution to this is to have some fairly low value that 
> is most cases safe. And if one received a PFB indicating a size below 
> that value do not lower the effective PMTU to the suggested value. 
> Initiate a probe to verify the safe value. If that is working I think we 
> have some different choices depending if one believes attacks are the 
> most likely or simply bad values in PTB. If one simultaneously with the 
> PTB message receives indication of loss of full size data packets, then 
> one need to reduce and it is likely a erroneous value in PTB. Thus 
> switch to the safe low value and probe up for a right value. If the old 
> effective still works, then increase the "attack_observed counter" and 
> when it gets high enough stop process PTB for this session.

Added text on on-path attackers to the security considerations

> My uncertainty for what is best here indicates that this algorithm 
> section needs to be much more explicit about several things.
> 
> First, an algorithm for triggering a effective MTU reduction based on 
> application data packet losses. Secondly, an algorithm for dealing with 
> PTBs with potentially wrong PMTU Values.
> 
> 12. Section 4.3:
> 
>    PROBE_TIMER:  Configured to expire after a period longer than the
>       maximum time to receive an acknowledgment to a probe packet. This
>       value MUST be larger than 1 second, and SHOULD be larger than 15
>       seconds.  Guidance on selection of the timer value are provide in
>       section 3.1.1 of the UDP Usage Guidelines [RFC8085].
> 
> I think mandating a particular span is fine. However, there need to be a 
> default value here that works in almost all applications. as the Probe 
> timer indicates how frequent a retransmission will occur I think SHOULD 
> for 15 seconds is unnecessary large. And with a default of 10 
> retransmission, assuming that any ACK within the PROBE_COUNT*PROBE_TIMER 
> will be counted as a success, this is a very long time (10*15 = 150 
> seconds) before a probe fails. I would think that 2 or 3 seconds for 
> PROBE_TIMER is more suitable default.
> 
> Secondly, is if the PL has an RTT estimate and timely ACKing, could 
> PROBE_TIMER be set to  max(100 ms, Smoothed PRTT*5) to (PRTT is protocol 
> RTT, i.e. time from sending a Protocol message until ACK has been 
> received by probe sender) speed this up even more. I guess the MUST be 
> at least 1 second is to prevent applications without any RTT estimate to 
> not use a value to short in relation to the RTT so that it thinks the 
> probes all failed prior to probe and ACK has had a chance to be returned.
> 
> I also think that protocol semantics are important here. For example if 
> one would implement this mechanism in RTP with AVP timings rather than 
> AVPF the smoothed PRTT would be approximately several seconds for any 
> network RTT below a second.
>

Will add text for PL with RTT estimates. Added a minimum value.

> 13. Section 4.3:
> 
> 
>    REACHABILITY_TIMER:  Configured to the period a sender ought to wait
>       before confirming the current effective PMTU is still supported.
>       This is less than the PMTU_RAISE_TIMER.
> 
> Also this more solid default value. Especially for this usage:
> 
>    If the PL is unable to verify reachability to the
>    destination endpoint after probing has completed, the method uses a
>    REACHABILITY_TIMER to periodically repeat a probe packet for the
>    current effective PMTU size, while the PMTU_RAISE_TIMER is running.
> 
> I assumed when reading that this is for PL that doesn't have explicit 
> ACKs so that you don't know which packets actually made it, and thus can 
> verify the path with application data, and instead needs to use explicit 
> probes for this. This verification from my perspective needs to be 
> frequent enough when data is flowing that you do not blackhole extensive 
> amount of traffic.

Added text on verification

> 14. Section 4.3:
> 
>       An application that needs to employ keep-alive messages to deliver
>       useful service over UDP SHOULD NOT transmit them more frequently
>       than once every 15 seconds and SHOULD use longer intervals when
>       possible.  DPLPMTUD ought to suspend reachability probes when no
>       application data has been sent since the previous probe packet.
>       Guidance on selection of the timer value are provide in section
>       3.1.1 of the UDP Usage Guidelines[RFC8085].
> 
> I would suggest that the first sentence is removed. I don't think it is 
> for this document to recommend these values. We already have documents 
> discussing this, like RFC4787. Although any application ensuring that 
> they don't loose any mappings will use 30 seconds as the value to ensure 
> you refresh your mapping (The RFC says at least 2 minutes).
> 

Removed first sentence. 

> But, I agree that the PMTUD discovery process should be suspended, or at 
> least only be sent in conjunction with other traffic during periods of 
> dormancy. The question is if there is rather a need to turn around this 
> statement to give a recommendation when DPLMTUD should consider an 
> application to be in an inactive phase and suspend active probing?
> 

I think so, will add some text.

> 15. Section 4.4:
> 
>   MIN_PMTU:  The smallest allowed probe packet size.  This value is
>       1280 bytes, as specified in [RFC2460].  For IPv4, the minimum
>       value is 68 bytes.  (An IPv4 routed is required to be able to
>       forward a datagram of 68 octets without further fragmentation.
>       This is the combined size of an IPv4 header and the minimum
>       fragment size of 8 octets.)
> 
> I wonder about the IPv6 Min PMTU value. Is that conservative enough 
> here? For the mechanism to be really robust, should it not probe a bit 
> under 1280, like 1200 actually. Thus enabling one set of tunnel headers 
> fit inside an IPv6 packet that is clamped to a 1280 MTU. The later still 
> a quite common practice.
> 
> Regarding IPv4, what about 576 bytes? Is that not widely enough 
> supported to be min value?
> 
> 

We will probably add some advisory text on minimum numbers, which are
larger than 68 bytes.

> 16. Section 4.4:
> 
>    MAX_PMTU:  The MAX_PMTU is the largest size of PMTU that is probed.
>       This has to be less than or equal to the minimum of the local MTU
>       of the outgoing interface and the destination effective MTU for
>       receiving.  An application or PL may reduce this when it knows
>       there is no need to send packets above a specific size.
> 
> Maybe there should be a default value here, for cases when the local 
> interface MTU isn't known? I would suggest 4096 as large enough to allow 
> for improved performance over larger than 1500 bytes MTUs, but this is 
> only a fallback for when Interface MTU can't be figured out, for example 
> due user land stacks with poor interfaces to the lower layers. Also, I 
> think such a value MUST be configurable.
> 

Default value when the local MTU isn't known has to be 65535.
Implementations that know better can choose a smaller value.

> 17. Section 4.6:
> 
> I think allowing for different algorithms are fine here. However, maybe 
> one reasonable method should be detailed in full using the defined 
> variables. When I reviewed an PMTUD probing algorithm in an earlier 
> version of draft-ietf-trill-over-ip it didn't conclude. Thus, I think 
> psuedo code for something that works is important.
> 
> I think a table driven approach is quite good. However, that requires 
> that we have reasonable good understanding over what values are more 
> frequent to support. Does anyone have good material for which values 
> that actually work out there today? If we had material to cite for such 
> research it would be great and help the implementor to select reasonable 
> values.
> 

We will add examples here.

> 18. Section 4.7:
> 
>  If
>    multihoming is supported, a state machine is needed for each active
>    path.
> 
> What are considered path identifiers here? Only the normal 5-tuple, or 
> do one need to recommend something additionally?
>

5-tuple

> 19. Section 4.7:
> 
>    PROBE_START:  The PROBE_START state is the initial state before
>       probing has started.  PLPMTUD is not performed in this state. The
>       state transitions to PROBE_BASE, when a path has been confirmed,
>       i.e. when a sent packet has been acknowledged on this path. The
>       effective PMTU is set to the BASE_PMTU size.  Probing ought to
>       start immediately after connection setup to prevent the loss of
>       user data.
> 
> The figure hints at using probe packets, while the text appears to say 
> that any packet will do, like a handshake packet for the protocol. And 
> if probe packets are used in this state, what size is used?
> 
> Maybe this step should be simplified to simply indicate that for 
> protocols that anyway generates ackable packets, upon confirmation from 
> peer that they have received a packet, one can move to PROBE_BASE state. 
> If the protocol is not sending ackable packets by it self initially, 
> then PROBE packets needs to be sent, using a MIN_PMTU size or even a 
> minimal Probe packet.
> 

We will add text to clarify which packets can move out of PROBE_START

> 20. Figure 1.
> 
> I find this state machine confusing due to its its transitions are not 
> what I would expect. Reading the text description I think the 
> transactions are more general than the more specific wording in the 
> figure. For example the PROBE_SEARCH -> PROBE_DONE. This is 
> transactioned when MAX_PMTU is acked, which is type fine. However, the  
> PROBE_TIMER expiry is not the right indication for a succesful 
> completion of a search for the MTU. If one implements a Search for the 
> MTU, one might during the Probe_SEARCH phase have some PROBED_SIZE 
> fails, while still making progress towards finding the highest supported 
> value in a reasonable amount of rounds.
> 
> Another example is the PROBE_SEARCH -> PROBE_BASE: PTB (< PROBED_SIZE). 
> I don't understand this transition. A PTB < PROBED_SIZE would just be an 
> indication that this SEARCH round can be concluded. Then assuming that 
> PTB > effective_MTU then the next step is to do PROBED_Size = PTB, which 
> if ACKED then likely tells that this round of searching is done.
> 
> Also the PROBE_BASE -> PROBE_DONE is not obvious. To my understanding 
> PROBE_BASE will only Probe BASE_PMTU. Thus, if this transition can only 
> make sense if MAX_PMTU = BASE_PMTU. And in that case no PTB should be 
> received. And if PTB > BASE_PMTU then it makes sense to do PROBE_SEARCH 
> using PTB.
> 
> My suggestion is that you redraw this figure to have the possible 
> transitions on the higher layer. Then acknowledge that PROBE_Search has 
> an internal state machine during its searching for MTU, and that is 
> going be dependent on the search algorithm implemented.

We will redraw to clarify and keep the text consistent.

> 21. Section 4.7:
> 
> PROBE_ERROR:
> 
> This is the state where the assumption of the safe starting point failed 
> to be the BASE value, and instead we have to go with the minimal ones. 
> The only effect is that given that MIN works, one will start the SEARCH 
> state with a different starting point than if one had come from 
> PROBE_BASE. Are there any other differences in the initiation of the 
> PROBE search?
> 

Search will start from the same point, but going into PROBE_ERROR stops
us signalling MPS to the upper layer.

Will add text to clarify this.

> 22. Section 4.7: PROBE_SEARCH
> 
>       The PROBE_COUNT is set to zero when the first probe packet is sent
>       for each probed size.  Each time a probe packet is acknowledged,
>       the effective PMTU is set to the PROBED_SIZE, and then the
>       PROBED_SIZE is increased.
> 
> Should there be an option for not updating the effective MTU until the 
> PROBE_SEARCH concludes this round of searching? The reason I am asking 
> is that in some application and application layers changing the 
> effective MTU is actually less than trivial. For a bulk transport layer 
> where change of effective MTU just changes how it fragements data to be 
> sent it is simple. But, for application where it has significant effect 
> and possibly needs to be synchronized with certain boundaries one may 
> want to hold off these changes. An example would be a transport using 
> FEC where the per packet MTU actually impacts the source blocks and how 
> they are structured.
> 

We will add text on robustly validating the discovered PLPMTU values.

> 23. Section 5.1.5.  Handling of PTB Messages by UDP
> 
>    Normal ICMP verification MUST be performed as specified in
>    Section 5.2 of [RFC8085].  This requires that the PL verifies each
>    received PTB messages to verify these are received in response to
>    transmitted traffic and that the reported LInk MTU is less than the
>    current probe size.  A verified PTB message MAY be used as input to
>    the PLPMTUD algorithm.
> 
> So, the assumption here is that the ECHOREQ will be part of the those 
> bytes included in the PMTUD so that it can be verified that the PTB 
> corresponds to a PROBE packet that actually was sent. I think you this 
> section should be more explicit about the need to verify this for probe 
> packets, as the ECHOREQ token is likely the only part of the packet that 
> can't be guessed by an attacker. I would also consider if the Token 
> should be larger than just 32 bits. Possibly the token should be allowed 
> to be of variable length between 4 and 64 bytes or so.
> 

4 bytes is the most we are going to get for the token. 

> 24. Section 5.1: UDP options and higher layer protocol
> 
> I don't know if this comment is for this document or the UDP options 
> one. However when probing with a UDP options packet with a user data 
> part of length 0, is that empty packet delivered to the higher layer, or 
> not. It is quite important in regards to how independent the PMTUD can 
> be from the higher layer?
>

All UDP applications should expect to receive 0 length packets and be
robust to them, UDP Options implementation shouldn't bother applications
with 0 length datagrams that result from probes.

> 25. Section 5.2: PROBE_START
> 
> With SCTP, I assume the reception of INIT_ACK is what is needed to be 
> moved from PROBE_START to PROBE_BASE? I think the specification should 
> be explicit on this. Similar but quite different text is needed for UDP 
> (5.1) and for QUIC (5.3)
>

Will add text

> 26. Detection of MTU blackholing due to path change.
> 
> I didn't see anything in this discussing the case when a PL with acks or 
> other type of feedback detects persistent or size related packet loss. 
> If a routing change would result in that the effective MTU is to big and 
> no PTB messages reach the sender, what happens then? My impression is 
> that the current recourse is to wait until the  PMTU_RAISE_TIMER fires 
> and the probe fails. With default case, that can be going on for 10 
> minutes (600 seconds). Shouldn't there be some other mechanism for those 
> layers that track their traffic (many) to detect the black hole and 
> initiate a probe to verify the MTU?

I think we have to wait for the black hole detection to trigger, but I
also think there are probably short cuts to save detection time. Either
way I hope implmentation experience will find an answer.

> 27. Mechanism for classifying a path as having alternating PMTU?
> 
> As the securing consideration notes, there can be cases where the actual 
> PMTU varies back and forth due the flow using multiple paths or rapidly 
> alternating paths. I think there should be recommendations for how to 
> deal with this case.
>

This is a 3 beer question

- Tom

-- 
- [tj]