Re: [MPTCP] [PATCH] mptcp: fall back to tcp when dss checksum are requested

[Florian Westphal <fw at strlen.de>]

[-- Attachment #1: Type: text/plain, Size: 3066 bytes --]

Florian Westphal <fw(a)strlen.de> wrote:
> Peter Krystad <peter.krystad(a)linux.intel.com> wrote:
> > 
> > Hi Florian -
> > 
> > My thinking here was that code in mptcp_parse_options() just parses the
> > options without having knowledge about configuration. There is existing logic
> > for peer requesting checksums in subflow_v4_init_req().
> 
> Uh, I don't understand this then.  Can you fix this?
> 
> mptcp-next can't talk to mptcp kernel unless one disables checksums on
> the mptcp kernel.
> 
> mptcp (out of tree) excepts checksums in default settings and will reset
> the connection quickly.  I don't see how to fallback to tcp in that case
> except with the patch I sent (not setting mp capable).

I hacked this thing together but I can't say I like it, but I found no
other solution to transport the needed information (csum requested)
to the spots that need to withdraw the mp_capable=1 decision.

 net/mptcp/options.c |  3 +++
 net/mptcp/subflow.c | 21 +++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 9f892478d336..2bfba0cee72c 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -320,6 +320,9 @@ void mptcp_rcv_synsent(struct sock *sk)
 	struct subflow_context *subflow = subflow_ctx(sk);
 
 	if (subflow->request_mptcp && tp->rx_opt.mptcp.mp_capable) {
+		if (tp->rx_opt.mptcp.flags & MPTCP_CAP_CHECKSUM_REQD)
+			subflow->use_checksum = 1;
+
 		subflow->mp_capable = 1;
 		subflow->remote_key = tp->rx_opt.mptcp.sndr_key;
 		pr_debug("subflow=%p, remote_key=%llu", subflow,
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index c9dcb601dce3..ec1e1ada657b 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -74,6 +74,22 @@ static void subflow_v4_init_req(struct request_sock *req,
 	subflow_req->mp_capable = 0;
 	subflow_req->mp_join = 0;
 
+	/* RFC 6824, Section 3.1:
+	 * "For the Checksum Required bit (labeled "A"), if either
+	 * host requires the use of checksums, checksums MUST be used.
+	 * In other words, the only way for checksums not to be used
+	 * is if both hosts in their SYNs set A=0."
+	 *
+	 * Section 3.3.0:
+	 * "If a checksum is not present when its use has been
+	 * negotiated, the receiver MUST close the subflow with a RST as it is
+	 * considered broken."
+	 *
+	 * We don't implement DSS checksum - fall back to TCP.
+	 */
+	if (rx_opt.mptcp.flags & MPTCP_CAP_CHECKSUM_REQD)
+		return;
+
 	if (rx_opt.mptcp.mp_capable && rx_opt.mptcp.mp_join)
 		return;
 
@@ -148,6 +164,11 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 	if (subflow->mp_capable && !subflow->conn_finished) {
 		pr_debug("subflow=%p, remote_key=%llu", subflow_ctx(sk),
 			 subflow->remote_key);
+
+		/* We don't support DSS checksums, no choice but to disable mptcp */
+		if (subflow->use_checksum)
+			subflow->mp_capable = 0;
+
 		mptcp_finish_connect(subflow->conn, subflow->mp_capable);
 		subflow->conn_finished = 1;
 
-- 
2.21.0