[MPTCP][PATCH v2 mptcp-next 0/4] add MP_CAPABLE 'C' flag

[MPTCP][PATCH v2 mptcp-next 0/4] add MP_CAPABLE 'C' flag

[Geliang Tang]

v2:
 - rename join_denied to allow_join_id0 in mptcp_out_options
 - rename join_denied to deny_join_id0 in mptcp_options_received
 - add a new function mptcp_pm_deny_join_id0_received
 - move deny_join_id0 flag from mptcp_sock to mptcp_pm_data
 - check deny_join_id0 flag in mptcp_pm_create_subflow_or_signal_addr
instead of in mptcp_syn_options.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/183

Geliang Tang (4):
  mptcp: add sysctl allow_join_initial_addr_port
  mptcp: add allow_join_id0 in mptcp_out_options
  mptcp: add deny_join_id0 in mptcp_options_received
  selftests: mptcp: add deny_join_id0 testcases

 Documentation/networking/mptcp-sysctl.rst     | 13 +++++
 include/net/mptcp.h                           |  3 +-
 net/mptcp/ctrl.c                              | 14 +++++
 net/mptcp/options.c                           | 16 +++++-
 net/mptcp/pm.c                                | 10 ++++
 net/mptcp/pm_netlink.c                        |  4 +-
 net/mptcp/protocol.h                          | 12 +++-
 net/mptcp/subflow.c                           |  3 +
 .../testing/selftests/net/mptcp/mptcp_join.sh | 55 ++++++++++++++++++-
 9 files changed, 122 insertions(+), 8 deletions(-)

-- 
2.30.2

[MPTCP][PATCH v2 mptcp-next 1/4] mptcp: add sysctl allow_join_initial_addr_port

[Geliang Tang]

This patch added a new sysctl, named allow_join_initial_addr_port, to
control whether allow peers to send join requests to the IP address and
port number used by the initial subflow.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 Documentation/networking/mptcp-sysctl.rst | 13 +++++++++++++
 net/mptcp/ctrl.c                          | 14 ++++++++++++++
 net/mptcp/protocol.h                      |  1 +
 3 files changed, 28 insertions(+)

diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index 6af0196c4297..9088836ded19 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -24,3 +24,16 @@ add_addr_timeout - INTEGER (seconds)
 	sysctl.
 
 	Default: 120
+
+allow_join_initial_addr_port - INTEGER
+	Allow peers to send join requests to the IP address and port number used
+	by the initial subflow if the value is nonzero. This controls a flag that
+	is sent to the peer at connection time, and whether such join requests are
+	accepted or denied.
+
+	Joins to addresses advertised with ADD_ADDR are not affected by this
+	value.
+
+	This is a per-namespace sysctl.
+
+	Default: 1
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 96ba616f59bf..3b963a25d5a8 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -19,6 +19,7 @@ struct mptcp_pernet {
 
 	int mptcp_enabled;
 	unsigned int add_addr_timeout;
+	int allow_join_initial_addr_port;
 };
 
 static struct mptcp_pernet *mptcp_get_pernet(struct net *net)
@@ -36,6 +37,11 @@ unsigned int mptcp_get_add_addr_timeout(struct net *net)
 	return mptcp_get_pernet(net)->add_addr_timeout;
 }
 
+int mptcp_is_allow_join_id0(struct net *net)
+{
+	return mptcp_get_pernet(net)->allow_join_initial_addr_port;
+}
+
 static struct ctl_table mptcp_sysctl_table[] = {
 	{
 		.procname = "enabled",
@@ -52,6 +58,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
 		.mode = 0644,
 		.proc_handler = proc_dointvec_jiffies,
 	},
+	{
+		.procname = "allow_join_initial_addr_port",
+		.maxlen = sizeof(int),
+		.mode = 0644,
+		.proc_handler = proc_dointvec,
+	},
 	{}
 };
 
