Subject: [multipathtcp] Review of draft-ietf-mptcp-multiaddressed-03

From: Andrew McGregor <[email-masked]>
Date: Thu, 16 Jun 2011 10:57:31 +1200

I finally got to do this review, what with earthquakes and reorganisations it was rather a mission.

I think this document is in good shape, most of my comments are little textual items rather than substantive issues needing clarifications.

Three substantive issues first:

Section 3.3.1, page 21:

   Note that checksumming relies on the TCP subflow containing
   contiguous data, and therefore a TCP subflow MUST NOT use the Urgent
   Pointer to interrupt an existing mapping.  Further note, however,
   that if Urgent data is received on a subflow, it SHOULD be mapped to
   the data sequence space and delivered to the application analogous to
   Urgent data in regular TCP.

Once you receive urgent data, the world is no longer safe for the checksum... so something needs to be said about what happens next after the urgent data is delivered.  RST everything and require new joins?

Section 3.3.1, page 22:

   Data sequence numbers are always 64-bit quantities, and MUST be
   maintained as such in implementations.

At the end of this paragraph, perhaps add: An implementation MAY choose to send 32 or 64 bit Data Sequence Numbers independently for each packet.

This is implicitly allowed by Appendix A, but I think it should be explicit in this section.

Section 3.4.1, page 33:

   A host that receives an ADD_ADDR but finds a connection setup to that
   address is unsuccessful SHOULD NOT perform further connection
   attempts to this address for this connection.  A sender that wants to
   trigger a new incoming connection attempt on a previously advertised
   address can therefore refresh ADD_ADDR information by sending the
   option again.

This is inconsistent with section 3.7.3, page 39:

   From the connection initiator's point of view, if an MP_JOIN
   fails, it SHOULD NOT attempt to connect to the same IP address during
   the lifetime of the connection, unless the other host refreshes the
   information with a REMOVE_ADDR and then an ADD_ADDR for the same
   address.

Should this read: it SHOULD NOT attempt to connect to the same IP address and port number

An MP_JOIN may fail if the initiator is behind a NAT and is not aware that it joined from the same IP address, so a join to the same remote port number fails where a join to a different port would succeed. Conversely, it may be desirable not to allow this situation to initiate multiple subflows.

And in any case, these two paragraphs should be consistent with each other as to how the address should be refreshed.

Nits:

Section 2, third paragraph, page 8: "The client are server keys" -> "The client and server keys". Also, I find this sentence unweildly, but I'm not sure how it could be reworded.

Section 2, third paragraph, page 8: "a 32 bits token" -> "a 32 bit token".  This same error occurs many times in the remainder of the document, sometimes with and without a hyphen. Some consistency would be good.

Section 2, fourth paragraph, page 8: "The MP_JOIN option contains the server's token that uniquely identifies the MPTCP connection to which the subflow must be associated and a random number." -> "The MP_JOIN option contains the server's token and a random number." since the function of the token was described in the previous paragraph.

Section 2, last paragraph, page 9: "possibly out-of-sequence over", add a comma after sequence

Section 3.1, page 11: remove "it just means that", since this doesn't add anything

Section 3.3.1, page 22: "and an unmapped data" -> "and any unmapped data"

Section 3.4.1, page 32: "and ordering it will not break" -> "and ordering will not break"

Bullet lists at the end of Section 6: each bullet has a : after the subject of the first sentence... this seems silly, it just breaks the reader's flow.

Appendix A: "24 bytes (for Join) - both of which" -> "24 bytes (for Join), both of which"

---

From: "Ford, Alan" <[email-masked]>
Date: Thu, 16 Jun 2011 09:50:06 +0100

Many thanks for this very valuable feedback. We'll apply these comments
and expect version -04 to be out soon.

Some stuff inline...

> -----Original Message-----
> From: Andrew McGregor [mailto:[email-masked]]
> Sent: 15 June 2011 23:58
> To: Yoshifumi Nishida; Philip Eardley; Ford, Alan; Costin Raiciu; Mark
> Handley; Olivier Bonaventure; [email-masked]
> Subject: Review of draft-ietf-mptcp-multiaddressed-03
> 
> I think this document is in good shape, most of my comments are little
> textual items rather than substantive issues needing clarifications.
> 
> Once you receive urgent data, the world is no longer safe for the
> checksum... so something needs to be said about what happens next
after
> the urgent data is delivered.  RST everything and require new joins?

The rule there is that Urgent data MUST NOT interrupt an existing data
sequence mapping. (However, if an urgent pointer aligns exactly with a
data sequence mapping, that can be treated as a chunk of urgent data
analogous to regular TCP).

As long as that rule is adhered to, no checksums should break. And if a
checksum does break, you go through the standard fallback procedure
(which usually involves closing the subflow).

So I don't think we need to do anything as drastic as RST everything.

> At the end of this paragraph, perhaps add: An implementation MAY
choose
> to send 32 or 64 bit Data Sequence Numbers independently for each
> packet.
> 

This should also be implied by much of the text in that paragraph, but
if you think it makes things clearer we can add that.

> Should this read: it SHOULD NOT attempt to connect to the same IP

Yes, it should, thanks.

> An MP_JOIN may fail if the initiator is behind a NAT and is not aware
> that it joined from the same IP address, so a join to the same remote
> port number fails where a join to a different port would succeed.
> Conversely, it may be desirable not to allow this situation to
initiate
> multiple subflows.
> 
> And in any case, these two paragraphs should be consistent with each
> other as to how the address should be refreshed.

Good point, I think we were converging on ADD_ADDR being a refresh with
no need for a REMOVE_ADDR so we'll remove that mention to REMOVE_ADDR in
3.7.3.

Many thanks again for spending the time looking at this, it's greatly
appreciated.

Cheers,
Alan

---

From: Andrew McGregor <[email-masked]>
Date: Thu, 16 Jun 2011 21:12:54 +1200

> So I don't think we need to do anything as drastic as RST everything.

Ah, right.  I see how that works now.

> if you think it makes things clearer we can add that.

I do, yes, I noted it on the first read as a bit confusing, and it was only cleared up by reading the appendix.

> Many thanks again for spending the time looking at this, it's greatly

No problem, I wanted to do it... but like I said, events surrounding me rather got in the way.

Andrew