[PATCH mptcp-next 0/4] add MPTCP_MIB_CURRESTAB

[PATCH mptcp-next 0/4] add MPTCP_MIB_CURRESTAB

[Geliang Tang]

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

Geliang Tang (4):
  mptcp: add mib counter dec helper
  mptcp: add a current established counter
  mptcp: add ftrace callback of tcp_set_state
  selftests: mptcp: join: check CURRESTAB counters

 net/mptcp/Makefile                            |  1 +
 net/mptcp/mib.c                               |  1 +
 net/mptcp/mib.h                               |  8 +++
 net/mptcp/trace.c                             | 68 +++++++++++++++++++
 .../testing/selftests/net/mptcp/mptcp_join.sh | 58 ++++++++++++++--
 5 files changed, 131 insertions(+), 5 deletions(-)
 create mode 100644 net/mptcp/trace.c

-- 
2.35.3

[PATCH mptcp-next 1/4] mptcp: add mib counter dec helper

[Geliang Tang]

Similar to MPTCP_INC_STATS() helper, this patch adds a new helper named
MPTCP_DEC_STATS() to decrement a MIB counter.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/mib.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index cae71d947252..9b7c366675ec 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -95,4 +95,11 @@ static inline void __MPTCP_INC_STATS(struct net *net,
 		__SNMP_INC_STATS(net->mib.mptcp_statistics, field);
 }
 
+static inline void MPTCP_DEC_STATS(struct net *net,
+				   enum linux_mptcp_mib_field field)
+{
+	if (likely(net->mib.mptcp_statistics))
+		SNMP_DEC_STATS(net->mib.mptcp_statistics, field);
+}
+
 bool mptcp_mib_alloc(struct net *net);
-- 
2.35.3

[PATCH mptcp-next 2/4] mptcp: add a current established counter

[Geliang Tang]

This patch adds a new MIB counter named MPTCP_MIB_CURRESTAB to count
current established MPTCP connections.

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

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index a0990c365a2e..c30405e76833 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -66,6 +66,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("RcvWndShared", MPTCP_MIB_RCVWNDSHARED),
 	SNMP_MIB_ITEM("RcvWndConflictUpdate", MPTCP_MIB_RCVWNDCONFLICTUPDATE),
 	SNMP_MIB_ITEM("RcvWndConflict", MPTCP_MIB_RCVWNDCONFLICT),
+	SNMP_MIB_ITEM("MPCurrEstab", MPTCP_MIB_CURRESTAB),
 	SNMP_MIB_SENTINEL
 };
 
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 9b7c366675ec..dd7fd1f246b5 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -65,6 +65,7 @@ enum linux_mptcp_mib_field {
 					 * conflict with another subflow while updating msk rcv wnd
 					 */
 	MPTCP_MIB_RCVWNDCONFLICT,	/* Conflict with while updating msk rcv wnd */
+	MPTCP_MIB_CURRESTAB,		/* Current established MPTCP connections */
 	__MPTCP_MIB_MAX
 };
 
-- 
2.35.3

[PATCH mptcp-next 3/4] mptcp: add ftrace callback of tcp_set_state

[Geliang Tang]

This patch adds a new function mptcp_check_state(), in it if switch from
or to ESTABLISH state, increment or decrement the newly added counter
MPTCP_MIB_CURRESTAB.

Instead of invoking mptcp_check_state() in tcp_set_state() directly, here
add a new file trace.c, in it use ftrace to hook a callback function to
tcp_set_state(), named mptcp_state_callback(). mptcp_check_state() is
invoked in the callback function.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/460
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/Makefile |  1 +
 net/mptcp/trace.c  | 68 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100644 net/mptcp/trace.c

diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
index 67cd565bb321..bf88bdb222b3 100644
--- a/net/mptcp/Makefile
+++ b/net/mptcp/Makefile
@@ -14,3 +14,4 @@ mptcp_token_test-objs := token_test.o
 obj-$(CONFIG_MPTCP_KUNIT_TEST) += mptcp_crypto_test.o mptcp_token_test.o
 
 obj-$(CONFIG_BPF_SYSCALL) += bpf.o
+obj-$(CONFIG_FUNCTION_TRACER) += trace.o
diff --git a/net/mptcp/trace.c b/net/mptcp/trace.c
new file mode 100644
index 000000000000..a930e1b50d56
--- /dev/null
+++ b/net/mptcp/trace.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Multipath TCP
+ *
+ * Copyright (c) 2023, SUSE.
+ */
+
+#define pr_fmt(fmt) "MPTCP: " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/ftrace.h>
+#include <net/sock.h>
+#include <net/mptcp.h>
+#include "protocol.h"
+#include "mib.h"
+
+static void mptcp_check_state(struct sock *sk, int oldstate, int state)
+{
+	switch (state) {
+	case TCP_ESTABLISHED:
+		if (oldstate != TCP_ESTABLISHED)
+			MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_CURRESTAB);
+		break;
+
+	default:
+		if (oldstate == TCP_ESTABLISHED)
+			MPTCP_DEC_STATS(sock_net(sk), MPTCP_MIB_CURRESTAB);
+	}
+}
+
+static void notrace mptcp_state_callback(unsigned long ip,
+					 unsigned long parent_ip,
+					 struct ftrace_ops *op,
+					 struct ftrace_regs *fregs)
+{
+	struct pt_regs *regs;
+	int oldstate, state;
+	struct sock *sk;
+
+	regs = ftrace_get_regs(fregs);
+	if (!regs)
+		return;
+
+	sk = (struct sock *)regs_get_kernel_argument(regs, 0);
+	if (!sk)
+		return;
+
+	oldstate = sk->sk_state;
+	state = regs_get_kernel_argument(regs, 1);
+
+	if (sk_is_mptcp(sk))
+		mptcp_check_state(sk, oldstate, state);
+}
+
+static struct ftrace_ops mptcp_state_ops __read_mostly = {
+	.func		= mptcp_state_callback,
+	.flags		= FTRACE_OPS_FL_SAVE_REGS,
+};
+
+static __init int mptcp_ftrace_init(void)
+{
+	int ret;
+
+	ret = ftrace_set_filter(&mptcp_state_ops, "tcp_set_state",
+				strlen("tcp_set_state"), 0);
+	return ret ?: register_ftrace_function(&mptcp_state_ops);
+}
+late_initcall(mptcp_ftrace_init);
-- 
2.35.3

Re: [PATCH mptcp-next 3/4] mptcp: add ftrace callback of tcp_set_state

[kernel test robot]

Hi Geliang,

kernel test robot noticed the following build errors:

[auto build test ERROR on mptcp/export]
[also build test ERROR on mptcp/export-net linus/master v6.7-rc3 next-20231128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Geliang-Tang/mptcp-add-mib-counter-dec-helper/20231128-221604
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
patch link:    https://lore.kernel.org/r/6f4baef54a7631cdc1460138bc7e52f2c432c69b.1701180777.git.geliang.tang%40suse.com
patch subject: [PATCH mptcp-next 3/4] mptcp: add ftrace callback of tcp_set_state
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20231129/202311290844.drzx9Vmn-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231129/202311290844.drzx9Vmn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311290844.drzx9Vmn-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/mptcp/trace.c: In function 'mptcp_state_callback':
>> net/mptcp/trace.c:44:29: error: implicit declaration of function 'regs_get_kernel_argument'; did you mean 'regs_get_kernel_stack_nth'? [-Werror=implicit-function-declaration]
      44 |         sk = (struct sock *)regs_get_kernel_argument(regs, 0);
         |                             ^~~~~~~~~~~~~~~~~~~~~~~~
         |                             regs_get_kernel_stack_nth
   cc1: some warnings being treated as errors


vim +44 net/mptcp/trace.c

    30	
    31	static void notrace mptcp_state_callback(unsigned long ip,
    32						 unsigned long parent_ip,
    33						 struct ftrace_ops *op,
    34						 struct ftrace_regs *fregs)
    35	{
    36		struct pt_regs *regs;
    37		int oldstate, state;
    38		struct sock *sk;
    39	
    40		regs = ftrace_get_regs(fregs);
    41		if (!regs)
    42			return;
    43	
  > 44		sk = (struct sock *)regs_get_kernel_argument(regs, 0);
    45		if (!sk)
    46			return;
    47	
    48		oldstate = sk->sk_state;
    49		state = regs_get_kernel_argument(regs, 1);
    50	
    51		if (sk_is_mptcp(sk))
    52			mptcp_check_state(sk, oldstate, state);
    53	}
    54	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH mptcp-next 3/4] mptcp: add ftrace callback of tcp_set_state