@@ -59,6 +71,7 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 {
 	pernet->mptcp_enabled = 1;
 	pernet->add_addr_timeout = TCP_RTO_MAX;
+	pernet->allow_join_initial_addr_port = 1;
 }
 
 static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
@@ -75,6 +88,7 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
 
 	table[0].data = &pernet->mptcp_enabled;
 	table[1].data = &pernet->add_addr_timeout;
+	table[2].data = &pernet->allow_join_initial_addr_port;
 
 	hdr = register_net_sysctl(net, MPTCP_SYSCTL_PATH, table);
 	if (!hdr)
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index d230a75af631..bb901435c2d4 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -524,6 +524,7 @@ static inline void mptcp_subflow_delegated_done(struct mptcp_subflow_context *su
 
 int mptcp_is_enabled(struct net *net);
 unsigned int mptcp_get_add_addr_timeout(struct net *net);
+int mptcp_is_allow_join_id0(struct net *net);
 void mptcp_subflow_fully_established(struct mptcp_subflow_context *subflow,
 				     struct mptcp_options_received *mp_opt);
 bool mptcp_subflow_data_available(struct sock *sk);
-- 
2.30.2

[MPTCP][PATCH v2 mptcp-next 2/4] mptcp: add allow_join_id0 in mptcp_out_options

[Geliang Tang]

This patch defined a new flag MPTCP_CAP_DENY_JOIN_ID0 for the third bit,
labeled "C" of the MP_CAPABLE option.

Add a new flag allow_join_id0 in struct mptcp_out_options. If this flag is
set, send out the MP_CAPABLE option with the flag MPTCP_CAP_DENY_JOIN_ID0.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 include/net/mptcp.h  |  3 ++-
 net/mptcp/options.c  | 10 ++++++++--
 net/mptcp/protocol.h |  6 ++++--
 net/mptcp/subflow.c  |  1 +
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 83f23774b908..d259796326ea 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -64,7 +64,8 @@ struct mptcp_out_options {
 	u8 join_id;
 	u8 backup;
 	u8 reset_reason:4;
-	u8 reset_transient:1;
+	u8 reset_transient:1,
+	   allow_join_id0:1;
 	u32 nonce;
 	u64 thmac;
 	u32 token;
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 99fc21406168..58ea22bf3d3d 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -381,6 +381,7 @@ bool mptcp_syn_options(struct sock *sk, const struct sk_buff *skb,
 	subflow->snd_isn = TCP_SKB_CB(skb)->end_seq;
 	if (subflow->request_mptcp) {
 		opts->suboptions = OPTION_MPTCP_MPC_SYN;
+		opts->allow_join_id0 = mptcp_is_allow_join_id0(sock_net(sk));
 		*size = TCPOLEN_MPTCP_MPC_SYN;
 		return true;
 	} else if (subflow->request_join) {
@@ -466,6 +467,7 @@ static bool mptcp_established_options_mp(struct sock *sk, struct sk_buff *skb,
 		opts->suboptions = OPTION_MPTCP_MPC_ACK;
 		opts->sndr_key = subflow->local_key;
 		opts->rcvr_key = subflow->remote_key;
+		opts->allow_join_id0 = mptcp_is_allow_join_id0(sock_net(sk));
 
 		/* Section 3.1.
 		 * The MP_CAPABLE option is carried on the SYN, SYN/ACK, and ACK
@@ -790,6 +792,7 @@ bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
 	if (subflow_req->mp_capable) {
 		opts->suboptions = OPTION_MPTCP_MPC_SYNACK;
 		opts->sndr_key = subflow_req->local_key;
+		opts->allow_join_id0 = subflow_req->allow_join_id0;
 		*size = TCPOLEN_MPTCP_MPC_SYNACK;
 		pr_debug("subflow_req=%p, local_key=%llu",
 			 subflow_req, subflow_req->local_key);
@@ -1124,7 +1127,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 {
 	if ((OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_SYNACK |
 	     OPTION_MPTCP_MPC_ACK) & opts->suboptions) {
-		u8 len;
+		u8 len, flag = MPTCP_CAP_HMAC_SHA256;
 
 		if (OPTION_MPTCP_MPC_SYN & opts->suboptions)
 			len = TCPOLEN_MPTCP_MPC_SYN;
@@ -1135,9 +1138,12 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 		else
 			len = TCPOLEN_MPTCP_MPC_ACK;
 
+		if (!opts->allow_join_id0)
+			flag |= MPTCP_CAP_DENY_JOIN_ID0;
+
 		*ptr++ = mptcp_option(MPTCPOPT_MP_CAPABLE, len,
 				      MPTCP_SUPPORTED_VERSION,
-				      MPTCP_CAP_HMAC_SHA256);
+				      flag);
 
 		if (!((OPTION_MPTCP_MPC_SYNACK | OPTION_MPTCP_MPC_ACK) &
 		    opts->suboptions))
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index bb901435c2d4..fcbaf47ce3bc 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -77,8 +77,9 @@
 #define MPTCP_VERSION_MASK	(0x0F)
 #define MPTCP_CAP_CHECKSUM_REQD	BIT(7)
 #define MPTCP_CAP_EXTENSIBILITY	BIT(6)
+#define MPTCP_CAP_DENY_JOIN_ID0	BIT(5)
 #define MPTCP_CAP_HMAC_SHA256	BIT(0)
-#define MPTCP_CAP_FLAG_MASK	(0x3F)
+#define MPTCP_CAP_FLAG_MASK	(0x1F)
 
 /* MPTCP DSS flags */
 #define MPTCP_DSS_DATA_FIN	BIT(4)
@@ -338,7 +339,8 @@ struct mptcp_subflow_request_sock {
 	struct	tcp_request_sock sk;
 	u16	mp_capable : 1,
 		mp_join : 1,
-		backup : 1;
+		backup : 1,
+		allow_join_id0 : 1;
 	u8	local_id;
 	u8	remote_id;
 	u64	local_key;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 15620bafc544..fb5b6eb5bec9 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -108,6 +108,7 @@ static void subflow_init_req(struct request_sock *req, const struct sock *sk_lis
 
 	subflow_req->mp_capable = 0;
 	subflow_req->mp_join = 0;
+	subflow_req->allow_join_id0 = mptcp_is_allow_join_id0(sock_net(sk_listener));
 	subflow_req->msk = NULL;
 	mptcp_token_init_request(req);
 }
-- 
2.30.2

[MPTCP][PATCH v2 mptcp-next 3/4] mptcp: add deny_join_id0 in mptcp_options_received

[Geliang Tang]

This patch added a new flag named deny_join_id0 in struct
mptcp_options_received. Set it when MP_CAPABLE with the flag
MPTCP_CAP_DENYJOIN_ID0 is received.

Also add a new flag deny_join_id0 in struct mptcp_pm_data. When the flag
mp_opt.deny_join_id0 is set, invoke mptcp_pm_deny_join_id0_received to
set the pm->deny_join_id0 flag.

In mptcp_pm_create_subflow_or_signal_addr, if the pm->deny_join_id0 flag
is set, and the remote address id is zero, stop this connection.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/options.c    |  6 ++++++
 net/mptcp/pm.c         | 10 ++++++++++
 net/mptcp/pm_netlink.c |  4 +++-
 net/mptcp/protocol.h   |  5 ++++-
 net/mptcp/subflow.c    |  2 ++
 5 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 58ea22bf3d3d..0618c2fd9887 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -77,6 +77,9 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		if (flags & MPTCP_CAP_CHECKSUM_REQD)
 			break;
 
+		if (flags & MPTCP_CAP_DENY_JOIN_ID0)
+			mp_opt->deny_join_id0 = 1;
+
 		mp_opt->mp_capable = 1;
 		if (opsize >= TCPOLEN_MPTCP_MPC_SYNACK) {
 			mp_opt->sndr_key = get_unaligned_be64(ptr);
@@ -342,6 +345,7 @@ void mptcp_get_options(const struct sk_buff *skb,
 	mp_opt->dss = 0;
 	mp_opt->mp_prio = 0;
 	mp_opt->reset = 0;
+	mp_opt->deny_join_id0 = 0;
 
 	length = (th->doff * 4) - sizeof(struct tcphdr);
 	ptr = (const unsigned char *)(th + 1);
@@ -1012,6 +1016,8 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 	}
 
 	mptcp_get_options(skb, &mp_opt);
+	if (mp_opt.deny_join_id0)
+		mptcp_pm_deny_join_id0_received(msk);
 	if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
 		return;
 
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 9d00fa6d22e9..e3489766caf3 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -247,6 +247,15 @@ void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
 	mptcp_event(MPTCP_EVENT_SUB_PRIORITY, mptcp_sk(subflow->conn), sk, GFP_ATOMIC);
 }
 
+void mptcp_pm_deny_join_id0_received(struct mptcp_sock *msk)
+{
+	struct mptcp_pm_data *pm = &msk->pm;
+
+	spin_lock_bh(&pm->lock);
+	WRITE_ONCE(pm->deny_join_id0, true);
+	spin_unlock_bh(&pm->lock);
+}
+
 /* path manager helpers */
 
 bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
@@ -320,6 +329,7 @@ void mptcp_pm_data_init(struct mptcp_sock *msk)
 	WRITE_ONCE(msk->pm.addr_signal, 0);
 	WRITE_ONCE(msk->pm.accept_addr, false);
 	WRITE_ONCE(msk->pm.accept_subflow, false);
+	WRITE_ONCE(msk->pm.deny_join_id0, false);
 	msk->pm.status = 0;
 
 	spin_lock_init(&msk->pm.lock);
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index d094588afad8..1491a759a04a 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -456,10 +456,12 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 		if (local) {
 			struct mptcp_addr_info remote = { 0 };
 
+			remote_address((struct sock_common *)sk, &remote);
+			if (!remote.id && READ_ONCE(msk->pm.deny_join_id0))
+				return;
 			msk->pm.local_addr_used++;
 			msk->pm.subflows++;
 			check_work_pending(msk);
-			remote_address((struct sock_common *)sk, &remote);
 			spin_unlock_bh(&msk->pm.lock);
 			__mptcp_subflow_connect(sk, &local->addr, &remote,
 						local->flags, local->ifindex);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index fcbaf47ce3bc..270feb0a1cd8 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -134,7 +134,8 @@ struct mptcp_options_received {
 		rm_addr : 1,
 		mp_prio : 1,
 		echo : 1,
-		backup : 1;
+		backup : 1,
+		deny_join_id0 : 1;
 	u32	token;
 	u32	nonce;
 	u64	thmac;
@@ -189,6 +190,7 @@ struct mptcp_pm_data {
 	bool		work_pending;
 	bool		accept_addr;
 	bool		accept_subflow;
+	bool		deny_join_id0;
 	u8		add_addr_signaled;
 	u8		add_addr_accepted;
 	u8		local_addr_used;
@@ -669,6 +671,7 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk);
 void mptcp_pm_rm_addr_received(struct mptcp_sock *msk,
 			       const struct mptcp_rm_list *rm_list);
 void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup);
+void mptcp_pm_deny_join_id0_received(struct mptcp_sock *msk);
 int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
 				 struct mptcp_addr_info *addr,
 				 u8 bkup);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index fb5b6eb5bec9..72d4a1520ba5 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -405,6 +405,8 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 			goto fallback;
 		}
 
+		if (mp_opt.deny_join_id0)
+			mptcp_pm_deny_join_id0_received(mptcp_sk(parent));
 		subflow->mp_capable = 1;
 		subflow->can_ack = 1;
 		subflow->remote_key = mp_opt.sndr_key;
-- 
2.30.2

[MPTCP][PATCH v2 mptcp-next 4/4] selftests: mptcp: add deny_join_id0 testcases

[Geliang Tang]

This patch added a new argument '-d' for mptcp_join.sh script, to invoke
the testcases for the MP_CAPABLE 'C' flag.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 55 ++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index fd99485cf2a4..feabc3ab88eb 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -124,6 +124,17 @@ reset_with_add_addr_timeout()
 		-j DROP
 }
 
+reset_with_allow_join_id0()
+{
+	local ns1_enable=$1
+	local ns2_enable=$2
+
+	reset
+
+	ip netns exec $ns1 sysctl -q net.mptcp.allow_join_initial_addr_port=$ns1_enable
+	ip netns exec $ns2 sysctl -q net.mptcp.allow_join_initial_addr_port=$ns2_enable
+}
+
 ip -Version > /dev/null 2>&1
 if [ $? -ne 0 ];then
 	echo "SKIP: Could not run test without ip tool"
@@ -1374,6 +1385,43 @@ syncookies_tests()
 	chk_add_nr 1 1
 }
 
+deny_join_id0_tests()
+{
+	# subflow allow join id0 ns1
+	reset_with_allow_join_id0 1 0
+	ip netns exec $ns1 ./pm_nl_ctl limits 1 1
+	ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+	ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
+	run_tests $ns1 $ns2 10.0.1.1
+	chk_join_nr "single subflow allow join id0 ns1" 1 1 1
+
+	# subflow allow join id0 ns2
+	reset_with_allow_join_id0 0 1
+	ip netns exec $ns1 ./pm_nl_ctl limits 1 1
+	ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+	ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
+	run_tests $ns1 $ns2 10.0.1.1
+	chk_join_nr "single subflow allow join id0 ns2" 0 0 0
+
+	# signal address allow join id0 ns1
+	reset_with_allow_join_id0 1 0
+	ip netns exec $ns1 ./pm_nl_ctl limits 1 1
+	ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+	ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
+	run_tests $ns1 $ns2 10.0.1.1
+	chk_join_nr "signal address allow join id0 ns1" 1 1 1
+	chk_add_nr 1 1
+
+	# signal address allow join id0 ns2
+	reset_with_allow_join_id0 0 1
+	ip netns exec $ns1 ./pm_nl_ctl limits 1 1
+	ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+	ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
+	run_tests $ns1 $ns2 10.0.1.1
+	chk_join_nr "signal address allow join id0 ns2" 1 1 1
+	chk_add_nr 1 1
+}
+
 all_tests()
 {
 	subflows_tests
@@ -1387,6 +1435,7 @@ all_tests()
 	backup_tests
 	add_addr_ports_tests
 	syncookies_tests
+	deny_join_id0_tests
 }
 
 usage()
@@ -1403,6 +1452,7 @@ usage()
 	echo "  -b backup_tests"
 	echo "  -p add_addr_ports_tests"
 	echo "  -k syncookies_tests"
+	echo "  -d deny_join_id0_tests"
 	echo "  -c capture pcap files"
 	echo "  -h help"
 }
@@ -1434,7 +1484,7 @@ if [ $do_all_tests -eq 1 ]; then
 	exit $ret
 fi
 
-while getopts 'fsltra64bpkch' opt; do
+while getopts 'fsltra64bpkdch' opt; do
 	case $opt in
 		f)
 			subflows_tests
@@ -1469,6 +1519,9 @@ while getopts 'fsltra64bpkch' opt; do
 		k)
 			syncookies_tests
 			;;
