public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat•com>
To: Daniel Borkmann <daniel@iogearbox•net>, bpf@vger•kernel.org
Cc: razor@blackwall•org, ast@kernel•org, andrii@kernel•org,
	martin.lau@linux•dev, john.fastabend@gmail•com,
	joannelkoong@gmail•com, memxor@gmail•com, joe@cilium•io,
	netdev@vger•kernel.org
Subject: Re: [PATCH bpf-next 01/10] bpf: Add initial fd-based API to attach tc BPF programs
Date: Wed, 05 Oct 2022 16:32:22 +0200	[thread overview]
Message-ID: <87wn9egx3d.fsf@toke.dk> (raw)
In-Reply-To: <3cc8a0c3-7767-12cf-f753-82e2df8ef293@iogearbox.net>

Daniel Borkmann <daniel@iogearbox•net> writes:

> On 10/5/22 12:33 PM, Toke Høiland-Jørgensen wrote:
>> Daniel Borkmann <daniel@iogearbox•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)
>
> Hehe, yeah agree, I don't have a strong opinion, but tcx (or just sticking
> with tc) is fully okay to me.

Either is fine with me; I don't have any strong opinions either, other
than "not XTC" ;)

>> [...]
>> 
>>> +/* (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?

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

-Toke


  reply	other threads:[~2022-10-05 14:32 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-04 23:11 [PATCH bpf-next 00/10] BPF link support for tc BPF programs Daniel Borkmann
2022-10-04 23:11 ` [PATCH bpf-next 01/10] bpf: Add initial fd-based API to attach " Daniel Borkmann
2022-10-05  0:55   ` sdf
2022-10-05 10:50     ` Toke Høiland-Jørgensen
2022-10-05 14:48       ` Daniel Borkmann
2022-10-05 12:35     ` Daniel Borkmann
2022-10-05 17:56       ` sdf
2022-10-05 18:21         ` Daniel Borkmann
2022-10-05 10:33   ` Toke Høiland-Jørgensen
2022-10-05 12:47     ` Daniel Borkmann
2022-10-05 14:32       ` Toke Høiland-Jørgensen [this message]
2022-10-05 14:53         ` Daniel Borkmann
2022-10-05 19:04   ` Jamal Hadi Salim
2022-10-06 20:49     ` Daniel Borkmann
2022-10-07 15:36       ` Jamal Hadi Salim
2022-10-06  0:22   ` Andrii Nakryiko
2022-10-06  5:00   ` Alexei Starovoitov
2022-10-06 14:40     ` Jamal Hadi Salim
2022-10-06 23:29       ` Alexei Starovoitov
2022-10-07 15:43         ` Jamal Hadi Salim
2022-10-06 21:29     ` Daniel Borkmann
2022-10-06 23:28       ` Alexei Starovoitov
2022-10-07 13:26         ` Daniel Borkmann
2022-10-07 14:32           ` Toke Høiland-Jørgensen
2022-10-07 16:55             ` sdf
2022-10-07 17:20               ` Toke Høiland-Jørgensen
2022-10-07 18:11                 ` sdf
2022-10-07 19:06                   ` Daniel Borkmann
2022-10-07 18:59                 ` Alexei Starovoitov
2022-10-07 19:37                   ` Daniel Borkmann
2022-10-07 22:45                     ` sdf
2022-10-07 23:41                       ` Alexei Starovoitov
2022-10-07 23:34                     ` Alexei Starovoitov
2022-10-08 11:38                       ` Toke Høiland-Jørgensen
2022-10-08 20:38                         ` Alexei Starovoitov
2022-10-13 18:30                           ` Andrii Nakryiko
2022-10-14 15:38                             ` Alexei Starovoitov
2022-10-27  9:01                               ` Daniel Xu
2022-10-06 20:15   ` Martin KaFai Lau
2022-10-06 20:54   ` Martin KaFai Lau
2022-10-04 23:11 ` [PATCH bpf-next 02/10] bpf: Implement BPF link handling for " Daniel Borkmann
2022-10-06  3:19   ` Andrii Nakryiko
2022-10-06 20:54     ` Daniel Borkmann
2022-10-06 17:56   ` Martin KaFai Lau
2022-10-06 20:10   ` Martin KaFai Lau
2022-10-04 23:11 ` [PATCH bpf-next 03/10] bpf: Implement link update for tc BPF link programs Daniel Borkmann
2022-10-06  3:19   ` Andrii Nakryiko
2022-10-04 23:11 ` [PATCH bpf-next 04/10] bpf: Implement link introspection " Daniel Borkmann
2022-10-06  3:19   ` Andrii Nakryiko
2022-10-06 23:14   ` Martin KaFai Lau
2022-10-04 23:11 ` [PATCH bpf-next 05/10] bpf: Implement link detach " Daniel Borkmann
2022-10-06  3:19   ` Andrii Nakryiko
2022-10-06 23:24   ` Martin KaFai Lau
2022-10-04 23:11 ` [PATCH bpf-next 06/10] libbpf: Change signature of bpf_prog_query Daniel Borkmann
2022-10-06  3:19   ` Andrii Nakryiko
2022-10-04 23:11 ` [PATCH bpf-next 07/10] libbpf: Add extended attach/detach opts Daniel Borkmann
2022-10-06  3:19   ` Andrii Nakryiko
2022-10-04 23:11 ` [PATCH bpf-next 08/10] libbpf: Add support for BPF tc link Daniel Borkmann
2022-10-06  3:19   ` Andrii Nakryiko
2022-10-04 23:11 ` [PATCH bpf-next 09/10] bpftool: Add support for tc fd-based attach types Daniel Borkmann
2022-10-04 23:11 ` [PATCH bpf-next 10/10] bpf, selftests: Add various BPF tc link selftests Daniel Borkmann
2022-10-06  3:19   ` Andrii Nakryiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wn9egx3d.fsf@toke.dk \
    --to=toke@redhat$(echo .)com \
    --cc=andrii@kernel$(echo .)org \
    --cc=ast@kernel$(echo .)org \
    --cc=bpf@vger$(echo .)kernel.org \
    --cc=daniel@iogearbox$(echo .)net \
    --cc=joannelkoong@gmail$(echo .)com \
    --cc=joe@cilium$(echo .)io \
    --cc=john.fastabend@gmail$(echo .)com \
    --cc=martin.lau@linux$(echo .)dev \
    --cc=memxor@gmail$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=razor@blackwall$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox