Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\))
From: Tim Chown <tjc.ietf@gmail.com>
In-Reply-To: <f35c1465-c511-facc-6f3b-96900a90c275@erg.abdn.ac.uk>
Date: Wed, 11 Mar 2020 13:20:44 +0000
Cc: Marc Petit-Huguenin <petithug@acm.org>, last-call@ietf.org,
 magnus.westerlund@ericsson.com, draft-ietf-tsvwg-datagram-plpmtud@ietf.org,
 tsvwg-chairs@ietf.org, tsvwg@ietf.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <12F31252-981A-4B54-B9B6-C4FEEC3E8A03@gmail.com>
References: <158264004537.15415.7388175321017685105.idtracker@ietfa.amsl.com>
 <babf588e-31b2-5cfd-9abf-cc0349a89be4@acm.org>
 <f35c1465-c511-facc-6f3b-96900a90c275@erg.abdn.ac.uk>
To: Gorry Fairhurst <gorry@erg.abdn.ac.uk>
Archived-At:
 <https://mailarchive.ietf.org/arch/msg/tsvwg/gsNP2K5qZX_2wzKdRK-rbCLCO4Q>
Subject: Re: [tsvwg] [Last-Call] Last Call:
 <draft-ietf-tsvwg-datagram-plpmtud-15.txt> (Packetization Layer Path MTU
 Discovery for Datagram Transports) to Proposed Standard
Precedence: list

Hi,

I also owe you an OPS-DIR review, which will be coming soon.  I could =
wait on the new version, but should have time in the coming few days.

Tim

