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