[PATCH mptcp-next v4 0/3] patches for new MPTCP subflow subtest

[PATCH mptcp-next v4 0/3] patches for new MPTCP subflow subtest

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v4:
 - "{*/,}" can't be used in MAINTAINERS, fix it.
 - fix checkpatch too.

v3:
 - update MAINTAINERS.
 - drop '-M' in _ss_search() as Matt suggested.

v2:
 - rename pm_nl_ctl to mptcp_pm_nl_ctl.

Geliang Tang (3):
  selftests/bpf: Add mptcp pm_nl_ctl link
  Squash to "selftests/bpf: Add mptcp subflow subtest"
  checkpatch: skip warnings for symbol links

 MAINTAINERS                                    |  1 +
 scripts/checkpatch.pl                          | 11 +++++++++--
 tools/testing/selftests/bpf/Makefile           |  2 +-
 tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c  |  1 +
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 17 ++++++-----------
 5 files changed, 18 insertions(+), 14 deletions(-)
 create mode 120000 tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c

-- 
2.43.0

[PATCH mptcp-next v4 1/3] selftests/bpf: Add mptcp pm_nl_ctl link

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds a link of mptcp pm_nl_ctl tool into bpf selftests, and
updates Makefile to compile it.

This is useful to run mptcp bpf selftests on a system with an old version
of IPRoite2 is installed. When 'ip mptcp' is not supported, this tool
pm_nl_ctl can be used instead.

MAINTAINERS needs to be updated since a new file is added.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 MAINTAINERS                                   | 1 +
 tools/testing/selftests/bpf/Makefile          | 2 +-
 tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)
 create mode 120000 tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b81b2be60b77..f973f1fb9e18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15510,6 +15510,7 @@ F:	include/trace/events/mptcp.h
 F:	include/uapi/linux/mptcp*.h
 F:	net/mptcp/
 F:	tools/testing/selftests/bpf/*/*mptcp*.c
+F:	tools/testing/selftests/bpf/*mptcp*.c
 F:	tools/testing/selftests/net/mptcp/
 
 NETWORKING [TCP]
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index b90c718218ae..6fe242275e14 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -131,7 +131,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
 	flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
 	test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
 	xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
-	xdp_features bpf_test_no_cfi.ko
+	xdp_features bpf_test_no_cfi.ko mptcp_pm_nl_ctl
 
 TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
 
diff --git a/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c b/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c
new file mode 120000
index 000000000000..5a08c255b278
--- /dev/null
+++ b/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c
@@ -0,0 +1 @@
+../net/mptcp/pm_nl_ctl.c
\ No newline at end of file
-- 
2.43.0

[PATCH mptcp-next v4 2/3] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Use pm_nl_ctl when 'ip mptcp' fails.
Use SYS_NOFAIL in _ss_search() and drop 'ss -M'.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 793b4b9c2bd2..2459c0e2a794 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -362,7 +362,10 @@ static int endpoint_init(char *flags)
 	SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
 	SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
 	SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
-	SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
+	if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags)) {
+		SYS(fail, "ip netns exec %s ./mptcp_pm_nl_ctl add %s flags %s",
+		    NS_TEST, ADDR_2, flags);
+	}
 
 	return 0;
 fail:
@@ -371,16 +374,8 @@ static int endpoint_init(char *flags)
 
 static int _ss_search(char *src, char *dst, char *port, char *keyword)
 {
-	char cmd[128];
-	int n;
-
-	n = snprintf(cmd, sizeof(cmd),
-		     "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
-		     NS_TEST, src, dst, port, PORT_1, keyword);
-	if (n < 0 || n >= sizeof(cmd))
-		return -1;
-
-	return system(cmd);
+	return SYS_NOFAIL("ip netns exec %s ss -enita src %s dst %s %s %d | grep -q '%s'",
+			  NS_TEST, src, dst, port, PORT_1, keyword);
 }
 
 static int ss_search(char *src, char *keyword)
-- 
2.43.0

[PATCH mptcp-next v4 3/3] checkpatch: skip warnings for symbol links

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

If there is a symbol link in the given patch, checkpatch.pl reports two
inaccurate warnings:

$ cat 0001-selftests-bpf-Add-mptcp-pm_nl_ctl-link.patch

 ... ...

 '''
 # diff --git a/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c \
 #            b/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c
 # new file mode 120000
 # index 000000000000..5a08c255b278
 # --- /dev/null
 # +++ b/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c
 # @@ -0,0 +1 @@
 # +../net/mptcp/pm_nl_ctl.c
 # \ No newline at end of file
 '''

$ ./scripts/checkpatch.pl 0001-selftests-bpf-Add-mptcp-pm_nl_ctl-link.patch

 '''
 WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
 #57: FILE: tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c:1:
 +../net/mptcp/pm_nl_ctl.c

 WARNING: adding a line without newline at end of file
 #57: FILE: tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c:1:
 +../net/mptcp/pm_nl_ctl.c

 total: 0 errors, 2 warnings, 16 lines checked
 '''

This patch fixes this by adding a new variable $symbol_link in checkpatch
script, set it if the new file mode is 120000. Skip these two checks
"missing SPDX-License-Identifier" and "adding a line without newline at
end of file" if this variable is set.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 scripts/checkpatch.pl | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9c4c4a61bc83..b31f13d643a7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -436,6 +436,7 @@ for my $key (keys %debug) {
 }
 
 my $rpt_cleaners = 0;
+my $symbol_link = 0;
 
 if ($terse) {
 	$emacs = 1;
@@ -3595,6 +3596,10 @@ sub process {
 				      "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet)
 		}
 
+		if ($line =~ /^new file mode 120000$/) {
+			$symbol_link = 1;
+		}
+
 # check for Kconfig help text having a real description
 # Only applies when adding the entry originally, after that we do not have
 # sufficient context to determine whether it is indeed long enough.
@@ -3756,7 +3761,8 @@ sub process {
 				}
 
 				if ($comment !~ /^$/ &&
-				    $rawline !~ m@^\+\Q$comment\E SPDX-License-Identifier: @) {
+				    $rawline !~ m@^\+\Q$comment\E SPDX-License-Identifier: @ &&
+				    $symbol_link =~ 1) {
 					WARN("SPDX_LICENSE_TAG",
 					     "Missing or malformed SPDX-License-Identifier tag in line $checklicenseline\n" . $herecurr);
 				} elsif ($rawline =~ /(SPDX-License-Identifier: .*)/) {
@@ -3867,7 +3873,8 @@ sub process {
 		}
 
 # check for adding lines without a newline.
-		if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) {
+		if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/ &&
+		    $symbol_link =~ 1) {
 			if (WARN("MISSING_EOF_NEWLINE",
 			         "adding a line without newline at end of file\n" . $herecurr) &&
 			    $fix) {
-- 
2.43.0

Re: [PATCH mptcp-next v4 0/3] patches for new MPTCP subflow subtest

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/9050206459

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/f24a9a4e12f1
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=852623


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-normal

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: [PATCH mptcp-next v4 2/3] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Matthieu Baerts]

Hi Geliang,

On 12/05/2024 10:41, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Use pm_nl_ctl when 'ip mptcp' fails.
> Use SYS_NOFAIL in _ss_search() and drop 'ss -M'.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/bpf/prog_tests/mptcp.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index 793b4b9c2bd2..2459c0e2a794 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -362,7 +362,10 @@ static int endpoint_init(char *flags)
>  	SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
>  	SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
>  	SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
> -	SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
> +	if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags)) {
> +		SYS(fail, "ip netns exec %s ./mptcp_pm_nl_ctl add %s flags %s",
> +		    NS_TEST, ADDR_2, flags);

As I mentioned in the v1, I would prefer only using 'ip mptcp', and I
find it strange to have a CI validating networking feature using an old
version of IPRoute2. But if we need an alternative method for the BPF
CI, I would prefer also having our CI using this method, to prevent any
issue with this method, and only realising that when upstreaming
patches, delaying them and causing confusions.

But feel free to add a command:

  /* It would be better to use  "ip -net %s mptcp endpoint add %s %s",
   * but the BPF CI is using an old version of IPRoute (5.5.0).
   */


