On 4/23/21 5:05 PM, Kumar Kartikeya Dwivedi wrote: [...]
tools/lib/bpf/libbpf.h | 92 ++++++++ tools/lib/bpf/libbpf.map | 5 + tools/lib/bpf/netlink.c | 478 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 574 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index bec4e6a6e31d..1c717c07b66e 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -775,6 +775,98 @@ 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); +enum bpf_tc_attach_point { + BPF_TC_INGRESS, + BPF_TC_EGRESS, + BPF_TC_CUSTOM_PARENT, + _BPF_TC_PARENT_MAX,
I don't think we need to expose _BPF_TC_PARENT_MAX as part of the API, I would drop the latter.
+}; + +/* The opts structure is also used to return the created filters attributes + * (e.g. in case the user left them unset). Some of the options that were left + * out default to a reasonable value, documented below. + * + * protocol - ETH_P_ALL + * chain index - 0 + * class_id - 0 (can be set by bpf program using skb->tc_classid) + * bpf_flags - TCA_BPF_FLAG_ACT_DIRECT (direct action mode) + * bpf_flags_gen - 0 + * + * The user must fulfill documented requirements for each function.
Not sure if this is overly relevant as part of the bpf_tc_opts in here. For the 2nd part, I would probably just mention that libbpf internally attaches the bpf programs with direct action mode. The hw offload may be future todo, and the other bits are little used anyway; mentioning them here, what value does it have to libbpf users? I'd rather just drop the 2nd part and/or simplify this paragraph just stating that the progs are attached in direct action mode.
+ */ +struct bpf_tc_opts { + size_t sz; + __u32 handle; + __u32 parent; + __u16 priority; + __u32 prog_id; + bool replace; + size_t :0; +}; + +#define bpf_tc_opts__last_field replace + +struct bpf_tc_ctx; + +struct bpf_tc_ctx_opts { + size_t sz; +}; + +#define bpf_tc_ctx_opts__last_field sz + +/* Requirements */ +/* + * @ifindex: Must be > 0. + * @parent: Must be one of the enum constants < _BPF_TC_PARENT_MAX + * @opts: Can be NULL, currently no options are supported. + */
Up to Andrii, but we don't have such API doc in general inside libbpf.h, I would drop it for the time being to be consistent with the rest (same for others below).
+LIBBPF_API struct bpf_tc_ctx *bpf_tc_ctx_init(__u32 ifindex,
nit: in user space s/__u32 ifindex/int ifindex/
+ enum bpf_tc_attach_point parent, + struct bpf_tc_ctx_opts *opts);
Should we enforce opts being NULL or non-NULL here, or drop the arg from here for now altogether? (And if later versions of the functions show up this could be mapped to the right one?)
+/* + * @ctx: Can be NULL, if not, must point to a valid object. + * If the qdisc was attached during ctx_init, it will be deleted if no + * filters are attached to it. + * When ctx == NULL, this is a no-op. + */ +LIBBPF_API int bpf_tc_ctx_destroy(struct bpf_tc_ctx *ctx); +/* + * @ctx: Cannot be NULL. + * @fd: Must be >= 0. + * @opts: Cannot be NULL, prog_id must be unset, all other fields can be + * optionally set. All fields except replace will be set as per created + * filter's attributes. parent must only be set when attach_point of ctx is + * BPF_TC_CUSTOM_PARENT, otherwise parent must be unset. + * + * Fills the following fields in opts: + * handle + * parent + * priority + * prog_id + */ +LIBBPF_API int bpf_tc_attach(struct bpf_tc_ctx *ctx, int fd, + struct bpf_tc_opts *opts); +/* + * @ctx: Cannot be NULL. + * @opts: Cannot be NULL, replace and prog_id must be unset, all other fields + * must be set. + */ +LIBBPF_API int bpf_tc_detach(struct bpf_tc_ctx *ctx, + const struct bpf_tc_opts *opts);
One thing that I find a bit odd from this API is that BPF_TC_INGRESS / BPF_TC_EGRESS needs to be set each time via bpf_tc_ctx_init(). So whenever a specific program would be attached to both we need to 're-init' in between just to change from hook a to b, whereas when you have BPF_TC_CUSTOM_PARENT, you could just use a different opts->parent without going this detour (unless the clsact wasn't loaded there in the first place). Could we add a BPF_TC_UNSPEC to enum bpf_tc_attach_point, which the user would pass to bpf_tc_ctx_init(), so that opts.direction = BPF_TC_INGRESS with subsequent bpf_tc_attach() can be called, and same opts.direction = BPF_TC_EGRESS with bpf_tc_attach() for different fd. The only thing we cared about in bpf_tc_ctx_init() resp. the ctx was that qdisc was ready.
+/* + * @ctx: Cannot be NULL. + * @opts: Cannot be NULL, replace and prog_id must be unset, all other fields + * must be set. + * + * Fills the following fields in opts: + * handle + * parent + * priority + * prog_id + */ +LIBBPF_API int bpf_tc_query(struct bpf_tc_ctx *ctx, + struct bpf_tc_opts *opts); + #ifdef __cplusplus } /* extern "C" */ #endif