[PATCH mptcp-next 0/2] reuse __mptcp_make_csum in validate_data_csum

[PATCH mptcp-next 0/2] reuse __mptcp_make_csum in validate_data_csum

[Geliang Tang]

Two cleanup patches for the data checksum.

Geliang Tang (2):
  mptcp: change the parameter of __mptcp_make_csum
  mptcp: reuse __mptcp_make_csum in validate_data_csum

 net/mptcp/options.c  |  8 ++++----
 net/mptcp/protocol.h |  1 +
 net/mptcp/subflow.c  | 15 ++++++---------
 3 files changed, 11 insertions(+), 13 deletions(-)

-- 
2.31.1

[PATCH mptcp-next 1/2] mptcp: change the parameter of __mptcp_make_csum

[Geliang Tang]

This patch changed the type of the last parameter of __mptcp_make_csum()
from __sum16 to __wsum. And export this function in protocol.h.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/options.c  | 8 ++++----
 net/mptcp/protocol.h | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 7a6a39b71633..c6726e8389ec 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1229,7 +1229,7 @@ static void mptcp_set_rwin(const struct tcp_sock *tp)
 		WRITE_ONCE(msk->rcv_wnd_sent, ack_seq);
 }
 
-static u16 __mptcp_make_csum(u64 data_seq, u32 subflow_seq, u16 data_len, __sum16 sum)
+u16 __mptcp_make_csum(u64 data_seq, u32 subflow_seq, u16 data_len, __wsum sum)
 {
 	struct csum_pseudo_header header;
 	__wsum csum;
@@ -1244,14 +1244,14 @@ static u16 __mptcp_make_csum(u64 data_seq, u32 subflow_seq, u16 data_len, __sum1
 	header.data_len = htons(data_len);
 	header.csum = 0;
 
-	csum = csum_partial(&header, sizeof(header), ~csum_unfold(sum));
+	csum = csum_partial(&header, sizeof(header), sum);
 	return (__force u16)csum_fold(csum);
 }
 
 static u16 mptcp_make_csum(const struct mptcp_ext *mpext)
 {
 	return __mptcp_make_csum(mpext->data_seq, mpext->subflow_seq, mpext->data_len,
-				 mpext->csum);
+				 ~csum_unfold(mpext->csum));
 }
 
 void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
@@ -1372,7 +1372,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 					   __mptcp_make_csum(opts->data_seq,
 							     opts->subflow_seq,
 							     opts->data_len,
-							     opts->csum), ptr);
+							     ~csum_unfold(opts->csum)), ptr);
 		} else {
 			put_unaligned_be32(opts->data_len << 16 |
 					   TCPOPT_NOP << 8 | TCPOPT_NOP, ptr);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index d1b46c0d8c40..ec077e52c475 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -737,6 +737,7 @@ void mptcp_token_destroy(struct mptcp_sock *msk);
 void mptcp_crypto_key_sha(u64 key, u32 *token, u64 *idsn);
 
 void mptcp_crypto_hmac_sha(u64 key1, u64 key2, u8 *msg, int len, void *hmac);
+u16 __mptcp_make_csum(u64 data_seq, u32 subflow_seq, u16 data_len, __wsum sum);
 
 void __init mptcp_pm_init(void);
 void mptcp_pm_data_init(struct mptcp_sock *msk);
-- 
2.31.1

[PATCH mptcp-next 2/2] mptcp: reuse __mptcp_make_csum in validate_data_csum

[Geliang Tang]

This patch reused __mptcp_make_csum() in validate_data_csum() instead of
open-coding.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/subflow.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index fd5fdb639088..c8126986793e 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -845,9 +845,8 @@ static enum mapping_status validate_data_csum(struct sock *ssk, struct sk_buff *
 					      bool csum_reqd)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
-	struct csum_pseudo_header header;
 	u32 offset, seq, delta;
-	__wsum csum;
+	u16 csum;
 	int len;
 
 	if (!csum_reqd)
@@ -908,13 +907,11 @@ static enum mapping_status validate_data_csum(struct sock *ssk, struct sk_buff *
 	 * while the pseudo header requires the original DSS data len,
 	 * including that
 	 */
-	header.data_seq = cpu_to_be64(subflow->map_seq);
-	header.subflow_seq = htonl(subflow->map_subflow_seq);
-	header.data_len = htons(subflow->map_data_len + subflow->map_data_fin);
-	header.csum = 0;
-
-	csum = csum_partial(&header, sizeof(header), subflow->map_data_csum);
-	if (unlikely(csum_fold(csum))) {
+	csum = __mptcp_make_csum(subflow->map_seq,
+				 subflow->map_subflow_seq,
+				 subflow->map_data_len + subflow->map_data_fin,
+				 subflow->map_data_csum);
+	if (unlikely(csum)) {
 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DATACSUMERR);
 		subflow->send_mp_fail = 1;
 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_MPFAILTX);
-- 
2.31.1

Re: [PATCH mptcp-next 0/2] reuse __mptcp_make_csum in validate_data_csum

[Mat Martineau]

On Fri, 17 Dec 2021, Geliang Tang wrote:

> Two cleanup patches for the data checksum.
>
> Geliang Tang (2):
>  mptcp: change the parameter of __mptcp_make_csum
>  mptcp: reuse __mptcp_make_csum in validate_data_csum
>
> net/mptcp/options.c  |  8 ++++----
> net/mptcp/protocol.h |  1 +
> net/mptcp/subflow.c  | 15 ++++++---------
> 3 files changed, 11 insertions(+), 13 deletions(-)
>
> -- 
> 2.31.1


Hi Geliang -

Thanks for the cleanup patches! The changes look good to me, and all the 
'mptcp_connect.sh -C' tests work on my system. I tested with only patch 1, 
or both patch 1 & 2, and since both the old and new validation code work 
we can be sure the behavior hasn't changed.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next 0/2] reuse __mptcp_make_csum in validate_data_csum

[Matthieu Baerts]

Hi Geliang, Mat,

On 17/12/2021 07:38, Geliang Tang wrote:
> Two cleanup patches for the data checksum.
> 
> Geliang Tang (2):
>   mptcp: change the parameter of __mptcp_make_csum
>   mptcp: reuse __mptcp_make_csum in validate_data_csum

Thank you for these patches and reviews!

Now in our tree (feat. for net-next) with Mat's RvB tag:

- 3d0fe9f4a4ff: mptcp: change the parameter of __mptcp_make_csum
- 4990d2f45261: mptcp: reuse __mptcp_make_csum in validate_data_csum
- Results: 079ed80abcff..23b272a3d65a

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20211218T090448
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net