[PATCH 0/7] mptcp: implement retransmit infrastructure
by Paolo Abeni
This is the first submission aming at git repo inclusion. It includes several
clean-ups, and covers feedback from Mat about una_seq updates and 32 bit
expansion.
In my testbet it survived a good number of self-tests iteration, but I can't
exclude more bugs are pending.
Please see the individual patches for the detailed changelog
RFC v2 -> v1
- update msk una_seq in tcp_incoming_options() and drop old patch 3/8
- cleanup timer management
- cleanup dfrag management
Paolo Abeni (7):
mptcp: move before/after64 into the header file
mptcp: update per unacked sequence on pkt reception
mptcp: queue data for mptcp level retransmission
mptcp: introduce MPTCP retransmission timer
mptcp: implement memory accounting for mptcp rtx queue
mptcp: rework mptcp_sendmsg_frag to accept optional dfrag
mptcp: implement and use MPTCP-level retransmission
net/mptcp/options.c | 40 ++++-
net/mptcp/protocol.c | 395 +++++++++++++++++++++++++++++++++++++++----
net/mptcp/protocol.h | 41 +++++
3 files changed, 439 insertions(+), 37 deletions(-)
--
2.21.0
8 months, 4 weeks
[PATCH v2] mptcp: Make MPTCP socket block/wakeup ignore sk_receive_queue
by Mat Martineau
The MPTCP-level socket doesn't use sk_receive_queue, so it was possible
for mptcp_recvmsg() to remain blocked when there was data ready for it
to read. When the MPTCP socket is waiting for additional data and it
releases the subflow socket lock, the subflow may have incoming packets
ready to process and it sometimes called subflow_data_ready() before the
MPTCP socket called sk_wait_data().
This change adds a new function for the MPTCP socket to use when waiting
for a data ready signal. Atomic bitops with memory barriers are used to
set, test, and clear a MPTCP socket flag that indicates waiting subflow
data. This flag replaces the sk_receive_queue checks used by other
socket types.
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
v2: Use existing sock_def_readable instead of creating a new function,
and add barriers around atomic ops.
net/mptcp/protocol.c | 31 ++++++++++++++++++++++++++++++-
net/mptcp/protocol.h | 4 ++++
net/mptcp/subflow.c | 5 +++++
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 4d2844bff36f..18ba430e8009 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -384,6 +384,31 @@ static enum mapping_status mptcp_get_mapping(struct sock *ssk)
return ret;
}
+static void mptcp_wait_data(struct sock *sk, long *timeo)
+{
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ struct mptcp_sock *msk = mptcp_sk(sk);
+ int data_ready;
+
+ add_wait_queue(sk_sleep(sk), &wait);
+ sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+
+ release_sock(sk);
+
+ smp_mb__before_atomic();
+ data_ready = test_and_clear_bit(MPTCP_DATA_READY, &msk->flags);
+ smp_mb__after_atomic();
+
+ if (!data_ready)
+ *timeo = wait_woken(&wait, TASK_INTERRUPTIBLE, *timeo);
+
+ sched_annotate_sleep();
+ lock_sock(sk);
+
+ sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+ remove_wait_queue(sk_sleep(sk), &wait);
+}
+
static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
int nonblock, int flags, int *addr_len)
{
@@ -434,6 +459,10 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
u64 old_ack;
u32 ssn;
+ smp_mb__before_atomic();
+ clear_bit(MPTCP_DATA_READY, &msk->flags);
+ smp_mb__after_atomic();
+
status = mptcp_get_mapping(ssk);
if (status == MAPPING_ADDED) {
@@ -567,7 +596,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
pr_debug("block");
release_sock(ssk);
- sk_wait_data(sk, &timeo, NULL);
+ mptcp_wait_data(sk, &timeo);
lock_sock(ssk);
}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 4c098acf4170..7a515d8e99d0 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -72,6 +72,9 @@
#define MPTCP_ADDR_IPVERSION_4 4
#define MPTCP_ADDR_IPVERSION_6 6
+/* MPTCP socket flags */
+#define MPTCP_DATA_READY BIT(0)
+
static inline u32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field)
{
return htonl((TCPOPT_MPTCP << 24) | (len << 16) | (subopt << 12) |
@@ -98,6 +101,7 @@ struct mptcp_sock {
u64 write_seq;
u64 ack_seq;
u32 token;
+ unsigned long flags;
struct list_head conn_list;
struct socket *subflow; /* outgoing connect/listener/!mp_capable */
struct pm_data pm;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index d91d817b8779..81c5cfae933a 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -191,6 +191,11 @@ static void subflow_data_ready(struct sock *sk)
if (parent) {
pr_debug("parent=%p", parent);
+
+ smp_mb__before_atomic();
+ set_bit(MPTCP_DATA_READY, &mptcp_sk(parent)->flags);
+ smp_mb__after_atomic();
+
parent->sk_data_ready(parent);
}
}
--
2.22.0
9 months
[PATCH v2] mptcp: sockopt: pass whitelisted setsockopt to subflows
by Florian Westphal
This allows to set tcp congestion control algorithm on all subflows.
In the future we could keep a record of sockopts so we can "replay"
them when another subflow is added at a later point in time.
Only TCP_CONGESTION is allowed at the moment, as some available
TCP knobs either have undesireable behaviour or should be later
implemented at the mptcp level rather than passing them through to
the subflows (TCP_CORK for example).
getsockopt is disabled for the time being to not expose a particular
behaviour ("first subflow on list") at this time.
v2: disable getsockopt, restrict setsockopt via whitelist.
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 51 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 47 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index aaa71b161c1a..a8cbd9741c32 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -707,13 +707,31 @@ static void mptcp_destroy(struct sock *sk)
token_destroy(msk->token);
}
+
+/* Only pass a small subset of level/optnames that are considered safe.
+ * We e.g. don't want MD5SIG (option space too limited) or TCP_CORK
+ * (meaning not entirely clear from mptcp point of view).
+ */
+static bool pass_to_subflows(int level, int optname)
+{
+ if (level != SOL_TCP)
+ return false;
+
+ switch (optname) {
+ case TCP_CONGESTION: return true;
+ }
+
+ return false;
+}
+
static int mptcp_setsockopt(struct sock *sk, int level, int optname,
char __user *uoptval, unsigned int optlen)
{
struct mptcp_sock *msk = mptcp_sk(sk);
+ const struct subflow_context *subflow;
char __kernel *optval;
struct socket *ssock;
- int ret;
+ int ret, err, subflows;
/* will be treated as __user in tcp_setsockopt */
optval = (char __kernel __force *)uoptval;
@@ -729,8 +747,33 @@ static int mptcp_setsockopt(struct sock *sk, int level, int optname,
/* @@ the meaning of setsockopt() when the socket is connected and
* there are multiple subflows is not defined.
+ *
+ * We attempt to perfom the operation on all current subflows, but
+ * only return error if all subflow setsockopt failed.
*/
- return 0;
+ if (!pass_to_subflows(level, optname))
+ return -EOPNOTSUPP;
+
+ ret = 0;
+ err = 0;
+ subflows = 0;
+ lock_sock(sk);
+ list_for_each_entry(subflow, &msk->conn_list, node) {
+ struct socket *tcp_sk = mptcp_subflow_tcp_socket(subflow);
+ int tmp;
+
+ tmp = kernel_setsockopt(tcp_sk, level, optname, optval, optlen);
+ if (tmp == 0)
+ subflows++;
+ else if (err == 0)
+ err = tmp;
+ }
+ release_sock(sk);
+
+ if (subflows == 0 && err)
+ ret = err;
+
+ return ret;
}
static int mptcp_getsockopt(struct sock *sk, int level, int optname,
@@ -755,10 +798,10 @@ static int mptcp_getsockopt(struct sock *sk, int level, int optname,
return ret;
}
- /* @@ the meaning of setsockopt() when the socket is connected and
+ /* @@ the meaning of getsockopt() when the socket is connected and
* there are multiple subflows is not defined.
*/
- return 0;
+ return -EOPNOTSUPP;
}
static int mptcp_get_port(struct sock *sk, unsigned short snum)
--
2.21.0
9 months
[PATCH] mptcp: fix mptcp socket cleanup
by Paolo Abeni
Currently the mptcp proto ->destruct() is never called.
With token accounting this causes a msk refcount leak.
Kmemleak can't detect that, due to msk <-> sk circular
pointer references.
Invoke the distructor via sk_common_release(), and reduce a
bit code duplication.
Additionally we need to acquire a msk refcount when we initialize
the msk token in mptcp_finish_connect(): not the destructor
is correctly called and the client sock will release the msk a
reference to self at close() time.
Fixes: cd2d4eeca99b ("ptcp: Add key generation and token tree")
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
--
I'm ok with squashing this one in the fixes commit
---
net/mptcp/protocol.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 52376b1fe64b..c862630897a6 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -653,8 +653,7 @@ static void mptcp_close(struct sock *sk, long timeout)
}
release_sock(sk);
- sock_orphan(sk);
- sock_put(sk);
+ sk_common_release(sk);
}
static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
@@ -830,9 +829,14 @@ void mptcp_finish_connect(struct sock *sk, int mp_capable)
local_bh_disable();
bh_lock_sock_nested(sk);
+ /* when assiging the token to the msk, we need to held
+ * an additional reference, will be released at msk token
+ * destruction time
+ */
msk->remote_key = subflow->remote_key;
msk->local_key = subflow->local_key;
msk->token = subflow->token;
+ sock_hold(sk);
pr_debug("msk=%p, token=%u", msk, msk->token);
msk->dport = ntohs(inet_sk(msk->subflow->sk)->inet_dport);
--
2.21.0
9 months
[Weekly meetings] MoM - 29nd of August 2019
by Matthieu Baerts
Hello,
Yesterday, we had our 64rd meeting with Mat, Peter, Ossama (Intel OTC),
Christoph (Apple), Paolo, Florian and Davide (RedHat) and myself (Tessares).
Thanks again for this new good meeting!
Here are the minutes of the meeting:
Accepted patches:
- mptcp: fall back to tcp when dss checksum are requested:
- by Florian
- accepted by Peter
- acked by Christoph
- "squashed"
- mptcp: remove msk->subflow->sk dereference in pr_debug statement:
- by Peter
- accepted by Mat
- "squashed"
- mptcp: Change options handling so in_addr are in network byte order:
- by Peter
- 2 patches to ease the "squashed"
- accepted by Mat
- proposed by Florian
- "squashed"
- tcp: fix hooking for mptcp_incoming_options:
- by Paolo
- Accepted by Mat
Pending patches:
- mptcp: Make MPTCP socket block/wakeup ignore sk_receive_queue :
- by Mat
- Mat has sent a new version (2 August)
- Waiting for reviews (especially regarding the new flags and
other modifications)
- Paolo replied during the meeting
- mptcp: Implement interim path manager:
- by Peter
- RFC v3
- still RFC? can we merge it?
- *@Peter* will merge the one from Matt (announce an addr) + use
(another?) sysctl to create sf
- mptcp: sockopt: pass whitelisted setsockopt to subflows:
- by Florian
- v2 sent
- Waiting to be accepted
- @Mat looked at that just after the meeting
- We might want to only take the getsockopt() (return an error)
part and think about the API stuff later
- mptcp: implement retransmit infrastructure:
- 8 patches
- by Paolo
- RFC v2
- Florian gave some comments
- Waiting for more comments
- *@Paolo* is working on it, fixing issues (one reported on the ML)
- will send a new version end of this week (+ other fixes
independent to that)
- mptcp:pm: sysctl to announce an addr:
- by Matth
- RFC
- to sit on top of "mptcp: Implement interim path manager":
- if the goal is to remove this later, better to also create
a new commit here?
- Waiting for the parent change to be merged
- Will also create a new subflow (like in Paolo's patch)
- Peter will look at merging this with his patch, see above
- mptcp: refactor token code:
- 10 patches
- by Florian
- RFC v2
- next step is to make the patches "squashed-friendly"
Checksum:
- (from Peter) where do we stand on checksums? Not necessary for
initial submission?
- *@Matth*: for multipath-tcp.org server, we should disable checksum
(only used if the client sets it)
→ request has been sent
MIB:
- multipath tcp MIB counter placement - share with tcp or extra?
(Florian)
- Eric:
There are about 40 counters.
Space for that will be per netns : num_possible_cpus * 40 * 8 bytes
The cost of folding all the values will make nstat slower even
if MPTCP is not used.
Maybe find a way to not having to fold the MPTCP percpu counters
if MPTCP is not loaded ?
- Florian:
Ok, so 'same proc file' would be fine but 'increase pcpu mem
cost unconditionally' isn't.
MPTCP is builtin (bool). However, we might be able to delay
allocation until first mptcp socket is requested, I will see if this can
be done somehow.
- maybe some static key and clever stuff, seems possible to do (by
Florian)
- like that, some tools will benefit from that (without having to
modify them)
- MIB are useful for quick debug
Memory usage:
- Paolo: we increased memory usage in structure (struct
tcp_options_received) by ~100 bytes for the parsing of the MPTCP options
- this is included in TCP structures (struct tcp_sock) and can be a
problem for upstream
- → a question we should ask before (now or at LPC)
- *@Peter* will look at making some optimisations there by using
unions (not everything is needed all the time)
Namespace pollution:
- prefix all exposed MPTCP functions (even internally in net/mptcp)
with "mptcp_"
- linked to the patches made by Florian (refactor token code)
- idea is to do the same for subflow / pm / crypto (all exposed
functions)
split protocol.[ch]:
- idea would be to send a list of functions that needs to be moved
where and Matth can do the work directly to avoid double work.
- please provide a list of what should be moved where to start
discussing on that (*@Paolo/@Peter*?)
[sg]etsockopt():
- do we want to be able to modify things for a specific subflow?
Using which FD? Another iface?
- related somehow to what Florian did (whitelist)
- there are already use cases for that, e.g. SO_MARK for Android per
subflow → we can ask Lorenzo for that, TCP CC per subflow, etc.
- Benjamin did some work on that for mptcp.org (Socket API):
https://inl.info.ucl.ac.be/system/files/main_8.pdf
- another way is to do that via the PM (Netlink)
- conclusion: we should think about that before freezing the API:
the idea for the moment is then not to allow any change and report this
for later.
LPC:
- Are we aiming for RFC6824 or RFC6824bis compliance in the initial
upstream patchset? Or is that to be determined at some time after LPC?:
- Do we need to explain the differences?:
- big difference is the initialisation (how we share the key
→ no longer in the SYN, only in the 3rd ACK + first data)
- add_addr is a bit different (with HMAC)
- reason of a remove
- fast-close with RST
- maybe interesting to support only the v1, not to have to deal
with both versions and add complexity the code
- (but not easy to test with others for the moment)
- maybe better to say that the intension is to support only the
v1 and ask the question @LPC
- Can we re-use abstract / intro / history from last time?:
- ok for the beginning of the paper: MPTCP didn't change.
- the presentation will of course be different
- What has been upstreamed already?:
- skb extensions
- ULP diag? → almost done. Should be in next-next soon (before LPC)
- do we still need to mention TCP Option framework that had been
rejected?
https://www.mail-archive.com/netdev@vger.kernel.org/msg214958.html → we
can skip this
- prepare a list of questions? what should be in the first
version? (ipv6, mib, checksum, memory usage for parsing TCP options,
which mptcp version to support etc.) → for next time, please start
thinking about it
- Looks like we need to upload our slides by the evening of 8 September
- We should send the draft sooner.
Next meeting:
- We propose to have it next Thursday, the 5th of September.
- Usual time: 16:00 UTC (9am PDT, 6pm CEST)
- Still open to everyone!
- https://annuel2.framapad.org/p/mptcp_upstreaming_20190905
Feel free to comment on these points and propose new ones for the next
meeting!
Talk to you next week,
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
[RFC v2 00/10] mptcp: cleanup token code
by Florian Westphal
This series again marked RFC so noone bothers to squash all of
this into the mptcp tree.
In this second round, I've addressed all comments from Matthieu
and Peter. As a result, even more code was pruned from token.c
and placed as static helper in subflow.c.
I've added a 'v2' mini-changelog to all affected patches.
pm.c | 6
protocol.c | 5
protocol.h | 23 +--
subflow.c | 152 +++++++++++++++++++--
token.c | 426 +++++++++++++++++++------------------------------------------
5 files changed, 277 insertions(+), 333 deletions(-)
9 months
[PATCH] tcp: fix hooking for mptcp_incoming_options()
by Paolo Abeni
Currently we don't process mptcp options for half-open sockets, e.g.
for packet with payload delivered to the rx queue via
tcp_rcv_state_process(), and for pure ack landing in the same code path.
We can deal with the first code path moving the existing
mptcp_incoming_options() hook inside tcp_data_queue(), but we need
to add an additional hook to deal with the specified pure acks
Fixes: d2d58ff48665 ("mptcp: Implement MPTCP receive path")
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
I'm ok with squashing this one into the fixed patch
Any other option not needing an additional hook more than welcome
The reported issue does not affect the existing code base,
but as soon as we start tracking the MPTCP level unacked sequence
number it become apparent, as we can't track it for
half-open sockets
---
net/ipv4/tcp_input.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 1c06cfb06374..cf2cf09d7d3d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4763,6 +4763,9 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
bool fragstolen;
int eaten;
+ if (sk_is_mptcp(sk))
+ mptcp_incoming_options(sk, skb, &tp->rx_opt);
+
if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
__kfree_skb(skb);
return;
@@ -5711,10 +5714,6 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
/* Process urgent data. */
tcp_urg(sk, skb, th);
- /* Process MPTCP options */
- if (sk_is_mptcp(sk))
- mptcp_incoming_options(sk, skb, &tp->rx_opt);
-
/* step 7: process the segment text */
tcp_data_queue(sk, skb);
@@ -6335,8 +6334,11 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
case TCP_CLOSE_WAIT:
case TCP_CLOSING:
case TCP_LAST_ACK:
- if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt))
+ if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
+ if (sk_is_mptcp(sk))
+ mptcp_incoming_options(sk, skb, &tp->rx_opt);
break;
+ }
/* fall through */
case TCP_FIN_WAIT1:
case TCP_FIN_WAIT2:
--
2.21.0
9 months
[PATCH 1/2] mptcp: Change options handling so in_addr are in network byte order
by Peter Krystad
squashto: Add ADD_ADDR handling
Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
---
net/mptcp/options.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 0e7abcea5bae..63ae21e573ef 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -209,7 +209,7 @@ void mptcp_parse_option(const unsigned char *ptr, int opsize,
mp_opt->addr_id = *ptr++;
if (mp_opt->family == MPTCP_ADDR_IPVERSION_4) {
mp_opt->add_addr = 1;
- mp_opt->addr.s_addr = get_unaligned_be32(ptr);
+ memcpy((u8 *)&mp_opt->addr.s_addr, (u8 *)ptr, 4);
pr_debug("ADD_ADDR: addr=%x, id=%d",
mp_opt->addr.s_addr, mp_opt->addr_id);
#if IS_ENABLED(CONFIG_IPV6)
@@ -593,7 +593,8 @@ void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts)
if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) {
*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR, TCPOLEN_MPTCP_ADD_ADDR,
MPTCP_ADDR_IPVERSION_4, opts->addr_id);
- *ptr++ = htonl(opts->addr.s_addr);
+ memcpy((u8 *)ptr, (u8 *)&opts->addr.s_addr, 4);
+ ptr += 1;
}
#if IS_ENABLED(CONFIG_IPV6)
--
2.17.2
9 months
[PATCH RFC 00/10] mptcp: refactor token code
by Florian Westphal
This series is intentionally marked RFC so noone bothers
to squash all of this into the mptcp tree.
The idea is to first do a review-friendly series, with
each change done in an individual patch.
Then, once consensus is reached (maybe over several iterations),
I would send a few larger patches suitable for squashing (rather
than being review friendly...).
The patches remove unneeded small helpers, add 'mptcp' prefix
to non-static functions, add error handling to those that may
fail and adds a few comments.
Feedback welcome.
pm.c | 6
protocol.c | 5
protocol.h | 27 +--
subflow.c | 76 ++++++++--
token.c | 447 +++++++++++++++++++++++++++----------------------------------
5 files changed, 278 insertions(+), 283 deletions(-)
9 months
[PATCH] mptcp: remove msk->subflow->sk dereference in pr_debug statement
by Peter Krystad
msk->subflow may be NULL when creation of an mptcp_sock fails due
to the protocol not being enabled.
squashto: Add key generation and token tree
Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
---
net/mptcp/protocol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index aaa71b161c1a..a58211d8568c 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -702,7 +702,7 @@ static void mptcp_destroy(struct sock *sk)
{
struct mptcp_sock *msk = mptcp_sk(sk);
- pr_debug("msk=%p, subflow=%p", sk, msk->subflow->sk);
+ pr_debug("msk=%p", sk);
token_destroy(msk->token);
}
--
2.17.2
9 months