All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: Andrii Nakryiko <andriin@fb.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org, ast@fb.com,
	daniel@iogearbox.net
Cc: andrii.nakryiko@gmail.com, kernel-team@fb.com
Subject: Re: [PATCH v3 bpf-next 2/9] bpf, xdp: maintain info on attached XDP BPF programs in net_device
Date: Thu, 16 Jul 2020 13:01:23 -0600	[thread overview]
Message-ID: <4cffee3d-6af9-57e6-a2d5-202925ee8e77@gmail.com> (raw)
In-Reply-To: <20200716045602.3896926-3-andriin@fb.com>

On 7/15/20 10:55 PM, Andrii Nakryiko wrote:
> Instead of delegating to drivers, maintain information about which BPF
> programs are attached in which XDP modes (generic/skb, driver, or hardware)
> locally in net_device. This effectively obsoletes XDP_QUERY_PROG command.
> 
> Such re-organization simplifies existing code already. But it also allows to
> further add bpf_link-based XDP attachments without drivers having to know
> about any of this at all, which seems like a good setup.
> XDP_SETUP_PROG/XDP_SETUP_PROG_HW are just low-level commands to driver to
> install/uninstall active BPF program. All the higher-level concerns about
> prog/link interaction will be contained within generic driver-agnostic logic.
> 
> All the XDP_QUERY_PROG calls to driver in dev_xdp_uninstall() were removed.
> It's not clear for me why dev_xdp_uninstall() were passing previous prog_flags
> when resetting installed programs. That seems unnecessary, plus most drivers
> don't populate prog_flags anyways. Having XDP_SETUP_PROG vs XDP_SETUP_PROG_HW
> should be enough of an indicator of what is required of driver to correctly
> reset active BPF program. dev_xdp_uninstall() is also generalized as an
> iteration over all three supported mode.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  include/linux/netdevice.h |  17 +++-
>  net/core/dev.c            | 158 +++++++++++++++++++++-----------------

Similar to my comment on a v1 patch, this change is doing multiple
things that really should be split into 2 patches - one moving code
around and the second making the change you want. As is the patch is
difficult to properly review.

Given that you need a v4 anyways, can you split this patch into 2?

  reply	other threads:[~2020-07-16 19:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16  4:55 [PATCH v3 bpf-next 0/9] BPF XDP link Andrii Nakryiko
2020-07-16  4:55 ` [PATCH v3 bpf-next 1/9] bpf: make bpf_link API available indepently of CONFIG_BPF_SYSCALL Andrii Nakryiko
2020-07-16  4:55 ` [PATCH v3 bpf-next 2/9] bpf, xdp: maintain info on attached XDP BPF programs in net_device Andrii Nakryiko
2020-07-16 19:01   ` David Ahern [this message]
2020-07-16 20:31     ` Andrii Nakryiko
2020-07-22  6:48       ` Andrii Nakryiko
2020-07-16  4:55 ` [PATCH v3 bpf-next 3/9] bpf, xdp: extract commong XDP program attachment logic Andrii Nakryiko
2020-07-16  4:55 ` [PATCH v3 bpf-next 4/9] bpf, xdp: add bpf_link-based XDP attachment API Andrii Nakryiko
2020-07-16  4:55 ` [PATCH v3 bpf-next 5/9] bpf, xdp: implement LINK_UPDATE for BPF XDP link Andrii Nakryiko
2020-07-16  4:55 ` [PATCH v3 bpf-next 6/9] bpf: implement BPF XDP link-specific introspection APIs Andrii Nakryiko
2020-07-16  4:55 ` [PATCH v3 bpf-next 7/9] libbpf: add support for BPF XDP link Andrii Nakryiko
2020-07-16  4:56 ` [PATCH v3 bpf-next 8/9] selftests/bpf: add BPF XDP link selftests Andrii Nakryiko
2020-07-16  4:56 ` [PATCH v3 bpf-next 9/9] bpf, xdp: remove XDP_QUERY_PROG and XDP_QUERY_PROG_HW XDP commands Andrii Nakryiko
2020-07-16 15:22   ` Jakub Kicinski
2020-07-16 17:00     ` 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=4cffee3d-6af9-57e6-a2d5-202925ee8e77@gmail.com \
    --to=dsahern@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    /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.