[MPTCP] Re: [PATCH] mptcp: Basic single-subflow DATA_FIN

Send a DATA_FIN along with any subflow TCP FIN flag. This seems to expose a bug in either the receive side disconnect handling or the mptcp_connect test program. The self test has intermittent problems with the receive temp file being truncated. Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com&gt;

--- I've tested this commit when applied to the end of the export branch, but I have not yet applied or tested it earlier in the history with the "initial feature set" patches.

net/mptcp/options.c | 38 +++++++++++++++++++++++++++++++++++--- net/mptcp/subflow.c | 32 ++++++++++++++++++-------------- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 9a18a3670cdf..79890b96afeb 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -384,18 +384,47 @@ static bool mptcp_established_options_mp(struct sock *sk, unsigned int *size, return false; } +static void mptcp_write_data_fin(struct mptcp_subflow_context *subflow, + struct mptcp_ext *ext) +{ + WARN_ONCE(!subflow->conn, "MPTCP: Missing MPTCP socket information");

+ /* Only send DATA_FIN if all data has been sent or this is the last + * mapping.

+ */ + ext->data_fin = 1; + + if (!ext->use_map) { + ext->use_map = 1; + ext->dsn64 = 1; + ext->data_seq = mptcp_sk(subflow->conn)->write_seq; + ext->subflow_seq = 0; + ext->data_len = 1; + } else { + ext->data_len++; + } +} + static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, unsigned int *size, unsigned int remaining, struct mptcp_out_options *opts) { + struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk); unsigned int dss_size = 0; struct mptcp_ext *mpext; unsigned int ack_size; + u8 tcp_fin;

- mpext = skb ? mptcp_get_ext(skb) : NULL; + if (skb) { + mpext = mptcp_get_ext(skb); + tcp_fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN; + } else { + mpext = NULL; + tcp_fin = 0; + } - if (!skb || (mpext && mpext->use_map)) { + if (!skb || (mpext && mpext->use_map) || tcp_fin) { unsigned int map_size; map_size = TCPOLEN_MPTCP_DSS_BASE + TCPOLEN_MPTCP_DSS_MAP64; @@ -405,6 +434,9 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, dss_size = map_size; if (mpext) opts->ext_copy = *mpext; + + if (skb && tcp_fin)

+ mptcp_write_data_fin(subflow, &opts->ext_copy); } else { opts->ext_copy.use_map = 0; WARN_ONCE(1, "MPTCP: Map dropped"); @@ -422,7 +454,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, dss_size += ack_size; - msk = mptcp_sk(mptcp_subflow_ctx(sk)->conn); + msk = mptcp_sk(subflow->conn); if (msk) { opts->ext_copy.data_ack = msk->ack_seq; } else { diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index ff38d54392cd..f6f4dbf2dbab 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -422,6 +422,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk) struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk); struct mptcp_ext *mpext; struct sk_buff *skb; + u16 data_len; u64 map_seq; skb = skb_peek(&ssk->sk_receive_queue); @@ -446,26 +447,29 @@ static enum mapping_status get_mapping_status(struct sock *ssk) if (!subflow->map_valid) return MAPPING_INVALID; + goto validate_seq; } - pr_debug("seq=%llu is64=%d ssn=%u data_len=%u", mpext->data_seq, - mpext->dsn64, mpext->subflow_seq, mpext->data_len); + pr_debug("seq=%llu is64=%d ssn=%u data_len=%u data_fin=%d", + mpext->data_seq, mpext->dsn64, mpext->subflow_seq, + mpext->data_len, mpext->data_fin); - if (mpext->data_len == 0) { + data_len = mpext->data_len; + if (data_len == 0) { pr_err("Infinite mapping not handled"); MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPRX); return MAPPING_INVALID; - } else if (mpext->subflow_seq == 0 && - mpext->data_fin == 1) { - if (WARN_ON_ONCE(mpext->data_len != 1)) - return false; + } - /* do not try hard to handle this any better, till we have - * real data_fin support - */ - pr_debug("DATA_FIN with no payload"); - return MAPPING_DATA_FIN; + if (mpext->data_fin == 1) { + if (data_len == 1) { + pr_debug("DATA_FIN with no payload"); > + return MAPPING_DATA_FIN;> + } + + /* Adjust for DATA_FIN using 1 byte of sequence space */ + data_len--;