Re: [tsvwg] WGLC for DPLPMTUD - WGLC comments, still to be resolved.

[Maksim Proshin <[e-mail]>]

Hi Gorry,

Please see below my follow-up.

BR, Maxim

-----Original Message-----
From: G Fairhurst <[e-mail]> 
Sent: Friday, December 20, 2019 6:21 PM
To: Maksim Proshin <[e-mail]>
Cc: [e-mail]
Subject: Re: [tsvwg] WGLC for DPLPMTUD - WGLC comments, still to be resolved.



Maxim, thanks again for all your comments. These comments below will require more help to resolve. Please help us discuss on the list, or help by offering some initial text proposals.

Best wishes,

Gorry

Major comments:

1.
RFC 4821 states that “an implementation SHOULD implement some form of MTU verification” and I don’t see how that aspect is addressed in DPLMTUD. Did I miss it or was it intentionally skipped?
GF: QUERY - I am unsure I understand the question - but maybe this relates to always probing a new PLPMTU before it is used - in which case, this is covered.
[Maxim] I refer to Section 7.8 of RFC 4821. My understanding of that section is that in some cases PLPMTUD might fail to find a correct PLPMTU value when e.g. a flow traverses multiple paths. In that case it might lead to packet losses. RFC 4821 recommends to check the loss rate and if it rises, to switch back to the previous value of eff_pmtu. I think that "always probing a new PLPMTU before it is used" is not enough. 
First of all I wanted to understand if DPLPMTUD recommends any other mechanism of the verification or refers to RFC 4821 for that or intentionally skips it (then why). If DPLMTUD relies on RFC 4821 (which I'm fine with), I would clearly state that. If you want to cover it in DPLMTUD, it will probably require a new state and a timer (or re-use SEARCH_COMPLETE).   

3.
Some PLs like SCTP can’t re-fragment data during retransmission which might lead to a problem if PMTU was decreased in run-time or the PLPMTU value was discovered wrongly. In some case the data would never be retransmitted. This aspect is not covered in the draft. I think it would be good at least to recommend to not set DF-bit for retransmitted data and to not fragment user data to pieces larger than BASE_PMTU (or even MIN_PMTU?).
GF: QUERY What does the WG think?

5.
Sec 3: “A method is REQUIRED to signal an appropriate MPS to the higher layer using the PL. ” – In general I don’t see how the draft addresses it. Is it up to the PL implementation or the PL specification? I assume it requires SCTP Socket API update. If so, is it out of scope of this doc?
GF: Should we specify this?
[Maxim] Now when I thought more about this, I'd like to understand the idea of this "REQUIRED". Do you want each PL regardless of whether it supports fragmentation or not to signal the MPS value every time the PLPMTU value is changed? If so I would probably replace it by "SHOULD" if fragmentation is supported.
Anyway, I personally think the signal should be specified if "REQUIRED" is used because implementers are forced to implement it and will need to care about the signal. If not specified, it will be many different implementations. If some PL already has such signal specified in another doc, it could be good to refer to it. In the SCTP case there is a way for the user to retrieve the current PMTU value but it is not a signal.   

Minor comments:

6.
Im: The default value is specified for some parameters but not for all and without clear explanation of why such values are used as default. 
The same is for recommended values. Shouldn’t it be consistent?
GF : Which parameters do you mean?
[Maxim] I meant mainly section 5.1.2. E.g. MAX_PROBES has the default value and BASE_PMTU has the recommended value. Other constants don't have any of them. For me it would be better to align it and e.g. put recommended values for all of them. Perhaps Section 5.1.1 could also be improved. 
This is really a minor comment so you can skip it if there are no recommendations for other constants.

9.
Mi: Sec 1.3. why don’t you refer to SCTP specification RFC4960 instead of or additionally to RFC4821. Section 7.3 of RFC4960 has a statement “An endpoint SHOULD apply these techniques, and SHOULD do so on a per-destination-address basis.” so for me it’s more relelvant to refer to RFC4960.
GF : Should this document update RFC4960?
[Maxim] Good question. I personally think it should update it. Let's see what Michael and Randall say. 

17.
Im/Q: Sec 4.4: “MUST suspend” – I assume it’s valid in general even when 
PL is using the other PL like QUIC uses UDP. If so, it would be good to 
clarify.
GF: I did not understand what text to add.
[Maxim] Let's agree on the issue first. If a PL uses another PL (like QUIC over UDP), does the draft assume that DPLPMTUD should only be enabled in the highest level (in QUIC)? 

25.
Im: Sec 6.2 (maybe general for Sec 6): It would be good to recommend to 
enable fragmentation (do not set DF bit) for all packets except for 
probes or at least for retransmitted packets.
GF:  i understand and know this is done. in some SCTP implementations. 
What specific text is required?
[Maxim] I think the following text should work:
"In IPv4, it is NOT RECOMMENDED to set the DF bit for any packet which is not a probe."
If you think it's too strong then probably the following text is a better choice:
"When retransmitting any packet which is not a probe, SCTP SHOULD NOT set the DF bit."

26.
Im/Q: Sec 6.2: Should or should not probe packets influence assoc and 
path error counters? I think it’s “SHOULD” when a probe is acknowledged 
and “SHOULD NOT” when it’s lost. In other words probes should only clear 
the error counters in my view.
GF: Is this correct below:
NEW:
  A successful probe updates the association and path
             counters, but an unsuccessful probe is discounted (assumed
             to be a result of choosing too large a PLPMTU).

[Maxim] It's fine with me.