[PATCH mptcp-next] Squash to "bpf: add bpf_skc_to_mptcp_sock_proto"

[PATCH mptcp-next] Squash to "bpf: add bpf_skc_to_mptcp_sock_proto"

[Geliang Tang]

Update as Daniel suggested.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 include/linux/bpf.h      | 1 +
 include/net/mptcp.h      | 2 +-
 kernel/bpf/verifier.c    | 1 +
 kernel/trace/bpf_trace.c | 2 ++
 net/core/filter.c        | 3 ++-
 net/mptcp/Makefile       | 2 --
 6 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index bdb5298735ce..2493f9601842 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2249,6 +2249,7 @@ extern const struct bpf_func_proto bpf_skc_to_tcp_timewait_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_tcp_request_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_udp6_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_unix_sock_proto;
+extern const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto;
 extern const struct bpf_func_proto bpf_copy_from_user_proto;
 extern const struct bpf_func_proto bpf_snprintf_btf_proto;
 extern const struct bpf_func_proto bpf_snprintf_proto;
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 6b07011c060d..4d761ad530c9 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -284,7 +284,7 @@ static inline int mptcpv6_init(void) { return 0; }
 static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
 #endif
 
-#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
+#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL)
 struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
 #else
 static inline struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk) { return NULL; }
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9c1a02b82ecd..40602ec20c6a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -517,6 +517,7 @@ static bool is_ptr_cast_function(enum bpf_func_id func_id)
 		func_id == BPF_FUNC_skc_to_tcp_sock ||
 		func_id == BPF_FUNC_skc_to_tcp6_sock ||
 		func_id == BPF_FUNC_skc_to_udp6_sock ||
+		func_id == BPF_FUNC_skc_to_mptcp_sock ||
 		func_id == BPF_FUNC_skc_to_tcp_timewait_sock ||
 		func_id == BPF_FUNC_skc_to_tcp_request_sock;
 }
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index b26f3da943de..c7bf10cf2fa5 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1685,6 +1685,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_skc_to_udp6_sock_proto;
 	case BPF_FUNC_skc_to_unix_sock:
 		return &bpf_skc_to_unix_sock_proto;
+	case BPF_FUNC_skc_to_mptcp_sock:
+		return &bpf_skc_to_mptcp_sock_proto;
 	case BPF_FUNC_sk_storage_get:
 		return &bpf_sk_storage_get_tracing_proto;
 	case BPF_FUNC_sk_storage_delete:
