All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Eelco Chaudron <echaudro@redhat.com>,
	KP Singh <kpsingh@chromium.org>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v9 04/11] bpf: move prog->aux->linked_prog and trampoline into bpf_link on attach
Date: Mon, 28 Sep 2020 17:05:22 -0700	[thread overview]
Message-ID: <20200929000522.n5g2hcahqjxwseye@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <160106910382.27725.8204173893583455016.stgit@toke.dk>

On Fri, Sep 25, 2020 at 11:25:03PM +0200, Toke Høiland-Jørgensen wrote:
>  
>  int bpf_check_attach_target(struct bpf_verifier_log *log,
>  			    const struct bpf_prog *prog,
> -			    const struct bpf_prog *tgt_prog,
> +			    const struct bpf_prog *dst_prog,

so you really did blind search and replace?
That's not at all what I was asking.
The function is called check_attach_target and the argument name
'tgt_prog' fits perfectly.

>  			    u32 btf_id,
>  			    struct bpf_attach_target_info *tgt_info);
>  
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 868c03a24d0a..faf57c6f8804 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -3706,10 +3706,10 @@ struct btf *btf_parse_vmlinux(void)
>  
>  struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog)
>  {
> -	struct bpf_prog *tgt_prog = prog->aux->linked_prog;
> +	struct bpf_prog *dst_prog = prog->aux->dst_prog;

same here. tgt_prog fits just fine as a name.

>  
> -	if (tgt_prog) {
> -		return tgt_prog->aux->btf;
> +	if (dst_prog) {
> +		return dst_prog->aux->btf;
>  	} else {
>  		return btf_vmlinux;
>  	}
> @@ -3733,7 +3733,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>  		    struct bpf_insn_access_aux *info)
>  {
>  	const struct btf_type *t = prog->aux->attach_func_proto;
> -	struct bpf_prog *tgt_prog = prog->aux->linked_prog;
> +	struct bpf_prog *dst_prog = prog->aux->dst_prog;

here as well.
it's a tgt_prog being checked.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 2740df19f55e..099a651efe8b 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2154,14 +2154,14 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
>  	prog->expected_attach_type = attr->expected_attach_type;
>  	prog->aux->attach_btf_id = attr->attach_btf_id;
>  	if (attr->attach_prog_fd) {
> -		struct bpf_prog *tgt_prog;
> +		struct bpf_prog *dst_prog;
>  
> -		tgt_prog = bpf_prog_get(attr->attach_prog_fd);
> -		if (IS_ERR(tgt_prog)) {
> -			err = PTR_ERR(tgt_prog);
> +		dst_prog = bpf_prog_get(attr->attach_prog_fd);
> +		if (IS_ERR(dst_prog)) {
> +			err = PTR_ERR(dst_prog);
>  			goto free_prog_nouncharge;
>  		}
> -		prog->aux->linked_prog = tgt_prog;
> +		prog->aux->dst_prog = dst_prog;

Here 'dst_prog' makes logical sense, but I wouldn't bother renaming.
You can keep this hunk, if you like.

>  int bpf_check_attach_target(struct bpf_verifier_log *log,
>  			    const struct bpf_prog *prog,
> -			    const struct bpf_prog *tgt_prog,
> +			    const struct bpf_prog *dst_prog,

pls keep it as 'tgt_prog' here and through the function.

>  static int check_attach_btf_id(struct bpf_verifier_env *env)
>  {
>  	struct bpf_prog *prog = env->prog;
> -	struct bpf_prog *tgt_prog = prog->aux->linked_prog;
> +	struct bpf_prog *dst_prog = prog->aux->dst_prog;

no need to rename either. It's a target program being checked.

  reply	other threads:[~2020-09-29  0:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25 21:24 [PATCH bpf-next v9 00/11] bpf: Support multi-attach for freplace programs Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 01/11] bpf: disallow attaching modify_return tracing functions to other BPF programs Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 02/11] bpf: change logging calls from verbose() to bpf_log() and use log pointer Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 03/11] bpf: verifier: refactor check_attach_btf_id() Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 04/11] bpf: move prog->aux->linked_prog and trampoline into bpf_link on attach Toke Høiland-Jørgensen
2020-09-29  0:05   ` Alexei Starovoitov [this message]
2020-09-29 10:47     ` Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 05/11] bpf: support attaching freplace programs to multiple attach points Toke Høiland-Jørgensen
2020-09-29  7:07   ` Dan Carpenter
2020-09-29  7:07     ` Dan Carpenter
2020-09-25 21:25 ` [PATCH bpf-next v9 06/11] bpf: Fix context type resolving for extension programs Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 07/11] libbpf: add support for freplace attachment in bpf_link_create Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 08/11] selftests: add test for multiple attachments of freplace program Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 09/11] selftests/bpf: Adding test for arg dereference in extension trace Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 10/11] selftests: Add selftest for disallowing modify_return attachment to freplace Toke Høiland-Jørgensen
2020-09-25 21:25 ` [PATCH bpf-next v9 11/11] selftests: Remove fmod_ret from test_overhead Toke Høiland-Jørgensen
2020-09-29  0:22   ` Alexei Starovoitov

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=20200929000522.n5g2hcahqjxwseye@ast-mbp.dhcp.thefacebook.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=echaudro@redhat.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=toke@redhat.com \
    --cc=yhs@fb.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.