+		d)
+			deny_join_id0_tests
+			;;
 		c)
 			;;
 		h | *)
-- 
2.30.2

Re: [MPTCP][PATCH v2 mptcp-next 1/4] mptcp: add sysctl allow_join_initial_addr_port

[Florian Westphal]

Geliang Tang <geliangtang@gmail.com> wrote:
> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index 96ba616f59bf..3b963a25d5a8 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -19,6 +19,7 @@ struct mptcp_pernet {
>  
>  	int mptcp_enabled;
>  	unsigned int add_addr_timeout;
> +	int allow_join_initial_addr_port;

u8 allow_join_initial_addr_port;

>  static struct mptcp_pernet *mptcp_get_pernet(struct net *net)
> @@ -36,6 +37,11 @@ unsigned int mptcp_get_add_addr_timeout(struct net *net)
>  	return mptcp_get_pernet(net)->add_addr_timeout;
>  }
>  
> +int mptcp_is_allow_join_id0(struct net *net)
> +{
> +	return mptcp_get_pernet(net)->allow_join_initial_addr_port;
> +}
> +
>  static struct ctl_table mptcp_sysctl_table[] = {
>  	{
>  		.procname = "enabled",
> @@ -52,6 +58,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
>  		.mode = 0644,
>  		.proc_handler = proc_dointvec_jiffies,
>  	},
> +	{
> +		.procname = "allow_join_initial_addr_port",
> +		.maxlen = sizeof(int),

sizeof(u8),

> +		.mode = 0644,
> +		.proc_handler = proc_dointvec,

.proc_handler   = proc_dou8vec_minmax,
.extra1         = SYSCTL_ZERO,
.extra2         = SYSCTL_ONE

Re: [MPTCP][PATCH v2 mptcp-next 3/4] mptcp: add deny_join_id0 in mptcp_options_received

[Florian Westphal]

Geliang Tang <geliangtang@gmail.com> wrote:
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 9d00fa6d22e9..e3489766caf3 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -247,6 +247,15 @@ void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
>  	mptcp_event(MPTCP_EVENT_SUB_PRIORITY, mptcp_sk(subflow->conn), sk, GFP_ATOMIC);
>  }
>  
> +void mptcp_pm_deny_join_id0_received(struct mptcp_sock *msk)
> +{
> +	struct mptcp_pm_data *pm = &msk->pm;
> +
> +	spin_lock_bh(&pm->lock);
> +	WRITE_ONCE(pm->deny_join_id0, true);
> +	spin_unlock_bh(&pm->lock);

Spinlock isn't needed.

Re: [MPTCP][PATCH v2 mptcp-next 1/4] mptcp: add sysctl allow_join_initial_addr_port

[Matthieu Baerts]

Hi Florian,

Thank you for this review!

On 28/04/2021 11:40, Florian Westphal wrote:
> Geliang Tang <geliangtang@gmail.com> wrote:
>> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
>> index 96ba616f59bf..3b963a25d5a8 100644
>> --- a/net/mptcp/ctrl.c
>> +++ b/net/mptcp/ctrl.c
>> @@ -19,6 +19,7 @@ struct mptcp_pernet {
>>  
>>  	int mptcp_enabled;
>>  	unsigned int add_addr_timeout;
>> +	int allow_join_initial_addr_port;
> 
> u8 allow_join_initial_addr_port;

Should we do the same modifications for 'mptcp_enabled'? Switch to a u8
with min 0 and max 1?

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

Re: [MPTCP][PATCH v2 mptcp-next 1/4] mptcp: add sysctl allow_join_initial_addr_port

[Matthieu Baerts]

On 28/04/2021 12:36, Matthieu Baerts wrote:
> Hi Florian,
> 
> Thank you for this review!
> 
> On 28/04/2021 11:40, Florian Westphal wrote:
>> Geliang Tang <geliangtang@gmail.com> wrote:
>>> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
>>> index 96ba616f59bf..3b963a25d5a8 100644
>>> --- a/net/mptcp/ctrl.c
>>> +++ b/net/mptcp/ctrl.c
>>> @@ -19,6 +19,7 @@ struct mptcp_pernet {
>>>  
>>>  	int mptcp_enabled;
>>>  	unsigned int add_addr_timeout;
>>> +	int allow_join_initial_addr_port;
>>
>> u8 allow_join_initial_addr_port;
> 
> Should we do the same modifications for 'mptcp_enabled'? Switch to a u8
> with min 0 and max 1?

Even if we typically use it with 0 and 1, modifying 'mptcp_enabled'
might be seen as breaking the exposed userspace API, no? Maybe too late
to change?

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

Re: [MPTCP][PATCH v2 mptcp-next 1/4] mptcp: add sysctl allow_join_initial_addr_port

[Florian Westphal]

Matthieu Baerts <matthieu.baerts@tessares.net> wrote:
> >>>  	int mptcp_enabled;
> >>>  	unsigned int add_addr_timeout;
> >>> +	int allow_join_initial_addr_port;
> >>
> >> u8 allow_join_initial_addr_port;
> > 
> > Should we do the same modifications for 'mptcp_enabled'? Switch to a u8
> > with min 0 and max 1?

It would be better yes, but I was more interested to not follow the
existing example.

> Even if we typically use it with 0 and 1, modifying 'mptcp_enabled'
> might be seen as breaking the exposed userspace API, no? Maybe too late
> to change?

Such changes have been done in the past, I think its ok.

Re: [MPTCP][PATCH v2 mptcp-next 3/4] mptcp: add deny_join_id0 in mptcp_options_received

[Geliang Tang]

Hi Florian,

Florian Westphal <fw@strlen.de> 于2021年4月28日周三 下午5:43写道：
>
> Geliang Tang <geliangtang@gmail.com> wrote:
> > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> > index 9d00fa6d22e9..e3489766caf3 100644
> > --- a/net/mptcp/pm.c
> > +++ b/net/mptcp/pm.c
> > @@ -247,6 +247,15 @@ void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
> >       mptcp_event(MPTCP_EVENT_SUB_PRIORITY, mptcp_sk(subflow->conn), sk, GFP_ATOMIC);
> >  }
> >
> > +void mptcp_pm_deny_join_id0_received(struct mptcp_sock *msk)
> > +{
> > +     struct mptcp_pm_data *pm = &msk->pm;
> > +
> > +     spin_lock_bh(&pm->lock);
> > +     WRITE_ONCE(pm->deny_join_id0, true);
> > +     spin_unlock_bh(&pm->lock);
>
> Spinlock isn't needed.

Thanks for your review, updated it in v3.

-Geliang

Re: [MPTCP][PATCH v2 mptcp-next 1/4] mptcp: add sysctl allow_join_initial_addr_port

[Geliang Tang]

Hi Florian,

Florian Westphal <fw@strlen.de> 于2021年4月28日周三 下午5:40写道：
>
> Geliang Tang <geliangtang@gmail.com> wrote:
> > diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> > index 96ba616f59bf..3b963a25d5a8 100644
> > --- a/net/mptcp/ctrl.c
> > +++ b/net/mptcp/ctrl.c
> > @@ -19,6 +19,7 @@ struct mptcp_pernet {
> >
> >       int mptcp_enabled;
> >       unsigned int add_addr_timeout;
> > +     int allow_join_initial_addr_port;
>
> u8 allow_join_initial_addr_port;
>
> >  static struct mptcp_pernet *mptcp_get_pernet(struct net *net)
> > @@ -36,6 +37,11 @@ unsigned int mptcp_get_add_addr_timeout(struct net *net)
> >       return mptcp_get_pernet(net)->add_addr_timeout;
> >  }
> >
> > +int mptcp_is_allow_join_id0(struct net *net)
> > +{
> > +     return mptcp_get_pernet(net)->allow_join_initial_addr_port;
> > +}
> > +
> >  static struct ctl_table mptcp_sysctl_table[] = {
> >       {
> >               .procname = "enabled",
> > @@ -52,6 +58,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
> >               .mode = 0644,
> >               .proc_handler = proc_dointvec_jiffies,
> >       },
> > +     {
> > +             .procname = "allow_join_initial_addr_port",
> > +             .maxlen = sizeof(int),
>
> sizeof(u8),
>
> > +             .mode = 0644,
> > +             .proc_handler = proc_dointvec,
>
> .proc_handler   = proc_dou8vec_minmax,
> .extra1         = SYSCTL_ZERO,
> .extra2         = SYSCTL_ONE

Updated in v3, thanks.

-Geliang