[PATCH mptcp-next] Squash-to "bpf: Add bpf_mptcp_sched_ops"

Matthieu Baerts posted 1 patch 1 year, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20221129142010.2886747-1-matthieu.baerts@tessares.net
Maintainers: Mat Martineau <mathew.j.martineau@linux.intel.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Kumar Kartikeya Dwivedi <memxor@gmail.com>, Alexei Starovoitov <ast@kernel.org>
net/mptcp/bpf.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
[PATCH mptcp-next] Squash-to "bpf: Add bpf_mptcp_sched_ops"
Posted by Matthieu Baerts 1 year, 4 months ago
The BPF API around btf_struct_access() has been modified recently, see
commit 6728aea7216c ("bpf: Refactor btf_struct_access").

Adaptations have been done, similar to what was done in the mentioned
commit, e.g. by looking at changes around bpf_tcp_ca_btf_struct_access().

Without this modification, we have compilation errors:

    net/mptcp/bpf.c: In function 'bpf_mptcp_sched_btf_struct_access':
    net/mptcp/bpf.c:46:47: error: passing argument 2 of 'btf_struct_access' from incompatible pointer type [-Werror=incompatible-pointer-types]
     46 |                 return btf_struct_access(log, btf, t, off, size, atype,
        |                                               ^~~
        |                                               |
        |                                               const struct btf *
  In file included from   net/mptcp/bpf.c:12:
    include/linux/bpf.h:2155:51: note: expected 'const struct bpf_reg_state *' but argument is of type 'const struct btf *'
   2155 |                       const struct bpf_reg_state *reg,
        |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
    net/mptcp/bpf.c:46:52: error: passing argument 3 of 'btf_struct_access' makes integer from pointer without a cast [-Werror=int-conversion]
     46 |                 return btf_struct_access(log, btf, t, off, size, atype,
        |                                                    ^
        |                                                    |
        |                                                    const struct btf_type *
  In file included from   net/mptcp/bpf.c:12:
    include/linux/bpf.h:2156:27: note: expected 'int' but argument is of type 'const struct btf_type *'
   2156 |                       int off, int size, enum bpf_access_type atype,
        |                       ~~~~^~~
    net/mptcp/bpf.c:46:66: error: incompatible type for argument 6 of 'btf_struct_access'
     46 |                 return btf_struct_access(log, btf, t, off, size, atype,
        |                                                                  ^~~~~
        |                                                                  |
        |                                                                  enum bpf_access_type
  In file included from   net/mptcp/bpf.c:12:
    include/linux/bpf.h:2157:28: note: expected 'u32 *' {aka 'unsigned int *'} but argument is of type 'enum bpf_access_type'
   2157 |                       u32 *next_btf_id, enum bpf_type_flag *flag);
        |                       ~~~~~^~~~~~~~~~~
    net/mptcp/bpf.c:46:24: error: too many arguments to function 'btf_struct_access'
     46 |                 return btf_struct_access(log, btf, t, off, size, atype,
        |                        ^~~~~~~~~~~~~~~~~
  In file included from   net/mptcp/bpf.c:12:
    include/linux/bpf.h:2154:5: note: declared here
   2154 | int btf_struct_access(struct bpf_verifier_log *log,
        |     ^~~~~~~~~~~~~~~~~
    net/mptcp/bpf.c: At top level:
    net/mptcp/bpf.c:76:35: error: initialization of 'int (*)(struct bpf_verifier_log *, const struct bpf_reg_state *, int,  int,  enum bpf_access_type,  u32 *, enum bpf_type_flag *)' {aka 'int (*)(struct bpf_verifier_log *, const struct bpf_reg_state *, int,  int,  enum bpf_access_type,  unsigned int *, enum bpf_type_flag *)'} from incompatible pointer type 'int (*)(struct bpf_verifier_log *, const struct btf *, const struct btf_type *, int,  int,  enum bpf_access_type,  u32 *, enum bpf_type_flag *)' {aka 'int (*)(struct bpf_verifier_log *, const struct btf *, const struct btf_type *, int,  int,  enum bpf_access_type,  unsigned int *, enum bpf_type_flag *)'} [-Werror=incompatible-pointer-types]
     76 |         .btf_struct_access      = bpf_mptcp_sched_btf_struct_access,
        |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    net/mptcp/bpf.c:76:35: note: (near initialization for 'bpf_mptcp_sched_verifier_ops.btf_struct_access')

Fixes: 6728aea7216c ("bpf: Refactor btf_struct_access")
Link: https://cirrus-ci.com/task/5509843558072320?logs=test#L3300-L3262
Cc: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---

Notes:
    I suggest not to wait before applying this patch to be able to continue
    compiling and validating the export branch.

 net/mptcp/bpf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 03decb05755f..f632c2f828e8 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -34,19 +34,21 @@ bpf_mptcp_sched_get_func_proto(enum bpf_func_id func_id,
 }
 
 static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
-					     const struct btf *btf,
-					     const struct btf_type *t, int off,
-					     int size, enum bpf_access_type atype,
+					     const struct bpf_reg_state *reg,
+					     int off, int size,
+					     enum bpf_access_type atype,
 					     u32 *next_btf_id,
 					     enum bpf_type_flag *flag)
 {
+	const struct btf_type *t;
 	size_t end;
 
 	if (atype == BPF_READ) {
-		return btf_struct_access(log, btf, t, off, size, atype,
+		return btf_struct_access(log, reg, off, size, atype,
 					 next_btf_id, flag);
 	}
 
+	t = btf_type_by_id(reg->btf, reg->btf_id);
 	if (t != mptcp_sched_type) {
 		bpf_log(log, "only access to mptcp_subflow_context is supported\n");
 		return -EACCES;

base-commit: 8c1a5e594de5220c783bb99a683a5c7e8f656632
-- 
2.37.2
Re: Squash-to "bpf: Add bpf_mptcp_sched_ops": Tests Results
Posted by MPTCP CI 1 year, 4 months ago
Hi Matthieu,

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/5929540917133312
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5929540917133312/summary/summary.txt

- KVM Validation: debug:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6040859421442048
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6040859421442048/summary/summary.txt

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


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: Squash-to "bpf: Add bpf_mptcp_sched_ops": Tests Results
Posted by MPTCP CI 1 year, 4 months ago
Hi Matthieu,

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/5929540917133312
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5929540917133312/summary/summary.txt

- KVM Validation: debug:
  - Critical: Global Timeout ❌:
  - Task: https://cirrus-ci.com/task/5366590963712000
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5366590963712000/summary/summary.txt

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


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_mptcp_sched_ops"
Posted by Matthieu Baerts 1 year, 4 months ago
Hello,

On 29/11/2022 15:20, Matthieu Baerts wrote:
> The BPF API around btf_struct_access() has been modified recently, see
> commit 6728aea7216c ("bpf: Refactor btf_struct_access").
> 
> Adaptations have been done, similar to what was done in the mentioned
> commit, e.g. by looking at changes around bpf_tcp_ca_btf_struct_access().
> 
> Without this modification, we have compilation errors:

(...)

> Fixes: 6728aea7216c ("bpf: Refactor btf_struct_access")
> Link: https://cirrus-ci.com/task/5509843558072320?logs=test#L3300-L3262
> Cc: Geliang Tang <geliang.tang@suse.com>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
> 
> Notes:
>     I suggest not to wait before applying this patch to be able to continue
>     compiling and validating the export branch.

As suggested, I just applied it:

- 3c77cc8397df: "squashed" in "bpf: Add bpf_mptcp_sched_ops"
- Results: 8c1a5e594de5..2f61edcc034c (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20221129T142751

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