diff --git a/net/core/filter.c b/net/core/filter.c
index 4bcf13b1d0e2..a0dd6f6b17f8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11282,10 +11282,11 @@ const struct bpf_func_proto bpf_skc_to_unix_sock_proto = {
 
 BPF_CALL_1(bpf_skc_to_mptcp_sock, struct sock *, sk)
 {
+	BTF_TYPE_EMIT(struct mptcp_sock);
 	return (unsigned long)bpf_mptcp_sock_from_subflow(sk);
 }
 
-static const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto = {
+const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto = {
 	.func		= bpf_skc_to_mptcp_sock,
 	.gpl_only	= false,
 	.ret_type	= RET_PTR_TO_BTF_ID_OR_NULL,
diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
index 4004347db47e..6e7df47c9584 100644
--- a/net/mptcp/Makefile
+++ b/net/mptcp/Makefile
@@ -11,6 +11,4 @@ mptcp_crypto_test-objs := crypto_test.o
 mptcp_token_test-objs := token_test.o
 obj-$(CONFIG_MPTCP_KUNIT_TEST) += mptcp_crypto_test.o mptcp_token_test.o
 
-ifeq ($(CONFIG_BPF_JIT),y)
 obj-$(CONFIG_BPF_SYSCALL) += bpf.o
-endif
-- 
2.34.1

[PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Geliang Tang]

Add CONFIG_MPTCP check.

When CONFIG_MPTCP is not enabled, we'll get a clearer error message:

libbpf: extern CONFIG_MPTCP (strong) not resolved
libbpf: failed to load object './mptcp_sock.o'

The message before is like this:

libbpf: prog '_sockops': BPF program load failed: Invalid argument
libbpf: prog '_sockops': -- BEGIN PROG LOAD LOG --

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/progs/mptcp_sock.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
index 5cfaec4e7245..7b6a25e37de8 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -7,6 +7,7 @@
 
 char _license[] SEC("license") = "GPL";
 __u32 _version SEC("version") = 1;
+extern bool CONFIG_MPTCP __kconfig;
 
 struct mptcp_storage {
 	__u32 invoked;
@@ -46,6 +47,9 @@ int _sockops(struct bpf_sock_ops *ctx)
 		if (!storage)
 			return 1;
 	} else {
+		if (!CONFIG_MPTCP)
+			return 1;
+
 		msk = bpf_skc_to_mptcp_sock(sk);
 		if (!msk)
 			return 1;
-- 
2.34.1

Re: Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock": Tests Results

[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! ✅:
  - Task: https://cirrus-ci.com/task/6067994064322560
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6067994064322560/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 1 failed test(s): selftest_diag 🔴:
  - Task: https://cirrus-ci.com/task/5505044110901248
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5505044110901248/summary/summary.txt

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


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 (Tessares)

Re: [PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Mat Martineau]

On Tue, 26 Apr 2022, Geliang Tang wrote:

> Add CONFIG_MPTCP check.
>
> When CONFIG_MPTCP is not enabled, we'll get a clearer error message:
>
> libbpf: extern CONFIG_MPTCP (strong) not resolved
> libbpf: failed to load object './mptcp_sock.o'
>
> The message before is like this:
>
> libbpf: prog '_sockops': BPF program load failed: Invalid argument
> libbpf: prog '_sockops': -- BEGIN PROG LOAD LOG --
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> tools/testing/selftests/bpf/progs/mptcp_sock.c | 4 ++++
> 1 file changed, 4 insertions(+)
>

Looks like this also requires CONFIG_IKCONFIG and CONFIG_IKCONFIG_PROC, 
can you also add those to tools/testing/selftests/bpf/config? Maybe that 
should be a separate commit, since other bpf test progs appear to rely on 
this undocumented config requirement.

- Mat

> diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> index 5cfaec4e7245..7b6a25e37de8 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
> +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> @@ -7,6 +7,7 @@
>
> char _license[] SEC("license") = "GPL";
> __u32 _version SEC("version") = 1;
> +extern bool CONFIG_MPTCP __kconfig;
>
> struct mptcp_storage {
> 	__u32 invoked;
> @@ -46,6 +47,9 @@ int _sockops(struct bpf_sock_ops *ctx)
> 		if (!storage)
> 			return 1;
> 	} else {
> +		if (!CONFIG_MPTCP)
> +			return 1;
> +
> 		msk = bpf_skc_to_mptcp_sock(sk);
> 		if (!msk)
> 			return 1;
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Mat Martineau]

On Tue, 26 Apr 2022, Mat Martineau wrote:

> On Tue, 26 Apr 2022, Geliang Tang wrote:
>
>> Add CONFIG_MPTCP check.
>> 
>> When CONFIG_MPTCP is not enabled, we'll get a clearer error message:
>> 
>> libbpf: extern CONFIG_MPTCP (strong) not resolved
>> libbpf: failed to load object './mptcp_sock.o'
>> 
>> The message before is like this:
>> 
>> libbpf: prog '_sockops': BPF program load failed: Invalid argument
>> libbpf: prog '_sockops': -- BEGIN PROG LOAD LOG --
>> 
>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
>> ---
>> tools/testing/selftests/bpf/progs/mptcp_sock.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>> 
>
> Looks like this also requires CONFIG_IKCONFIG and CONFIG_IKCONFIG_PROC, can 
> you also add those to tools/testing/selftests/bpf/config? Maybe that should 
> be a separate commit, since other bpf test progs appear to rely on this 
> undocumented config requirement.
>

I hit 'send' too soon... We should still squash this, and add that second 
commit for the config information.

- Mat

>
>> diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c 
>> b/tools/testing/selftests/bpf/progs/mptcp_sock.c
>> index 5cfaec4e7245..7b6a25e37de8 100644
>> --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
>> +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
>> @@ -7,6 +7,7 @@
>> 
>> char _license[] SEC("license") = "GPL";
>> __u32 _version SEC("version") = 1;
>> +extern bool CONFIG_MPTCP __kconfig;
>> 
>> struct mptcp_storage {
>> 	__u32 invoked;
>> @@ -46,6 +47,9 @@ int _sockops(struct bpf_sock_ops *ctx)
>> 		if (!storage)
>> 			return 1;
>> 	} else {
>> +		if (!CONFIG_MPTCP)
>> +			return 1;
>> +
>> 		msk = bpf_skc_to_mptcp_sock(sk);
>> 		if (!msk)
>> 			return 1;
>> -- 
>> 2.34.1
>> 
>> 
>> 
>
> --
> Mat Martineau
> Intel
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Geliang Tang]

Hi Mat,

Mat Martineau <mathew.j.martineau@linux.intel.com> 于2022年4月27日周三 08:20写道：
>
> On Tue, 26 Apr 2022, Geliang Tang wrote:
>
> > Add CONFIG_MPTCP check.
> >
> > When CONFIG_MPTCP is not enabled, we'll get a clearer error message:
> >
> > libbpf: extern CONFIG_MPTCP (strong) not resolved
> > libbpf: failed to load object './mptcp_sock.o'
> >
> > The message before is like this:
> >
> > libbpf: prog '_sockops': BPF program load failed: Invalid argument
> > libbpf: prog '_sockops': -- BEGIN PROG LOAD LOG --
> >
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > ---
> > tools/testing/selftests/bpf/progs/mptcp_sock.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
>
> Looks like this also requires CONFIG_IKCONFIG and CONFIG_IKCONFIG_PROC,
> can you also add those to tools/testing/selftests/bpf/config? Maybe that
> should be a separate commit, since other bpf test progs appear to rely on
> this undocumented config requirement.

In my test, this bpf base test still works without CONFIG_IKCONFIG and
CONFIG_IKCONFIG_PROC:

> ls /proc/config.gz
ls: cannot access '/proc/config.gz': No such file or directory
> sudo ./test_progs -n 103
#103 mptcp:OK
Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED

Thanks,
-Geliang




>
> - Mat
>
> > diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> > index 5cfaec4e7245..7b6a25e37de8 100644
> > --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
> > +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> > @@ -7,6 +7,7 @@
> >
> > char _license[] SEC("license") = "GPL";
> > __u32 _version SEC("version") = 1;
> > +extern bool CONFIG_MPTCP __kconfig;
> >
> > struct mptcp_storage {
> >       __u32 invoked;
> > @@ -46,6 +47,9 @@ int _sockops(struct bpf_sock_ops *ctx)
> >               if (!storage)
> >                       return 1;
> >       } else {
> > +             if (!CONFIG_MPTCP)
> > +                     return 1;
> > +
> >               msk = bpf_skc_to_mptcp_sock(sk);
> >               if (!msk)
> >                       return 1;
> > --
> > 2.34.1
> >
> >
> >
>
> --
> Mat Martineau
> Intel
>

Re: [PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Mat Martineau]

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

On Wed, 27 Apr 2022, Geliang Tang wrote:

> Hi Mat,
>
> Mat Martineau <mathew.j.martineau@linux.intel.com> 于2022年4月27日周三 08:20写道：
>>
>> On Tue, 26 Apr 2022, Geliang Tang wrote:
>>
>>> Add CONFIG_MPTCP check.
>>>
>>> When CONFIG_MPTCP is not enabled, we'll get a clearer error message:
>>>
>>> libbpf: extern CONFIG_MPTCP (strong) not resolved
>>> libbpf: failed to load object './mptcp_sock.o'
>>>
>>> The message before is like this:
>>>
>>> libbpf: prog '_sockops': BPF program load failed: Invalid argument
>>> libbpf: prog '_sockops': -- BEGIN PROG LOAD LOG --
>>>
>>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
>>> ---
>>> tools/testing/selftests/bpf/progs/mptcp_sock.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>
>> Looks like this also requires CONFIG_IKCONFIG and CONFIG_IKCONFIG_PROC,
>> can you also add those to tools/testing/selftests/bpf/config? Maybe that
>> should be a separate commit, since other bpf test progs appear to rely on
>> this undocumented config requirement.
>
> In my test, this bpf base test still works without CONFIG_IKCONFIG and
> CONFIG_IKCONFIG_PROC:
>
>> ls /proc/config.gz
> ls: cannot access '/proc/config.gz': No such file or directory
>> sudo ./test_progs -n 103
> #103 mptcp:OK
> Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED
>

Huh, I wonder what's going on? It fails for me:

$ sudo ./test_progs -v -a mptcp
bpf_testmod.ko is already unloaded.
Loading bpf_testmod.ko...
Successfully loaded bpf_testmod.ko.
libbpf: failed to open system Kconfig
libbpf: failed to load object './mptcp_sock.o'
run_test:FAIL:165
test_base:FAIL:227
libbpf: failed to open system Kconfig
libbpf: failed to load object './mptcp_sock.o'
run_test:FAIL:165
test_base:FAIL:244
#103/1 mptcp/base:FAIL
(cgroup_helpers.c:146: errno: Device or resource busy) Removing cgroup: /mnt/cgroup-test-work-dir831/mptcp
(cgroup_helpers.c:146: errno: Device or resource busy) Removing cgroup: /mnt/cgroup-test-work-dir831
#103 mptcp:FAIL

All error logs:
#103 mptcp:FAIL

Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED

This is with Fedora 35. I'll check the BPF CI to see if there's any 
information about their configuration that might explain the difference.


- Mat

>
>
>>
>> - Mat
>>
>>> diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
>>> index 5cfaec4e7245..7b6a25e37de8 100644
>>> --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
>>> +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
>>> @@ -7,6 +7,7 @@
>>>
>>> char _license[] SEC("license") = "GPL";
>>> __u32 _version SEC("version") = 1;
>>> +extern bool CONFIG_MPTCP __kconfig;
>>>
>>> struct mptcp_storage {
>>>       __u32 invoked;
>>> @@ -46,6 +47,9 @@ int _sockops(struct bpf_sock_ops *ctx)
>>>               if (!storage)
>>>                       return 1;
>>>       } else {
>>> +             if (!CONFIG_MPTCP)
>>> +                     return 1;
>>> +
>>>               msk = bpf_skc_to_mptcp_sock(sk);
>>>               if (!msk)
>>>                       return 1;
>>> --
>>> 2.34.1
>>>
>>>
>>>
>>
>> --
>> Mat Martineau
>> Intel
>>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Geliang Tang]

Hi Mat,


Mat Martineau <mathew.j.martineau@linux.intel.com> 于2022年4月28日周四 05:47写道：
>
> On Wed, 27 Apr 2022, Geliang Tang wrote:
>
> > Hi Mat,
> >
> > Mat Martineau <mathew.j.martineau@linux.intel.com> 于2022年4月27日周三 08:20写道：
> >>
> >> On Tue, 26 Apr 2022, Geliang Tang wrote:
> >>
> >>> Add CONFIG_MPTCP check.
> >>>
> >>> When CONFIG_MPTCP is not enabled, we'll get a clearer error message:
> >>>
> >>> libbpf: extern CONFIG_MPTCP (strong) not resolved
> >>> libbpf: failed to load object './mptcp_sock.o'
> >>>
> >>> The message before is like this:
> >>>
> >>> libbpf: prog '_sockops': BPF program load failed: Invalid argument
> >>> libbpf: prog '_sockops': -- BEGIN PROG LOAD LOG --
> >>>
> >>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> >>> ---
> >>> tools/testing/selftests/bpf/progs/mptcp_sock.c | 4 ++++
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>
> >> Looks like this also requires CONFIG_IKCONFIG and CONFIG_IKCONFIG_PROC,
> >> can you also add those to tools/testing/selftests/bpf/config? Maybe that
> >> should be a separate commit, since other bpf test progs appear to rely on
> >> this undocumented config requirement.
> >
> > In my test, this bpf base test still works without CONFIG_IKCONFIG and
> > CONFIG_IKCONFIG_PROC:
> >
> >> ls /proc/config.gz
> > ls: cannot access '/proc/config.gz': No such file or directory
> >> sudo ./test_progs -n 103
> > #103 mptcp:OK
> > Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED
> >
>
> Huh, I wonder what's going on? It fails for me:
>
> $ sudo ./test_progs -v -a mptcp
> bpf_testmod.ko is already unloaded.
> Loading bpf_testmod.ko...
> Successfully loaded bpf_testmod.ko.
> libbpf: failed to open system Kconfig
> libbpf: failed to load object './mptcp_sock.o'
> run_test:FAIL:165
> test_base:FAIL:227
> libbpf: failed to open system Kconfig
> libbpf: failed to load object './mptcp_sock.o'
> run_test:FAIL:165
> test_base:FAIL:244
> #103/1 mptcp/base:FAIL
> (cgroup_helpers.c:146: errno: Device or resource busy) Removing cgroup: /mnt/cgroup-test-work-dir831/mptcp
> (cgroup_helpers.c:146: errno: Device or resource busy) Removing cgroup: /mnt/cgroup-test-work-dir831
> #103 mptcp:FAIL
>
> All error logs:
> #103 mptcp:FAIL
>
> Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
>
> This is with Fedora 35. I'll check the BPF CI to see if there's any
> information about their configuration that might explain the difference.

I figure it out. libbpf reads /boot/config-* first, then /proc/config.gz:

 1840 static int bpf_object__read_kconfig_file(struct bpf_object *obj,
void *data)
 1841 {
 1842         char buf[PATH_MAX];
 1843         struct utsname uts;
 1844         int len, err = 0;
 1845         gzFile file;
 1846
 1847         uname(&uts);
 1848         len = snprintf(buf, PATH_MAX, "/boot/config-%s", uts.release);
 1849         if (len < 0)
 1850                 return -EINVAL;
 1851         else if (len >= PATH_MAX)
 1852                 return -ENAMETOOLONG;
 1853
 1854         /* gzopen also accepts uncompressed files. */
 1855         file = gzopen(buf, "r");
 1856         if (!file)
 1857                 file = gzopen("/proc/config.gz", "r");

So we do need to add CONFIG_IKCONFIG and CONFIG_IKCONFIG_PROC in
config. I'll send a v2 as a dedicated patch.

Thanks,
-Geliang


>
>
> - Mat
>
> >
> >
> >>
> >> - Mat
> >>
> >>> diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> >>> index 5cfaec4e7245..7b6a25e37de8 100644
> >>> --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
> >>> +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> >>> @@ -7,6 +7,7 @@
> >>>
> >>> char _license[] SEC("license") = "GPL";
> >>> __u32 _version SEC("version") = 1;
> >>> +extern bool CONFIG_MPTCP __kconfig;
> >>>
> >>> struct mptcp_storage {
> >>>       __u32 invoked;
> >>> @@ -46,6 +47,9 @@ int _sockops(struct bpf_sock_ops *ctx)
> >>>               if (!storage)
> >>>                       return 1;
> >>>       } else {
> >>> +             if (!CONFIG_MPTCP)
> >>> +                     return 1;
> >>> +
> >>>               msk = bpf_skc_to_mptcp_sock(sk);
> >>>               if (!msk)
> >>>                       return 1;
> >>> --
> >>> 2.34.1
> >>>
> >>>
> >>>
> >>
> >> --
> >> Mat Martineau
> >> Intel
> >>
> >
>
> --
> Mat Martineau
> Intel

Re: [PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Matthieu Baerts]

Hi Geliang, Mat,

On 26/04/2022 06:26, Geliang Tang wrote:
> Add CONFIG_MPTCP check.
> 
> When CONFIG_MPTCP is not enabled, we'll get a clearer error message:
> 
> libbpf: extern CONFIG_MPTCP (strong) not resolved
> libbpf: failed to load object './mptcp_sock.o'
> 
> The message before is like this:
> 
> libbpf: prog '_sockops': BPF program load failed: Invalid argument
> libbpf: prog '_sockops': -- BEGIN PROG LOAD LOG --

Thank you for the two patches and reviews!

Now in our tree:

- f456d87ca214: "squashed" in "bpf: add bpf_skc_to_mptcp_sock_proto"
- 8aa57e042f44: "squashed" in "selftests: bpf: test bpf_skc_to_mptcp_sock"
- Results: c2a1191f33d3..440ea2eff5af (export)

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220427T162930
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

Re: Squash to "bpf: add bpf_skc_to_mptcp_sock_proto": Tests Results

[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:
  - Unstable: 1 failed test(s): selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/5696628827881472
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5696628827881472/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 2 failed test(s): selftest_diag selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/5133678874460160
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5133678874460160/summary/summary.txt

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


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 (Tessares)

Re: [PATCH mptcp-next] Squash to "bpf: add bpf_skc_to_mptcp_sock_proto"

[Mat Martineau]

On Tue, 26 Apr 2022, Geliang Tang wrote:

> Update as Daniel suggested.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>

Looks good to squash, thanks. The 'base' bpf tests pass, which is what 
Daniel noted as failing.

- Mat


> ---
> include/linux/bpf.h      | 1 +
> include/net/mptcp.h      | 2 +-
> kernel/bpf/verifier.c    | 1 +
> kernel/trace/bpf_trace.c | 2 ++
> net/core/filter.c        | 3 ++-
> net/mptcp/Makefile       | 2 --
> 6 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index bdb5298735ce..2493f9601842 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2249,6 +2249,7 @@ extern const struct bpf_func_proto bpf_skc_to_tcp_timewait_sock_proto;
> extern const struct bpf_func_proto bpf_skc_to_tcp_request_sock_proto;
> extern const struct bpf_func_proto bpf_skc_to_udp6_sock_proto;
> extern const struct bpf_func_proto bpf_skc_to_unix_sock_proto;
> +extern const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto;
> extern const struct bpf_func_proto bpf_copy_from_user_proto;
> extern const struct bpf_func_proto bpf_snprintf_btf_proto;
> extern const struct bpf_func_proto bpf_snprintf_proto;
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 6b07011c060d..4d761ad530c9 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -284,7 +284,7 @@ static inline int mptcpv6_init(void) { return 0; }
> static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
> #endif
>
> -#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
> +#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL)
> struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
> #else
> static inline struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk) { return NULL; }
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9c1a02b82ecd..40602ec20c6a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -517,6 +517,7 @@ static bool is_ptr_cast_function(enum bpf_func_id func_id)
> 		func_id == BPF_FUNC_skc_to_tcp_sock ||
> 		func_id == BPF_FUNC_skc_to_tcp6_sock ||
> 		func_id == BPF_FUNC_skc_to_udp6_sock ||
> +		func_id == BPF_FUNC_skc_to_mptcp_sock ||
> 		func_id == BPF_FUNC_skc_to_tcp_timewait_sock ||
> 		func_id == BPF_FUNC_skc_to_tcp_request_sock;
> }
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index b26f3da943de..c7bf10cf2fa5 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1685,6 +1685,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> 		return &bpf_skc_to_udp6_sock_proto;
> 	case BPF_FUNC_skc_to_unix_sock:
> 		return &bpf_skc_to_unix_sock_proto;
> +	case BPF_FUNC_skc_to_mptcp_sock:
> +		return &bpf_skc_to_mptcp_sock_proto;
> 	case BPF_FUNC_sk_storage_get:
> 		return &bpf_sk_storage_get_tracing_proto;
> 	case BPF_FUNC_sk_storage_delete:
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 4bcf13b1d0e2..a0dd6f6b17f8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -11282,10 +11282,11 @@ const struct bpf_func_proto bpf_skc_to_unix_sock_proto = {
>
> BPF_CALL_1(bpf_skc_to_mptcp_sock, struct sock *, sk)
> {
> +	BTF_TYPE_EMIT(struct mptcp_sock);
> 	return (unsigned long)bpf_mptcp_sock_from_subflow(sk);
> }
>
> -static const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto = {
> +const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto = {
> 	.func		= bpf_skc_to_mptcp_sock,
> 	.gpl_only	= false,
> 	.ret_type	= RET_PTR_TO_BTF_ID_OR_NULL,
> diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
> index 4004347db47e..6e7df47c9584 100644
> --- a/net/mptcp/Makefile
> +++ b/net/mptcp/Makefile
> @@ -11,6 +11,4 @@ mptcp_crypto_test-objs := crypto_test.o
> mptcp_token_test-objs := token_test.o
> obj-$(CONFIG_MPTCP_KUNIT_TEST) += mptcp_crypto_test.o mptcp_token_test.o
>
> -ifeq ($(CONFIG_BPF_JIT),y)
> obj-$(CONFIG_BPF_SYSCALL) += bpf.o
> -endif
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel

[PATCH mptcp-next] Squash to "selftests: bpf: add MPTCP test base"

[Geliang Tang]

Address to Andrii's comments:
 - add copyright 2022
 - use ASSERT_* instead of CHECK_FAIL
 - drop SEC("version")

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 24 +++++++++----------
 .../testing/selftests/bpf/progs/mptcp_sock.c  |  2 +-
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index cd548bb2828f..760374f1fce4 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2020, Tessares SA. */
+/* Copyright (c) 2022, SUSE. */
 
 #include <test_progs.h>
 #include "cgroup_helpers.h"
@@ -12,16 +13,15 @@ struct mptcp_storage {
 
 static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp)
 {
-	int err = 0, cfd = client_fd;
+	int err, cfd = client_fd;
 	struct mptcp_storage val;
 
 	if (is_mptcp == 1)
 		return 0;
 
-	if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
-		perror("Failed to read socket storage");
-		return -1;
-	}
+	err = bpf_map_lookup_elem(map_fd, &cfd, &val);
+	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
+		return err;
 
 	if (val.invoked != 1) {
 		log_err("%s: unexpected invoked count %d != 1",
@@ -50,40 +50,40 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
 		return -EIO;
 
 	err = bpf_object__load(obj);
-	if (CHECK_FAIL(err))
+	if (!ASSERT_OK(err, "bpf_object__load"))
 		goto out;
 
 	prog = bpf_object__find_program_by_name(obj, "_sockops");
-	if (CHECK_FAIL(!prog)) {
+	if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name")) {
 		err = -EIO;
 		goto out;
 	}
 
 	prog_fd = bpf_program__fd(prog);
-	if (CHECK_FAIL(prog_fd < 0)) {
+	if (!ASSERT_GT(prog_fd, 0, "bpf_program__fd")) {
 		err = -EIO;
 		goto out;
 	}
 
 	map = bpf_object__find_map_by_name(obj, "socket_storage_map");
-	if (CHECK_FAIL(!map)) {
+	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name")) {
 		err = -EIO;
 		goto out;
 	}
 
 	map_fd = bpf_map__fd(map);
-	if (CHECK_FAIL(map_fd < 0)) {
+	if (!ASSERT_GT(map_fd, 0, "bpf_map__fd")) {
 		err = -EIO;
 		goto out;
 	}
 
 	err = bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_SOCK_OPS, 0);
-	if (CHECK_FAIL(err))
+	if (!ASSERT_OK(err, "bpf_prog_attach"))
 		goto out;
 
 	client_fd = is_mptcp ? connect_to_mptcp_fd(server_fd, 0) :
 			       connect_to_fd(server_fd, 0);
-	if (client_fd < 0) {
+	if (!ASSERT_GT(client_fd, 0, "connect to fd")) {
 		err = -EIO;
 		goto out;
 	}
diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
index 0d65fb889d03..19f5f8a183f5 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -1,11 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2020, Tessares SA. */
+/* Copyright (c) 2022, SUSE. */
 
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 
 char _license[] SEC("license") = "GPL";
-__u32 _version SEC("version") = 1;
 
 struct mptcp_storage {
 	__u32 invoked;
-- 
2.34.1

[PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Geliang Tang]

Address to Andrii's comments:
- use ASSERT_* instead of CHECK_FAIL

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index b74df17b47ba..2f2b110d7f56 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -39,13 +39,12 @@ static int verify_tsk(int map_fd, int client_fd)
 static int verify_msk(int map_fd, int client_fd)
 {
 	char *msg = "MPTCP subflow socket";
-	int err = 0, cfd = client_fd;
+	int err, cfd = client_fd;
 	struct mptcp_storage val;
 
-	if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
-		perror("Failed to read socket storage");
-		return -1;
-	}
+	err = bpf_map_lookup_elem(map_fd, &cfd, &val);
+	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
+		return err;
 
 	if (val.invoked != 1) {
 		log_err("%s: unexpected invoked count %d != 1",
@@ -127,25 +126,25 @@ void test_base(void)
 	int server_fd, cgroup_fd;
 
 	cgroup_fd = test__join_cgroup("/mptcp");
-	if (CHECK_FAIL(cgroup_fd < 0))
+	if (!ASSERT_GT(cgroup_fd, 0, "test__join_cgroup"))
 		return;
 
 	/* without MPTCP */
 	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
-	if (CHECK_FAIL(server_fd < 0))
+	if (!ASSERT_GT(server_fd, 0, "start_server"))
 		goto with_mptcp;
 
-	CHECK_FAIL(run_test(cgroup_fd, server_fd, false));
+	ASSERT_OK(run_test(cgroup_fd, server_fd, false), "run_test tcp");
 
 	close(server_fd);
 
 with_mptcp:
 	/* with MPTCP */
 	server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
-	if (CHECK_FAIL(server_fd < 0))
+	if (!ASSERT_GT(server_fd, 0, "start_mptcp_server"))
 		goto close_cgroup_fd;
 
-	CHECK_FAIL(run_test(cgroup_fd, server_fd, true));
+	ASSERT_OK(run_test(cgroup_fd, server_fd, true), "run_test mptcp");
 
 	close(server_fd);
 
-- 
2.34.1

[PATCH mptcp-next] Squash to "selftests: bpf: test bpf_skc_to_mptcp_sock"

[Geliang Tang]

Use dedicated bpf_mptcp_helpers.h for MPTCP tests, instead of sharing
with bpf_tcp_helpers.h.

This patch will conflict with the next three patches.

1. selftests: bpf: verify token of struct mptcp_sock

CONFLICT (content): Merge conflict in tools/testing/selftests/bpf/bpf_tcp_helpers.h

Drop these lines in bpf_tcp_helpers.h:

'''
<<<<<<< HEAD
=======
struct mptcp_sock {
	struct inet_connection_sock	sk;

	__u32		token;
} __attribute__((preserve_access_index));

>>>>>>> 81f7e2dd50fa (selftests: bpf: verify token of struct mptcp_sock)
'''

Merge it into bpf_mptcp_helpers.h:

'''
struct mptcp_sock {
	struct inet_connection_sock	sk;

	__u32		token;
} __attribute__((preserve_access_index));
'''

2. selftests: bpf: verify ca_name of struct mptcp_sock

CONFLICT (content): Merge conflict in tools/testing/selftests/bpf/bpf_tcp_helpers.h

Drop these lines in bpf_tcp_helpers.h:

'''
<<<<<<< HEAD
=======
struct mptcp_sock {
	struct inet_connection_sock	sk;

	__u32		token;
	char		ca_name[TCP_CA_NAME_MAX];
} __attribute__((preserve_access_index));

>>>>>>> dc3031fdfe35 (selftests: bpf: verify ca_name of struct mptcp_sock)
'''

Merge it into bpf_mptcp_helpers.h:

'''
struct mptcp_sock {
	struct inet_connection_sock	sk;

	__u32		token;
	char		ca_name[TCP_CA_NAME_MAX];
} __attribute__((preserve_access_index));
'''

3. selftests: bpf: verify first of struct mptcp_sock

CONFLICT (content): Merge conflict in tools/testing/selftests/bpf/bpf_tcp_helpers.h

Drop these lines in bpf_tcp_helpers.h:

'''
<<<<<<< HEAD
=======
struct mptcp_sock {
	struct inet_connection_sock	sk;

	__u32		token;
	struct sock	*first;
	char		ca_name[TCP_CA_NAME_MAX];
} __attribute__((preserve_access_index));

>>>>>>> afb45ac4de7f (selftests: bpf: verify first of struct mptcp_sock)
'''

Merge it into bpf_mptcp_helpers.h:

'''
struct mptcp_sock {
	struct inet_connection_sock	sk;

	__u32		token;
	struct sock	*first;
	char		ca_name[TCP_CA_NAME_MAX];
} __attribute__((preserve_access_index));
'''

Note: 'first' is inserted between 'token' and 'ca_name', to keep the order of
mptcp_sock in kernel.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/bpf_mptcp_helpers.h | 14 ++++++++++++++
 tools/testing/selftests/bpf/bpf_tcp_helpers.h   |  5 -----
 tools/testing/selftests/bpf/progs/mptcp_sock.c  |  2 +-
 3 files changed, 15 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/bpf_mptcp_helpers.h

diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
new file mode 100644
index 000000000000..18da4cc65e89
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2022, SUSE. */
+
+#ifndef __BPF_MPTCP_HELPERS_H
+#define __BPF_MPTCP_HELPERS_H
+
+#include "bpf_tcp_helpers.h"
+
+struct mptcp_sock {
+	struct inet_connection_sock	sk;
+
+} __attribute__((preserve_access_index));
+
+#endif
diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index d71616873a3c..b1ede6f0b821 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -83,11 +83,6 @@ struct tcp_sock {
 	__u64	tcp_mstamp;	/* most recent packet received/sent */
 } __attribute__((preserve_access_index));
 
-struct mptcp_sock {
-	struct inet_connection_sock	sk;
-
-} __attribute__((preserve_access_index));
-
 static __always_inline struct inet_connection_sock *inet_csk(const struct sock *sk)
 {
 	return (struct inet_connection_sock *)sk;
diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
index a58cd24da777..5cfaec4e7245 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -3,7 +3,7 @@
 
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
-#include "bpf_tcp_helpers.h"
+#include "bpf_mptcp_helpers.h"
 
 char _license[] SEC("license") = "GPL";
 __u32 _version SEC("version") = 1;
-- 
2.34.1