netdev - Re: [PATCH bpf-next 01/10] bpf: Add initial fd-based API to attach tc BPF programs

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 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]

Date:   Wed, 05 Oct 2022 12:33:29 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Daniel Borkmann <daniel@...earbox.net>, bpf@...r.kernel.org
Cc:     razor@...ckwall.org, ast@...nel.org, andrii@...nel.org,
        martin.lau@...ux.dev, john.fastabend@...il.com,
        joannelkoong@...il.com, memxor@...il.com, joe@...ium.io,
        netdev@...r.kernel.org, Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH bpf-next 01/10] bpf: Add initial fd-based API to attach
 tc BPF programs

Daniel Borkmann <daniel@...earbox.net> writes:

> As part of the feedback from LPC, there was a suggestion to provide a
> name for this infrastructure to more easily differ between the classic
> cls_bpf attachment and the fd-based API. As for most, the XDP vs tc
> layer is already the default mental model for the pkt processing
> pipeline. We refactored this with an xtc internal prefix aka 'express
> traffic control' in order to avoid to deviate too far (and 'express'
> given its more lightweight/faster entry point).

Woohoo, bikeshed time! :)

I am OK with having a separate name for this, but can we please pick one
that doesn't sound like 'XDP' when you say it out loud? You really don't
have to mumble much for 'XDP' and 'XTC' to sound exactly alike; this is
bound to lead to confusion!

Alternatives, in the same vein:
- ltc (lightweight)
- etc (extended/express/ebpf/et cetera ;))
- tcx (keep the cool X, but put it at the end)

[...]

> +/* (Simplified) user return codes for tc prog type.
> + * A valid tc program must return one of these defined values. All other
> + * return codes are reserved for future use. Must remain compatible with
> + * their TC_ACT_* counter-parts. For compatibility in behavior, unknown
> + * return codes are mapped to TC_NEXT.
> + */
> +enum tc_action_base {
> +	TC_NEXT		= -1,
> +	TC_PASS		= 0,
> +	TC_DROP		= 2,
> +	TC_REDIRECT	= 7,
> +};

Looking at things like this, though, I wonder if having a separate name
(at least if it's too prominent) is not just going to be more confusing
than not? I.e., we go out of our way to make it compatible with existing
TC-BPF programs (which is a good thing!), so do we really need a
separate name? Couldn't it just be an implementation detail that "it's
faster now"?

Oh, and speaking of compatibility should 'tc' (the iproute2 binary) be
taught how to display these new bpf_link attachments so that users can
see that they're there?

-Toke