> On 11 Mar 2020, at 13:02, Gorry Fairhurst <gorry@erg.abdn.ac.uk> =
wrote:
>=20
> Thank you for reading this and the review comments. We now plan to =
look at each of these turn and prepare a new revision. We will also get =
back in touch to note the corrections and ask where we need =
clarification.
>=20
> Best wishes,
>=20
> Gorry and the other editors for datagram-plpmtud.
>=20
>=20
> On 10/03/2020 22:00, Marc Petit-Huguenin wrote:
>> Please find below my Last Call review of =
draft-ietf-tsvwg-datagram-plpmtud-15.  Note that this review does not =
cover sections 6.2, 6.3 and 9.  Also I believe that an RFC should be =
implementable without reading the informative parts, so I skipped the =
abstract and section 1.
>>=20
>> Let's start with the most general comments:
>>=20
>> It seems that the goal of this standard track document is to =
prescribe one single method (from now on: "method") to find the =
effective PMTU, something that RFC 4821 did not do.  By doing so, this =
draft effectively restricts the number of ways that RFC 4821 can be =
implemented.  A non-exhaustive list of things that the method would =
prevent could be:
>>=20
>> - Doing parallel probing, i.e. sending a few probes of different =
sizes at the same time.  Instead the method uses a lockstep mechanism so =
a new size can be tried only when an acknowledgement is received or the =
PROBE_TIMER expired MAX_PROBES times.
>> - Using the possibility in RFC 4821 section 6.1 to take in account =
the packets surrounding a Probe (including probes of different size sent =
at the same time) to differentiate between congestion and a probe lost =
because of its size.
>>=20
>> As a software developer specialized in communication protocols, I do =
not particularly like the idea that my options to implement a protocol =
are constrained, especially when the constraints are that I can only do =
things sequentially.  I think that a better option would be to simply =
constrain RFC 4821 by defining some limits (like the number of =
retransmission, and the rate probes should be sent) and let developers =
do their job.  That said that draft certainly has value for a beginner =
or unsupervised developer, in which case that whole state machine would =
be useful in an Informative draft, as the simplest and safest way to do =
PLPMTUD.
>>=20
>> Now going more in detail about the draft:
>>=20
>> - I would suggest to say something about RFC 6864, which would =
rate-limits the probes sent between a pair of IPv4 addresses for a =
particular protocol (in that case UDP).
>>=20
>> - MAX_PMTU is defined as the minimum of the local link MTU and the =
destination link MTU.  =46rom the top of my mind I could not find a =
protocol that actually carries that value back to the local side, but I =
suppose that can be easily done.  It would be useful to say something =
about that, that the size of the packet used to retrieve that value =
(also the size of the packet used for connectivity check) should be =
lower than MIN_MTU, and also what happen when that value becomes =
available when the state machine is in another state than DISABLED.
>>=20
>> - About MAX_PMTU, this name and others are defined after their first =
use.  Maybe adding all these to section 2 would make it easier to find =
definitions (and may even result in discovering some unnecessary =
aliasing).
>>=20
>> - It could be useful to state that a probe should carry a unique =
identifier, and that it needs to be reflected in the acknowledgement, so =
to be able to process out-of-order and delayed packets.  In that case an =
additional variable in section 5.1.3 would contain the last probe =
identifier used.
>>=20
>> - =46rom a developer point of view, the information needed to =
implement PLPMTUD seems to be spread in different sections, making it =
difficult to get a complete picture of what is going on.  In fact I had =
to convert the text into a Petri Net -- a non-trivial and time-consuming =
task -- to be able to understand how bits from various sections fit =
together.
>>=20
>> So I would suggest to merge sections 4.6.2, 5.1.1, 5.1.2, 5.1.3, 5.2 =
and 5.3 into one single state machine, listing (a) the set of states, =
(b) the state context (aka variables, adding PLPMTU to it), (c) the list =
of transitions conditions (effectively merging timers and packet types =
received -- destination MTU size, connectivity acknowledgment, probe =
acknowledgement, and PTB) and finally (d) the exhaustive list of =
transitions between states, including for each the list of actions on =
the context and/or the packets sent.  I would either forgo completely =
the state machine diagram, or use Cosmogol =
(draft-bortzmeyer-language-state-machines) to include a formal state =
machine that can be converted into an SVG picture.
>>=20
>> Having such exhaustive list of transitions between states would 1) =
put all the information needed in one single place and 2) add more =
clarity to the whole state machine.  E.g. it is not clear if a Probe =
should also be sent when entering the Base and Search state, or just =
when PROBE_TIMER expires (delaying the first probe by PROBE_TIMER).  =
There is other ambiguities like this that could be resolved by a =
systematic listing of the transitions actions.  And the formalization =
would permit to check the model for completeness and a few other =
properties, which cannot be a bad thing in itself.
>>=20
>> Some minor comments:
>>=20
>> - Section 3, Bullet point 8:  Why not "MUST NOT"?
>> - Section 4.4: "The MPS is smaller than the PLPMTU because of the =
presence of Pl headers and any IP options or extensions added to the PL =
packet."  Obviously also because of the presence of the IP header =
itself, as shown in the diagram.
>> - Figure 2: "UDPO" is never defined.
>> - Section 5.1.1: "When an acknowledged PL is used..."  I do not =
understand what an "acknowledged PL" is.
>> - Section 5.1.1: "An implementation..." Should be replaced by a more =
general statement saying that implementers can do whatever they want, as =
long as the external behavior of the implementation behaves exactly as =
the external behavior of how that state machine would behave.
>> - Section 5.1.4: "sends an acknowledged probe packet"  I do not know =
what that is.
>> - Section 5.2: "Not all changes are shown to simplify the diagram."  =
See above.
>> - Section 5.2: "uses an unacknowledged PL": I do not know what that =
is.
>>=20
>> Some nits:
>>=20
>> - Section 3, first bullet point: s/For datagram PLs,]/For datagram =
PLs,]/
>> - Section 4.3: s/MUST NOT rely soley/MUST NOT rely solely/
>> - Section 4.3: s/up-to-data/up-to-date/
>> - Section 4.6.1: s/speed at the which/speed at which/
>> - Section 4.6.2: s/(e. g.  PLPMTU/(e.g. PLPMTU/
>> - Section 4.6.2: s/to trigger enabling a resilience/to enable a =
resilience/
>> - Section 5.2: s/This state is left, once/This state is left once/
>> - Section 6.1.3: s/A probe packet that could/A probe packet could/
>> - Section 6.1.6: s/the application to check each/the application =
checks that/
>>=20
>> On 2/25/20 6:14 AM, The IESG wrote:
>>> The IESG has received a request from the Transport Area Working =
Group WG
>>> (tsvwg) to consider the following document: - 'Packetization Layer =
Path MTU
>>> Discovery for Datagram Transports'
>>>   <draft-ietf-tsvwg-datagram-plpmtud-15.txt> as Proposed Standard
>>>=20
>>> The IESG plans to make a decision in the next few weeks, and =
solicits final
>>> comments on this action. Please send substantive comments to the
>>> last-call@ietf.org mailing lists by 2020-03-10. Exceptionally, =
comments may
>>> be sent to iesg@ietf.org instead. In either case, please retain the =
beginning
>>> of the Subject line to allow automated sorting.
>>>=20
>>> Abstract
>>>=20
>>>=20
>>>    This document describes a robust method for Path MTU Discovery
>>>    (PMTUD) for datagram Packetization Layers (PLs).  It describes an
>>>    extension to RFC 1191 and RFC 8201, which specifies ICMP-based =
Path
>>>    MTU Discovery for IPv4 and IPv6.  The method allows a PL, or a
>>>    datagram application that uses a PL, to discover whether a =
network
>>>    path can support the current size of datagram.  This can be used =
to
>>>    detect and reduce the message size when a sender encounters a =
packet
>>>    black hole (where packets are discarded).  The method can probe a
>>>    network path with progressively larger packets to discover =
whether
>>>    the maximum packet size can be increased.  This allows a sender =
to
>>>    determine an appropriate packet size, providing functionality for
>>>    datagram transports that is equivalent to the Packetization Layer
>>>    PMTUD specification for TCP, specified in RFC 4821.
>>>=20
>>>    The document updates RFC 4821 to specify the method for datagram =
PLs,
>>>    and updates RFC 8085 as the method to use in place of RFC 4821 =
with
>>>    UDP datagrams.  Section 7.3 of RFC4960 recommends an endpoint =
apply
>>>    the techniques in RFC 4821 on a per-destination-address basis.  =
RFC
>>>    4960, RFC 6951 and RFC 8261 are updated to recommend that SCTP, =
SCTP
>>>    encapsulated in UDP and SCTP encapsulated in DTLS use the method
>>>    specified in this document instead of the method in RFC 4821.
>>>=20
>>>    The document also provides implementation notes for incorporating
>>>    Datagram PMTUD into IETF datagram transports or applications that =
use
>>>    datagram transports.
>>>=20
>>>    When published, this specification updates RFC 4960, RFC 4821, =
RFC
>>>    8085 and RFC 8261.
>>>=20
>>>=20
>>>=20
>>>=20
>>> The file can be obtained via
>>> https://datatracker.ietf.org/doc/draft-ietf-tsvwg-datagram-plpmtud/
>>>=20
>>> IESG discussion can be tracked via
>>> =
https://datatracker.ietf.org/doc/draft-ietf-tsvwg-datagram-plpmtud/ballot/=

>>>=20
>>>=20
>>> No IPR declarations have been submitted directly on this I-D.
>>>=20
>>>=20
>>=20
>>=20
>=20
> --=20
> last-call mailing list
> last-call@ietf.org
> https://www.ietf.org/mailman/listinfo/last-call