netdev - Re: [PATCH bpf-next v3 2/3] libbpf: add low level TC-BPF API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <27b90b27-ce90-7b2c-23be-24cbc0781fbe@fb.com>
Date:   Tue, 20 Apr 2021 23:58:01 -0700
From:   Yonghong Song <yhs@...com>
To:     Kumar Kartikeya Dwivedi <memxor@...il.com>, <bpf@...r.kernel.org>
CC:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next v3 2/3] libbpf: add low level TC-BPF API



On 4/20/21 12:37 PM, Kumar Kartikeya Dwivedi wrote:
> This adds functions that wrap the netlink API used for adding,
> manipulating, and removing traffic control filters. These functions
> operate directly on the loaded prog's fd, and return a handle to the
> filter using an out parameter named id.
> 
> The basic featureset is covered to allow for attaching and removal of
> filters. Some additional features like TCA_BPF_POLICE and TCA_RATE for
> the API have been omitted. These can added on top later by extending the

"later" => "layer"?

> bpf_tc_opts struct.
> 
> Support for binding actions directly to a classifier by passing them in
> during filter creation has also been omitted for now. These actions have
> an auto clean up property because their lifetime is bound to the filter
> they are attached to. This can be added later, but was omitted for now
> as direct action mode is a better alternative to it, which is enabled by
> default.
> 
> An API summary:
> 
> bpf_tc_attach may be used to attach, and replace SCHED_CLS bpf
> classifier. The protocol is always set as ETH_P_ALL. The replace option
> in bpf_tc_opts is used to control replacement behavior.  Attachment
> fails if filter with existing attributes already exists.
> 
> bpf_tc_detach may be used to detach existing SCHED_CLS filter. The
> bpf_tc_attach_id object filled in during attach must be passed in to the
> detach functions for them to remove the filter and its attached
> classififer correctly.
> 
> bpf_tc_get_info is a helper that can be used to obtain attributes
> for the filter and classififer.
> 
> Examples:
> 
> 	struct bpf_tc_attach_id id = {};
> 	struct bpf_object *obj;
> 	struct bpf_program *p;
> 	int fd, r;
> 
> 	obj = bpf_object_open("foo.o");
> 	if (IS_ERR_OR_NULL(obj))
> 		return PTR_ERR(obj);
> 
> 	p = bpf_object__find_program_by_title(obj, "classifier");

Please use bpf_object__find_program_by_name() API.
bpf_object__find_program_by_title() is not recommended as now
libbpf supports multiple programs within the same section.

> 	if (IS_ERR_OR_NULL(p))
> 		return PTR_ERR(p);
> 
> 	if (bpf_object__load(obj) < 0)
> 		return -1;
> 
> 	fd = bpf_program__fd(p);
> 
> 	r = bpf_tc_attach(fd, if_nametoindex("lo"),
> 			  BPF_TC_CLSACT_INGRESS,
> 			  NULL, &id);
> 	if (r < 0)
> 		return r;
> 
> ... which is roughly equivalent to:
>    # tc qdisc add dev lo clsact
>    # tc filter add dev lo ingress bpf obj foo.o sec classifier da
> 
> ... as direct action mode is always enabled.
> 
> To replace an existing filter:
> 
> 	DECLARE_LIBBPF_OPTS(bpf_tc_opts, opts, .handle = id.handle,
> 			    .priority = id.priority, .replace = true);
> 	r = bpf_tc_attach(fd, if_nametoindex("lo"),
> 			  BPF_TC_CLSACT_INGRESS,
> 			  &opts, &id);
> 	if (r < 0)
> 		return r;
> 
> To obtain info of a particular filter, the example above can be extended
> as follows:
> 
> 	struct bpf_tc_info info = {};
> 
> 	r = bpf_tc_get_info(if_nametoindex("lo"),
> 			    BPF_TC_CLSACT_INGRESS,
> 			    &id, &info);
> 	if (r < 0)
> 		return r;
> 
> ... where id corresponds to the bpf_tc_attach_id filled in during an
> attach operation.
> 
> Reviewed-by: Toke Høiland-Jørgensen <toke@...hat.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@...il.com>
> ---
>   tools/lib/bpf/libbpf.h   |  44 ++++++
>   tools/lib/bpf/libbpf.map |   3 +
>   tools/lib/bpf/netlink.c  | 319 ++++++++++++++++++++++++++++++++++++++-
>   3 files changed, 360 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index bec4e6a6e31d..b4ed6a41ea70 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -16,6 +16,8 @@
>   #include <stdbool.h>
>   #include <sys/types.h>  // for size_t
>   #include <linux/bpf.h>
> +#include <linux/pkt_sched.h>
> +#include <linux/tc_act/tc_bpf.h>
>   
>   #include "libbpf_common.h"
>   
> @@ -775,6 +777,48 @@ LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker, const char *filen
>   LIBBPF_API int bpf_linker__finalize(struct bpf_linker *linker);
>   LIBBPF_API void bpf_linker__free(struct bpf_linker *linker);
>   
> +/* Convenience macros for the clsact attach hooks */
> +#define BPF_TC_CLSACT_INGRESS TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS)
> +#define BPF_TC_CLSACT_EGRESS TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_EGRESS)
> +
> +struct bpf_tc_opts {
> +	size_t sz;
> +	__u32 handle;
> +	__u32 class_id;
> +	__u16 priority;
> +	bool replace;
> +	size_t :0;

Did you see any error without "size_t :0"?

> +};
> +
> +#define bpf_tc_opts__last_field replace
> +
> +/* Acts as a handle for an attached filter */
> +struct bpf_tc_attach_id {
> +	__u32 handle;
> +	__u16 priority;
> +};
> +
[...]