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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/5/22 4:32 PM, Toke Høiland-Jørgensen wrote:
Daniel Borkmann <daniel@xxxxxxxxxxxxx> writes:
On 10/5/22 12:33 PM, Toke Høiland-Jørgensen wrote:
Daniel Borkmann <daniel@xxxxxxxxxxxxx> writes:
[...]
+/* (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"?

Yep, faster is an implementation detail; and developers can stick to existing
opcodes. I added this here given Andrii suggested to add the action codes as
enum so they land in vmlinux BTF. My thinking was that if we go this route,
we could also make them more user friendly. This part is 100% optional,
but for new developers it might lower the barrier a bit I was hoping given
it makes it clear which subset of actions BPF supports explicitly and with
less cryptic name.

Oh, I didn't mean that we shouldn't define these helpers; that's totally
fine, and probably useful. Just that when everything is named 'TC'
anyway, having a different name (like TCX) is maybe not that important
anyway?

I thought about this initially, but then also it has nothing to do with tcx
given it can just as well be used on both old/new style attachments, thus
wanted to avoid potential confusion around this.

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?

Sounds reasonable, I can follow-up with the iproute2 support as well.

Cool!

Thanks,
Daniel



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux