On Wed, Apr 28, 2021 at 04:44:23AM IST, Daniel Borkmann wrote:

On 4/28/21 12:51 AM, Toke Høiland-Jørgensen wrote:

quoted

Daniel Borkmann [off-list ref] writes:

quoted

On 4/28/21 12:36 AM, Toke Høiland-Jørgensen wrote:

quoted

Daniel Borkmann [off-list ref] writes:

[...]

quoted

quoted

Small addendum:

       DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = 42, .which = BPF_TC_INGRESS|BPF_TC_EGRESS);

       err = bpf_tc_hook_create(&hook);
       [...]

... is also possible, of course, and then both bpf_tc_hook_{create,destroy}() are symmetric.

It should be allowed, but it wouldn't actually make any difference which
combination of TC_INGRESS and TC_EGRESS you specify, as long as one of
them is set, right? I.e., we just attach the clsact qdisc in both
cases...

Yes, that is correct, for the bpf_tc_hook_create() whether you pass in BPF_TC_INGRESS,
BPF_TC_EGRESS or BPF_TC_INGRESS|BPF_TC_EGRESS, you'll end up creating clsact qdisc in
either of the three cases. Only the bpf_tc_hook_destroy() differs
between all of them.

Right, just checking. Other than that, I like your proposal; it loses
the "automatic removal of qdisc if we added it" feature, but that's
probably OK: less magic is good. And as long as bpf_tc_hook_create()
returns EEXIST if the qdisc already exists, the caller can do the same
thing if they want.

Yes exactly. Less magic the better, especially given this has global effect.

Everything sounds good. I'll do a resend.

Thanks,
Daniel

--
Kartikeya