[tsvwg] Opsdir last call review of draft-ietf-tsvwg-datagram-plpmtud-17

[Tim Chown via Datatracker <[e-mail]>]

Reviewer: Tim Chown
Review result: Has Nits

Hi,

I have reviewed this document as part of the Operational directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written with the intent of improving the operational aspects of
the IETF drafts. Comments that are not addressed in last call may be included
in AD reviews during the IESG review.  Document editors and WG chairs should
treat these comments just like any other last call comments.

This document defines Datagram Packetization Layer Path MTU Discovery
(DPLPMTUD) as a new, robust mechanism for applications or transport protocols
to handle path MTU discovery, rather than relying on the somewhat problematic
classic PMTUD.

Overall, the document is well written with a good structure.  In my view it is
Ready for publication, subject to some nits being considered and addressed.  I
had held off on this review at Gorry's request until -17 was out.  My comments
follow below; these are all generally nit-level.

General comments:

None.

There are some ordering issues, esp. around Sec 4.6.2 where terms defined later
in the doc are used before they are defined.  But that should be easy to fix.

I did notice a few extra commas in the text; I’ve not called these out, and
assume the RFC editor will pick these up, probably better than me!

Specific comments:

Sec 1.1

Is there anything to be added here regarding problematic PTB delivery wrt
protocols that insert headers into packets, rathe than encapsulating them, such
as the case of one variant of IPv6 segment routing?

Sec 1.2

Para 3 - if there is no response to the probe, might the PLPMTU remain the same
rather than being reduced?  Just steady state operation?  There is a separate
case of a larger probe failing, as against the path MTU changing (becoming
smaller) over time?  (This is implicitly mentioned in case 2 of section 4.3)

Sec 2

The first mention here of DPLPMTUD doesn’t actually say what the acronym is. 
It’s only expanded later in the document.

Definition of Link - is that “layer and tunnels” or :”layer tunnels” ?

Sec 4.1

First mention of AEAD not expanded.

Last para - maybe add “Maximum Segment Lifetime” to the definitions in section
2?

Sec 4.3

I think it would be clearer if
“There are three ways to detect black holes:”
read
“There are three such indicators that allow detection of black holes:”

The three methods may have different responsiveness to a change in underlying
MTU; should that be mentioned and briefly discussed?   There’s a brief mention
of this issue in the last para of 4.6.1.

Perhaps this section could have - “Reducing PLPMTU” appended to the subject.

Section 4.6.1

First line, maybe “utilization and validation of”

Penultimate para, maybe say “as discussed in the Security Considerations”
section or similar.

Section 4.6.2

Many of the terms here are only defined later in Sec 5.1.2, e.g., BASE_PLPMTU. 
So you need to either add a foreword pointer, or change the order of things
around.  Took me a little while to find that the terms were not yet mentioned.

Sec 5

Second para - is there any way to detect if both layers are running DPLPMTUD?
Also, here you say “SHOULD NOT”, later the doc says it’s “desired” that this
doesn’t happen.

Section 5.1.2

Last term, BASE_PLPMTU - why is 1200 RECOMMENDED for IPv4?  Any reference to
point to to explain?

Section 5.1.4

Figure 4 is useful as a “light” version of Figure 5, but there are one or two
inconsistencies, or omissions.   In 5.2 you say there are omissions to Fig 5
for simplicity, maybe the same note should be here too for Fig 4? e.g., there
is no “black hole detection” path from “Search Complete” to “Base” (which I
*think* is different to “PLPMTU confirmation failed”. I’d also suggest saying
“PMTU Raise Timer” rather than just “raise timer”, or even use the proper
“PMTU_RAISE_TIMER” in the figure.

I also feel the “search complete” state is a little misleading; search complete
is how you get to this state, not so much what it is, i.e., the search is
complete, but this is the “DPLPMTUD steady state” or maintenance state, isn’t
it?  From it, every so often you probe to see if you can raise the MTU, or test
that the current MTU is OK, or if you detect a PTB you MAY trigger a probe.
Maybe consider renaming this state?

Last para of Base: spec - change “If this phase” to “if the Base Phase” else it
could be read that “this” is the Search Phase.

Sec 5.2

In Fig 5, should there be a path from “Search Complete” to “Base” about PLPMTU
Confirmation Failed, as per Fig 4?

Definition of SEARCHING, 2nd para, I’d suggest adding “as described in Section
5.3” at the end.

Definition of SEARCH COMPLETE - is it really always a “successful end”?   It
may be that PROBE_COUNT = MAX_PROBES that gets you there.  Is that successful? 
Maybe just say after the completion, and not say successful?  Depends on the
definition of success I guess :)

Sec 5.3.2

Maybe details of the advice here could be a separate Informational RFC?  I’m
curious now.