All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands
@ 2018-11-09 13:03 Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 1/9] nfp: bpf: move nfp_bpf_analyzer_ops from verifier.c to offload.c Quentin Monnet
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

For passing device functions for offloaded eBPF programs, there used to
be no place where to store the pointer without making the non-offloaded
programs pay a memory price.

As a consequence, three functions were called with ndo_bpf() through
specific commands. Now that we have struct bpf_offload_dev, and since none
of those operations rely on RTNL, we can turn these three commands into
hooks inside the struct bpf_prog_offload_ops, and pass them as part of
bpf_offload_dev_create().

This patch set changes the offload architecture to do so, and brings the
relevant changes to the nfp and netdevsim drivers.

Quentin Monnet (9):
  nfp: bpf: move nfp_bpf_analyzer_ops from verifier.c to offload.c
  bpf: pass a struct with offload callbacks to bpf_offload_dev_create()
  bpf: call verify_insn from its callback in struct bpf_offload_dev
  bpf: call finalize() from its callback in struct bpf_offload_dev
  bpf: call verifier_prep from its callback in struct bpf_offload_dev
  bpf: pass translate() as a callback and remove its ndo_bpf subcommand
  bpf: pass destroy() as a callback and remove its ndo_bpf subcommand
  bpf: pass prog instead of env to bpf_prog_offload_verifier_prep()
  bpf: do not pass netdev to translate() and prepare() offload callbacks

 drivers/net/ethernet/netronome/nfp/bpf/main.c |  2 +-
 drivers/net/ethernet/netronome/nfp/bpf/main.h |  6 +-
 .../net/ethernet/netronome/nfp/bpf/offload.c  | 34 ++++-----
 .../net/ethernet/netronome/nfp/bpf/verifier.c | 11 +--
 drivers/net/netdevsim/bpf.c                   | 51 +++++++------
 include/linux/bpf.h                           |  8 +-
 include/linux/bpf_verifier.h                  |  2 +-
 include/linux/netdevice.h                     | 12 ---
 kernel/bpf/offload.c                          | 75 +++++++------------
 kernel/bpf/verifier.c                         |  2 +-
 10 files changed, 85 insertions(+), 118 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 1/9] nfp: bpf: move nfp_bpf_analyzer_ops from verifier.c to offload.c
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 2/9] bpf: pass a struct with offload callbacks to bpf_offload_dev_create() Quentin Monnet
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

We are about to add several new callbacks to the struct, all of them
defined in offload.c. Move the struct bpf_prog_offload_ops object in
that file. As a consequence, nfp_verify_insn() and nfp_finalize() can no
longer be static.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/main.h     |  4 ++++
 drivers/net/ethernet/netronome/nfp/bpf/offload.c  |  5 +++++
 drivers/net/ethernet/netronome/nfp/bpf/verifier.c | 11 +++--------
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 7f591d71ab28..abdd93d14439 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -509,6 +509,10 @@ void nfp_bpf_jit_prepare(struct nfp_prog *nfp_prog, unsigned int cnt);
 int nfp_bpf_jit(struct nfp_prog *prog);
 bool nfp_bpf_supported_opcode(u8 code);
 
+int nfp_verify_insn(struct bpf_verifier_env *env, int insn_idx,
+		    int prev_insn_idx);
+int nfp_bpf_finalize(struct bpf_verifier_env *env);
+
 extern const struct bpf_prog_offload_ops nfp_bpf_analyzer_ops;
 
 struct netdev_bpf;
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 07bdc1f61996..dc548bb4089e 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -601,3 +601,8 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
 
 	return 0;
 }
+
+const struct bpf_prog_offload_ops nfp_bpf_analyzer_ops = {
+	.insn_hook	= nfp_verify_insn,
+	.finalize	= nfp_bpf_finalize,
+};
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
index 99f977bfd8cc..337bb862ec1d 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
@@ -623,8 +623,8 @@ nfp_bpf_check_alu(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
 	return 0;
 }
 
-static int
-nfp_verify_insn(struct bpf_verifier_env *env, int insn_idx, int prev_insn_idx)
+int nfp_verify_insn(struct bpf_verifier_env *env, int insn_idx,
+		    int prev_insn_idx)
 {
 	struct nfp_prog *nfp_prog = env->prog->aux->offload->dev_priv;
 	struct nfp_insn_meta *meta = nfp_prog->verifier_meta;
@@ -745,7 +745,7 @@ nfp_bpf_get_stack_usage(struct nfp_prog *nfp_prog, unsigned int cnt)
 	goto continue_subprog;
 }
 
-static int nfp_bpf_finalize(struct bpf_verifier_env *env)
+int nfp_bpf_finalize(struct bpf_verifier_env *env)
 {
 	struct bpf_subprog_info *info;
 	struct nfp_prog *nfp_prog;
@@ -788,8 +788,3 @@ static int nfp_bpf_finalize(struct bpf_verifier_env *env)
 
 	return 0;
 }
-
-const struct bpf_prog_offload_ops nfp_bpf_analyzer_ops = {
-	.insn_hook	= nfp_verify_insn,
-	.finalize	= nfp_bpf_finalize,
-};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 2/9] bpf: pass a struct with offload callbacks to bpf_offload_dev_create()
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 1/9] nfp: bpf: move nfp_bpf_analyzer_ops from verifier.c to offload.c Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 3/9] bpf: call verify_insn from its callback in struct bpf_offload_dev Quentin Monnet
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

For passing device functions for offloaded eBPF programs, there used to
be no place where to store the pointer without making the non-offloaded
programs pay a memory price.

As a consequence, three functions were called with ndo_bpf() through
specific commands. Now that we have struct bpf_offload_dev, and since
none of those operations rely on RTNL, we can turn these three commands
into hooks inside the struct bpf_prog_offload_ops, and pass them as part
of bpf_offload_dev_create().

