* [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization
@ 2021-08-19 15:16 Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 1/4] mptcp: do not set unconditionally csum_reqd on incoming opt Paolo Abeni
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Paolo Abeni @ 2021-08-19 15:16 UTC (permalink / raw)
To: mptcp
This series aims at closing:
https://github.com/multipath-tcp/mptcp_net-next/issues/42
The relevant code is introduced by patch 4/4.
Do do the above in hopefully clean way, the series needs some
changes to the input options struct. That means a lot of
noise in the option-related code in the previous patches,
anyhow no functional change is intended here.
v1 -> v2:
- fixed = -> |= (Mat)
- fixed bug in ADD_ADDR processing (still in 3/4)
Paolo Abeni (4):
mptcp: do not set unconditionally csum_reqd on incoming opt
mptcp: better binary layout for mptcp_options_received
mptcp: consolidate in_opt sub-options fields in a bitmask
mptcp: optimize the input options processing.
net/mptcp/options.c | 140 +++++++++++++++++++------------------------
net/mptcp/protocol.c | 4 +-
net/mptcp/protocol.h | 36 ++++++-----
net/mptcp/subflow.c | 40 +++++++------
4 files changed, 104 insertions(+), 116 deletions(-)
--
2.26.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 mptcp-next 1/4] mptcp: do not set unconditionally csum_reqd on incoming opt
2021-08-19 15:16 [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Paolo Abeni
@ 2021-08-19 15:16 ` Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 2/4] mptcp: better binary layout for mptcp_options_received Paolo Abeni
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Paolo Abeni @ 2021-08-19 15:16 UTC (permalink / raw)
To: mptcp
Should be set only if the ingress packets present it, otherwise
we can confuse csum validation.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/options.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index bec3ed82e253..f012a71dd996 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -355,8 +355,6 @@ void mptcp_get_options(const struct sock *sk,
const struct sk_buff *skb,
struct mptcp_options_received *mp_opt)
{
- struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
- struct mptcp_sock *msk = mptcp_sk(subflow->conn);
const struct tcphdr *th = tcp_hdr(skb);
const unsigned char *ptr;
int length;
@@ -372,7 +370,7 @@ void mptcp_get_options(const struct sock *sk,
mp_opt->dss = 0;
mp_opt->mp_prio = 0;
mp_opt->reset = 0;
- mp_opt->csum_reqd = READ_ONCE(msk->csum_enabled);
+ mp_opt->csum_reqd = 0;
mp_opt->deny_join_id0 = 0;
mp_opt->mp_fail = 0;
--
2.26.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 mptcp-next 2/4] mptcp: better binary layout for mptcp_options_received
2021-08-19 15:16 [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 1/4] mptcp: do not set unconditionally csum_reqd on incoming opt Paolo Abeni
@ 2021-08-19 15:16 ` Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 3/4] mptcp: consolidate in_opt sub-options fields in a bitmask Paolo Abeni
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Paolo Abeni @ 2021-08-19 15:16 UTC (permalink / raw)
To: mptcp
This change reorder the mptcp_options_received fields
to shrink the structure a bit and to ensure the most
frequently used fields are all in the first cacheline.
Sub-opt specific flags are are moved out of the suboptions
area, and we must now explicitly set them when the relevant
suboption is parsed.
There is a notable exception: 'csum_reqd' is used by both DSS
and MPC suboptions, and keeping such field in the suboptions
flag area will simplfy the next patch.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/options.c | 8 +++-----
net/mptcp/protocol.h | 20 ++++++++++----------
2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index f012a71dd996..79b68ae9ef4d 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -83,8 +83,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
if (flags & MPTCP_CAP_CHECKSUM_REQD)
mp_opt->csum_reqd = 1;
- if (flags & MPTCP_CAP_DENY_JOIN_ID0)
- mp_opt->deny_join_id0 = 1;
+ mp_opt->deny_join_id0 = !!(flags & MPTCP_CAP_DENY_JOIN_ID0);
mp_opt->mp_capable = 1;
if (opsize >= TCPOLEN_MPTCP_MPC_SYNACK) {
@@ -262,6 +261,8 @@ static void mptcp_parse_option(const struct sk_buff *skb,
mp_opt->add_addr = 1;
mp_opt->addr.id = *ptr++;
+ mp_opt->addr.port = 0;
+ mp_opt->ahmac = 0;
if (mp_opt->addr.family == AF_INET) {
memcpy((u8 *)&mp_opt->addr.addr.s_addr, (u8 *)ptr, 4);
ptr += 4;
@@ -363,15 +364,12 @@ void mptcp_get_options(const struct sock *sk,
mp_opt->mp_capable = 0;
mp_opt->mp_join = 0;
mp_opt->add_addr = 0;
- mp_opt->ahmac = 0;
mp_opt->fastclose = 0;
- mp_opt->addr.port = 0;
mp_opt->rm_addr = 0;
mp_opt->dss = 0;
mp_opt->mp_prio = 0;
mp_opt->reset = 0;
mp_opt->csum_reqd = 0;
- mp_opt->deny_join_id0 = 0;
mp_opt->mp_fail = 0;
length = (th->doff * 4) - sizeof(struct tcphdr);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 57a50b1194a9..9a0d91f92bbc 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -140,28 +140,28 @@ struct mptcp_options_received {
add_addr : 1,
rm_addr : 1,
mp_prio : 1,
- mp_fail : 1,
- echo : 1,
csum_reqd : 1,
- backup : 1,
- deny_join_id0 : 1;
+ mp_fail : 1;
u32 token;
u32 nonce;
- u64 thmac;
- u8 hmac[MPTCPOPT_HMAC_LEN];
- u8 join_id;
- u8 use_map:1,
+ u16 use_map:1,
dsn64:1,
data_fin:1,
use_ack:1,
ack64:1,
mpc_map:1,
+ reset_reason:4,
+ reset_transient:1,
+ echo:1,
+ backup:1,
+ deny_join_id0:1,
__unused:2;
+ u8 join_id;
+ u64 thmac;
+ u8 hmac[MPTCPOPT_HMAC_LEN];
struct mptcp_addr_info addr;
struct mptcp_rm_list rm_list;
u64 ahmac;
- u8 reset_reason:4;
- u8 reset_transient:1;
u64 fail_seq;
};
--
2.26.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 mptcp-next 3/4] mptcp: consolidate in_opt sub-options fields in a bitmask
2021-08-19 15:16 [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 1/4] mptcp: do not set unconditionally csum_reqd on incoming opt Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 2/4] mptcp: better binary layout for mptcp_options_received Paolo Abeni
@ 2021-08-19 15:16 ` Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 4/4] mptcp: optimize the input options processing Paolo Abeni
2021-08-20 8:05 ` [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Matthieu Baerts
4 siblings, 0 replies; 9+ messages in thread
From: Paolo Abeni @ 2021-08-19 15:16 UTC (permalink / raw)
To: mptcp
This makes input options processing more consistent with
output ones and will simplify the next patch.
Also avoid clearing the suboption field after processing
it, since it's not needed.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/options.c | 74 ++++++++++++++++++--------------------------
net/mptcp/protocol.c | 4 +--
net/mptcp/protocol.h | 18 +++++------
net/mptcp/subflow.c | 40 ++++++++++++++----------
4 files changed, 63 insertions(+), 73 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 79b68ae9ef4d..0d33c020062f 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -81,11 +81,11 @@ static void mptcp_parse_option(const struct sk_buff *skb,
* is if both hosts in their SYNs set A=0."
*/
if (flags & MPTCP_CAP_CHECKSUM_REQD)
- mp_opt->csum_reqd = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_CSUMREQD;
mp_opt->deny_join_id0 = !!(flags & MPTCP_CAP_DENY_JOIN_ID0);
- mp_opt->mp_capable = 1;
+ mp_opt->suboptions |= OPTIONS_MPTCP_MPC;
if (opsize >= TCPOLEN_MPTCP_MPC_SYNACK) {
mp_opt->sndr_key = get_unaligned_be64(ptr);
ptr += 8;
@@ -100,7 +100,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
* equivalent to those in a DSS option and can be used
* interchangeably."
*/
- mp_opt->dss = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_DSS;
mp_opt->use_map = 1;
mp_opt->mpc_map = 1;
mp_opt->data_len = get_unaligned_be16(ptr);
@@ -108,7 +108,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
}
if (opsize == TCPOLEN_MPTCP_MPC_ACK_DATA_CSUM) {
mp_opt->csum = (__force __sum16)get_unaligned_be16(ptr);
- mp_opt->csum_reqd = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_CSUMREQD;
ptr += 2;
}
pr_debug("MP_CAPABLE version=%x, flags=%x, optlen=%d sndr=%llu, rcvr=%llu len=%d csum=%u",
@@ -117,7 +117,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
break;
case MPTCPOPT_MP_JOIN:
- mp_opt->mp_join = 1;
+ mp_opt->suboptions |= OPTIONS_MPTCP_MPJ;
if (opsize == TCPOLEN_MPTCP_MPJ_SYN) {
mp_opt->backup = *ptr++ & MPTCPOPT_BACKUP;
mp_opt->join_id = *ptr++;
@@ -143,7 +143,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
memcpy(mp_opt->hmac, ptr, MPTCPOPT_HMAC_LEN);
pr_debug("MP_JOIN hmac");
} else {
- mp_opt->mp_join = 0;
+ mp_opt->suboptions &= ~OPTIONS_MPTCP_MPJ;
}
break;
@@ -191,8 +191,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
opsize != expected_opsize + TCPOLEN_MPTCP_DSS_CHECKSUM)
break;
- mp_opt->dss = 1;
-
+ mp_opt->suboptions |= OPTION_MPTCP_DSS;
if (mp_opt->use_ack) {
if (mp_opt->ack64) {
mp_opt->data_ack = get_unaligned_be64(ptr);
@@ -221,14 +220,15 @@ static void mptcp_parse_option(const struct sk_buff *skb,
ptr += 2;
if (opsize == expected_opsize + TCPOLEN_MPTCP_DSS_CHECKSUM) {
- mp_opt->csum_reqd = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_CSUMREQD;
mp_opt->csum = (__force __sum16)get_unaligned_be16(ptr);
ptr += 2;
}
pr_debug("data_seq=%llu subflow_seq=%u data_len=%u csum=%d:%u",
mp_opt->data_seq, mp_opt->subflow_seq,
- mp_opt->data_len, mp_opt->csum_reqd, mp_opt->csum);
+ mp_opt->data_len, !!(mp_opt->suboptions & OPTION_MPTCP_CSUMREQD),
+ mp_opt->csum);
}
break;
@@ -259,7 +259,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
break;
}
- mp_opt->add_addr = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_ADD_ADDR;
mp_opt->addr.id = *ptr++;
mp_opt->addr.port = 0;
mp_opt->ahmac = 0;
@@ -299,7 +299,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
ptr++;
- mp_opt->rm_addr = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_RM_ADDR;
mp_opt->rm_list.nr = opsize - TCPOLEN_MPTCP_RM_ADDR_BASE;
for (i = 0; i < mp_opt->rm_list.nr; i++)
mp_opt->rm_list.ids[i] = *ptr++;
@@ -310,7 +310,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
if (opsize != TCPOLEN_MPTCP_PRIO)
break;
- mp_opt->mp_prio = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_PRIO;
mp_opt->backup = *ptr++ & MPTCP_PRIO_BKUP;
pr_debug("MP_PRIO: prio=%d", mp_opt->backup);
break;
@@ -322,7 +322,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
ptr += 2;
mp_opt->rcvr_key = get_unaligned_be64(ptr);
ptr += 8;
- mp_opt->fastclose = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_FASTCLOSE;
break;
case MPTCPOPT_RST:
@@ -331,7 +331,8 @@ static void mptcp_parse_option(const struct sk_buff *skb,
if (!(TCP_SKB_CB(skb)->tcp_flags & TCPHDR_RST))
break;
- mp_opt->reset = 1;
+
+ mp_opt->suboptions |= OPTION_MPTCP_RST;
flags = *ptr++;
mp_opt->reset_transient = flags & MPTCP_RST_TRANSIENT;
mp_opt->reset_reason = *ptr;
@@ -342,7 +343,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
break;
ptr += 2;
- mp_opt->mp_fail = 1;
+ mp_opt->suboptions |= OPTION_MPTCP_FAIL;
mp_opt->fail_seq = get_unaligned_be64(ptr);
pr_debug("MP_FAIL: data_seq=%llu", mp_opt->fail_seq);
break;
@@ -361,16 +362,7 @@ void mptcp_get_options(const struct sock *sk,
int length;
/* initialize option status */
- mp_opt->mp_capable = 0;
- mp_opt->mp_join = 0;
- mp_opt->add_addr = 0;
- mp_opt->fastclose = 0;
- mp_opt->rm_addr = 0;
- mp_opt->dss = 0;
- mp_opt->mp_prio = 0;
- mp_opt->reset = 0;
- mp_opt->csum_reqd = 0;
- mp_opt->mp_fail = 0;
+ mp_opt->suboptions = 0;
length = (th->doff * 4) - sizeof(struct tcphdr);
ptr = (const unsigned char *)(th + 1);
@@ -924,7 +916,7 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
*/
if (TCP_SKB_CB(skb)->seq == subflow->ssn_offset + 1 &&
TCP_SKB_CB(skb)->end_seq == TCP_SKB_CB(skb)->seq &&
- subflow->mp_join && mp_opt->mp_join &&
+ subflow->mp_join && (mp_opt->suboptions & OPTIONS_MPTCP_MPJ) &&
READ_ONCE(msk->pm.server_side))
tcp_send_ack(ssk);
goto fully_established;
@@ -941,8 +933,8 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
return subflow->mp_capable;
}
- if ((mp_opt->dss && mp_opt->use_ack) ||
- (mp_opt->add_addr && !mp_opt->echo)) {
+ if (((mp_opt->suboptions & OPTION_MPTCP_DSS) && mp_opt->use_ack) ||
+ ((mp_opt->suboptions & OPTION_MPTCP_ADD_ADDR) && !mp_opt->echo)) {
/* subflows are fully established as soon as we get any
* additional ack, including ADD_ADDR.
*/
@@ -955,7 +947,7 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
* then fallback to TCP. Fallback scenarios requires a reset for
* MP_JOIN subflows.
*/
- if (!mp_opt->mp_capable) {
+ if (!(mp_opt->suboptions & OPTIONS_MPTCP_MPC)) {
if (subflow->mp_join)
goto reset;
subflow->mp_capable = 0;
@@ -1119,13 +1111,13 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
return sk->sk_state != TCP_CLOSE;
- if (mp_opt.fastclose &&
+ if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
msk->local_key == mp_opt.rcvr_key) {
WRITE_ONCE(msk->rcv_fastclose, true);
mptcp_schedule_work((struct sock *)msk);
}
- if (mp_opt.add_addr && add_addr_hmac_valid(msk, &mp_opt)) {
+ if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) && add_addr_hmac_valid(msk, &mp_opt)) {
if (!mp_opt.echo) {
mptcp_pm_add_addr_received(msk, &mp_opt.addr);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
@@ -1137,34 +1129,28 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
if (mp_opt.addr.port)
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_PORTADD);
-
- mp_opt.add_addr = 0;
}
- if (mp_opt.rm_addr) {
+ if (mp_opt.suboptions & OPTION_MPTCP_RM_ADDR)
mptcp_pm_rm_addr_received(msk, &mp_opt.rm_list);
- mp_opt.rm_addr = 0;
- }
- if (mp_opt.mp_prio) {
+ if (mp_opt.suboptions & OPTION_MPTCP_PRIO) {
mptcp_pm_mp_prio_received(sk, mp_opt.backup);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPPRIORX);
- mp_opt.mp_prio = 0;
}
- if (mp_opt.mp_fail) {
+ if (mp_opt.suboptions & OPTION_MPTCP_FAIL) {
mptcp_pm_mp_fail_received(sk, mp_opt.fail_seq);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPFAILRX);
- mp_opt.mp_fail = 0;
}
- if (mp_opt.reset) {
+ if (mp_opt.suboptions & OPTION_MPTCP_RST) {
subflow->reset_seen = 1;
subflow->reset_reason = mp_opt.reset_reason;
subflow->reset_transient = mp_opt.reset_transient;
}
- if (!mp_opt.dss)
+ if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
return true;
/* we can't wait for recvmsg() to update the ack_seq, otherwise
@@ -1213,7 +1199,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
}
mpext->data_len = mp_opt.data_len;
mpext->use_map = 1;
- mpext->csum_reqd = mp_opt.csum_reqd;
+ mpext->csum_reqd = !!(mp_opt.suboptions & OPTION_MPTCP_CSUMREQD);
if (mpext->csum_reqd)
mpext->csum = mp_opt.csum;
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 8eb2626503d7..b5f8ad634571 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2836,7 +2836,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk,
msk->token = subflow_req->token;
msk->subflow = NULL;
WRITE_ONCE(msk->fully_established, false);
- if (mp_opt->csum_reqd)
+ if (mp_opt->suboptions & OPTION_MPTCP_CSUMREQD)
WRITE_ONCE(msk->csum_enabled, true);
msk->write_seq = subflow_req->idsn + 1;
@@ -2845,7 +2845,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk,
msk->wnd_end = msk->snd_nxt + req->rsk_rcv_wnd;
msk->setsockopt_seq = mptcp_sk(sk)->setsockopt_seq;
- if (mp_opt->mp_capable) {
+ if (mp_opt->suboptions & OPTIONS_MPTCP_MPC) {
msk->can_ack = true;
msk->remote_key = mp_opt->sndr_key;
mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 9a0d91f92bbc..d7aba1c4dc48 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -29,6 +29,13 @@
#define OPTION_MPTCP_DSS BIT(11)
#define OPTION_MPTCP_FAIL BIT(12)
+#define OPTION_MPTCP_CSUMREQD BIT(13)
+
+#define OPTIONS_MPTCP_MPC (OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_SYNACK | \
+ OPTION_MPTCP_MPC_ACK)
+#define OPTIONS_MPTCP_MPJ (OPTION_MPTCP_MPJ_SYN | OPTION_MPTCP_MPJ_SYNACK | \
+ OPTION_MPTCP_MPJ_SYNACK)
+
/* MPTCP option subtypes */
#define MPTCPOPT_MP_CAPABLE 0
#define MPTCPOPT_MP_JOIN 1
@@ -132,16 +139,7 @@ struct mptcp_options_received {
u32 subflow_seq;
u16 data_len;
__sum16 csum;
- u16 mp_capable : 1,
- mp_join : 1,
- fastclose : 1,
- reset : 1,
- dss : 1,
- add_addr : 1,
- rm_addr : 1,
- mp_prio : 1,
- csum_reqd : 1,
- mp_fail : 1;
+ u16 suboptions;
u32 token;
u32 nonce;
u16 use_map:1,
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 54b7ffc21861..1de7ce883c37 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -141,6 +141,7 @@ static int subflow_check_req(struct request_sock *req,
struct mptcp_subflow_context *listener = mptcp_subflow_ctx(sk_listener);
struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
struct mptcp_options_received mp_opt;
+ bool opt_mp_capable, opt_mp_join;
pr_debug("subflow_req=%p, listener=%p", subflow_req, listener);
@@ -154,16 +155,18 @@ static int subflow_check_req(struct request_sock *req,
mptcp_get_options(sk_listener, skb, &mp_opt);
- if (mp_opt.mp_capable) {
+ opt_mp_capable = !!(mp_opt.suboptions & OPTIONS_MPTCP_MPC);
+ opt_mp_join = !!(mp_opt.suboptions & OPTIONS_MPTCP_MPJ);
+ if (opt_mp_capable) {
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MPCAPABLEPASSIVE);
- if (mp_opt.mp_join)
+ if (opt_mp_join)
return 0;
- } else if (mp_opt.mp_join) {
+ } else if (opt_mp_join) {
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINSYNRX);
}
- if (mp_opt.mp_capable && listener->request_mptcp) {
+ if (opt_mp_capable && listener->request_mptcp) {
int err, retries = MPTCP_TOKEN_MAX_RETRIES;
subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq;
@@ -194,7 +197,7 @@ static int subflow_check_req(struct request_sock *req,
else
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_TOKENFALLBACKINIT);
- } else if (mp_opt.mp_join && listener->request_mptcp) {
+ } else if (opt_mp_join && listener->request_mptcp) {
subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq;
subflow_req->mp_join = 1;
subflow_req->backup = mp_opt.backup;
@@ -243,15 +246,18 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req,
struct mptcp_subflow_context *listener = mptcp_subflow_ctx(sk_listener);
struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
struct mptcp_options_received mp_opt;
+ bool opt_mp_capable, opt_mp_join;
int err;
subflow_init_req(req, sk_listener);
mptcp_get_options(sk_listener, skb, &mp_opt);
- if (mp_opt.mp_capable && mp_opt.mp_join)
+ opt_mp_capable = !!(mp_opt.suboptions & OPTIONS_MPTCP_MPC);
+ opt_mp_join = !!(mp_opt.suboptions & OPTIONS_MPTCP_MPJ);
+ if (opt_mp_capable && opt_mp_join)
return -EINVAL;
- if (mp_opt.mp_capable && listener->request_mptcp) {
+ if (opt_mp_capable && listener->request_mptcp) {
if (mp_opt.sndr_key == 0)
return -EINVAL;
@@ -262,7 +268,7 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req,
subflow_req->mp_capable = 1;
subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq - 1;
- } else if (mp_opt.mp_join && listener->request_mptcp) {
+ } else if (opt_mp_join && listener->request_mptcp) {
if (!mptcp_token_join_cookie_init_state(subflow_req, skb))
return -EINVAL;
@@ -394,7 +400,6 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
subflow->icsk_af_ops->sk_rx_dst_set(sk, skb);
-
/* be sure no special action on any packet other than syn-ack */
if (subflow->conn_finished)
return;
@@ -407,7 +412,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
mptcp_get_options(sk, skb, &mp_opt);
if (subflow->request_mptcp) {
- if (!mp_opt.mp_capable) {
+ if (!(mp_opt.suboptions & OPTIONS_MPTCP_MPC)) {
MPTCP_INC_STATS(sock_net(sk),
MPTCP_MIB_MPCAPABLEACTIVEFALLBACK);
mptcp_do_fallback(sk);
@@ -415,7 +420,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
goto fallback;
}
- if (mp_opt.csum_reqd)
+ if (mp_opt.suboptions & OPTION_MPTCP_CSUMREQD)
WRITE_ONCE(mptcp_sk(parent)->csum_enabled, true);
if (mp_opt.deny_join_id0)
WRITE_ONCE(mptcp_sk(parent)->pm.remote_deny_join_id0, true);
@@ -430,7 +435,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
} else if (subflow->request_join) {
u8 hmac[SHA256_DIGEST_SIZE];
- if (!mp_opt.mp_join) {
+ if (!(mp_opt.suboptions & OPTIONS_MPTCP_MPJ)) {
subflow->reset_reason = MPTCP_RST_EMPTCP;
goto do_reset;
}
@@ -636,10 +641,10 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
pr_debug("listener=%p, req=%p, conn=%p", listener, req, listener->conn);
- /* After child creation we must look for 'mp_capable' even when options
+ /* After child creation we must look for MPC even when options
* are not parsed
*/
- mp_opt.mp_capable = 0;
+ mp_opt.suboptions = 0;
/* hopefully temporary handling for MP_JOIN+syncookie */
subflow_req = mptcp_subflow_rsk(req);
@@ -659,7 +664,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
* options.
*/
mptcp_get_options(sk, skb, &mp_opt);
- if (!mp_opt.mp_capable) {
+ if (!(mp_opt.suboptions & OPTIONS_MPTCP_MPC)) {
fallback = true;
goto create_child;
}
@@ -669,7 +674,8 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
fallback = true;
} else if (subflow_req->mp_join) {
mptcp_get_options(sk, skb, &mp_opt);
- if (!mp_opt.mp_join || !subflow_hmac_valid(req, &mp_opt) ||
+ if (!(mp_opt.suboptions & OPTIONS_MPTCP_MPJ) ||
+ !subflow_hmac_valid(req, &mp_opt) ||
!mptcp_can_accept_new_subflow(subflow_req->msk)) {
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKMAC);
fallback = true;
@@ -726,7 +732,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
/* with OoO packets we can reach here without ingress
* mpc option
*/
- if (mp_opt.mp_capable)
+ if (mp_opt.suboptions & OPTIONS_MPTCP_MPC)
mptcp_subflow_fully_established(ctx, &mp_opt);
} else if (ctx->mp_join) {
struct mptcp_sock *owner;
--
2.26.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 mptcp-next 4/4] mptcp: optimize the input options processing.
2021-08-19 15:16 [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Paolo Abeni
` (2 preceding siblings ...)
2021-08-19 15:16 ` [PATCH v2 mptcp-next 3/4] mptcp: consolidate in_opt sub-options fields in a bitmask Paolo Abeni
@ 2021-08-19 15:16 ` Paolo Abeni
2021-08-20 0:24 ` Mat Martineau
2021-08-20 8:05 ` [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Matthieu Baerts
4 siblings, 1 reply; 9+ messages in thread
From: Paolo Abeni @ 2021-08-19 15:16 UTC (permalink / raw)
To: mptcp
Most MPTCP packets carries a single MPTCP subption: the
DSS containing the mapping for the current packet.
Check explicitly for the above, so that is such scenario we
replace most conditional statements with a single likely() one.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/options.c | 70 +++++++++++++++++++++++----------------------
1 file changed, 36 insertions(+), 34 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 0d33c020062f..af22d76f1699 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1111,48 +1111,50 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
return sk->sk_state != TCP_CLOSE;
- if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
- msk->local_key == mp_opt.rcvr_key) {
- WRITE_ONCE(msk->rcv_fastclose, true);
- mptcp_schedule_work((struct sock *)msk);
- }
+ if (unlikely(mp_opt.suboptions != OPTION_MPTCP_DSS)) {
+ if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
+ msk->local_key == mp_opt.rcvr_key) {
+ WRITE_ONCE(msk->rcv_fastclose, true);
+ mptcp_schedule_work((struct sock *)msk);
+ }
- if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) && add_addr_hmac_valid(msk, &mp_opt)) {
- if (!mp_opt.echo) {
- mptcp_pm_add_addr_received(msk, &mp_opt.addr);
- MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
- } else {
- mptcp_pm_add_addr_echoed(msk, &mp_opt.addr);
- mptcp_pm_del_add_timer(msk, &mp_opt.addr, true);
- MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADD);
+ if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) && add_addr_hmac_valid(msk, &mp_opt)) {
+ if (!mp_opt.echo) {
+ mptcp_pm_add_addr_received(msk, &mp_opt.addr);
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
+ } else {
+ mptcp_pm_add_addr_echoed(msk, &mp_opt.addr);
+ mptcp_pm_del_add_timer(msk, &mp_opt.addr, true);
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADD);
+ }
+
+ if (mp_opt.addr.port)
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_PORTADD);
}
- if (mp_opt.addr.port)
- MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_PORTADD);
- }
+ if (mp_opt.suboptions & OPTION_MPTCP_RM_ADDR)
+ mptcp_pm_rm_addr_received(msk, &mp_opt.rm_list);
- if (mp_opt.suboptions & OPTION_MPTCP_RM_ADDR)
- mptcp_pm_rm_addr_received(msk, &mp_opt.rm_list);
+ if (mp_opt.suboptions & OPTION_MPTCP_PRIO) {
+ mptcp_pm_mp_prio_received(sk, mp_opt.backup);
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPPRIORX);
+ }
- if (mp_opt.suboptions & OPTION_MPTCP_PRIO) {
- mptcp_pm_mp_prio_received(sk, mp_opt.backup);
- MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPPRIORX);
- }
+ if (mp_opt.suboptions & OPTION_MPTCP_FAIL) {
+ mptcp_pm_mp_fail_received(sk, mp_opt.fail_seq);
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPFAILRX);
+ }
- if (mp_opt.suboptions & OPTION_MPTCP_FAIL) {
- mptcp_pm_mp_fail_received(sk, mp_opt.fail_seq);
- MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPFAILRX);
- }
+ if (mp_opt.suboptions & OPTION_MPTCP_RST) {
+ subflow->reset_seen = 1;
+ subflow->reset_reason = mp_opt.reset_reason;
+ subflow->reset_transient = mp_opt.reset_transient;
+ }
- if (mp_opt.suboptions & OPTION_MPTCP_RST) {
- subflow->reset_seen = 1;
- subflow->reset_reason = mp_opt.reset_reason;
- subflow->reset_transient = mp_opt.reset_transient;
+ if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
+ return true;
}
- if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
- return true;
-
/* we can't wait for recvmsg() to update the ack_seq, otherwise
* monodirectional flows will stuck
*/
@@ -1179,7 +1181,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
memset(mpext, 0, sizeof(*mpext));
- if (mp_opt.use_map) {
+ if (likely(mp_opt.use_map)) {
if (mp_opt.mpc_map) {
/* this is an MP_CAPABLE carrying MPTCP data
* we know this map the first chunk of data
--
2.26.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 mptcp-next 4/4] mptcp: optimize the input options processing.
2021-08-19 15:16 ` [PATCH v2 mptcp-next 4/4] mptcp: optimize the input options processing Paolo Abeni
@ 2021-08-20 0:24 ` Mat Martineau
2021-08-20 7:58 ` Paolo Abeni
0 siblings, 1 reply; 9+ messages in thread
From: Mat Martineau @ 2021-08-20 0:24 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp
On Thu, 19 Aug 2021, Paolo Abeni wrote:
> Most MPTCP packets carries a single MPTCP subption: the
> DSS containing the mapping for the current packet.
>
> Check explicitly for the above, so that is such scenario we
> replace most conditional statements with a single likely() one.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/options.c | 70 +++++++++++++++++++++++----------------------
> 1 file changed, 36 insertions(+), 34 deletions(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 0d33c020062f..af22d76f1699 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1111,48 +1111,50 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
> if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
> return sk->sk_state != TCP_CLOSE;
>
> - if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
> - msk->local_key == mp_opt.rcvr_key) {
> - WRITE_ONCE(msk->rcv_fastclose, true);
> - mptcp_schedule_work((struct sock *)msk);
> - }
> + if (unlikely(mp_opt.suboptions != OPTION_MPTCP_DSS)) {
This diff is a lot shorter when ignoring whitespace :)
One question - is it worth optimizing for the mp_opt.suboptions = 0 case
too? While our stack will send duplicate DSS mappings when the mapped data
is split across multiple TCP packets, other stacks might send each mapping
only once (hopefully not, since that will affect GRO/etc).
I think this series is good for the export branch now, though:
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> + if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
> + msk->local_key == mp_opt.rcvr_key) {
> + WRITE_ONCE(msk->rcv_fastclose, true);
> + mptcp_schedule_work((struct sock *)msk);
> + }
>
> - if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) && add_addr_hmac_valid(msk, &mp_opt)) {
> - if (!mp_opt.echo) {
> - mptcp_pm_add_addr_received(msk, &mp_opt.addr);
> - MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
> - } else {
> - mptcp_pm_add_addr_echoed(msk, &mp_opt.addr);
> - mptcp_pm_del_add_timer(msk, &mp_opt.addr, true);
> - MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADD);
> + if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) && add_addr_hmac_valid(msk, &mp_opt)) {
> + if (!mp_opt.echo) {
> + mptcp_pm_add_addr_received(msk, &mp_opt.addr);
> + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
> + } else {
> + mptcp_pm_add_addr_echoed(msk, &mp_opt.addr);
> + mptcp_pm_del_add_timer(msk, &mp_opt.addr, true);
> + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADD);
> + }
> +
> + if (mp_opt.addr.port)
> + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_PORTADD);
> }
>
> - if (mp_opt.addr.port)
> - MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_PORTADD);
> - }
> + if (mp_opt.suboptions & OPTION_MPTCP_RM_ADDR)
> + mptcp_pm_rm_addr_received(msk, &mp_opt.rm_list);
>
> - if (mp_opt.suboptions & OPTION_MPTCP_RM_ADDR)
> - mptcp_pm_rm_addr_received(msk, &mp_opt.rm_list);
> + if (mp_opt.suboptions & OPTION_MPTCP_PRIO) {
> + mptcp_pm_mp_prio_received(sk, mp_opt.backup);
> + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPPRIORX);
> + }
>
> - if (mp_opt.suboptions & OPTION_MPTCP_PRIO) {
> - mptcp_pm_mp_prio_received(sk, mp_opt.backup);
> - MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPPRIORX);
> - }
> + if (mp_opt.suboptions & OPTION_MPTCP_FAIL) {
> + mptcp_pm_mp_fail_received(sk, mp_opt.fail_seq);
> + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPFAILRX);
> + }
>
> - if (mp_opt.suboptions & OPTION_MPTCP_FAIL) {
> - mptcp_pm_mp_fail_received(sk, mp_opt.fail_seq);
> - MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPFAILRX);
> - }
> + if (mp_opt.suboptions & OPTION_MPTCP_RST) {
> + subflow->reset_seen = 1;
> + subflow->reset_reason = mp_opt.reset_reason;
> + subflow->reset_transient = mp_opt.reset_transient;
> + }
>
> - if (mp_opt.suboptions & OPTION_MPTCP_RST) {
> - subflow->reset_seen = 1;
> - subflow->reset_reason = mp_opt.reset_reason;
> - subflow->reset_transient = mp_opt.reset_transient;
> + if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
> + return true;
> }
>
> - if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
> - return true;
> -
> /* we can't wait for recvmsg() to update the ack_seq, otherwise
> * monodirectional flows will stuck
> */
> @@ -1179,7 +1181,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>
> memset(mpext, 0, sizeof(*mpext));
>
> - if (mp_opt.use_map) {
> + if (likely(mp_opt.use_map)) {
> if (mp_opt.mpc_map) {
> /* this is an MP_CAPABLE carrying MPTCP data
> * we know this map the first chunk of data
> --
> 2.26.3
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 mptcp-next 4/4] mptcp: optimize the input options processing.
2021-08-20 0:24 ` Mat Martineau
@ 2021-08-20 7:58 ` Paolo Abeni
2021-08-20 8:02 ` Matthieu Baerts
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Abeni @ 2021-08-20 7:58 UTC (permalink / raw)
To: Mat Martineau; +Cc: mptcp
On Thu, 2021-08-19 at 17:24 -0700, Mat Martineau wrote:
> On Thu, 19 Aug 2021, Paolo Abeni wrote:
>
> > Most MPTCP packets carries a single MPTCP subption: the
> > DSS containing the mapping for the current packet.
> >
> > Check explicitly for the above, so that is such scenario we
> > replace most conditional statements with a single likely() one.
> >
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > net/mptcp/options.c | 70 +++++++++++++++++++++++----------------------
> > 1 file changed, 36 insertions(+), 34 deletions(-)
> >
> > diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> > index 0d33c020062f..af22d76f1699 100644
> > --- a/net/mptcp/options.c
> > +++ b/net/mptcp/options.c
> > @@ -1111,48 +1111,50 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
> > if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
> > return sk->sk_state != TCP_CLOSE;
> >
> > - if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
> > - msk->local_key == mp_opt.rcvr_key) {
> > - WRITE_ONCE(msk->rcv_fastclose, true);
> > - mptcp_schedule_work((struct sock *)msk);
> > - }
> > + if (unlikely(mp_opt.suboptions != OPTION_MPTCP_DSS)) {
>
> This diff is a lot shorter when ignoring whitespace :)
>
> One question - is it worth optimizing for the mp_opt.suboptions = 0 case
> too? While our stack will send duplicate DSS mappings when the mapped data
> is split across multiple TCP packets, other stacks might send each mapping
> only once (hopefully not, since that will affect GRO/etc).
I *think* even mptcp.org is sending DSS on every packets, to help
GRO/GSO.
Generally speaking, with intermittent DSS, we kill both TSO and GRO. It
does not looks like a fast path. I guess we could add that check with
time, if we see it being useful.
Thanks!
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 mptcp-next 4/4] mptcp: optimize the input options processing.
2021-08-20 7:58 ` Paolo Abeni
@ 2021-08-20 8:02 ` Matthieu Baerts
0 siblings, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2021-08-20 8:02 UTC (permalink / raw)
To: Paolo Abeni, Mat Martineau; +Cc: mptcp
Hi Paolo,
On 20/08/2021 09:58, Paolo Abeni wrote:
> On Thu, 2021-08-19 at 17:24 -0700, Mat Martineau wrote:
>> On Thu, 19 Aug 2021, Paolo Abeni wrote:
>>
>>> Most MPTCP packets carries a single MPTCP subption: the
>>> DSS containing the mapping for the current packet.
>>>
>>> Check explicitly for the above, so that is such scenario we
>>> replace most conditional statements with a single likely() one.
>>>
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>> ---
>>> net/mptcp/options.c | 70 +++++++++++++++++++++++----------------------
>>> 1 file changed, 36 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>>> index 0d33c020062f..af22d76f1699 100644
>>> --- a/net/mptcp/options.c
>>> +++ b/net/mptcp/options.c
>>> @@ -1111,48 +1111,50 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>>> if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
>>> return sk->sk_state != TCP_CLOSE;
>>>
>>> - if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
>>> - msk->local_key == mp_opt.rcvr_key) {
>>> - WRITE_ONCE(msk->rcv_fastclose, true);
>>> - mptcp_schedule_work((struct sock *)msk);
>>> - }
>>> + if (unlikely(mp_opt.suboptions != OPTION_MPTCP_DSS)) {
>>
>> This diff is a lot shorter when ignoring whitespace :)
>>
>> One question - is it worth optimizing for the mp_opt.suboptions = 0 case
>> too? While our stack will send duplicate DSS mappings when the mapped data
>> is split across multiple TCP packets, other stacks might send each mapping
>> only once (hopefully not, since that will affect GRO/etc).
>
> I *think* even mptcp.org is sending DSS on every packets, to help
> GRO/GSO.
Yes it does. (or at least, that's what is expected :) )
> Generally speaking, with intermittent DSS, we kill both TSO and GRO.
Hopefully, NIC vendors could recognise MPTCP options and aggregate
packets if they are in-order from MPTCP point of view as well.
> It does not looks like a fast path. I guess we could add that check with
> time, if we see it being useful.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization
2021-08-19 15:16 [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Paolo Abeni
` (3 preceding siblings ...)
2021-08-19 15:16 ` [PATCH v2 mptcp-next 4/4] mptcp: optimize the input options processing Paolo Abeni
@ 2021-08-20 8:05 ` Matthieu Baerts
4 siblings, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2021-08-20 8:05 UTC (permalink / raw)
To: Paolo Abeni, Mat Martineau; +Cc: mptcp
Hi Paolo, Mat,
On 19/08/2021 17:16, Paolo Abeni wrote:
> This series aims at closing:
>
> https://github.com/multipath-tcp/mptcp_net-next/issues/42
>
> The relevant code is introduced by patch 4/4.
>
> Do do the above in hopefully clean way, the series needs some
> changes to the input options struct. That means a lot of
> noise in the option-related code in the previous patches,
> anyhow no functional change is intended here.
Thank you for the patches and the reviews!
Now in our tree with Mat's RvB tags and without two small things
reported by checkpatch (duplicated "are" word + over 100 chars).
- 43bb18ab33aa: mptcp: do not set unconditionally csum_reqd on incoming opt
- a232edf92729: mptcp: better binary layout for mptcp_options_received
- 8b88edd207eb: mptcp: consolidate in_opt sub-options fields in a bitmask
- 2cea5d97a2ff: mptcp: optimize the input options processing
- Results: 720da4811476..6d620b4d8c2b
Builds and tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210820T080437
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210820T08043
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-08-20 8:05 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 15:16 [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 1/4] mptcp: do not set unconditionally csum_reqd on incoming opt Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 2/4] mptcp: better binary layout for mptcp_options_received Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 3/4] mptcp: consolidate in_opt sub-options fields in a bitmask Paolo Abeni
2021-08-19 15:16 ` [PATCH v2 mptcp-next 4/4] mptcp: optimize the input options processing Paolo Abeni
2021-08-20 0:24 ` Mat Martineau
2021-08-20 7:58 ` Paolo Abeni
2021-08-20 8:02 ` Matthieu Baerts
2021-08-20 8:05 ` [PATCH v2 mptcp-next 0/4] mptcp: minor receive path optimization Matthieu Baerts
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.