On Thu, Jun 3, 2021 at 11:32 PM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> wrote:
>
> This adds userspace TC-BPF management API based on bpf_link.
>
> Reviewed-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>.
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> ---
> tools/lib/bpf/bpf.c | 8 +++++-
> tools/lib/bpf/bpf.h | 8 +++++-
> tools/lib/bpf/libbpf.c | 59 ++++++++++++++++++++++++++++++++++++++--
> tools/lib/bpf/libbpf.h | 17 ++++++++++++
> tools/lib/bpf/libbpf.map | 1 +
> tools/lib/bpf/netlink.c | 5 ++--
> tools/lib/bpf/netlink.h | 8 ++++++
> 7 files changed, 100 insertions(+), 6 deletions(-)
> create mode 100644 tools/lib/bpf/netlink.h
>
[...]
> + link = calloc(1, sizeof(*link));
> + if (!link)
> + return ERR_PTR(-ENOMEM);
> + link->detach = &bpf_link__detach_fd;
> +
> + link_fd = bpf_link_create(prog_fd, OPTS_GET(hook, ifindex, 0), BPF_TC, &lopts);
> + if (link_fd < 0) {
> + link_fd = -errno;
> + free(link);
> + pr_warn("prog '%s': failed to attach tc filter: %s\n",
> + prog->name, libbpf_strerror_r(link_fd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(link_fd);
like Yonghong mentioned, all these error returns have to be either
`return libbpf_err_ptr(err);` or, given it's a new API, we can do just
direct `return errno = err, NULL;`. I'd probably use libbpf_err_ptr()
for now for consistency.
> + }
> + link->fd = link_fd;
> +
> + return link;
> +}
> +
> struct bpf_link *bpf_program__attach_freplace(struct bpf_program *prog,
> int target_fd,
> const char *attach_func_name)
[...]
> diff --git a/tools/lib/bpf/netlink.h b/tools/lib/bpf/netlink.h
> new file mode 100644
> index 000000000000..c89133d56eb4
> --- /dev/null
> +++ b/tools/lib/bpf/netlink.h
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> +#pragma once
we don't use "#pragma once" in libbpf. And I saw some kernel
discussions discouraging its use. So please just stick to the classic
#ifdef/#define/#endif combo.
> +
> +#include <linux/types.h>
> +#include "libbpf.h"
> +
> +int tc_get_tcm_parent(enum bpf_tc_attach_point attach_point,
> + __u32 *parent);
> --
> 2.31.1
>