This commit effectively passes a pointer to the struct to
bpf_offload_dev_create(). We temporarily have two struct
bpf_prog_offload_ops instances, one under offdev->ops and one under
offload->dev_ops. The next patches will make the transition towards the
former, so that offload->dev_ops can be removed, and callbacks relying
on ndo_bpf() added to offdev->ops as well.

While at it, rename "nfp_bpf_analyzer_ops" as "nfp_bpf_dev_ops" (and
similarly for netdevsim).

Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/main.c    | 2 +-
 drivers/net/ethernet/netronome/nfp/bpf/main.h    | 2 +-
 drivers/net/ethernet/netronome/nfp/bpf/offload.c | 4 ++--
 drivers/net/netdevsim/bpf.c                      | 6 +++---
 include/linux/bpf.h                              | 3 ++-
 kernel/bpf/offload.c                             | 5 ++++-
 6 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index 6243af0ab025..dccae0319204 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -465,7 +465,7 @@ static int nfp_bpf_init(struct nfp_app *app)
 		app->ctrl_mtu = nfp_bpf_ctrl_cmsg_mtu(bpf);
 	}
 
-	bpf->bpf_dev = bpf_offload_dev_create();
+	bpf->bpf_dev = bpf_offload_dev_create(&nfp_bpf_dev_ops);
 	err = PTR_ERR_OR_ZERO(bpf->bpf_dev);
 	if (err)
 		goto err_free_neutral_maps;
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index abdd93d14439..941277936475 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -513,7 +513,7 @@ int nfp_verify_insn(struct bpf_verifier_env *env, int insn_idx,
 		    int prev_insn_idx);
 int nfp_bpf_finalize(struct bpf_verifier_env *env);
 
-extern const struct bpf_prog_offload_ops nfp_bpf_analyzer_ops;
+extern const struct bpf_prog_offload_ops nfp_bpf_dev_ops;
 
 struct netdev_bpf;
 struct nfp_app;
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index dc548bb4089e..2fca996a7e77 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -209,7 +209,7 @@ nfp_bpf_verifier_prep(struct nfp_app *app, struct nfp_net *nn,
 		goto err_free;
 
 	nfp_prog->verifier_meta = nfp_prog_first_meta(nfp_prog);
-	bpf->verifier.ops = &nfp_bpf_analyzer_ops;
+	bpf->verifier.ops = &nfp_bpf_dev_ops;
 
 	return 0;
 
@@ -602,7 +602,7 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
 	return 0;
 }
 
-const struct bpf_prog_offload_ops nfp_bpf_analyzer_ops = {
+const struct bpf_prog_offload_ops nfp_bpf_dev_ops = {
 	.insn_hook	= nfp_verify_insn,
 	.finalize	= nfp_bpf_finalize,
 };
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index cb3518474f0e..135aee864162 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -91,7 +91,7 @@ static int nsim_bpf_finalize(struct bpf_verifier_env *env)
 	return 0;
 }
 
-static const struct bpf_prog_offload_ops nsim_bpf_analyzer_ops = {
+static const struct bpf_prog_offload_ops nsim_bpf_dev_ops = {
 	.insn_hook	= nsim_bpf_verify_insn,
 	.finalize	= nsim_bpf_finalize,
 };
@@ -547,7 +547,7 @@ int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 		if (err)
 			return err;
 
-		bpf->verifier.ops = &nsim_bpf_analyzer_ops;
+		bpf->verifier.ops = &nsim_bpf_dev_ops;
 		return 0;
 	case BPF_OFFLOAD_TRANSLATE:
 		state = bpf->offload.prog->aux->offload->dev_priv;
@@ -599,7 +599,7 @@ int nsim_bpf_init(struct netdevsim *ns)
 		if (IS_ERR_OR_NULL(ns->sdev->ddir_bpf_bound_progs))
 			return -ENOMEM;
 
-		ns->sdev->bpf_dev = bpf_offload_dev_create();
+		ns->sdev->bpf_dev = bpf_offload_dev_create(&nsim_bpf_dev_ops);
 		err = PTR_ERR_OR_ZERO(ns->sdev->bpf_dev);
 		if (err)
 			return err;
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b6a296e01f6a..c0197c37b2b2 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -692,7 +692,8 @@ int bpf_map_offload_get_next_key(struct bpf_map *map,
 
 bool bpf_offload_prog_map_match(struct bpf_prog *prog, struct bpf_map *map);
 
-struct bpf_offload_dev *bpf_offload_dev_create(void);
+struct bpf_offload_dev *
+bpf_offload_dev_create(const struct bpf_prog_offload_ops *ops);
 void bpf_offload_dev_destroy(struct bpf_offload_dev *offdev);
 int bpf_offload_dev_netdev_register(struct bpf_offload_dev *offdev,
 				    struct net_device *netdev);
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 8e93c47f0779..d513fbf9ca53 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -33,6 +33,7 @@
 static DECLARE_RWSEM(bpf_devs_lock);
 
 struct bpf_offload_dev {
+	const struct bpf_prog_offload_ops *ops;
 	struct list_head netdevs;
 };
 
@@ -655,7 +656,8 @@ void bpf_offload_dev_netdev_unregister(struct bpf_offload_dev *offdev,
 }
 EXPORT_SYMBOL_GPL(bpf_offload_dev_netdev_unregister);
 
-struct bpf_offload_dev *bpf_offload_dev_create(void)
+struct bpf_offload_dev *
+bpf_offload_dev_create(const struct bpf_prog_offload_ops *ops)
 {
 	struct bpf_offload_dev *offdev;
 	int err;
@@ -673,6 +675,7 @@ struct bpf_offload_dev *bpf_offload_dev_create(void)
 	if (!offdev)
 		return ERR_PTR(-ENOMEM);
 
+	offdev->ops = ops;
 	INIT_LIST_HEAD(&offdev->netdevs);
 
 	return offdev;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 3/9] bpf: call verify_insn from its callback in struct bpf_offload_dev
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 1/9] nfp: bpf: move nfp_bpf_analyzer_ops from verifier.c to offload.c Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 2/9] bpf: pass a struct with offload callbacks to bpf_offload_dev_create() Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 4/9] bpf: call finalize() " Quentin Monnet
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

