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

[Kumar Kartikeya Dwivedi]

On Wed, Mar 31, 2021 at 02:55:47AM IST, Daniel Borkmann wrote:
> Do we even need the _block variant? I would rather prefer to take the chance
> and make it as simple as possible, and only iff really needed extend with
> other APIs, for example:

The block variant can be dropped, I'll use the TC_BLOCK/TC_DEV alternative which
sets parent_id/ifindex properly.

>
>   bpf_tc_attach(prog_fd, ifindex, {INGRESS,EGRESS});
>
> Internally, this will create the sch_clsact qdisc & cls_bpf filter instance
> iff not present yet, and attach to a default prio 1 handle 1, and _always_ in
> direct-action mode. This is /as simple as it gets/ and we don't need to bother
> users with more complex tc/cls_bpf internals unless desired. For example,
> extended APIs could add prio/parent so that multi-prog can be attached to a
> single cls_bpf instance, but even that could be a second step, imho.
>

I am not opposed to clsact qdisc setup if INGRESS/EGRESS is supplied (not sure
how others feel about it).

We could make direct_action mode default, and similarly choose prio
as 1 by default instead of letting the kernel do it. Then you can just pass in
NULL for bpf_tc_cls_opts and be close to what you're proposing. For protocol we
can choose ETH_P_ALL by default too if the user doesn't set it.

With these modifications, the equivalent would look like
	bpf_tc_cls_attach(prog_fd, TC_DEV(ifindex, INGRESS), NULL, &id);

So as long as the user doesn't care about other details, they can just pass opts
as NULL.

WDYT?

> Thanks,
> Daniel

--
Kartikeya