[kernel test robot]

Hi Geliang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mptcp/export]
[also build test WARNING on mptcp/export-net linus/master v6.7-rc3 next-20231128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Geliang-Tang/mptcp-add-mib-counter-dec-helper/20231128-221604
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
patch link:    https://lore.kernel.org/r/6f4baef54a7631cdc1460138bc7e52f2c432c69b.1701180777.git.geliang.tang%40suse.com
patch subject: [PATCH mptcp-next 3/4] mptcp: add ftrace callback of tcp_set_state
config: sparc-allmodconfig (https://download.01.org/0day-ci/archive/20231129/202311290855.aEWX2Ohy-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231129/202311290855.aEWX2Ohy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311290855.aEWX2Ohy-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/mptcp/trace.c: In function 'mptcp_state_callback':
   net/mptcp/trace.c:44:29: error: implicit declaration of function 'regs_get_kernel_argument'; did you mean 'regs_get_kernel_stack_nth'? [-Werror=implicit-function-declaration]
      44 |         sk = (struct sock *)regs_get_kernel_argument(regs, 0);
         |                             ^~~~~~~~~~~~~~~~~~~~~~~~
         |                             regs_get_kernel_stack_nth
>> net/mptcp/trace.c:44:14: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      44 |         sk = (struct sock *)regs_get_kernel_argument(regs, 0);
         |              ^
   cc1: some warnings being treated as errors


vim +44 net/mptcp/trace.c

    30	
    31	static void notrace mptcp_state_callback(unsigned long ip,
    32						 unsigned long parent_ip,
    33						 struct ftrace_ops *op,
    34						 struct ftrace_regs *fregs)
    35	{
    36		struct pt_regs *regs;
    37		int oldstate, state;
    38		struct sock *sk;
    39	
    40		regs = ftrace_get_regs(fregs);
    41		if (!regs)
    42			return;
    43	
  > 44		sk = (struct sock *)regs_get_kernel_argument(regs, 0);
    45		if (!sk)
    46			return;
    47	
    48		oldstate = sk->sk_state;
    49		state = regs_get_kernel_argument(regs, 1);
    50	
    51		if (sk_is_mptcp(sk))
    52			mptcp_check_state(sk, oldstate, state);
    53	}
    54	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH mptcp-next 4/4] selftests: mptcp: join: check CURRESTAB counters

[Geliang Tang]

This patch adds a new helper chk_cestab_nr() to check the current
established connections counter MIB_CURRESTAB. Set the newly added
variables cestab_ns1 and cestab_ns2 to indicate how many connections
are expected in ns1 or ns2. These checks are embedded in add_tests().

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 58 +++++++++++++++++--
 1 file changed, 53 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 947b60cc2512..10beaa6ec626 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -36,6 +36,8 @@ unset FAILING_LINKS
 unset test_linkfail
 unset addr_nr_ns1
 unset addr_nr_ns2
+unset cestab_ns1
+unset cestab_ns2
 unset sflags
 unset fastclose
 unset fullmesh
@@ -905,6 +907,46 @@ pm_nl_set_endpoint()
 	fi
 }
 
