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. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
