[multipathtcp] AD review detailed comments [was: Re: AD review is coming]

[Mirja Kuehlewind <[e-mail]>]

Hi again,

Here are the more detailed comment of my review as promised. Most of these comments are editorial, however, there are a few comments on use of normative language as well as clarifications and also one point about specifying the registration policy for the new registry (comment 13 below) which really must be address in the next version.

Thanks!
Mirja

----------------------------

1) Sec 3.1: “If a responder does not support (or does not want to support)
   any of the initiator's proposals, it can respond without an
   MP_CAPABLE option, thus forcing a fallback to regular TCP.”
   I wonder if this should be normative: “…it MUST respond without an MP_CAPABLE option…”..?

2) Sec 3.3.1: “… and any later data for that sequence space should be ignored.”
   Maybe SHOULD?

3) Sec 3.3.2: “If the DSN received is
   32 bits, it is valid for the implementation to choose whether to send
   a 32-bit or 64-bit Data ACK.”
  Maybe “… the implementation MAY choose whether to send…”

4) Sec 3.3.3: I guess if there is only one subflow it is probably safe to send the TCP FIN together with a DATA_FIN (as an optimisation), or would there be a reason to not do that? Should that be mentioned in the text?

5) Sec 3.3.8: Is the reference to ECN really needed? I feel that this idea is hard to understand without further context.

6) Sec 3.6: Maybe I missed it that but it seems that the error codes “Unacceptable performance” is not further used in the document while I would think further guidance might be needed. 

7) Sec 3.7: Minor editorial point I noticed here but there might be other cases:
“If lost options on data packets occur on any other subflow apart from the initial subflow…”
Not sure if “initial” is correct here because if you move you may not have the initial subflow anymore… I guess it would be something like “all expect one” here…

8) Sec 3.9.2: I wondering about this text, given this is a bis doc and we have an deployment experience report:
“We expect that experience gathered from deployments will provide further guidance on this, and will be affected by particular application characteristics (which are likely to change over time).  […] Results from experimental deployments are needed in order to verify the correctness of this proposal.”
Maybe just rephrase it say something like “We got some experience but more is needed/there is no generics solution.”.. or just remove…

9) Maybe section 3.9.2 should also say again that receiving a second ADD_ADDRESS (with the same ID) is a request from the other end to open a subflow immediately?

10) Sec 3.9.3: Should learned information really be cached “forever”, or should the text maybe says also something about retrying after some time…?

11) Sec 8.1: “This document defines one additional subtype
   (ADD_ADDR) and updates the references to this document for all sub-
   types except ADD_ADDR, which is deprecated.”
   What?

12) Also sec 8.1: RFC6824 also has this half-sentence:
“future assignments are to be defined by Standards Action as defined by [25].”
which got lost in this document. However, given this document obsoletes RFC6824 it should probably also mention the assignment policy for this registry.

13) Sec 8.3. doesn’t specify an assignment policy.

14) Given RFC6824 will be obsoleted by this spec, it does not to be listed in the reference section.

Nits:
- Sec 3.3.6: s/if it so desired for reliability reasons./if it is desired for reliability reasons./
- Sec 3.3.&: s/Multiple retransmissions are triggers that/Multiple retransmissions are triggered that/ or actually
                         s/Multiple retransmissions are triggers that/If multiple retransmissions are triggered that/
- Sec 3.7: s/Middlebox interferance/Middlebox interference/
- Sec 3.9.2: s/If the the same ports are used on all subflows/If the same ports are used on all subflows/



> On 12. Apr 2019, at 18:40, Mirja Kuehlewind <[e-mail]> wrote:
> 
> Hi authors, hi shepherd/Phil,
> 
> Just wanted to quickly let you know that I’ve started the IETF last call just now in order to hopefully get this document on the telechat in 3 weeks. 
> 
> I’ve not finished my AD review completely but I am nearly done and convinced that this doc is ready to start IETF last call. So far I have only some editorial or smallish comments and nits which I will send in detail beginning of next week and can be address during or after IETF last call.
> 
> Only one request for now (mostly with an eye on the IESG evaluation): The draft says that changes are made which are not backward compatible (see sec 3.1). That’s fine, however, it would be good to also explain a little bit in the draft as well as in the shepherd write up (!) why this is okay. My understanding is that in case of a v0 server the connection will “just" fall-back to non-MPTCP capable and that this is acceptable in current deployment situation. Please confirm and at least update the write-up accordingly as soon as possible!
> 
> Thanks!
> Mirja
> 
> 
>