[PATCH v2 0/4] mptcp: fallback socket cleanup
by Paolo Abeni
This addresses feedback from Mat on v1
From the original cover letter:
---
After all the above, performance for connection fallen back to plain TCP should
be almost the same of TCP connection on unpacked kernel.
Addittionally 'nc' and 'netperf' now run successfully when forced to MPTCP.
The latter still report a non fatal error at connection closing time as it
performs:
getsockopt(4, SOL_TCP, TCP_MAXSEG, //...);
getsockopt(4, SOL_TCP, TCP_INFO, //...);
which is still unsupported after that the mp capable handshake is started.
For the records on the debug build I'm using netperf scores better result
with MPTCP that with plain TCP ;) [agreed, completely irrelevant, still nice]
The series applies at selftests and is unsquashed - will require quite a bit
of work there and more work to rebase/adjust the patches after kselftest.
I'll start that after the pending patches are in.
_Any_ feedback _very_ welcome!
Paolo Abeni (4):
mptcp: clear 'is_tcp' socket flag when the MP_CAPABLE handshake fails
mptcp: cleanup fallback handling
mptcp: add subflow to conn_list early
mptcp: avoid acquiring the msk lock in mptcp_finisch_connect()
net/mptcp/options.c | 3 +
net/mptcp/protocol.c | 353 ++++++++++++++++++++-----------------------
net/mptcp/subflow.c | 2 +
3 files changed, 169 insertions(+), 189 deletions(-)
--
2.21.0
9 months, 1 week
[PATCH v1 0/4] mptcp: fallback socket cleanup
by Paolo Abeni
This is a rebase on top of last rebased by Florian.
The old patch 2/5 has been dropped - obsoleted by Florian's patches - and the
following patches has been reworked a bit, due to mptcp_subflow_tcp_socket() ->
mptcp_subflow_tcp_sock() change.
Last patch now also deal with mptcp_copy_inaddrs() in order to remove the lock
from mptcp_finish_connect() we must move that to connect() and calling it also
from bind(), listen() allows simplifying mptcp_getname(), too.
From the original cover letter:
---
After all the above, performance for connection fallen back to plain TCP should
be almost the same of TCP connection on unpacked kernel.
Addittionally 'nc' and 'netperf' now run successfully when forced to MPTCP.
The latter still report a non fatal error at connection closing time as it
performs:
getsockopt(4, SOL_TCP, TCP_MAXSEG, //...);
getsockopt(4, SOL_TCP, TCP_INFO, //...);
which is still unsupported after that the mp capable handshake is started.
For the records on the debug build I'm using netperf scores better result
with MPTCP that with plain TCP ;) [agreed, completely irrelevant, still nice]
The series applies at selftests and is unsquashed - will require quite a bit
of work there and more work to rebase/adjust the patches after kselftest.
I'll start that after the pending patches are in.
_Any_ feedback _very_ welcome!
Paolo Abeni (4):
mptcp: clear 'is_tcp' socket flag when the MP_CAPABLE handshake fails
imptcp: cleanup fallback handling
mptcp: add subflow to conn_list early
mptcp: avoid acquiring the msk lock in mptcp_finisch_connect()
net/mptcp/options.c | 3 +
net/mptcp/protocol.c | 320 +++++++++++++++++++++----------------------
net/mptcp/subflow.c | 2 +
3 files changed, 163 insertions(+), 162 deletions(-)
--
2.21.0
9 months, 2 weeks
[RFC PATCH 0/5] mptcp: fallback socket cleanup
by Paolo Abeni
This is a quite early RFC to acquire as early feedback as possible.
I did self-tests a few times on 3/5 and on patch 5/5 without failure, but
likely some bugs are still present.
The first 3 patches in this series:
- clarify and cleanup the TCP fallback handling
- fixes setsockopt()/getsockopt() for connected/unbound sockets
The last 2 patches do more aggressive/invasive cleanup:
- drop the 'fallback' field so that we always use the conn_list
- drop the nested socket in mptcp_finish_connect()
- remove the lock/lookup/get/unlock dance in most case (due the the above)
After all the above, performance for connection fallen back to plain TCP should
be almost the same of TCP connection on unpacked kernel.
Addittionally 'nc' and 'netperf' now run successfully when forced to MPTCP.
The latter still report a non fatal error at connection closing time as it
performs:
getsockopt(4, SOL_TCP, TCP_MAXSEG, //...);
getsockopt(4, SOL_TCP, TCP_INFO, //...);
which is still unsupported after that the mp capable handshake is started.
For the records on the debug build I'm using netperf scores better result
with MPTCP that with plain TCP ;) [agreed, completely irrelevant, still nice]
The series applies at selftests and is unsquashed - will require quite a bit
of work there and more work to rebase/adjust the patches after kselftest.
I'll start that after the pending patches are in.
_Any_ feedback _very_ welcome!
Paolo Abeni (5):
mptcp: clear 'is_tcp' socket flag when the MP_CAPABLE handshake fails
mptcp: pass subflow to mptcp_finish_connect()
mptcp: cleanup fallback handling
mptcp: remove 'fallback' field
mptcp: avoid acquiring the msk lock in mptcp_finisch_connect()
net/mptcp/options.c | 3 +
net/mptcp/protocol.c | 342 +++++++++++++++++++++++--------------------
net/mptcp/protocol.h | 4 +-
net/mptcp/subflow.c | 4 +-
4 files changed, 189 insertions(+), 164 deletions(-)
--
2.21.0
9 months, 2 weeks
[RFC 0/1] Optimize mptcp_received_options
by Peter Krystad
This reduces the size from 104 to 32, at the cost of some loss of
clarity. I deliberately did not change the names of any fields,
reducing the impact on code (and the squasher).
This also seems like a good opportunity to remove all of the pr_debug()
statements from options processing which occupy so much of the log.
Peter Krystad (1):
mptcp: Optimize struct mptcp_received_options.
include/linux/tcp.h | 97 ++++++++++++++++++++++++----------------
net/mptcp/options.c | 102 ++++++++++++++++++++++++-------------------
net/mptcp/pm.c | 2 +-
net/mptcp/protocol.h | 2 +-
4 files changed, 118 insertions(+), 85 deletions(-)
--
2.17.2
9 months, 2 weeks
[PATCH 1/3] MAINTAINERS: Add MPTCP maintainer
by Mat Martineau
When we discussed maintainership in an earlier meeting
(https://lists.01.org/hyperkitty/list/mptcp@lists.01.org/thread/HCII2622GW...)
there was a loose consensus that it would be best to have multiple
maintainers. I nominate Matthieu Baerts <matthieu.baerts(a)tessares.net>
as a co-maintainer, what do you think?
squashto: mptcp: Add MPTCP to skb extensions
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8608724835dd..0576b17abd44 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11538,6 +11538,12 @@ F: net/ipv6/calipso.c
F: net/netfilter/xt_CONNSECMARK.c
F: net/netfilter/xt_SECMARK.c
+NETWORKING [MPTCP]
+M: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
+L: netdev(a)vger.kernel.org
+S: Maintained
+F: include/net/mptcp.h
+
NETWORKING [TCP]
M: Eric Dumazet <edumazet(a)google.com>
L: netdev(a)vger.kernel.org
--
2.24.0
9 months, 2 weeks
Preparation of patches for netdev: status
by Matthieu Baerts
Hello,
It took me more time than expected but I applied all the patches that
needed to be applied, the list is now empty:
https://patchwork.ozlabs.org/project/mptcp/list/?state=8
There are some new patches that needs review:
https://patchwork.ozlabs.org/project/mptcp/list/
But in summary, here are the items that still need to be done:
1) Rebase of Florian's patches added at the end of the series (at
least the ones impacting code before MPTCPv1 patches)
2) Remove export of __tcp_poll()
3) Review fixes related to TCP fallback from Paolo (RFC)
4) Review the update of the MAINTAINERS file / apply comments (?)
5) Check if there is no issue with MPTCPv1 patches (+ fixes if needed)
6) Review Mat's cover letters (part 1 & 2)
7) New cover letter for part 3
8) Review all commit messages (part 1, 2, 3)
9) Add signed-off of the submitters
10) Tests
*Florian* : is it still OK for you to do the rebase, items 1 & 2? :)
no urgency because we no longer target Monday but the end
of the week.
*Christoph* : may you look at the item 7 please? :)
I can do the item 9.
For the rest, they can be done by anybody, sometimes even better if
there are multiple people.
I hope I listed everything! Please tell me if something is missing or
not correct!
Cheers,
Matt
--
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
9 months, 2 weeks
[PATCH 0/9] mptcp: support mptcp joins
by Florian Westphal
This set aims to improve handling of multiple subflows.
There are a few remaining problems, see below, but I wanted to send what
I have now rather than keep this back.
The first two patches are new, following few patches are a resend of the
earlier RFC series, I think they're good for acceptance now.
Patch 6 had a few changes since the RFC based on Paolos feedback, please
see patch description for details.
The last two patches are there to allow cancellation/teardown of
not-yet completed join requests. The connecting subflow flows
are placed on a 'join_list' so mptcp_shutdown can close these sockets
as well.
Last patch moves the 'eof hack' to a work queue so we don't set EOF on the
mptcp socket until all subflows have closed. Maybe this isn't needed, I
would be fine with dropping it in favor of mptcp-level DATA_FIN and
reset handling.
The rest of this work is to have msk->conn_list contain tcp_sock structs
without an outer 'struct socket'.
The problem is that for incoming join requests (which are not subject
to accept/request socket queue), there is no 'struct socket', so the kernel
will crash. Even when fixing those up, a NULL sk->sk_socket is problematic
as tcp stack will assume that such tcp sk is already detached from userspace,
i.e. NOSPACE wakeups do not work for them.
The solution here (suggested by Paolo) is to have sk->sk_socket refer
to the mptcp parent socket structure. This assignment is done by
a private function similar to sock_graft().
mptcp.org kernel will be able to establish multiple connections per
mptcp socket but does not consider those extra joins as established, as the
"4th ack" is missing.
A simple "tcp_send_ack" is enough, but there are two problems with this:
1. It can be lost
2. Given we are not expecting to receive any data on such flows this
doesn't seem to be a problem.
During testing I saw that sometimes mptcp.org kernel will emit further
join requests to mptcp-next but those trigger no response (and those
packets do not even show up in perfs network drop monitor, sigh).
I also saw a UAF at mptcp_close where a sock_hold() in tcp_close()
caused a 0->1 recount transition, but i've been unable to reproduce it so far
(might have been caused by a bug that has been fixed in the mean time).
The following changes since commit dd25ff04840135581b833762571aac08a2a94c6c:
sendmsg: truncate source buffer if mptcp sndbuf size was set from userspace (2019-11-30 02:47:59 +0000)
are available in the Git repository at:
git://git.breakpoint.cc/fw/mptcp-next.git mptcp_multijoin_11
for you to fetch changes up to 010c2e19c2d643a87c991bf1e7d4bfc140dfdba5:
don't flag parent socket as RCV_SHUTDOWN if one one subflow has closed (2019-11-30 23:21:25 +0100)
----------------------------------------------------------------
Florian Westphal (9):
token: handle join-before-accept-completion case
mptcp: fix race from mptcp_close/mptcp join
copy connection id from first subflow to mptcp socket
store tcp_sk, not socket
add mptcp_subflow_shutdown
make accept not allocate kernel socket struct
pass subflow socket to mptcp_finish_connect
subflow: place further subflows on new 'join_list'
don't flag parent socket as RCV_SHUTDOWN if one one subflow has closed
net/mptcp/pm.c | 18 +--
net/mptcp/protocol.c | 313 +++++++++++++++++++++++++++++++++++++--------------
net/mptcp/protocol.h | 16 +--
net/mptcp/subflow.c | 56 +++++----
net/mptcp/token.c | 9 +-
5 files changed, 292 insertions(+), 120 deletions(-)
9 months, 2 weeks
[PATCH] mptcp: Remove dss_flags field from struct mptcp_options_received
by Peter Krystad
This field is used only temporarily while parsing options,
use a local variable instead.
Formatted to apply directly to Implement MPTCP receive path.
squashto: Implement MPTCP receive path
Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
---
include/linux/tcp.h | 1 -
net/mptcp/options.c | 12 ++++++------
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 184fc88a140b..a6d6438b27b1 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -107,7 +107,6 @@ struct tcp_options_received {
mp_join : 1,
dss : 1,
version : 4;
- u8 dss_flags;
u8 use_map:1,
dsn64:1,
data_fin:1,
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index c5c770d2e711..e69d56bfabc4 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -99,12 +99,12 @@ void mptcp_parse_option(const unsigned char *ptr, int opsize,
pr_debug("DSS");
ptr++;
- mp_opt->dss_flags = (*ptr++) & MPTCP_DSS_FLAG_MASK;
- mp_opt->data_fin = (mp_opt->dss_flags & MPTCP_DSS_DATA_FIN) != 0;
- mp_opt->dsn64 = (mp_opt->dss_flags & MPTCP_DSS_DSN64) != 0;
- mp_opt->use_map = (mp_opt->dss_flags & MPTCP_DSS_HAS_MAP) != 0;
- mp_opt->ack64 = (mp_opt->dss_flags & MPTCP_DSS_ACK64) != 0;
- mp_opt->use_ack = (mp_opt->dss_flags & MPTCP_DSS_HAS_ACK);
+ flags = (*ptr++) & MPTCP_DSS_FLAG_MASK;
+ mp_opt->data_fin = (flags & MPTCP_DSS_DATA_FIN) != 0;
+ mp_opt->dsn64 = (flags & MPTCP_DSS_DSN64) != 0;
+ mp_opt->use_map = (flags & MPTCP_DSS_HAS_MAP) != 0;
+ mp_opt->ack64 = (flags & MPTCP_DSS_ACK64) != 0;
+ mp_opt->use_ack = (flags & MPTCP_DSS_HAS_ACK);
pr_debug("data_fin=%d dsn64=%d use_map=%d ack64=%d use_ack=%d",
mp_opt->data_fin, mp_opt->dsn64,
--
2.17.2
9 months, 2 weeks
[PATCH] mptcp: Remove flags field from struct mptcp_options_received
by Peter Krystad
This field is only used temporarily while parsing options,
use a local variable instead.
Also move key fields to final position and fix indents.
Formatted to apply directly to Handle MPTCP TCP options.
squashto: Handle MPTCP TCP options
Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
---
include/linux/tcp.h | 7 +++----
net/mptcp/options.c | 17 ++++++++---------
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index da2a281e2c98..7c8a6f877900 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -97,13 +97,12 @@ struct tcp_options_received {
u16 mss_clamp; /* Maximal mss, negotiated at connection setup */
#if IS_ENABLED(CONFIG_MPTCP)
struct mptcp_options_received {
- u8 mp_capable : 1,
+ u64 sndr_key;
+ u64 rcvr_key;
+ u8 mp_capable : 1,
mp_join : 1,
dss : 1,
version : 4;
- u8 flags;
- u64 sndr_key;
- u64 rcvr_key;
} mptcp;
#endif
};
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index adf65d3ff27b..a3dc787e7408 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -14,6 +14,7 @@ void mptcp_parse_option(const unsigned char *ptr, int opsize,
{
struct mptcp_options_received *mp_opt = &opt_rx->mptcp;
u8 subtype = *ptr >> 4;
+ u8 flags;
switch (subtype) {
/* MPTCPOPT_MP_CAPABLE
@@ -31,9 +32,9 @@ void mptcp_parse_option(const unsigned char *ptr, int opsize,
if (mp_opt->version != 0)
break;
- mp_opt->flags = *ptr++;
- if (!((mp_opt->flags & MPTCP_CAP_FLAG_MASK) == MPTCP_CAP_HMAC_SHA1) ||
- (mp_opt->flags & MPTCP_CAP_EXTENSIBILITY))
+ flags = *ptr++;
+ if (!((flags & MPTCP_CAP_FLAG_MASK) == MPTCP_CAP_HMAC_SHA1) ||
+ (flags & MPTCP_CAP_EXTENSIBILITY))
break;
/* RFC 6824, Section 3.1:
@@ -49,7 +50,7 @@ void mptcp_parse_option(const unsigned char *ptr, int opsize,
*
* We don't implement DSS checksum - fall back to TCP.
*/
- if (mp_opt->flags & MPTCP_CAP_CHECKSUM_REQD)
+ if (flags & MPTCP_CAP_CHECKSUM_REQD)
break;
mp_opt->mp_capable = 1;
@@ -59,12 +60,10 @@ void mptcp_parse_option(const unsigned char *ptr, int opsize,
if (opsize == TCPOLEN_MPTCP_MPC_SYNACK) {
mp_opt->rcvr_key = get_unaligned_be64(ptr);
ptr += 8;
- pr_debug("MP_CAPABLE flags=%x, sndr=%llu, rcvr=%llu",
- mp_opt->flags, mp_opt->sndr_key,
- mp_opt->rcvr_key);
+ pr_debug("MP_CAPABLE sndr=%llu, rcvr=%llu",
+ mp_opt->sndr_key, mp_opt->rcvr_key);
} else {
- pr_debug("MP_CAPABLE flags=%x, sndr=%llu",
- mp_opt->flags, mp_opt->sndr_key);
+ pr_debug("MP_CAPABLE sndr=%llu", mp_opt->sndr_key);
}
break;
--
2.17.2
9 months, 2 weeks