> +	}
>  
>  	return 0;
>  fail:
> @@ -371,16 +374,8 @@ static int endpoint_init(char *flags)
>  
>  static int _ss_search(char *src, char *dst, char *port, char *keyword)
>  {
> -	char cmd[128];
> -	int n;
> -
> -	n = snprintf(cmd, sizeof(cmd),
> -		     "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
> -		     NS_TEST, src, dst, port, PORT_1, keyword);
> -	if (n < 0 || n >= sizeof(cmd))
> -		return -1;
> -
> -	return system(cmd);
> +	return SYS_NOFAIL("ip netns exec %s ss -enita src %s dst %s %s %d | grep -q '%s'",
> +			  NS_TEST, src, dst, port, PORT_1, keyword);
>  }
>  
>  static int ss_search(char *src, char *keyword)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next v4 3/3] checkpatch: skip warnings for symbol links

[Matthieu Baerts]

Hi Geliang,

On 12/05/2024 10:41, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> If there is a symbol link in the given patch, checkpatch.pl reports two
> inaccurate warnings:

Good idea to fix that!

Do you mind sending it as a separated patch to CheckPatch maintainers
directly?

It might be good to split it in 2, with 2 different Fixes tag, to help
with the backports.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next v4 3/3] checkpatch: skip warnings for symbol links

[Matthieu Baerts]

Hi Geliang,

On 12/05/2024 10:41, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> If there is a symbol link in the given patch, checkpatch.pl reports two
> inaccurate warnings:
> 
> $ cat 0001-selftests-bpf-Add-mptcp-pm_nl_ctl-link.patch
> 
>  ... ...
> 
>  '''
>  # diff --git a/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c \
>  #            b/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c
>  # new file mode 120000
>  # index 000000000000..5a08c255b278
>  # --- /dev/null
>  # +++ b/tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c
>  # @@ -0,0 +1 @@
>  # +../net/mptcp/pm_nl_ctl.c
>  # \ No newline at end of file
>  '''
> 
> $ ./scripts/checkpatch.pl 0001-selftests-bpf-Add-mptcp-pm_nl_ctl-link.patch
> 
>  '''
>  WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
>  #57: FILE: tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c:1:
>  +../net/mptcp/pm_nl_ctl.c
> 
>  WARNING: adding a line without newline at end of file
>  #57: FILE: tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c:1:
>  +../net/mptcp/pm_nl_ctl.c
> 
>  total: 0 errors, 2 warnings, 16 lines checked
>  '''

With 'checkpatch.pl --strict', I also had these warnings:

> CHECK: spaces preferred around that '/' (ctx:VxV)
> #63: FILE: tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c:1:
> +../net/mptcp/pm_nl_ctl.c
>    ^
> 
> CHECK: spaces preferred around that '/' (ctx:VxV)
> #63: FILE: tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c:1:
> +../net/mptcp/pm_nl_ctl.c
>        ^
> 
> CHECK: spaces preferred around that '/' (ctx:VxV)
> #63: FILE: tools/testing/selftests/bpf/mptcp_pm_nl_ctl.c:1:
> +../net/mptcp/pm_nl_ctl.c
>              ^

I guess a similar fix could be done for this check as well :)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.