This patch exports the member last_snd of struct mptcp_sock in
bpf_mptcp_helpers.h, and adds BPF write access to it.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/bpf.c                                 | 16 ++++++++++++++++
 tools/testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index e849fc3fb6c5..bd3c50b07ab2 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -40,6 +40,7 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
 {
 	const struct btf_type *state;
 	u32 type_id;
+	size_t end;
 
 	if (atype == BPF_READ)
 		return btf_struct_access(log, btf, t, off, size, atype,
@@ -55,6 +56,21 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
 		return -EACCES;
 	}
 
+	switch (off) {
+	case offsetof(struct mptcp_sock, last_snd):
+		end = offsetofend(struct mptcp_sock, last_snd);
+		break;
+	default:
+		bpf_log(log, "no write support to mptcp_sock at off %d\n", off);
+		return -EACCES;
+	}
+
+	if (off + size > end) {
+		bpf_log(log, "access beyond mptcp_sock at off %u size %u ended at %lu",
+			off, size, end);
+		return -EACCES;
+	}
+
 	return NOT_INIT;
 }
 
diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
index eb51fc1f54d5..90accd94e947 100644
--- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
@@ -21,6 +21,7 @@ struct mptcp_sched_ops {
 struct mptcp_sock {
 	struct inet_connection_sock	sk;
 
+	struct sock	*last_snd;
 	__u32		token;
 	struct sock	*first;
 	struct mptcp_sched_ops *sched;
-- 
2.34.1

Hi Geliang,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mptcp/export]
[cannot apply to bpf-next/master bpf/master linus/master v5.18-rc3 next-20220420]
[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]

url:    https://github.com/intel-lab-lkp/linux/commits/Geliang-Tang/BPF-packet-scheduler/20220420-130015
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220421/202204210456.03kTcuXu-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/c711e7d26efa699f1cfe5e2c9619bbf5cc98a7a7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Geliang-Tang/BPF-packet-scheduler/20220420-130015
        git checkout c711e7d26efa699f1cfe5e2c9619bbf5cc98a7a7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/mptcp/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   net/mptcp/bpf.c: In function 'bpf_mptcp_sched_btf_struct_access':
>> net/mptcp/bpf.c:69:85: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
      69 |                 bpf_log(log, "access beyond mptcp_sock at off %u size %u ended at %lu",
         |                                                                                   ~~^
         |                                                                                     |
         |                                                                                     long unsigned int
         |                                                                                   %u
      70 |                         off, size, end);
         |                                    ~~~                                               
         |                                    |
         |                                    size_t {aka unsigned int}


vim +69 net/mptcp/bpf.c

    33	
    34	static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
    35						     const struct btf *btf,
    36						     const struct btf_type *t, int off,
    37						     int size, enum bpf_access_type atype,
    38						     u32 *next_btf_id,
    39						     enum bpf_type_flag *flag)
    40	{
    41		const struct btf_type *state;
    42		u32 type_id;
    43		size_t end;
    44	
    45		if (atype == BPF_READ)
    46			return btf_struct_access(log, btf, t, off, size, atype,
    47						 next_btf_id, flag);
    48	
    49		type_id = btf_find_by_name_kind(btf, "mptcp_sock", BTF_KIND_STRUCT);
    50		if (type_id < 0)
    51			return -EINVAL;
    52	
    53		state = btf_type_by_id(btf, type_id);
    54		if (t != state) {
    55			bpf_log(log, "only read is supported\n");
    56			return -EACCES;
    57		}
    58	
    59		switch (off) {
    60		case offsetof(struct mptcp_sock, last_snd):
    61			end = offsetofend(struct mptcp_sock, last_snd);
    62			break;
    63		default:
    64			bpf_log(log, "no write support to mptcp_sock at off %d\n", off);
    65			return -EACCES;
    66		}
    67	
    68		if (off + size > end) {
  > 69			bpf_log(log, "access beyond mptcp_sock at off %u size %u ended at %lu",
    70				off, size, end);
    71			return -EACCES;
    72		}
    73	
    74		return NOT_INIT;
    75	}
    76	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Hi Geliang,

On 20/04/2022 06:57, Geliang Tang wrote:
> This patch exports the member last_snd of struct mptcp_sock in
> bpf_mptcp_helpers.h, and adds BPF write access to it.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  net/mptcp/bpf.c                                 | 16 ++++++++++++++++
>  tools/testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index e849fc3fb6c5..bd3c50b07ab2 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -40,6 +40,7 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
>  {
>  	const struct btf_type *state;
>  	u32 type_id;
> +	size_t end;
>  
>  	if (atype == BPF_READ)
>  		return btf_struct_access(log, btf, t, off, size, atype,
> @@ -55,6 +56,21 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
>  		return -EACCES;
>  	}
>  
> +	switch (off) {
> +	case offsetof(struct mptcp_sock, last_snd):
> +		end = offsetofend(struct mptcp_sock, last_snd);
> +		break;
> +	default:
> +		bpf_log(log, "no write support to mptcp_sock at off %d\n", off);
> +		return -EACCES;
> +	}
> +
> +	if (off + size > end) {
> +		bpf_log(log, "access beyond mptcp_sock at off %u size %u ended at %lu",
> +			off, size, end);

A small note about this line: the CI[1] seems to complain about it when
compiling in i386:

>   net/mptcp/bpf.c: In function 'bpf_mptcp_sched_btf_struct_access':
>   net/mptcp/bpf.c:69:71: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
>      69 |   bpf_log(log, "access beyond mptcp_sock at off %u size %u ended at %lu",
>         |                                                                     ~~^
>         |                                                                       |
>         |                                                                       long unsigned int
>         |                                                                     %u
>      70 |    off, size, end);
>         |               ~~~                                                      
>         |               |
>         |               size_t {aka unsigned int}
>   cc1: all warnings being treated as errors

I think you need to use %zu.

https://github.com/multipath-tcp/mptcp_net-next/runs/6089328349?check_suite_focus=true

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

Hi Geliang,

On 20/04/2022 06:57, Geliang Tang wrote:
> This patch exports the member last_snd of struct mptcp_sock in
> bpf_mptcp_helpers.h, and adds BPF write access to it.

Do we really need the have the different schedulers modifying it? Could
we not let the caller of "sched->get_subflow()" setting it?

I mean: 'last_snd' should always be the same as what get_subflow() will
return, no? If yes, why not setting it in mptcp_sched_get_subflow()?

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