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.kicin...@netronome.com>
> > Reviewed-by: Quentin Monnet <quentin.mon...@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.
