From: Jakub Kicinski <jakub.kicinski@netronome•com>
To: Alexei Starovoitov <alexei.starovoitov@gmail•com>
Cc: daniel@iogearbox•net, oss-drivers@netronome•com, netdev@vger•kernel.org
Subject: Re: [PATCH bpf-next 6/9] bpf: offload: allow program and map sharing per-ASIC
Date: Mon, 16 Jul 2018 20:18:54 -0700 [thread overview]
Message-ID: <20180716201854.603cb2d8@cakuba.lan> (raw)
In-Reply-To: <20180717030046.4ylnjs3lkdar4uld@ast-mbp.dhcp.thefacebook.com>
On Mon, 16 Jul 2018 20:00:47 -0700, Alexei Starovoitov wrote:
> On Mon, Jul 16, 2018 at 07:37:20PM -0700, Jakub Kicinski wrote:
> > Create a higher-level entity to represent a device/ASIC to allow
> > programs and maps to be shared between device ports. The extra
> > work is required to make sure we don't destroy BPF objects as
> > soon as the netdev for which they were loaded gets destroyed,
> > as other ports may still be using them. When netdev goes away
> > all of its BPF objects will be moved to other netdevs of the
> > device, and only destroyed when last netdev is unregistered.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome•com>
> > Reviewed-by: Quentin Monnet <quentin.monnet@netronome•com>
> ..
> > -bool bpf_offload_dev_match(struct bpf_prog *prog, struct bpf_map *map)
> > +static bool __bpf_offload_dev_match(struct bpf_prog *prog,
> > + struct net_device *netdev)
> > {
> > - struct bpf_offloaded_map *offmap;
> > + struct bpf_offload_netdev *ondev1, *ondev2;
> > struct bpf_prog_offload *offload;
> > - bool ret;
> >
> > if (!bpf_prog_is_dev_bound(prog->aux))
> > return false;
> > - if (!bpf_map_is_dev_bound(map))
> > - return bpf_map_offload_neutral(map);
> >
> > - down_read(&bpf_devs_lock);
> > offload = prog->aux->offload;
> > + if (!offload)
> > + return false;
> > + if (offload->netdev == netdev)
> > + return true;
> > +
> > + ondev1 = bpf_offload_find_netdev(offload->netdev);
> > + ondev2 = bpf_offload_find_netdev(netdev);
> > +
> > + return ondev1 && ondev2 && ondev1->offdev == ondev2->offdev;
> > +}
> > +
> > +bool bpf_offload_dev_match(struct bpf_prog *prog, struct net_device *netdev)
> > +{
> > + bool ret;
> > +
> > + down_read(&bpf_devs_lock);
> > + ret = __bpf_offload_dev_match(prog, netdev);
> > + up_read(&bpf_devs_lock);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(bpf_offload_dev_match);
> > +
> > +bool bpf_offload_match(struct bpf_prog *prog, struct bpf_map *map)
> > +{
> > + struct bpf_offloaded_map *offmap;
> > + bool ret;
> > +
> > + if (!bpf_map_is_dev_bound(map))
> > + return bpf_map_offload_neutral(map);
> > offmap = map_to_offmap(map);
> >
> > - ret = offload && offload->netdev == offmap->netdev;
> > + down_read(&bpf_devs_lock);
> > + ret = __bpf_offload_dev_match(prog, offmap->netdev);
> > up_read(&bpf_devs_lock);
> >
> > return ret;
> > }
> >
> ..
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 9e2bf834f13a..2c5b923eef75 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -5054,7 +5054,7 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
> > }
> >
> > if ((bpf_prog_is_dev_bound(prog->aux) || bpf_map_is_dev_bound(map)) &&
> > - !bpf_offload_dev_match(prog, map)) {
> > + !bpf_offload_match(prog, map)) {
>
> I'm confused with new names and renaming.
> May be split renaming into separate patch?
> Should new bpf_offload_match() be called bpf_offload_prog_map_match() ?
> or some other name?
> May be adding comments to these functions will make it clear...
It is messy. The new functions to register/unregister ASIC are called
bpf_offload_dev_*, hence it seemed like a good idea to call the
function exported to the drivers bpf_offload_dev_match() (see patches 7
and 8) to keep the driver API consistent. But then the old function
which is only used by the verivier has to be renamed.
I will use bpf_offload_prog_map_match() and split to a separate patch.
next prev parent reply other threads:[~2018-07-17 3:49 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-17 2:37 [PATCH bpf-next 0/9] bpf: offload program and map sharing Jakub Kicinski
2018-07-17 2:37 ` [PATCH bpf-next 1/9] netdevsim: add switch_id attribute Jakub Kicinski
2018-07-17 2:37 ` [PATCH bpf-next 2/9] netdevsim: add shared netdevsim devices Jakub Kicinski
2018-07-17 2:37 ` [PATCH bpf-next 3/9] netdevsim: associate bound programs with shared dev Jakub Kicinski
2018-07-17 2:37 ` [PATCH bpf-next 4/9] nfp: add .ndo_init() and .ndo_uninit() callbacks Jakub Kicinski
2018-07-17 2:37 ` [PATCH bpf-next 5/9] bpf: offload: aggregate offloads per-device Jakub Kicinski
2018-07-17 2:37 ` [PATCH bpf-next 6/9] bpf: offload: allow program and map sharing per-ASIC Jakub Kicinski
2018-07-17 3:00 ` Alexei Starovoitov
2018-07-17 3:18 ` Jakub Kicinski [this message]
2018-07-17 2:37 ` [PATCH bpf-next 7/9] netdevsim: allow program sharing between devices Jakub Kicinski
2018-07-17 2:37 ` [PATCH bpf-next 8/9] nfp: bpf: allow program sharing within ASIC Jakub Kicinski
2018-07-17 2:37 ` [PATCH bpf-next 9/9] selftests/bpf: add test for sharing objects between netdevs Jakub Kicinski
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=20180716201854.603cb2d8@cakuba.lan \
--to=jakub.kicinski@netronome$(echo .)com \
--cc=alexei.starovoitov@gmail$(echo .)com \
--cc=daniel@iogearbox$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=oss-drivers@netronome$(echo .)com \
/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