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>,
	KP Singh <kpsingh@chromium.org>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 3/6] bpf: support attaching freplace programs to multiple attach points
Date: Wed, 15 Jul 2020 13:44:06 -0700	[thread overview]
Message-ID: <20200715204406.vt64vgvzsbr6kolm@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <159481854255.454654.15065796817034016611.stgit@toke.dk>

On Wed, Jul 15, 2020 at 03:09:02PM +0200, Toke Høiland-Jørgensen wrote:
>  
> +	if (tgt_prog_fd) {
> +		/* For now we only allow new targets for BPF_PROG_TYPE_EXT */
> +		if (prog->type != BPF_PROG_TYPE_EXT ||
> +		    !btf_id) {
> +			err = -EINVAL;
> +			goto out_put_prog;
> +		}
> +		tgt_prog = bpf_prog_get(tgt_prog_fd);
> +		if (IS_ERR(tgt_prog)) {
> +			err = PTR_ERR(tgt_prog);
> +			tgt_prog = NULL;
> +			goto out_put_prog;
> +		}
> +
> +	} else if (btf_id) {
> +		err = -EINVAL;
> +		goto out_put_prog;
> +	} else {
> +		btf_id = prog->aux->attach_btf_id;
> +		tgt_prog = prog->aux->linked_prog;
> +		if (tgt_prog)
> +			bpf_prog_inc(tgt_prog); /* we call bpf_prog_put() on link release */

so the first prog_load cmd will beholding the first target prog?
This is complete non starter.
You didn't mention such decision anywhere.
The first ext prog will attach to the first dispatcher xdp prog,
then that ext prog will multi attach to second dispatcher xdp prog and
the first dispatcher prog will live in the kernel forever.
That's not what we discussed back in April.

> +	}
> +	err = bpf_check_attach_target(NULL, prog, tgt_prog, btf_id,
> +				      &fmodel, &addr, NULL, NULL);

This is a second check for btf id match?
What's the point? The first one was done at load time.
When tgt_prog_fd/tgt_btf_id are zero there is no need to recheck.

I really hope I'm misreading these patches, because they look very raw.

  reply	other threads:[~2020-07-15 20:44 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-15 13:08 [PATCH bpf-next v2 0/6] bpf: Support multi-attach for freplace programs Toke Høiland-Jørgensen
2020-07-15 13:09 ` [PATCH bpf-next v2 1/6] bpf: change logging calls from verbose() to bpf_log() and use log pointer Toke Høiland-Jørgensen
2020-07-16 10:10   ` Dan Carpenter
2020-07-16 10:10     ` Dan Carpenter
2020-07-16 10:10     ` Dan Carpenter
2020-07-15 13:09 ` [PATCH bpf-next v2 2/6] bpf: verifier: refactor check_attach_btf_id() Toke Høiland-Jørgensen
2020-07-15 13:09 ` [PATCH bpf-next v2 3/6] bpf: support attaching freplace programs to multiple attach points Toke Høiland-Jørgensen
2020-07-15 20:44   ` Alexei Starovoitov [this message]
2020-07-16 10:50     ` Toke Høiland-Jørgensen
2020-07-17  2:05       ` Alexei Starovoitov
2020-07-17 10:52         ` Toke Høiland-Jørgensen
2020-07-20 22:57           ` Alexei Starovoitov
2020-07-20  5:02         ` Andrii Nakryiko
2020-07-20 23:34           ` Alexei Starovoitov
2020-07-21  3:48             ` Andrii Nakryiko
2020-07-22  0:29               ` Alexei Starovoitov
2020-07-22  6:02                 ` Andrii Nakryiko
2020-07-23  0:32                   ` Alexei Starovoitov
2020-07-23  0:56                     ` Andrii Nakryiko
2020-07-16 10:14   ` Dan Carpenter
2020-07-16 10:14     ` Dan Carpenter
2020-07-16 10:14     ` Dan Carpenter
2020-07-15 13:09 ` [PATCH bpf-next v2 4/6] tools: add new members to bpf_attr.raw_tracepoint in bpf.h Toke Høiland-Jørgensen
2020-07-15 13:09 ` [PATCH bpf-next v2 5/6] libbpf: add support for supplying target to bpf_raw_tracepoint_open() Toke Høiland-Jørgensen
2020-07-15 13:09 ` [PATCH bpf-next v2 6/6] selftests: add test for multiple attachments of freplace program Toke Høiland-Jørgensen
2020-07-15 20:59 [PATCH bpf-next v2 3/6] bpf: support attaching freplace programs to multiple attach points kernel test robot

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=20200715204406.vt64vgvzsbr6kolm@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=john.fastabend@gmail.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.