All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cong Wang <xiyou.wangcong@gmail.com>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Martin KaFai Lau" <kafai@fb.com>,
	"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"KP Singh" <kpsingh@kernel.org>,
	"Jamal Hadi Salim" <jhs@mojatatu.com>,
	"Vlad Buslov" <vladbu@nvidia.com>,
	"Jiri Pirko" <jiri@resnulli.us>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Joe Stringer" <joe@cilium.io>,
	"Quentin Monnet" <quentin@isovalent.com>,
	"Jesper Dangaard Brouer" <brouer@redhat.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Linux Kernel Network Developers" <netdev@vger.kernel.org>
Subject: Re: [PATCH RFC bpf-next 0/7] Add bpf_link based TC-BPF API
Date: Thu, 10 Jun 2021 19:00:49 -0700	[thread overview]
Message-ID: <CAM_iQpXFmsWhMA-RO2j5Ph5Ak8yJgUVBppGj2_5NS3BuyjkvzQ@mail.gmail.com> (raw)
In-Reply-To: <20210608071908.sos275adj3gunewo@apollo>

On Tue, Jun 8, 2021 at 12:20 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> So we're not really creating a qdisc here, we're just tying the filter (which in
> the current semantics exists only while attached) to the bpf_link. The filter is
> the attachment, so tying its lifetime to bpf_link makes sense. When you destroy
> the bpf_link, the filter goes away too, which means classification at that
> hook (parent/class) in the qdisc stops working. This is why creating the filter
> from the bpf_link made sense to me.

I see why you are creating TC filters now, because you are trying to
force the lifetime of a bpf target to align with the bpf program itself.
The deeper reason seems to be that a cls_bpf filter looks so small
that it appears to you that it has nothing but a bpf_prog, right?

I offer two different views here:

1. If you view a TC filter as an instance as a netdev/qdisc/action, they
are no different from this perspective. Maybe the fact that a TC filter
resides in a qdisc makes a slight difference here, but like I mentioned, it
actually makes sense to let TC filters be standalone, qdisc's just have to
bind with them, like how we bind TC filters with standalone TC actions.
These are all updated independently, despite some of them residing in
another. There should not be an exceptional TC filter which can not
be updated via `tc filter` command.

2. For cls_bpf specifically, it is also an instance, like all other TC filters.
You can update it in the same way: tc filter change [...] The only difference
is a bpf program can attach to such an instance. So you can view the bpf
program attached to cls_bpf as a property of it. From this point of view,
there is no difference with XDP to netdev, where an XDP program
attached to a netdev is also a property of netdev. A netdev can still
function without XDP. Same for cls_bpf, it can be just a nop returns
TC_ACT_SHOT (or whatever) if no ppf program is attached. Thus,
the lifetime of a bpf program can be separated from the target it
attaches too, like all other bpf_link targets. bpf_link is just a
supplement to `tc filter change cls_bpf`, not to replace it.

This is actually simpler, you do not need to worry about whether
netdev is destroyed when you detach the XDP bpf_link anyway,
same for cls_bpf filters. Likewise, TC filters don't need to worry
about bpf_links associated.

Thanks.

  parent reply	other threads:[~2021-06-11  2:01 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-28 19:59 [PATCH RFC bpf-next 0/7] Add bpf_link based TC-BPF API Kumar Kartikeya Dwivedi
2021-05-28 19:59 ` [PATCH RFC bpf-next 1/7] net: sched: refactor cls_bpf creation code Kumar Kartikeya Dwivedi
2021-05-28 19:59 ` [PATCH RFC bpf-next 2/7] bpf: export bpf_link functions for modules Kumar Kartikeya Dwivedi
2021-05-28 19:59 ` [PATCH RFC bpf-next 3/7] net: sched: add bpf_link API for bpf classifier Kumar Kartikeya Dwivedi
2021-05-28 22:37   ` kernel test robot
2021-05-28 23:18   ` kernel test robot
2021-06-02 20:56   ` Andrii Nakryiko
2021-05-28 19:59 ` [PATCH RFC bpf-next 4/7] net: sched: add lightweight update path for cls_bpf Kumar Kartikeya Dwivedi
2021-05-28 19:59 ` [PATCH RFC bpf-next 5/7] tools: bpf.h: sync with kernel sources Kumar Kartikeya Dwivedi
2021-05-28 19:59 ` [PATCH RFC bpf-next 6/7] libbpf: add bpf_link based TC-BPF management API Kumar Kartikeya Dwivedi
2021-06-02 21:03   ` Andrii Nakryiko
2021-05-28 19:59 ` [PATCH RFC bpf-next 7/7] libbpf: add selftest for " Kumar Kartikeya Dwivedi
2021-06-02 21:09 ` [PATCH RFC bpf-next 0/7] Add bpf_link based TC-BPF API Andrii Nakryiko
2021-06-02 21:45   ` Kumar Kartikeya Dwivedi
2021-06-02 23:50     ` Alexei Starovoitov
2021-06-04  6:43       ` Kumar Kartikeya Dwivedi
2021-06-06 23:37 ` Cong Wang
2021-06-07  3:37   ` Kumar Kartikeya Dwivedi
2021-06-07  5:18     ` Cong Wang
2021-06-07  6:07       ` Kumar Kartikeya Dwivedi
2021-06-08  2:00         ` Cong Wang
2021-06-08  7:19           ` Kumar Kartikeya Dwivedi
2021-06-08 15:39             ` Alexei Starovoitov
2021-06-11  2:10               ` Cong Wang
2021-06-11  2:00             ` Cong Wang [this message]
2021-06-13  2:53               ` Kumar Kartikeya Dwivedi
2021-06-13 20:27                 ` Jamal Hadi Salim
2021-06-13 20:34                   ` Kumar Kartikeya Dwivedi
2021-06-13 21:10                     ` Jamal Hadi Salim
2021-06-14 13:03                       ` Marcelo Ricardo Leitner
2021-06-15 23:07                       ` Daniel Borkmann
2021-06-16 14:40                         ` Jamal Hadi Salim
2021-06-16 15:32                           ` Kumar Kartikeya Dwivedi
2021-06-16 16:00                             ` Daniel Borkmann
2021-06-18 11:40                               ` Jamal Hadi Salim
2021-06-18 14:38                                 ` Alexei Starovoitov
2021-06-18 14:50                                   ` Jamal Hadi Salim
2021-06-18 16:23                                     ` Alexei Starovoitov
2021-06-18 16:41                                       ` Jamal Hadi Salim
2021-06-18 22:42                                 ` Daniel Borkmann
2021-06-21 13:55                                   ` Jamal Hadi Salim
2021-06-15  4:33                 ` Cong Wang
2021-06-15 11:54                   ` Toke Høiland-Jørgensen
2021-06-15 23:44                     ` Daniel Borkmann
2021-06-16 12:03                       ` Toke Høiland-Jørgensen
2021-06-16 15:33                       ` Jamal Hadi Salim
2021-06-13  3:08               ` Kumar Kartikeya Dwivedi

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=CAM_iQpXFmsWhMA-RO2j5Ph5Ak8yJgUVBppGj2_5NS3BuyjkvzQ@mail.gmail.com \
    --to=xiyou.wangcong@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=joe@cilium.io \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=quentin@isovalent.com \
    --cc=songliubraving@fb.com \
    --cc=toke@redhat.com \
    --cc=vladbu@nvidia.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.