We intend to remove the dev_ops in struct bpf_prog_offload, and to only
keep the ops in struct bpf_offload_dev instead, which is accessible from
more locations for passing function pointers.

But dev_ops is used for calling the verify_insn hook. Switch to the
newly added ops in struct bpf_prog_offload instead.

To avoid table lookups for each eBPF instruction to verify, we remember
the offdev attached to a netdev and modify bpf_offload_find_netdev() to
avoid performing more than once a lookup for a given offload object.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/linux/bpf.h  | 1 +
 kernel/bpf/offload.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c0197c37b2b2..672714cd904f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -273,6 +273,7 @@ struct bpf_prog_offload_ops {
 struct bpf_prog_offload {
 	struct bpf_prog		*prog;
 	struct net_device	*netdev;
+	struct bpf_offload_dev	*offdev;
 	void			*dev_priv;
 	struct list_head	offloads;
 	bool			dev_state;
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index d513fbf9ca53..2cd3c0d0417b 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -107,6 +107,7 @@ int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr)
 		err = -EINVAL;
 		goto err_unlock;
 	}
+	offload->offdev = ondev->offdev;
 	prog->aux->offload = offload;
 	list_add_tail(&offload->offloads, &ondev->progs);
 	dev_put(offload->netdev);
@@ -167,7 +168,8 @@ int bpf_prog_offload_verify_insn(struct bpf_verifier_env *env,
 	down_read(&bpf_devs_lock);
 	offload = env->prog->aux->offload;
 	if (offload)
-		ret = offload->dev_ops->insn_hook(env, insn_idx, prev_insn_idx);
+		ret = offload->offdev->ops->insn_hook(env, insn_idx,
+						      prev_insn_idx);
 	up_read(&bpf_devs_lock);
 
 	return ret;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 4/9] bpf: call finalize() from its callback in struct bpf_offload_dev
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
                   ` (2 preceding siblings ...)
  2018-11-09 13:03 ` [PATCH bpf-next 3/9] bpf: call verify_insn from its callback in struct bpf_offload_dev Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 5/9] bpf: call verifier_prep " Quentin Monnet
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

In a way similar to the change previously brought to the verify_insn
hook, switch to the newly added ops in struct bpf_prog_offload for
calling the functions used to perform final verification steps for
offloaded programs.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 kernel/bpf/offload.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 2cd3c0d0417b..2c88cb4ddfd8 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -183,8 +183,8 @@ int bpf_prog_offload_finalize(struct bpf_verifier_env *env)
 	down_read(&bpf_devs_lock);
 	offload = env->prog->aux->offload;
 	if (offload) {
-		if (offload->dev_ops->finalize)
-			ret = offload->dev_ops->finalize(env);
+		if (offload->offdev->ops->finalize)
+			ret = offload->offdev->ops->finalize(env);
 		else
 			ret = 0;
 	}
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 5/9] bpf: call verifier_prep from its callback in struct bpf_offload_dev
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
                   ` (3 preceding siblings ...)
  2018-11-09 13:03 ` [PATCH bpf-next 4/9] bpf: call finalize() " Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 6/9] bpf: pass translate() as a callback and remove its ndo_bpf subcommand Quentin Monnet
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

In a way similar to the change previously brought to the verify_insn
hook and to the finalize callback, switch to the newly added ops in
struct bpf_prog_offload for calling the functions used to prepare driver
verifiers.

Since the dev_ops pointer in struct bpf_prog_offload is no longer used
by any callback, we can now remove it from struct bpf_prog_offload.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/bpf/offload.c  | 11 +++----
 drivers/net/netdevsim/bpf.c                   | 32 ++++++++++---------
 include/linux/bpf.h                           |  2 +-
 include/linux/netdevice.h                     |  6 ----
 kernel/bpf/offload.c                          | 22 ++++++-------
 5 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 2fca996a7e77..16a3a9c55852 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -188,10 +188,11 @@ static void nfp_prog_free(struct nfp_prog *nfp_prog)
 }
 
 static int
-nfp_bpf_verifier_prep(struct nfp_app *app, struct nfp_net *nn,
-		      struct netdev_bpf *bpf)
+nfp_bpf_verifier_prep(struct net_device *netdev, struct bpf_verifier_env *env)
 {
-	struct bpf_prog *prog = bpf->verifier.prog;
+	struct nfp_net *nn = netdev_priv(netdev);
+	struct bpf_prog *prog = env->prog;
+	struct nfp_app *app = nn->app;
 	struct nfp_prog *nfp_prog;
 	int ret;
 
@@ -209,7 +210,6 @@ nfp_bpf_verifier_prep(struct nfp_app *app, struct nfp_net *nn,
 		goto err_free;
 
 	nfp_prog->verifier_meta = nfp_prog_first_meta(nfp_prog);
-	bpf->verifier.ops = &nfp_bpf_dev_ops;
 
 	return 0;
 
@@ -422,8 +422,6 @@ nfp_bpf_map_free(struct nfp_app_bpf *bpf, struct bpf_offloaded_map *offmap)
 int nfp_ndo_bpf(struct nfp_app *app, struct nfp_net *nn, struct netdev_bpf *bpf)
 {
 	switch (bpf->command) {
-	case BPF_OFFLOAD_VERIFIER_PREP:
-		return nfp_bpf_verifier_prep(app, nn, bpf);
 	case BPF_OFFLOAD_TRANSLATE:
 		return nfp_bpf_translate(nn, bpf->offload.prog);
 	case BPF_OFFLOAD_DESTROY:
@@ -605,4 +603,5 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
 const struct bpf_prog_offload_ops nfp_bpf_dev_ops = {
 	.insn_hook	= nfp_verify_insn,
 	.finalize	= nfp_bpf_finalize,
+	.prepare	= nfp_bpf_verifier_prep,
 };
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 135aee864162..d045b7d666d9 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -91,11 +91,6 @@ static int nsim_bpf_finalize(struct bpf_verifier_env *env)
 	return 0;
 }
 
-static const struct bpf_prog_offload_ops nsim_bpf_dev_ops = {
-	.insn_hook	= nsim_bpf_verify_insn,
-	.finalize	= nsim_bpf_finalize,
-};
-
 static bool nsim_xdp_offload_active(struct netdevsim *ns)
 {
 	return ns->xdp_hw.prog;
@@ -263,6 +258,17 @@ static int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
 	return 0;
 }
 
+static int
+nsim_bpf_verifier_prep(struct net_device *dev, struct bpf_verifier_env *env)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	if (!ns->bpf_bind_accept)
+		return -EOPNOTSUPP;
+
+	return nsim_bpf_create_prog(ns, env->prog);
+}
+
 static void nsim_bpf_destroy_prog(struct bpf_prog *prog)
 {
 	struct nsim_bpf_bound_prog *state;
@@ -275,6 +281,12 @@ static void nsim_bpf_destroy_prog(struct bpf_prog *prog)
 	kfree(state);
 }
 
+static const struct bpf_prog_offload_ops nsim_bpf_dev_ops = {
+	.insn_hook	= nsim_bpf_verify_insn,
+	.finalize	= nsim_bpf_finalize,
+	.prepare	= nsim_bpf_verifier_prep,
+};
+
 static int nsim_setup_prog_checks(struct netdevsim *ns, struct netdev_bpf *bpf)
 {
 	if (bpf->prog && bpf->prog->aux->offload) {
@@ -539,16 +551,6 @@ int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 	ASSERT_RTNL();
 
 	switch (bpf->command) {
-	case BPF_OFFLOAD_VERIFIER_PREP:
-		if (!ns->bpf_bind_accept)
-			return -EOPNOTSUPP;
-
-		err = nsim_bpf_create_prog(ns, bpf->verifier.prog);
-		if (err)
-			return err;
-
-		bpf->verifier.ops = &nsim_bpf_dev_ops;
-		return 0;
 	case BPF_OFFLOAD_TRANSLATE:
 		state = bpf->offload.prog->aux->offload->dev_priv;
 
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 672714cd904f..f250494a4f56 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -268,6 +268,7 @@ struct bpf_prog_offload_ops {
 	int (*insn_hook)(struct bpf_verifier_env *env,
 			 int insn_idx, int prev_insn_idx);
 	int (*finalize)(struct bpf_verifier_env *env);
+	int (*prepare)(struct net_device *netdev, struct bpf_verifier_env *env);
 };
 
 struct bpf_prog_offload {
@@ -277,7 +278,6 @@ struct bpf_prog_offload {
 	void			*dev_priv;
 	struct list_head	offloads;
 	bool			dev_state;
-	const struct bpf_prog_offload_ops *dev_ops;
 	void			*jited_image;
 	u32			jited_len;
 };
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 857f8abf7b91..0fa2c2744928 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -863,7 +863,6 @@ enum bpf_netdev_command {
 	XDP_QUERY_PROG,
 	XDP_QUERY_PROG_HW,
 	/* BPF program for offload callbacks, invoked at program load time. */
-	BPF_OFFLOAD_VERIFIER_PREP,
 	BPF_OFFLOAD_TRANSLATE,
 	BPF_OFFLOAD_DESTROY,
 	BPF_OFFLOAD_MAP_ALLOC,
@@ -891,11 +890,6 @@ struct netdev_bpf {
 			/* flags with which program was installed */
 			u32 prog_flags;
 		};
-		/* BPF_OFFLOAD_VERIFIER_PREP */
-		struct {
-			struct bpf_prog *prog;
-			const struct bpf_prog_offload_ops *ops; /* callee set */
-		} verifier;
 		/* BPF_OFFLOAD_TRANSLATE, BPF_OFFLOAD_DESTROY */
 		struct {
 			struct bpf_prog *prog;
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 2c88cb4ddfd8..1f7ac00a494d 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -142,21 +142,17 @@ static int __bpf_offload_ndo(struct bpf_prog *prog, enum bpf_netdev_command cmd,
 
 int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env)
 {
-	struct netdev_bpf data = {};
-	int err;
-
-	data.verifier.prog = env->prog;
+	struct bpf_prog_offload *offload;
+	int ret = -ENODEV;
 
-	rtnl_lock();
-	err = __bpf_offload_ndo(env->prog, BPF_OFFLOAD_VERIFIER_PREP, &data);
-	if (err)
-		goto exit_unlock;
+	down_read(&bpf_devs_lock);
+	offload = env->prog->aux->offload;
+	if (offload)
+		ret = offload->offdev->ops->prepare(offload->netdev, env);
+	offload->dev_state = !ret;
+	up_read(&bpf_devs_lock);
 
-	env->prog->aux->offload->dev_ops = data.verifier.ops;
-	env->prog->aux->offload->dev_state = true;
-exit_unlock:
-	rtnl_unlock();
-	return err;
+	return ret;
 }
 
 int bpf_prog_offload_verify_insn(struct bpf_verifier_env *env,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 6/9] bpf: pass translate() as a callback and remove its ndo_bpf subcommand
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
                   ` (4 preceding siblings ...)
  2018-11-09 13:03 ` [PATCH bpf-next 5/9] bpf: call verifier_prep " Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 7/9] bpf: pass destroy() " Quentin Monnet
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

As part of the transition from ndo_bpf() to callbacks attached to struct
bpf_offload_dev for some of the eBPF offload operations, move the
functions related to code translation to the struct and remove the
subcommand that was used to call them through the NDO.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/offload.c | 11 +++--------
 drivers/net/netdevsim/bpf.c                      | 14 +++++++++-----
 include/linux/bpf.h                              |  1 +
 include/linux/netdevice.h                        |  3 +--
 kernel/bpf/offload.c                             | 14 +++++++-------
 5 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 16a3a9c55852..8653a2189c19 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -33,9 +33,6 @@ nfp_map_ptr_record(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog,
 	struct nfp_bpf_neutral_map *record;
 	int err;
 
-	/* Map record paths are entered via ndo, update side is protected. */
-	ASSERT_RTNL();
-
 	/* Reuse path - other offloaded program is already tracking this map. */
 	record = rhashtable_lookup_fast(&bpf->maps_neutral, &map->id,
 					nfp_bpf_maps_neutral_params);
@@ -84,8 +81,6 @@ nfp_map_ptrs_forget(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog)
 	bool freed = false;
 	int i;
 
-	ASSERT_RTNL();
-
 	for (i = 0; i < nfp_prog->map_records_cnt; i++) {
 		if (--nfp_prog->map_records[i]->count) {
 			nfp_prog->map_records[i] = NULL;
@@ -219,9 +214,10 @@ nfp_bpf_verifier_prep(struct net_device *netdev, struct bpf_verifier_env *env)
 	return ret;
 }
 
-static int nfp_bpf_translate(struct nfp_net *nn, struct bpf_prog *prog)
+static int nfp_bpf_translate(struct net_device *netdev, struct bpf_prog *prog)
 {
 	struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
+	struct nfp_net *nn = netdev_priv(netdev);
 	unsigned int max_instr;
 	int err;
 
@@ -422,8 +418,6 @@ nfp_bpf_map_free(struct nfp_app_bpf *bpf, struct bpf_offloaded_map *offmap)
 int nfp_ndo_bpf(struct nfp_app *app, struct nfp_net *nn, struct netdev_bpf *bpf)
 {
 	switch (bpf->command) {
-	case BPF_OFFLOAD_TRANSLATE:
-		return nfp_bpf_translate(nn, bpf->offload.prog);
 	case BPF_OFFLOAD_DESTROY:
 		return nfp_bpf_destroy(nn, bpf->offload.prog);
 	case BPF_OFFLOAD_MAP_ALLOC:
@@ -604,4 +598,5 @@ const struct bpf_prog_offload_ops nfp_bpf_dev_ops = {
 	.insn_hook	= nfp_verify_insn,
 	.finalize	= nfp_bpf_finalize,
 	.prepare	= nfp_bpf_verifier_prep,
+	.translate	= nfp_bpf_translate,
 };
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index d045b7d666d9..30c2cd516d1c 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -269,6 +269,14 @@ nsim_bpf_verifier_prep(struct net_device *dev, struct bpf_verifier_env *env)
 	return nsim_bpf_create_prog(ns, env->prog);
 }
 
+static int nsim_bpf_translate(struct net_device *dev, struct bpf_prog *prog)
+{
+	struct nsim_bpf_bound_prog *state = prog->aux->offload->dev_priv;
+
+	state->state = "xlated";
+	return 0;
+}
+
 static void nsim_bpf_destroy_prog(struct bpf_prog *prog)
 {
 	struct nsim_bpf_bound_prog *state;
@@ -285,6 +293,7 @@ static const struct bpf_prog_offload_ops nsim_bpf_dev_ops = {
 	.insn_hook	= nsim_bpf_verify_insn,
 	.finalize	= nsim_bpf_finalize,
 	.prepare	= nsim_bpf_verifier_prep,
+	.translate	= nsim_bpf_translate,
 };
 
 static int nsim_setup_prog_checks(struct netdevsim *ns, struct netdev_bpf *bpf)
@@ -551,11 +560,6 @@ int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 	ASSERT_RTNL();
 
 	switch (bpf->command) {
-	case BPF_OFFLOAD_TRANSLATE:
-		state = bpf->offload.prog->aux->offload->dev_priv;
-
-		state->state = "xlated";
-		return 0;
 	case BPF_OFFLOAD_DESTROY:
 		nsim_bpf_destroy_prog(bpf->offload.prog);
 		return 0;
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f250494a4f56..d1eb3c8a3fa9 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -269,6 +269,7 @@ struct bpf_prog_offload_ops {
 			 int insn_idx, int prev_insn_idx);
 	int (*finalize)(struct bpf_verifier_env *env);
 	int (*prepare)(struct net_device *netdev, struct bpf_verifier_env *env);
+	int (*translate)(struct net_device *netdev, struct bpf_prog *prog);
 };
 
 struct bpf_prog_offload {
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0fa2c2744928..27499127e038 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -863,7 +863,6 @@ enum bpf_netdev_command {
 	XDP_QUERY_PROG,
 	XDP_QUERY_PROG_HW,
 	/* BPF program for offload callbacks, invoked at program load time. */
-	BPF_OFFLOAD_TRANSLATE,
 	BPF_OFFLOAD_DESTROY,
 	BPF_OFFLOAD_MAP_ALLOC,
 	BPF_OFFLOAD_MAP_FREE,
@@ -890,7 +889,7 @@ struct netdev_bpf {
 			/* flags with which program was installed */
 			u32 prog_flags;
 		};
-		/* BPF_OFFLOAD_TRANSLATE, BPF_OFFLOAD_DESTROY */
+		/* BPF_OFFLOAD_DESTROY */
 		struct {
 			struct bpf_prog *prog;
 		} offload;
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 1f7ac00a494d..ae0167366c12 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -219,14 +219,14 @@ void bpf_prog_offload_destroy(struct bpf_prog *prog)
 
 static int bpf_prog_offload_translate(struct bpf_prog *prog)
 {
-	struct netdev_bpf data = {};
-	int ret;
-
-	data.offload.prog = prog;
+	struct bpf_prog_offload *offload;
+	int ret = -ENODEV;
 
-	rtnl_lock();
-	ret = __bpf_offload_ndo(prog, BPF_OFFLOAD_TRANSLATE, &data);
-	rtnl_unlock();
+	down_read(&bpf_devs_lock);
+	offload = prog->aux->offload;
+	if (offload)
+		ret = offload->offdev->ops->translate(offload->netdev, prog);
+	up_read(&bpf_devs_lock);
 
 	return ret;
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 7/9] bpf: pass destroy() as a callback and remove its ndo_bpf subcommand
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
                   ` (5 preceding siblings ...)
  2018-11-09 13:03 ` [PATCH bpf-next 6/9] bpf: pass translate() as a callback and remove its ndo_bpf subcommand Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 8/9] bpf: pass prog instead of env to bpf_prog_offload_verifier_prep() Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 9/9] bpf: do not pass netdev to translate() and prepare() offload callbacks Quentin Monnet
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

As part of the transition from ndo_bpf() to callbacks attached to struct
bpf_offload_dev for some of the eBPF offload operations, move the
functions related to program destruction to the struct and remove the
subcommand that was used to call them through the NDO.

Remove function __bpf_offload_ndo(), which is no longer used.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/bpf/offload.c  |  7 ++----
 drivers/net/netdevsim/bpf.c                   |  4 +---
 include/linux/bpf.h                           |  1 +
 include/linux/netdevice.h                     |  5 ----
 kernel/bpf/offload.c                          | 24 +------------------
 5 files changed, 5 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 8653a2189c19..91085cc3c843 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -238,15 +238,13 @@ static int nfp_bpf_translate(struct net_device *netdev, struct bpf_prog *prog)
 	return nfp_map_ptrs_record(nfp_prog->bpf, nfp_prog, prog);
 }
 
-static int nfp_bpf_destroy(struct nfp_net *nn, struct bpf_prog *prog)
+static void nfp_bpf_destroy(struct bpf_prog *prog)
 {
 	struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
 
 	kvfree(nfp_prog->prog);
 	nfp_map_ptrs_forget(nfp_prog->bpf, nfp_prog);
 	nfp_prog_free(nfp_prog);
-
-	return 0;
 }
 
 /* Atomic engine requires values to be in big endian, we need to byte swap
@@ -418,8 +416,6 @@ nfp_bpf_map_free(struct nfp_app_bpf *bpf, struct bpf_offloaded_map *offmap)
 int nfp_ndo_bpf(struct nfp_app *app, struct nfp_net *nn, struct netdev_bpf *bpf)
 {
 	switch (bpf->command) {
-	case BPF_OFFLOAD_DESTROY:
-		return nfp_bpf_destroy(nn, bpf->offload.prog);
 	case BPF_OFFLOAD_MAP_ALLOC:
 		return nfp_bpf_map_alloc(app->priv, bpf->offmap);
 	case BPF_OFFLOAD_MAP_FREE:
@@ -599,4 +595,5 @@ const struct bpf_prog_offload_ops nfp_bpf_dev_ops = {
 	.finalize	= nfp_bpf_finalize,
 	.prepare	= nfp_bpf_verifier_prep,
 	.translate	= nfp_bpf_translate,
+	.destroy	= nfp_bpf_destroy,
 };
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 30c2cd516d1c..33e3d54c3a0a 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -294,6 +294,7 @@ static const struct bpf_prog_offload_ops nsim_bpf_dev_ops = {
 	.finalize	= nsim_bpf_finalize,
 	.prepare	= nsim_bpf_verifier_prep,
 	.translate	= nsim_bpf_translate,
+	.destroy	= nsim_bpf_destroy_prog,
 };
 
 static int nsim_setup_prog_checks(struct netdevsim *ns, struct netdev_bpf *bpf)
@@ -560,9 +561,6 @@ int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 	ASSERT_RTNL();
 
 	switch (bpf->command) {
-	case BPF_OFFLOAD_DESTROY:
-		nsim_bpf_destroy_prog(bpf->offload.prog);
-		return 0;
 	case XDP_QUERY_PROG:
 		return xdp_attachment_query(&ns->xdp, bpf);
 	case XDP_QUERY_PROG_HW:
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d1eb3c8a3fa9..867d2801db64 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -270,6 +270,7 @@ struct bpf_prog_offload_ops {
 	int (*finalize)(struct bpf_verifier_env *env);
 	int (*prepare)(struct net_device *netdev, struct bpf_verifier_env *env);
 	int (*translate)(struct net_device *netdev, struct bpf_prog *prog);
+	void (*destroy)(struct bpf_prog *prog);
 };
 
 struct bpf_prog_offload {
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 27499127e038..17d52a647fe5 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -863,7 +863,6 @@ enum bpf_netdev_command {
 	XDP_QUERY_PROG,
 	XDP_QUERY_PROG_HW,
 	/* BPF program for offload callbacks, invoked at program load time. */
-	BPF_OFFLOAD_DESTROY,
 	BPF_OFFLOAD_MAP_ALLOC,
 	BPF_OFFLOAD_MAP_FREE,
 	XDP_QUERY_XSK_UMEM,
@@ -889,10 +888,6 @@ struct netdev_bpf {
 			/* flags with which program was installed */
 			u32 prog_flags;
 		};
-		/* BPF_OFFLOAD_DESTROY */
-		struct {
-			struct bpf_prog *prog;
-		} offload;
 		/* BPF_OFFLOAD_MAP_ALLOC, BPF_OFFLOAD_MAP_FREE */
 		struct {
 			struct bpf_offloaded_map *offmap;
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index ae0167366c12..d665e75a0ac3 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -123,23 +123,6 @@ int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr)
 	return err;
 }
 
-static int __bpf_offload_ndo(struct bpf_prog *prog, enum bpf_netdev_command cmd,
-			     struct netdev_bpf *data)
-{
-	struct bpf_prog_offload *offload = prog->aux->offload;
-	struct net_device *netdev;
-
-	ASSERT_RTNL();
-
-	if (!offload)
-		return -ENODEV;
-	netdev = offload->netdev;
-
-	data->command = cmd;
-
-	return netdev->netdev_ops->ndo_bpf(netdev, data);
-}
-
 int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env)
 {
 	struct bpf_prog_offload *offload;
@@ -192,12 +175,9 @@ int bpf_prog_offload_finalize(struct bpf_verifier_env *env)
 static void __bpf_prog_offload_destroy(struct bpf_prog *prog)
 {
 	struct bpf_prog_offload *offload = prog->aux->offload;
-	struct netdev_bpf data = {};
-
-	data.offload.prog = prog;
 
 	if (offload->dev_state)
-		WARN_ON(__bpf_offload_ndo(prog, BPF_OFFLOAD_DESTROY, &data));
+		offload->offdev->ops->destroy(prog);
 
 	/* Make sure BPF_PROG_GET_NEXT_ID can't find this dead program */
 	bpf_prog_free_id(prog, true);
@@ -209,12 +189,10 @@ static void __bpf_prog_offload_destroy(struct bpf_prog *prog)
 
 void bpf_prog_offload_destroy(struct bpf_prog *prog)
 {
-	rtnl_lock();
 	down_write(&bpf_devs_lock);
 	if (prog->aux->offload)
 		__bpf_prog_offload_destroy(prog);
 	up_write(&bpf_devs_lock);
-	rtnl_unlock();
 }
 
 static int bpf_prog_offload_translate(struct bpf_prog *prog)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 8/9] bpf: pass prog instead of env to bpf_prog_offload_verifier_prep()
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
                   ` (6 preceding siblings ...)
  2018-11-09 13:03 ` [PATCH bpf-next 7/9] bpf: pass destroy() " Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  2018-11-09 13:03 ` [PATCH bpf-next 9/9] bpf: do not pass netdev to translate() and prepare() offload callbacks Quentin Monnet
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

Function bpf_prog_offload_verifier_prep(), called from the kernel BPF
verifier to run a driver-specific callback for preparing for the
verification step for offloaded programs, takes a pointer to a struct
bpf_verifier_env object. However, no driver callback needs the whole
structure at this time: the two drivers supporting this, nfp and
netdevsim, only need a pointer to the struct bpf_prog instance held by
env.

Update the callback accordingly, on kernel side and in these two
drivers.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/offload.c | 3 +--
 drivers/net/netdevsim/bpf.c                      | 4 ++--
 include/linux/bpf.h                              | 2 +-
 include/linux/bpf_verifier.h                     | 2 +-
 kernel/bpf/offload.c                             | 6 +++---
 kernel/bpf/verifier.c                            | 2 +-
 6 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 91085cc3c843..e6b26d2f651d 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -183,10 +183,9 @@ static void nfp_prog_free(struct nfp_prog *nfp_prog)
 }
 
 static int
-nfp_bpf_verifier_prep(struct net_device *netdev, struct bpf_verifier_env *env)
+nfp_bpf_verifier_prep(struct net_device *netdev, struct bpf_prog *prog)
 {
 	struct nfp_net *nn = netdev_priv(netdev);
-	struct bpf_prog *prog = env->prog;
 	struct nfp_app *app = nn->app;
 	struct nfp_prog *nfp_prog;
 	int ret;
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 33e3d54c3a0a..560bdaf1c98b 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -259,14 +259,14 @@ static int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
 }
 
 static int
-nsim_bpf_verifier_prep(struct net_device *dev, struct bpf_verifier_env *env)
+nsim_bpf_verifier_prep(struct net_device *dev, struct bpf_prog *prog)
 {
 	struct netdevsim *ns = netdev_priv(dev);
 
 	if (!ns->bpf_bind_accept)
 		return -EOPNOTSUPP;
 
-	return nsim_bpf_create_prog(ns, env->prog);
+	return nsim_bpf_create_prog(ns, prog);
 }
 
 static int nsim_bpf_translate(struct net_device *dev, struct bpf_prog *prog)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 867d2801db64..888111350d0e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -268,7 +268,7 @@ struct bpf_prog_offload_ops {
 	int (*insn_hook)(struct bpf_verifier_env *env,
 			 int insn_idx, int prev_insn_idx);
 	int (*finalize)(struct bpf_verifier_env *env);
-	int (*prepare)(struct net_device *netdev, struct bpf_verifier_env *env);
+	int (*prepare)(struct net_device *netdev, struct bpf_prog *prog);
 	int (*translate)(struct net_device *netdev, struct bpf_prog *prog);
 	void (*destroy)(struct bpf_prog *prog);
 };
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index d93e89761a8b..11f5df1092d9 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -245,7 +245,7 @@ static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
 	return cur_func(env)->regs;
 }
 
-int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env);
+int bpf_prog_offload_verifier_prep(struct bpf_prog *prog);
 int bpf_prog_offload_verify_insn(struct bpf_verifier_env *env,
 				 int insn_idx, int prev_insn_idx);
 int bpf_prog_offload_finalize(struct bpf_verifier_env *env);
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index d665e75a0ac3..397d206e184b 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -123,15 +123,15 @@ int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr)
 	return err;
 }
 
-int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env)
+int bpf_prog_offload_verifier_prep(struct bpf_prog *prog)
 {
 	struct bpf_prog_offload *offload;
 	int ret = -ENODEV;
 
 	down_read(&bpf_devs_lock);
-	offload = env->prog->aux->offload;
+	offload = prog->aux->offload;
 	if (offload)
-		ret = offload->offdev->ops->prepare(offload->netdev, env);
+		ret = offload->offdev->ops->prepare(offload->netdev, prog);
 	offload->dev_state = !ret;
 	up_read(&bpf_devs_lock);
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 75dab40b19a3..8d0977980cfa 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6368,7 +6368,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
 		goto skip_full_check;
 
 	if (bpf_prog_is_dev_bound(env->prog->aux)) {
-		ret = bpf_prog_offload_verifier_prep(env);
+		ret = bpf_prog_offload_verifier_prep(env->prog);
 		if (ret)
 			goto skip_full_check;
 	}
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next 9/9] bpf: do not pass netdev to translate() and prepare() offload callbacks
  2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
                   ` (7 preceding siblings ...)
  2018-11-09 13:03 ` [PATCH bpf-next 8/9] bpf: pass prog instead of env to bpf_prog_offload_verifier_prep() Quentin Monnet
@ 2018-11-09 13:03 ` Quentin Monnet
  8 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, oss-drivers, Quentin Monnet

The kernel functions to prepare verifier and translate for offloaded
program retrieve "offload" from "prog", and "netdev" from "offload".
Then both "prog" and "netdev" are passed to the callbacks.

Simplify this by letting the drivers retrieve the net device themselves
from the offload object attached to prog - if they need it at all. There
is currently no need to pass the netdev as an argument to those
functions.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/offload.c | 9 ++++-----
 drivers/net/netdevsim/bpf.c                      | 7 +++----
 include/linux/bpf.h                              | 4 ++--
 kernel/bpf/offload.c                             | 4 ++--
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index e6b26d2f651d..f0283854fade 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -182,10 +182,9 @@ static void nfp_prog_free(struct nfp_prog *nfp_prog)
 	kfree(nfp_prog);
 }
 
-static int
-nfp_bpf_verifier_prep(struct net_device *netdev, struct bpf_prog *prog)
+static int nfp_bpf_verifier_prep(struct bpf_prog *prog)
 {
-	struct nfp_net *nn = netdev_priv(netdev);
+	struct nfp_net *nn = netdev_priv(prog->aux->offload->netdev);
 	struct nfp_app *app = nn->app;
 	struct nfp_prog *nfp_prog;
 	int ret;
@@ -213,10 +212,10 @@ nfp_bpf_verifier_prep(struct net_device *netdev, struct bpf_prog *prog)
 	return ret;
 }
 
-static int nfp_bpf_translate(struct net_device *netdev, struct bpf_prog *prog)
+static int nfp_bpf_translate(struct bpf_prog *prog)
 {
+	struct nfp_net *nn = netdev_priv(prog->aux->offload->netdev);
 	struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
-	struct nfp_net *nn = netdev_priv(netdev);
 	unsigned int max_instr;
 	int err;
 
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 560bdaf1c98b..6a5b7bd9a1f9 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -258,10 +258,9 @@ static int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
 	return 0;
 }
 
-static int
-nsim_bpf_verifier_prep(struct net_device *dev, struct bpf_prog *prog)
+static int nsim_bpf_verifier_prep(struct bpf_prog *prog)
 {
-	struct netdevsim *ns = netdev_priv(dev);
+	struct netdevsim *ns = netdev_priv(prog->aux->offload->netdev);
 
 	if (!ns->bpf_bind_accept)
 		return -EOPNOTSUPP;
@@ -269,7 +268,7 @@ nsim_bpf_verifier_prep(struct net_device *dev, struct bpf_prog *prog)
 	return nsim_bpf_create_prog(ns, prog);
 }
 
-static int nsim_bpf_translate(struct net_device *dev, struct bpf_prog *prog)
+static int nsim_bpf_translate(struct bpf_prog *prog)
 {
 	struct nsim_bpf_bound_prog *state = prog->aux->offload->dev_priv;
 
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 888111350d0e..987815152629 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -268,8 +268,8 @@ struct bpf_prog_offload_ops {
 	int (*insn_hook)(struct bpf_verifier_env *env,
 			 int insn_idx, int prev_insn_idx);
 	int (*finalize)(struct bpf_verifier_env *env);
-	int (*prepare)(struct net_device *netdev, struct bpf_prog *prog);
-	int (*translate)(struct net_device *netdev, struct bpf_prog *prog);
+	int (*prepare)(struct bpf_prog *prog);
+	int (*translate)(struct bpf_prog *prog);
 	void (*destroy)(struct bpf_prog *prog);
 };
 
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 397d206e184b..52c5617e3716 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -131,7 +131,7 @@ int bpf_prog_offload_verifier_prep(struct bpf_prog *prog)
 	down_read(&bpf_devs_lock);
 	offload = prog->aux->offload;
 	if (offload)
-		ret = offload->offdev->ops->prepare(offload->netdev, prog);
+		ret = offload->offdev->ops->prepare(prog);
 	offload->dev_state = !ret;
 	up_read(&bpf_devs_lock);
 
@@ -203,7 +203,7 @@ static int bpf_prog_offload_translate(struct bpf_prog *prog)
 	down_read(&bpf_devs_lock);
 	offload = prog->aux->offload;
 	if (offload)
-		ret = offload->offdev->ops->translate(offload->netdev, prog);
+		ret = offload->offdev->ops->translate(prog);
 	up_read(&bpf_devs_lock);
 
 	return ret;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-11-09 22:45 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 13:03 [PATCH bpf-next 0/9] bpf: pass device ops as callbacks and remove some ndo_bpf subcommands Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 1/9] nfp: bpf: move nfp_bpf_analyzer_ops from verifier.c to offload.c Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 2/9] bpf: pass a struct with offload callbacks to bpf_offload_dev_create() Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 3/9] bpf: call verify_insn from its callback in struct bpf_offload_dev Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 4/9] bpf: call finalize() " Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 5/9] bpf: call verifier_prep " Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 6/9] bpf: pass translate() as a callback and remove its ndo_bpf subcommand Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 7/9] bpf: pass destroy() " Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 8/9] bpf: pass prog instead of env to bpf_prog_offload_verifier_prep() Quentin Monnet
2018-11-09 13:03 ` [PATCH bpf-next 9/9] bpf: do not pass netdev to translate() and prepare() offload callbacks Quentin Monnet

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.