bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Florian Westphal <fw@strlen.de>
Cc: bpf@vger.kernel.org, ast@kernel.org, netdev@vger.kernel.org,
	dxu@dxuuu.xyz,  qde@naccy.de
Subject: Re: [PATCH bpf-next 1/2] tools: libbpf: add netfilter link attach helper
Date: Thu, 25 May 2023 14:01:46 -0700	[thread overview]
Message-ID: <CAEf4BzZ69YgrQW7DHCJUT_X+GqMq_ZQQPBwopaJJVGFD5=d5Vg@mail.gmail.com> (raw)
In-Reply-To: <20230525110100.8212-2-fw@strlen.de>

On Thu, May 25, 2023 at 4:01 AM Florian Westphal <fw@strlen.de> wrote:
>
> Add new api function: bpf_program__attach_netfilter_opts.
>
> It takes a bpf program (netfilter type), and a pointer to a option struct
> that contains the desired attachment (protocol family, priority, hook
> location, ...).
>
> It returns a pointer to a 'bpf_link' structure or NULL on error.
>
> Next patch adds new netfilter_basic test that uses this function to
> attach a program to a few pf/hook/priority combinations.
>
> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  tools/lib/bpf/libbpf.c   | 51 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   | 15 ++++++++++++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 67 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 5cca00979aae..033447aa0773 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -11811,6 +11811,57 @@ static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_l
>         return libbpf_get_error(*link);
>  }
>
> +struct bpf_link *bpf_program__attach_netfilter_opts(const struct bpf_program *prog,
> +                                                   const struct bpf_netfilter_opts *opts)

let's just call it `bpf_program__attach_netfilter`. We add "_opts" if
we had variant without opts. This doesn't apply here, so a shorter
name is preferable.

> +{
> +       const size_t attr_sz = offsetofend(union bpf_attr, link_create);
> +       struct bpf_link *link;
> +       int prog_fd, link_fd;
> +       union bpf_attr attr;
> +
> +       if (!OPTS_VALID(opts, bpf_netfilter_opts))
> +               return libbpf_err_ptr(-EINVAL);
> +
> +       prog_fd = bpf_program__fd(prog);
> +       if (prog_fd < 0) {
> +               pr_warn("prog '%s': can't attach before loaded\n", prog->name);
> +               return libbpf_err_ptr(-EINVAL);
> +       }
> +
> +       link = calloc(1, sizeof(*link));
> +       if (!link)
> +               return libbpf_err_ptr(-ENOMEM);
> +       link->detach = &bpf_link__detach_fd;
> +
> +       memset(&attr, 0, attr_sz);
> +
> +       attr.link_create.prog_fd = prog_fd;
> +       attr.link_create.netfilter.pf = OPTS_GET(opts, pf, 0);
> +       attr.link_create.netfilter.hooknum = OPTS_GET(opts, hooknum, 0);
> +       attr.link_create.netfilter.priority = OPTS_GET(opts, priority, 0);
> +       attr.link_create.netfilter.flags = OPTS_GET(opts, flags, 0);
> +
> +       link_fd = syscall(__NR_bpf, BPF_LINK_CREATE, &attr, attr_sz);

this code shouldn't do direct syscall, these high-level APIs should go
through libbpf low-level API. In this case, you need to call
bpf_link_create().

Except bpf_link_create() doesn't really support NETLINK links yet,
which is what we'll need to fix first. bpf_link_create() determines
what kind of parameters to pass to kernel based on bpf_attach_type.
And we currently don't have an attach type for NETLINK BPF link.
Thankfully it's not too late to add it. I see that link_create() in
kernel/bpf/syscall.c just bypasses attach_type check. We shouldn't
have done that. Instead we need to add BPF_NETLINK attach type to enum
bpf_attach_type. And wire all that properly throughout the kernel and
libbpf itself. Thankfully kernel release is not finalized and we can
still fix that up, but please prioritize it before we get too far into
rc releases.

> +
> +       link->fd = ensure_good_fd(link_fd);
> +
> +       if (link->fd < 0) {
> +               char errmsg[STRERR_BUFSIZE];
> +
> +               link_fd = -errno;
> +               free(link);
> +               pr_warn("prog '%s': failed to attach to pf:%d,hooknum:%d:prio:%d: %s\n",
> +                       prog->name,
> +                       OPTS_GET(opts, pf, 0),
> +                       OPTS_GET(opts, hooknum, 0),
> +                       OPTS_GET(opts, priority, 0),
> +                       libbpf_strerror_r(link_fd, errmsg, sizeof(errmsg)));
> +               return libbpf_err_ptr(link_fd);
> +       }
> +
> +       return link;
> +}
> +
>  struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
>  {
>         struct bpf_link *link = NULL;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 754da73c643b..081beb95a097 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -718,6 +718,21 @@ LIBBPF_API struct bpf_link *
>  bpf_program__attach_freplace(const struct bpf_program *prog,
>                              int target_fd, const char *attach_func_name);
>
> +struct bpf_netfilter_opts {
> +       /* size of this struct, for forward/backward compatibility */
> +       size_t sz;
> +
> +       __u32 pf;
> +       __u32 hooknum;
> +       __s32 priority;
> +       __u32 flags;
> +};
> +#define bpf_netfilter_opts__last_field flags
> +
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_netfilter_opts(const struct bpf_program *prog,
> +                                  const struct bpf_netfilter_opts *opts);
> +
>  struct bpf_map;
>
>  LIBBPF_API struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map);
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 7521a2fb7626..e13d60608bf3 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -395,4 +395,5 @@ LIBBPF_1.2.0 {
>  LIBBPF_1.3.0 {
>         global:
>                 bpf_obj_pin_opts;
> +               bpf_program__attach_netfilter_opts;

opts and the rest looks good, thanks

>  } LIBBPF_1.2.0;
> --
> 2.39.3
>

  reply	other threads:[~2023-05-25 21:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-25 11:00 [PATCH bpf-next 0/2] libbpf: add netfilter link attach helper Florian Westphal
2023-05-25 11:00 ` [PATCH bpf-next 1/2] tools: " Florian Westphal
2023-05-25 21:01   ` Andrii Nakryiko [this message]
2023-05-26 12:11     ` [PATCH bpf] bpf: netfilter: add BPF_NETFILTER bpf_attach_type Florian Westphal
2023-05-30 17:10       ` Andrii Nakryiko
2023-05-25 11:01 ` [PATCH bpf-next 2/2] selftests/bpf: Add bpf_program__attach_netfilter_opts helper test Florian Westphal
2023-05-25 21:05   ` Andrii Nakryiko

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='CAEf4BzZ69YgrQW7DHCJUT_X+GqMq_ZQQPBwopaJJVGFD5=d5Vg@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dxu@dxuuu.xyz \
    --cc=fw@strlen.de \
    --cc=netdev@vger.kernel.org \
    --cc=qde@naccy.de \
    /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 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).