+chk_cestab_nr()
+{
+	local ns=$1
+	local cestab=$2
+	local count
+
+	print_check "current establish"
+	count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPCurrEstab")
+	if [ -z "$count" ]; then
+		print_skip
+	elif [ "$count" != "$cestab" ]; then
+		fail_test "got $count current establish[s] expected $cestab"
+	else
+		print_ok
+	fi
+}
+
+check_cestab()
+{
+	local cestab_ns1=${cestab_ns1:-0}
+	local cestab_ns2=${cestab_ns2:-0}
+
+	if ! mptcp_lib_kallsyms_has "register_ftrace_function"; then
+		return
+	fi
+
+	if [ $cestab_ns1 -gt 0 ]; then
+		if mptcp_lib_is_v6 $3; then
+			sleep 2
+		fi
+		chk_cestab_nr $1 $cestab_ns1
+	fi
+	if [ $cestab_ns2 -gt 0 ]; then
+		if mptcp_lib_is_v6 $3; then
+			sleep 2
+		fi
+		chk_cestab_nr $2 $cestab_ns2
+	fi
+}
+
 do_transfer()
 {
 	local listener_ns="$1"
@@ -1018,6 +1060,7 @@ do_transfer()
 	local cpid=$!
 
 	pm_nl_set_endpoint $listener_ns $connector_ns $connect_addr
+	check_cestab $listener_ns $connector_ns $connect_addr
 
 	wait $cpid
 	local retc=$?
@@ -2406,47 +2449,52 @@ add_tests()
 	if reset "add single subflow"; then
 		pm_nl_set_limits $ns1 0 1
 		pm_nl_set_limits $ns2 0 1
-		addr_nr_ns2=1 speed=slow \
+		addr_nr_ns2=1 speed=slow cestab_ns2=2 \
 			run_tests $ns1 $ns2 10.0.1.1
 		chk_join_nr 1 1 1
+		chk_cestab_nr $ns2 0
 	fi
 
 	# add signal address
 	if reset "add signal address"; then
 		pm_nl_set_limits $ns1 0 1
 		pm_nl_set_limits $ns2 1 1
-		addr_nr_ns1=1 speed=slow \
+		addr_nr_ns1=1 speed=slow cestab_ns1=2 \
 			run_tests $ns1 $ns2 10.0.1.1
 		chk_join_nr 1 1 1
 		chk_add_nr 1 1
+		chk_cestab_nr $ns1 0
 	fi
 
 	# add multiple subflows
 	if reset "add multiple subflows"; then
 		pm_nl_set_limits $ns1 0 2
 		pm_nl_set_limits $ns2 0 2
-		addr_nr_ns2=2 speed=slow \
+		addr_nr_ns2=2 speed=slow cestab_ns2=3 \
 			run_tests $ns1 $ns2 10.0.1.1
 		chk_join_nr 2 2 2
+		chk_cestab_nr $ns2 0
 	fi
 
 	# add multiple subflows IPv6
 	if reset "add multiple subflows IPv6"; then
 		pm_nl_set_limits $ns1 0 2
 		pm_nl_set_limits $ns2 0 2
-		addr_nr_ns2=2 speed=slow \
+		addr_nr_ns2=2 speed=slow cestab_ns2=3 \
 			run_tests $ns1 $ns2 dead:beef:1::1
 		chk_join_nr 2 2 2
+		chk_cestab_nr $ns2 0
 	fi
 
 	# add multiple addresses IPv6
 	if reset "add multiple addresses IPv6"; then
 		pm_nl_set_limits $ns1 0 2
 		pm_nl_set_limits $ns2 2 2
-		addr_nr_ns1=2 speed=slow \
+		addr_nr_ns1=2 speed=slow cestab_ns1=3 \
 			run_tests $ns1 $ns2 dead:beef:1::1
 		chk_join_nr 2 2 2
 		chk_add_nr 2 2
+		chk_cestab_nr $ns1 0
 	fi
 }
 
-- 
2.35.3

Re: selftests: mptcp: join: check CURRESTAB counters: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/6003812935139328
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6003812935139328/summary/summary.txt

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/6566762888560640
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6566762888560640/summary/summary.txt

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/4737175539941376
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4737175539941376/summary/summary.txt

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/5440862981718016
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5440862981718016/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/9fc088de6572


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: selftests: mptcp: join: check CURRESTAB counters: Build Failure

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/0b59f639265bcd844b1d51597ffd5e172127732a.1701180777.git.geliang.tang@suse.com/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7020263867

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/9fc088de6572

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)