Re: [tsvwg] Murray Kucherawy's No Objection on draft-ietf-tsvwg-datagram-plpmtud-17: (with COMMENT)

[Gorry Fairhurst <[e-mail]>]

On 31/03/2020 17:26, Murray Kucherawy via Datatracker wrote:
> Murray Kucherawy has entered the following ballot position for
> draft-ietf-tsvwg-datagram-plpmtud-17: No Objection
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------

We think revision -18 adddreses these issues (details below).

Best wishes,

Gorry & the team.

> Who knew we liked commas so much!
>
>> Kudos on a very approachable document.  The thin air up in the ART layers had
>> me fearing something that said "TSVWG" on it.
>>
>> Section 2:
>> * The definition for EMTU_R appears to double up on itself.
>>>> GF This mistake was already caught and will be resolved in the next rev.
>
>>   Section 3:
>> * In bullet 2, "On request, a DPLPMTUD sender is REQUIRED to be able to
>> transmit a packet  ..."
>> -- I read this as "If I ask you to do X, you MUST be
>> able to do X", versus "you MUST do X".  Was that the intent?
>>>> GF - I think so, no change.
>
>> * In bullet 3,
>> there's reference to a "feedback method" that is REQUIRED, but this method is
>> unspecified.  Is that defined elsewhere, or is out of scope here?
>>>> GF -  it must exist within the PL, or be added to it. I think the wisest
>> Add a sentence:
>>
>> OLD:
>>          The destination PL endpoint is REQUIRED to
>>          provide a feedback method that indicates to the DPLPMTUD sender
>>          when a probe packet has been received by the destination PL
>>          endpoint.
>> NEW:
>>          The destination PL endpoint is REQUIRED to
>>          provide a feedback method that indicates to the DPLPMTUD sender
>>          when a probe packet has been received by the destination PL
>>          endpoint. Section 6 provides examples of how a PL can
>>          provide this acknowledgment of received probe packets.
>>
>
>>   Section 4.1:
>> Nits:
>> * "... uses a probe packet carrying an application data ..." -- s/an//
>>>> GF Mistake was already caught and will be resolved in the next rev.
>
>>   * "... this probe packet, could perform ..." -- s/,//
>>>> GF - to fix.
>
>>   * "This retransmited data block ..." --  typo: "retransmitted"
>>>> GF - to fix.
>
>>   Section 4.3:
>> * PROBE_COUNT and MAX_PROBES are first used here, though they are not defined
>> until later in the document in the DPLPMTUD section.
>>>> GF - Annoying, this is what happens we you later decide to use consistent names,
>> We propose a forward reference to the terms in Section 5.1, unless
>> someone pushes to move section 5.1.2 and 5.1.3 earlier.
>> NEW: The desciption that follows uses the set of constants defined in
>> section 5.1.2 and variable defined in 5.1.3.
>>
>
>> Nit: * "... data is sent
>> again, MAY choose ..." -- s/,//
>>>> GF: To fix with no comma.
>
>> Section 4.4:
>> Nit:
>> * "... over clearing the DF-bit in the IPv4 header ..." -- s/-/ /
>>>> GF: To fix with no hyphen.
>
>>   Section 4.6.1:
>> * "For example, by checking the value ..."
>> -- Suggest: "For example, it could check the value..."
>>>> GF: To fix.
>
>> Nit: * "... from a router or middlebox, performs ..." --
>> s/,//
>> GF: To fix with no comma.
>
>> Section 5:
>> Nit:
>> * "... in a lower layer, DPLPMTUD SHOULD only ..." -- s/,/./
>>>> GF: To fix. Change comma to stop.
>
>
>> Section 5.1.1:
>> Nit:
>> * "... use an up-to-data PMTU once ..." -- I think you mean "up-to-date"
>>>> GF Mistake was hard to see - thanks and will be resolved in the next rev.
>
>>   Section 5.1.2:
>> Nit:
>> * "... from a MAX_PROBES valugreater than 1 because ..." -- s/valugreater/value
>> greater/
>>>> GF To fix add space.
>
>>   Section 5.3.3:
>> Nit:
>> * "... inconsistent, when, for example, ..." -- remove the first comma
>>>> GF To fix with no comma.
>
>> Section 6.1.1:
>> Nit:
>> * "... from off-path insertion of data [RFC8085], suitable methods include ..."
>> -- s/, s/. S/
>>>> GF To fix. Change comma to stop, capital S.
>
>> Section 6.3.2:
>> Nit:
>> * "... packets of the required size, this sets the ..." -- either s/, t/. T/,
>> or s/this/which/
>>>> GF To fix.
>
>>   Section 9:
>> "Parallel forwarding paths SHOULD be considered."  What is the specific action
>> being recommended here?
>>
>> Nits: * "This protection if provided ..."
>> -- s/if/is/
>>>> GF: To fix.
>   
>
>> *
>> "... design (see Section 1.1), this method therefore ..."
>> -- I think "this" should begin a new sentence.
>>>> GF: To fix. Change comma to stop, capital T.
>
>>   * "An on-path attacker, able to create ..."
>> -- remove comma
>>>> GF: To fix with no comma.
>>>>
>> * "This could occur when there are multiple paths are concurrently
>> in use." -- s/there are//
>> GF: To remove.
>>
>>