5:30 p.m.
his series refactor the mptcp xmit path trying to make sendmsg behavior
more consistent, memory efficent and improve its performances.
The code is also available here:
https://github.com/pabeni/mptcp/tree/mptcp-proposal-devel
To support MP_JOIN and multiple subflows, the idea is adding, with later
patches, an RB-tree structure containing references to the pending page
fragments, their length and the associated mptcp write_seq number (is subflow
rel_write_seq needed, too?). RB-tree node can be allocated using the same
page frag used in this series for skb data. Each RB-tree node will own a
reference to the page frag.
RFC -> v1:
- addressed Mat's feedback, dropping patch 3/4 and switching
to per msk page frag use, and checking mptcp seq number for coalescing
Paolo Abeni (3):
mptcp: use sk_page_frag() in sendmsg
mptcp: sendmsg() do spool all the provided data
mptcp: allow collapsing consecutive sendpages on the same substream
net/mptcp/protocol.c | 181 ++++++++++++++++++++++++++++---------------
1 file changed, 119 insertions(+), 62 deletions(-)
--
2.20.1
5:30 p.m.
New subject: [PATCH 1/3] mptcp: use sk_page_frag() in sendmsg
This clean-up a bit the send path, and allows better performances.
RFC -> v1:
- use sk's page_frag
- fix error handling on memory pressure
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
net/mptcp/protocol.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1e73e307a6d2..1b843c5e2dbc 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -52,10 +52,11 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
struct mptcp_sock *msk = mptcp_sk(sk);
int mss_now, size_goal, poffset, ret;
struct mptcp_ext *mpext = NULL;
- struct page *page = NULL;
+ struct page_frag *pfrag;
struct sk_buff *skb;
struct sock *ssk;
size_t psize;
+ long timeo;
pr_debug("msk=%p", msk);
if (msk->subflow) {
@@ -80,33 +81,33 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
goto put_out;
}
- /* Initial experiment: new page per send. Real code will
- * maintain list of active pages and DSS mappings, append to the
- * end and honor zerocopy
+ lock_sock(sk);
+ lock_sock(ssk);
+ timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
+
+ /* use the mptcp page cache so that we can easily move the data
+ * from one substream to another, but do per subflow memory accounting
*/
- page = alloc_page(GFP_KERNEL);
- if (!page) {
- ret = -ENOMEM;
- goto put_out;
+ pfrag = sk_page_frag(sk);
+ while (!sk_page_frag_refill(ssk, pfrag)) {
+ ret = sk_stream_wait_memory(ssk, &timeo);
+ if (ret)
+ goto release_out;
}
/* Copy to page */
- poffset = 0;
+ poffset = pfrag->offset;
pr_debug("left=%zu", msg_data_left(msg));
- psize = copy_page_from_iter(page, poffset,
+ psize = copy_page_from_iter(pfrag->page, poffset,
min_t(size_t, msg_data_left(msg),
- PAGE_SIZE),
+ pfrag->size - poffset),
&msg->msg_iter);
pr_debug("left=%zu", msg_data_left(msg));
-
if (!psize) {
ret = -EINVAL;
- goto put_out;
+ goto release_out;
}
- lock_sock(sk);
- lock_sock(ssk);
-
/* Mark the end of the previous write so the beginning of the
* next write (with its own mptcp skb extension data) is not
* collapsed.
@@ -116,8 +117,8 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
TCP_SKB_CB(skb)->eor = 1;
mss_now = tcp_send_mss(ssk, &size_goal, msg->msg_flags);
-
- ret = do_tcp_sendpages(ssk, page, poffset, min_t(int, size_goal, psize),
+ psize = min_t(int, size_goal, psize);
+ ret = do_tcp_sendpages(ssk, pfrag->page, poffset, psize,
msg->msg_flags | MSG_SENDPAGE_NOTLAST);
if (ret <= 0)
goto release_out;
@@ -143,6 +144,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
mpext->checksum, mpext->dsn64);
} /* TODO: else fallback */
+ pfrag->offset += ret;
msk->write_seq += ret;
subflow_ctx(ssk)->rel_write_seq += ret;
@@ -153,9 +155,6 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
release_sock(sk);
put_out:
- if (page)
- put_page(page);
-
sock_put(ssk);
return ret;
}
--
2.20.1
5:30 p.m.
New subject: [PATCH 2/3] mptcp: sendmsg() do spool all the provided data
This makes mptcp sendmsg() behaviour more consistent and
improves xmit performances.
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
net/mptcp/protocol.c | 91 ++++++++++++++++++++++++++------------------
1 file changed, 53 insertions(+), 38 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1b843c5e2dbc..085d658af772 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -47,52 +47,24 @@ static struct sock *mptcp_subflow_get_ref(const struct mptcp_sock
*msk)
return NULL;
}
-static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
+static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
+ struct msghdr *msg, long *timeo)
{
struct mptcp_sock *msk = mptcp_sk(sk);
int mss_now, size_goal, poffset, ret;
struct mptcp_ext *mpext = NULL;
struct page_frag *pfrag;
struct sk_buff *skb;
- struct sock *ssk;
size_t psize;
- long timeo;
-
- pr_debug("msk=%p", msk);
- if (msk->subflow) {
- pr_debug("fallback passthrough");
- return sock_sendmsg(msk->subflow, msg);
- }
-
- ssk = mptcp_subflow_get_ref(msk);
- if (!ssk)
- return -ENOTCONN;
-
- if (!msg_data_left(msg)) {
- pr_debug("empty send");
- ret = sock_sendmsg(ssk->sk_socket, msg);
- goto put_out;
- }
-
- pr_debug("conn_list->subflow=%p", ssk);
-
- if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL)) {
- ret = -ENOTSUPP;
- goto put_out;
- }
-
- lock_sock(sk);
- lock_sock(ssk);
- timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
/* use the mptcp page cache so that we can easily move the data
* from one substream to another, but do per subflow memory accounting
*/
pfrag = sk_page_frag(sk);
while (!sk_page_frag_refill(ssk, pfrag)) {
- ret = sk_stream_wait_memory(ssk, &timeo);
+ ret = sk_stream_wait_memory(ssk, timeo);
if (ret)
- goto release_out;
+ return ret;
}
/* Copy to page */
@@ -103,10 +75,8 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
pfrag->size - poffset),
&msg->msg_iter);
pr_debug("left=%zu", msg_data_left(msg));
- if (!psize) {
- ret = -EINVAL;
- goto release_out;
- }
+ if (!psize)
+ return -EINVAL;
/* Mark the end of the previous write so the beginning of the
* next write (with its own mptcp skb extension data) is not
@@ -121,7 +91,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
ret = do_tcp_sendpages(ssk, pfrag->page, poffset, psize,
msg->msg_flags | MSG_SENDPAGE_NOTLAST);
if (ret <= 0)
- goto release_out;
+ return ret;
if (skb == tcp_write_queue_tail(ssk))
pr_err("no new skb %p/%p", sk, ssk);
@@ -149,8 +119,53 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
subflow_ctx(ssk)->rel_write_seq += ret;
tcp_push(ssk, msg->msg_flags, mss_now, tcp_sk(ssk)->nonagle, size_goal);
+ return ret;
+}
+
+static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
+{
+ struct mptcp_sock *msk = mptcp_sk(sk);
+ size_t copied = 0;
+ struct sock *ssk;
+ int ret = 0;
+ long timeo;
+
+ pr_debug("msk=%p", msk);
+ if (msk->subflow) {
+ pr_debug("fallback passthrough");
+ return sock_sendmsg(msk->subflow, msg);
+ }
+
+ ssk = mptcp_subflow_get_ref(msk);
+ if (!ssk)
+ return -ENOTCONN;
+
+ if (!msg_data_left(msg)) {
+ pr_debug("empty send");
+ ret = sock_sendmsg(ssk->sk_socket, msg);
+ goto put_out;
+ }
+
+ pr_debug("conn_list->subflow=%p", ssk);
+
+ if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL)) {
+ ret = -ENOTSUPP;
+ goto put_out;
+ }
+
+ lock_sock(sk);
+ lock_sock(ssk);
+ timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
+ while (msg_data_left(msg)) {
+ ret = mptcp_sendmsg_frag(sk, ssk, msg, &timeo);
+ if (ret < 0)
+ break;
+
+ copied += ret;
+ }
+ if (copied > 0)
+ ret = copied;
-release_out:
release_sock(ssk);
release_sock(sk);
--
2.20.1
5:30 p.m.
New subject: [PATCH 3/3] mptcp: allow collapsing consecutive sendpages on the same substream
If the current sendmsg() lands on the same subflow we used last, we
can try to collapse the data.
RFC -> v1
- claried collasping schema
- update data_len for collapsed skb in mptcp_sendmsg_frag()
- fix collapsing decision on mptcp-level OoO
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
net/mptcp/protocol.c | 77 ++++++++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 17 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 085d658af772..0a947f44f712 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -47,11 +47,24 @@ static struct sock *mptcp_subflow_get_ref(const struct mptcp_sock
*msk)
return NULL;
}
+static inline bool mptcp_skb_can_collapse_to(const struct mptcp_sock *msk,
+ const struct sk_buff *skb,
+ const struct mptcp_ext *mpext)
+{
+ if (!tcp_skb_can_collapse_to(skb))
+ return false;
+
+ /* can't collapse if there is a MPTCP level out-of-order */
+ return mpext->data_seq + mpext->data_len == msk->write_seq;
+}
+
static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
- struct msghdr *msg, long *timeo)
+ struct msghdr *msg, long *timeo, int *pmss_now,
+ int *ps_goal)
{
+ int mss_now, avail_size, size_goal, poffset, ret;
struct mptcp_sock *msk = mptcp_sk(sk);
- int mss_now, size_goal, poffset, ret;
+ bool collapsed, can_collapse = false;
struct mptcp_ext *mpext = NULL;
struct page_frag *pfrag;
struct sk_buff *skb;
@@ -78,25 +91,48 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
if (!psize)
return -EINVAL;
- /* Mark the end of the previous write so the beginning of the
- * next write (with its own mptcp skb extension data) is not
- * collapsed.
- */
+ mss_now = tcp_send_mss(ssk, &size_goal, msg->msg_flags);
+ *pmss_now = mss_now;
+ *ps_goal = size_goal;
+ avail_size = size_goal;
skb = tcp_write_queue_tail(ssk);
- if (skb)
- TCP_SKB_CB(skb)->eor = 1;
+ if (skb) {
+ mpext = skb_ext_find(skb, SKB_EXT_MPTCP);
+ BUG_ON(!mpext);
+
+ /* Limit the write to the size available in the
+ * current skb, if any, so that we create at most a new skb.
+ * If we run out of space in the current skb (e.g. the window
+ * size shrunk from last sent) a new skb will be allocated even
+ * is collapsing was allowed: collapsing is effectively
+ * disabled.
+ */
+ can_collapse = mptcp_skb_can_collapse_to(msk, skb, mpext);
+ if (!can_collapse)
+ TCP_SKB_CB(skb)->eor = 1;
+ else if (size_goal - skb->len > 0)
+ avail_size = size_goal - skb->len;
+ else
+ can_collapse = false;
+ }
- mss_now = tcp_send_mss(ssk, &size_goal, msg->msg_flags);
- psize = min_t(int, size_goal, psize);
+ /* tell the TCP stack to delay the push so that we can safely
+ * access the skb after the sendpages call
+ */
+ psize = min_t(int, avail_size, psize);
ret = do_tcp_sendpages(ssk, pfrag->page, poffset, psize,
msg->msg_flags | MSG_SENDPAGE_NOTLAST);
if (ret <= 0)
return ret;
- if (skb == tcp_write_queue_tail(ssk))
- pr_err("no new skb %p/%p", sk, ssk);
+ collapsed = skb == tcp_write_queue_tail(ssk);
+ BUG_ON(collapsed && !can_collapse);
skb = tcp_write_queue_tail(ssk);
+ if (collapsed) {
+ mpext->data_len += ret;
+ goto out;
+ }
mpext = skb_ext_add(skb, SKB_EXT_MPTCP);
@@ -112,22 +148,25 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
pr_debug("data_seq=%llu subflow_seq=%u data_len=%u checksum=%u, dsn64=%d",
mpext->data_seq, mpext->subflow_seq, mpext->data_len,
mpext->checksum, mpext->dsn64);
- } /* TODO: else fallback */
+ }
+ /* TODO: else fallback; allocation can fail, but we can't easily retire
+ * skbs from the write_queue, as we need to roll-back TCP status
+ */
+out:
pfrag->offset += ret;
msk->write_seq += ret;
subflow_ctx(ssk)->rel_write_seq += ret;
- tcp_push(ssk, msg->msg_flags, mss_now, tcp_sk(ssk)->nonagle, size_goal);
return ret;
}
static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
{
+ int mss_now = 0, size_goal = 0, ret = 0;
struct mptcp_sock *msk = mptcp_sk(sk);
size_t copied = 0;
struct sock *ssk;
- int ret = 0;
long timeo;
pr_debug("msk=%p", msk);
@@ -157,14 +196,18 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t
len)
lock_sock(ssk);
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
while (msg_data_left(msg)) {
- ret = mptcp_sendmsg_frag(sk, ssk, msg, &timeo);
+ ret = mptcp_sendmsg_frag(sk, ssk, msg, &timeo, &mss_now,
+ &size_goal);
if (ret < 0)
break;
copied += ret;
}
- if (copied > 0)
+ if (copied) {
ret = copied;
+ tcp_push(ssk, msg->msg_flags, mss_now, tcp_sk(ssk)->nonagle,
+ size_goal);
+ }
release_sock(ssk);
release_sock(sk);
--
2.20.1
1:56 a.m.
New subject: [PATCH 3/3] mptcp: allow collapsing consecutive sendpages on the same substream
On Mon, 15 Apr 2019, Paolo Abeni wrote:
If the current sendmsg() lands on the same subflow we used last, we
can try to collapse the data.
RFC -> v1
- claried collasping schema
- update data_len for collapsed skb in mptcp_sendmsg_frag()
- fix collapsing decision on mptcp-level OoO
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
net/mptcp/protocol.c | 77 ++++++++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 17 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 085d658af772..0a947f44f712 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -47,11 +47,24 @@ static struct sock *mptcp_subflow_get_ref(const struct mptcp_sock
*msk)
return NULL;
}
+static inline bool mptcp_skb_can_collapse_to(const struct mptcp_sock *msk,
+ const struct sk_buff *skb,
+ const struct mptcp_ext *mpext)
+{
+ if (!tcp_skb_can_collapse_to(skb))
+ return false;
+
+ /* can't collapse if there is a MPTCP level out-of-order */
+ return mpext->data_seq + mpext->data_len == msk->write_seq;
Ok, looks like a good in-order check.
+}
+
static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
- struct msghdr *msg, long *timeo)
+ struct msghdr *msg, long *timeo, int *pmss_now,
+ int *ps_goal)
{
+ int mss_now, avail_size, size_goal, poffset, ret;
struct mptcp_sock *msk = mptcp_sk(sk);
- int mss_now, size_goal, poffset, ret;
+ bool collapsed, can_collapse = false;
struct mptcp_ext *mpext = NULL;
struct page_frag *pfrag;
struct sk_buff *skb;
@@ -78,25 +91,48 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
if (!psize)
return -EINVAL;
- /* Mark the end of the previous write so the beginning of the
- * next write (with its own mptcp skb extension data) is not
- * collapsed.
- */
+ mss_now = tcp_send_mss(ssk, &size_goal, msg->msg_flags);
+ *pmss_now = mss_now;
+ *ps_goal = size_goal;
+ avail_size = size_goal;
skb = tcp_write_queue_tail(ssk);
- if (skb)
- TCP_SKB_CB(skb)->eor = 1;
+ if (skb) {
+ mpext = skb_ext_find(skb, SKB_EXT_MPTCP);
+ BUG_ON(!mpext);
+
+ /* Limit the write to the size available in the
+ * current skb, if any, so that we create at most a new skb.
+ * If we run out of space in the current skb (e.g. the window
+ * size shrunk from last sent) a new skb will be allocated even
+ * is collapsing was allowed: collapsing is effectively
+ * disabled.
+ */
+ can_collapse = mptcp_skb_can_collapse_to(msk, skb, mpext);
+ if (!can_collapse)
+ TCP_SKB_CB(skb)->eor = 1;
+ else if (size_goal - skb->len > 0)
+ avail_size = size_goal - skb->len;
+ else
+ can_collapse = false;
In this final clause, should it set eor as well? If we're not expecting a
collapse, it might be better to make sure it does not happen.
+ }
- mss_now = tcp_send_mss(ssk, &size_goal, msg->msg_flags);
- psize = min_t(int, size_goal, psize);
+ /* tell the TCP stack to delay the push so that we can safely
+ * access the skb after the sendpages call
+ */
+ psize = min_t(int, avail_size, psize);
ret = do_tcp_sendpages(ssk, pfrag->page, poffset, psize,
msg->msg_flags | MSG_SENDPAGE_NOTLAST);
if (ret <= 0)
return ret;
- if (skb == tcp_write_queue_tail(ssk))
- pr_err("no new skb %p/%p", sk, ssk);
+ collapsed = skb == tcp_write_queue_tail(ssk);
+ BUG_ON(collapsed && !can_collapse);
skb = tcp_write_queue_tail(ssk);
Minor: seems like this skb assignment is coupled with the mpext line below
rather than the conditional to update mpext and exit, and could be moved
after the collapse check.
Thanks,
Mat
+ if (collapsed) {
+ mpext->data_len += ret;
+ goto out;
+ }
mpext = skb_ext_add(skb, SKB_EXT_MPTCP);
@@ -112,22 +148,25 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
pr_debug("data_seq=%llu subflow_seq=%u data_len=%u checksum=%u, dsn64=%d",
mpext->data_seq, mpext->subflow_seq, mpext->data_len,
mpext->checksum, mpext->dsn64);
- } /* TODO: else fallback */
+ }
+ /* TODO: else fallback; allocation can fail, but we can't easily retire
+ * skbs from the write_queue, as we need to roll-back TCP status
+ */
+out:
pfrag->offset += ret;
msk->write_seq += ret;
subflow_ctx(ssk)->rel_write_seq += ret;
- tcp_push(ssk, msg->msg_flags, mss_now, tcp_sk(ssk)->nonagle, size_goal);
return ret;
}
static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
{
+ int mss_now = 0, size_goal = 0, ret = 0;
struct mptcp_sock *msk = mptcp_sk(sk);
size_t copied = 0;
struct sock *ssk;
- int ret = 0;
long timeo;
pr_debug("msk=%p", msk);
@@ -157,14 +196,18 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg,
size_t len)
lock_sock(ssk);
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
while (msg_data_left(msg)) {
- ret = mptcp_sendmsg_frag(sk, ssk, msg, &timeo);
+ ret = mptcp_sendmsg_frag(sk, ssk, msg, &timeo, &mss_now,
+ &size_goal);
if (ret < 0)
break;
copied += ret;
}
- if (copied > 0)
+ if (copied) {
ret = copied;
+ tcp_push(ssk, msg->msg_flags, mss_now, tcp_sk(ssk)->nonagle,
+ size_goal);
+ }
release_sock(ssk);
release_sock(sk);
--
2.20.1
--
Mat Martineau
Intel
6:19 p.m.
New subject: [PATCH 3/3] mptcp: allow collapsing consecutive sendpages on the same substream
Hi,
Thank you for the feedback
On Thu, 2019-04-18 at 16:56 -0700, Mat Martineau wrote:
@@ -78,25 +91,48 @@ static int mptcp_sendmsg_frag(struct sock *sk,
struct sock *ssk,
> if (!psize)
> return -EINVAL;
>
> - /* Mark the end of the previous write so the beginning of the
> - * next write (with its own mptcp skb extension data) is not
> - * collapsed.
> - */
> + mss_now = tcp_send_mss(ssk, &size_goal, msg->msg_flags);
> + *pmss_now = mss_now;
> + *ps_goal = size_goal;
> + avail_size = size_goal;
> skb = tcp_write_queue_tail(ssk);
> - if (skb)
> - TCP_SKB_CB(skb)->eor = 1;
> + if (skb) {
> + mpext = skb_ext_find(skb, SKB_EXT_MPTCP);
> + BUG_ON(!mpext);
> +
> + /* Limit the write to the size available in the
> + * current skb, if any, so that we create at most a new skb.
> + * If we run out of space in the current skb (e.g. the window
> + * size shrunk from last sent) a new skb will be allocated even
> + * is collapsing was allowed: collapsing is effectively
> + * disabled.
> + */
> + can_collapse = mptcp_skb_can_collapse_to(msk, skb, mpext);
> + if (!can_collapse)
> + TCP_SKB_CB(skb)->eor = 1;
> + else if (size_goal - skb->len > 0)
> + avail_size = size_goal - skb->len;
> + else
> + can_collapse = false;
In this final clause, should it set eor as well? If we're not expecting a
collapse, it might be better to make sure it does not happen.
yes, we could set eor here, but if there is no available size in the
current skb, collapsing can't happen, otherwise is a bug. I think
setting it will not add additional safety.
> - if (skb == tcp_write_queue_tail(ssk))
> - pr_err("no new skb %p/%p", sk, ssk);
> + collapsed = skb == tcp_write_queue_tail(ssk);
> + BUG_ON(collapsed && !can_collapse);
>
> skb = tcp_write_queue_tail(ssk);
Minor: seems like this skb assignment is coupled with the mpext line below
rather than the conditional to update mpext and exit, and could be moved
after the collapse check.
Agreed, I will do that in the next iteration.
Final note: it looks like this patch is not the root cause of the
stream corruption I was observing, it just uncovers a bug present in
current code. I'll try to send a fix soon.
Thanks,
Paolo
1:58 a.m.
On Mon, 15 Apr 2019, Paolo Abeni wrote:
his series refactor the mptcp xmit path trying to make sendmsg
behavior
more consistent, memory efficent and improve its performances.
The code is also available here:
https://github.com/pabeni/mptcp/tree/mptcp-proposal-devel
To support MP_JOIN and multiple subflows, the idea is adding, with later
patches, an RB-tree structure containing references to the pending page
fragments, their length and the associated mptcp write_seq number (is subflow
rel_write_seq needed, too?). RB-tree node can be allocated using the same
page frag used in this series for skb data. Each RB-tree node will own a
reference to the page frag.
RFC -> v1:
- addressed Mat's feedback, dropping patch 3/4 and switching
to per msk page frag use, and checking mptcp seq number for coalescing
Paolo Abeni (3):
mptcp: use sk_page_frag() in sendmsg
mptcp: sendmsg() do spool all the provided data
mptcp: allow collapsing consecutive sendpages on the same substream
net/mptcp/protocol.c | 181 ++++++++++++++++++++++++++++---------------
1 file changed, 119 insertions(+), 62 deletions(-)
--
2.20.1
Paolo -
Thanks for the updated patch set. I had a few comments on patch 3, and the
rest look good. I'll wait on applying them for now, I think you said
you're checking on a possible bug. If I should go ahead and apply them,
please let me know.
--
Mat Martineau
Intel
640
days inactive
644
days old
6 comments
2 participants
participants (2)
-
Mat Martineau -
Paolo Abeni