bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hou Tao <houtao1@huawei.com>
To: Martin KaFai Lau <kafai@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>, <netdev@vger.kernel.org>,
	<bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 2/5] bpf: factor out a helper to prepare trampoline for struct_ops prog
Date: Thu, 30 Sep 2021 18:17:33 +0800	[thread overview]
Message-ID: <74247c43-39df-6872-4de6-8f4136ac37cd@huawei.com> (raw)
In-Reply-To: <20210929175620.yi4jfpllhugys6eo@kafai-mbp.dhcp.thefacebook.com>

Hi

On 9/30/2021 1:56 AM, Martin KaFai Lau wrote:
> On Tue, Sep 28, 2021 at 10:52:25AM +0800, Hou Tao wrote:
>> Factor out a helper bpf_prepare_st_ops_prog() to prepare trampoline
>> for BPF_PROG_TYPE_STRUCT_OPS prog. It will be used by .test_run
>> callback in following patch.
> Thanks for the patches.
Thanks for you review.
>
> This preparation change should be the first patch instead.
Will do.
>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 155dfcfb8923..002bbb2c8bc7 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -2224,4 +2224,9 @@ int bpf_bprintf_prepare(char *fmt, u32 fmt_size, const u64 *raw_args,
>>  			u32 **bin_buf, u32 num_args);
>>  void bpf_bprintf_cleanup(void);
>>  
>> +int bpf_prepare_st_ops_prog(struct bpf_tramp_progs *tprogs,
>> +			    struct bpf_prog *prog,
>> +			    const struct btf_func_model *model,
>> +			    void *image, void *image_end);
> Move it under where other bpf_struct_ops_.*() resides in bpf.h.
>
>> +
>>  #endif /* _LINUX_BPF_H */
>> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
>> index 9abcc33f02cf..ec3c25174923 100644
>> --- a/kernel/bpf/bpf_struct_ops.c
>> +++ b/kernel/bpf/bpf_struct_ops.c
>> @@ -312,6 +312,20 @@ static int check_zero_holes(const struct btf_type *t, void *data)
>>  	return 0;
>>  }
>>  
>> +int bpf_prepare_st_ops_prog(struct bpf_tramp_progs *tprogs,
>> +			    struct bpf_prog *prog,
>> +			    const struct btf_func_model *model,
>> +			    void *image, void *image_end)
> The existing struct_ops functions in the kernel now have naming like
> bpf_struct_ops_.*().  How about renaming it to
> bpf_struct_ops_prepare_trampoline()?
bpf_struct_ops_prepare_trampoline() may be a little long, and it will make
the indentations of its parameters look ugly, so how about
bpf_struct_ops_prep_prog() ?
>
>> +{
>> +	u32 flags;
>> +
>> +	tprogs[BPF_TRAMP_FENTRY].progs[0] = prog;
>> +	tprogs[BPF_TRAMP_FENTRY].nr_progs = 1;
>> +	flags = model->ret_size > 0 ? BPF_TRAMP_F_RET_FENTRY_RET : 0;
>> +	return arch_prepare_bpf_trampoline(NULL, image, image_end,
>> +					   model, flags, tprogs, NULL);
>> +}
>> +
>>  static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
>>  					  void *value, u64 flags)
>>  {
>> @@ -368,7 +382,6 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
>>  		const struct btf_type *mtype, *ptype;
>>  		struct bpf_prog *prog;
>>  		u32 moff;
>> -		u32 flags;
>>  
>>  		moff = btf_member_bit_offset(t, member) / 8;
>>  		ptype = btf_type_resolve_ptr(btf_vmlinux, member->type, NULL);
>> @@ -430,14 +443,9 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
>>  			goto reset_unlock;
>>  		}
>>  
>> -		tprogs[BPF_TRAMP_FENTRY].progs[0] = prog;
>> -		tprogs[BPF_TRAMP_FENTRY].nr_progs = 1;
>> -		flags = st_ops->func_models[i].ret_size > 0 ?
>> -			BPF_TRAMP_F_RET_FENTRY_RET : 0;
> This change can't apply to bpf-next now because
> commit 356ed64991c6 ("bpf: Handle return value of BPF_PROG_TYPE_STRUCT_OPS prog")
> is not pulled into bpf-next yet.  Please mention the dependency
> in the cover letter if it is still the case in v2.
Thanks for the reminder. Will do.
>
>> -		err = arch_prepare_bpf_trampoline(NULL, image,
>> -						  st_map->image + PAGE_SIZE,
>> -						  &st_ops->func_models[i],
>> -						  flags, tprogs, NULL);
>> +		err = bpf_prepare_st_ops_prog(tprogs, prog,
>> +					      &st_ops->func_models[i],
>> +					      image, st_map->image + PAGE_SIZE);
>>  		if (err < 0)
>>  			goto reset_unlock;
>>  
>> -- 
>> 2.29.2
>>
> .


  reply	other threads:[~2021-09-30 10:17 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28  2:52 [PATCH bpf-next 0/5] introduce dummy BPF STRUCT_OPS Hou Tao
2021-09-28  2:52 ` [PATCH bpf-next 1/5] bpf: add dummy BPF STRUCT_OPS for test purpose Hou Tao
2021-09-28  2:52 ` [PATCH bpf-next 2/5] bpf: factor out a helper to prepare trampoline for struct_ops prog Hou Tao
2021-09-29 17:56   ` Martin KaFai Lau
2021-09-30 10:17     ` Hou Tao [this message]
2021-10-01 17:39       ` Martin KaFai Lau
2021-09-28  2:52 ` [PATCH bpf-next 3/5] bpf: do .test_run in dummy BPF STRUCT_OPS Hou Tao
2021-09-29 18:55   ` Martin KaFai Lau
2021-09-30 11:05     ` Hou Tao
2021-10-01 19:09       ` Martin KaFai Lau
2021-09-28  2:52 ` [PATCH bpf-next 4/5] bpf: hook .test_run for struct_ops program Hou Tao
2021-09-28  2:52 ` [PATCH bpf-next 5/5] selftests/bpf: test return value handling for struct_ops prog Hou Tao
2021-09-28 23:19   ` Andrii Nakryiko
2021-09-30 11:08     ` Hou Tao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74247c43-39df-6872-4de6-8f4136ac37cd@huawei.com \
    --to=houtao1@huawei.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kafai@fb.com \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH bpf-next 2/5] bpf: factor out a helper to prepare trampoline for struct_ops prog' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).