netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list
@ 2017-04-07 19:00 Johannes Berg
  2017-04-07 19:00 ` [PATCH v2 2/2] bpf: remove struct bpf_map_type_list Johannes Berg
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Johannes Berg @ 2017-04-07 19:00 UTC (permalink / raw)
  To: netdev; +Cc: Alexei Starovoitov, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There's no need to have struct bpf_prog_type_list since
it just contains a list_head, the type, and the ops
pointer. Since the types are densely packed and not
actually dynamically registered, it's much easier and
smaller to have an array of type->ops pointer. Also
initialize this array statically to remove code needed
to initialize it.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/bpf.h      | 22 +++++++-------
 kernel/bpf/syscall.c     | 39 ++++++++++++++-----------
 kernel/trace/bpf_trace.c | 30 ++-----------------
 net/core/filter.c        | 75 +++++-------------------------------------------
 4 files changed, 44 insertions(+), 122 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 909fc033173a..75cea2890751 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -169,12 +169,6 @@ struct bpf_verifier_ops {
 				  struct bpf_prog *prog);
 };
 
-struct bpf_prog_type_list {
-	struct list_head list_node;
-	const struct bpf_verifier_ops *ops;
-	enum bpf_prog_type type;
-};
-
 struct bpf_prog_aux {
 	atomic_t refcnt;
 	u32 used_map_cnt;
@@ -234,7 +228,17 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
 #ifdef CONFIG_BPF_SYSCALL
 DECLARE_PER_CPU(int, bpf_prog_active);
 
-void bpf_register_prog_type(struct bpf_prog_type_list *tl);
+extern const struct bpf_verifier_ops sk_filter_prog_ops;
+extern const struct bpf_verifier_ops tc_cls_act_prog_ops;
+extern const struct bpf_verifier_ops xdp_prog_ops;
+extern const struct bpf_verifier_ops cg_skb_prog_ops;
+extern const struct bpf_verifier_ops cg_sock_prog_ops;
+extern const struct bpf_verifier_ops lwt_inout_prog_ops;
+extern const struct bpf_verifier_ops lwt_xmit_prog_ops;
+extern const struct bpf_verifier_ops kprobe_prog_ops;
+extern const struct bpf_verifier_ops tracepoint_prog_ops;
+extern const struct bpf_verifier_ops perf_event_prog_ops;
+
 void bpf_register_map_type(struct bpf_map_type_list *tl);
 
 struct bpf_prog *bpf_prog_get(u32 ufd);
@@ -295,10 +299,6 @@ static inline void bpf_long_memcpy(void *dst, const void *src, u32 size)
 /* verify correctness of eBPF program */
 int bpf_check(struct bpf_prog **fp, union bpf_attr *attr);
 #else
-static inline void bpf_register_prog_type(struct bpf_prog_type_list *tl)
-{
-}
-
 static inline struct bpf_prog *bpf_prog_get(u32 ufd)
 {
 	return ERR_PTR(-EOPNOTSUPP);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 7af0dcc5d755..421aa81ef5d4 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -564,26 +564,33 @@ static int map_get_next_key(union bpf_attr *attr)
 	return err;
 }
 
-static LIST_HEAD(bpf_prog_types);
+static const struct bpf_verifier_ops * const bpf_prog_types[] = {
+#ifdef CONFIG_NET
+	[BPF_PROG_TYPE_SOCKET_FILTER] = &sk_filter_prog_ops,
+	[BPF_PROG_TYPE_SCHED_CLS] = &tc_cls_act_prog_ops,
+	[BPF_PROG_TYPE_SCHED_ACT] = &tc_cls_act_prog_ops,
+	[BPF_PROG_TYPE_XDP] = &xdp_prog_ops,
+	[BPF_PROG_TYPE_CGROUP_SKB] = &cg_skb_prog_ops,
+	[BPF_PROG_TYPE_CGROUP_SOCK] = &cg_sock_prog_ops,
+	[BPF_PROG_TYPE_LWT_IN] = &lwt_inout_prog_ops,
+	[BPF_PROG_TYPE_LWT_OUT] = &lwt_inout_prog_ops,
+	[BPF_PROG_TYPE_LWT_XMIT] = &lwt_xmit_prog_ops,
+#endif
+#ifdef CONFIG_BPF_EVENTS
+	[BPF_PROG_TYPE_KPROBE] = &kprobe_prog_ops,
+	[BPF_PROG_TYPE_TRACEPOINT] = &tracepoint_prog_ops,
+	[BPF_PROG_TYPE_PERF_EVENT] = &perf_event_prog_ops,
+#endif
+};
 
 static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog)
 {
-	struct bpf_prog_type_list *tl;
-
-	list_for_each_entry(tl, &bpf_prog_types, list_node) {
-		if (tl->type == type) {
-			prog->aux->ops = tl->ops;
-			prog->type = type;
-			return 0;
-		}
-	}
-
-	return -EINVAL;
-}
+	if (type >= ARRAY_SIZE(bpf_prog_types) || !bpf_prog_types[type])
+		return -EINVAL;
 
-void bpf_register_prog_type(struct bpf_prog_type_list *tl)
-{
-	list_add(&tl->list_node, &bpf_prog_types);
+	prog->aux->ops = bpf_prog_types[type];
+	prog->type = type;
+	return 0;
 }
 
 /* fixup insn->imm field of bpf_call instructions:
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index cee9802cf3e0..8a4efac28710 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -501,16 +501,11 @@ static bool kprobe_prog_is_valid_access(int off, int size, enum bpf_access_type
 	return true;
 }
 
-static const struct bpf_verifier_ops kprobe_prog_ops = {
+const struct bpf_verifier_ops kprobe_prog_ops = {
 	.get_func_proto  = kprobe_prog_func_proto,
 	.is_valid_access = kprobe_prog_is_valid_access,
 };
 
-static struct bpf_prog_type_list kprobe_tl __ro_after_init = {
-	.ops	= &kprobe_prog_ops,
-	.type	= BPF_PROG_TYPE_KPROBE,
-};
-
 BPF_CALL_5(bpf_perf_event_output_tp, void *, tp_buff, struct bpf_map *, map,
 	   u64, flags, void *, data, u64, size)
 {
@@ -584,16 +579,11 @@ static bool tp_prog_is_valid_access(int off, int size, enum bpf_access_type type
 	return true;
 }
 
-static const struct bpf_verifier_ops tracepoint_prog_ops = {
+const struct bpf_verifier_ops tracepoint_prog_ops = {
 	.get_func_proto  = tp_prog_func_proto,
 	.is_valid_access = tp_prog_is_valid_access,
 };
 
-static struct bpf_prog_type_list tracepoint_tl __ro_after_init = {
-	.ops	= &tracepoint_prog_ops,
-	.type	= BPF_PROG_TYPE_TRACEPOINT,
-};
-
 static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
 				    enum bpf_reg_type *reg_type)
 {
@@ -642,22 +632,8 @@ static u32 pe_prog_convert_ctx_access(enum bpf_access_type type,
 	return insn - insn_buf;
 }
 
-static const struct bpf_verifier_ops perf_event_prog_ops = {
+const struct bpf_verifier_ops perf_event_prog_ops = {
 	.get_func_proto		= tp_prog_func_proto,
 	.is_valid_access	= pe_prog_is_valid_access,
 	.convert_ctx_access	= pe_prog_convert_ctx_access,
 };
-
-static struct bpf_prog_type_list perf_event_tl __ro_after_init = {
-	.ops	= &perf_event_prog_ops,
-	.type	= BPF_PROG_TYPE_PERF_EVENT,
-};
-
-static int __init register_kprobe_prog_ops(void)
-{
-	bpf_register_prog_type(&kprobe_tl);
-	bpf_register_prog_type(&tracepoint_tl);
-	bpf_register_prog_type(&perf_event_tl);
-	return 0;
-}
-late_initcall(register_kprobe_prog_ops);
diff --git a/net/core/filter.c b/net/core/filter.c
index ebaeaf2e46e8..a55d93980d3e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3252,111 +3252,50 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
 	return insn - insn_buf;
 }
 
-static const struct bpf_verifier_ops sk_filter_ops = {
+const struct bpf_verifier_ops sk_filter_prog_ops = {
 	.get_func_proto		= sk_filter_func_proto,
 	.is_valid_access	= sk_filter_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-static const struct bpf_verifier_ops tc_cls_act_ops = {
+const struct bpf_verifier_ops tc_cls_act_prog_ops = {
 	.get_func_proto		= tc_cls_act_func_proto,
 	.is_valid_access	= tc_cls_act_is_valid_access,
 	.convert_ctx_access	= tc_cls_act_convert_ctx_access,
 	.gen_prologue		= tc_cls_act_prologue,
 };
 
-static const struct bpf_verifier_ops xdp_ops = {
+const struct bpf_verifier_ops xdp_prog_ops = {
 	.get_func_proto		= xdp_func_proto,
 	.is_valid_access	= xdp_is_valid_access,
 	.convert_ctx_access	= xdp_convert_ctx_access,
 };
 
-static const struct bpf_verifier_ops cg_skb_ops = {
+const struct bpf_verifier_ops cg_skb_prog_ops = {
 	.get_func_proto		= cg_skb_func_proto,
 	.is_valid_access	= sk_filter_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-static const struct bpf_verifier_ops lwt_inout_ops = {
+const struct bpf_verifier_ops lwt_inout_prog_ops = {
 	.get_func_proto		= lwt_inout_func_proto,
 	.is_valid_access	= lwt_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-static const struct bpf_verifier_ops lwt_xmit_ops = {
+const struct bpf_verifier_ops lwt_xmit_prog_ops = {
 	.get_func_proto		= lwt_xmit_func_proto,
 	.is_valid_access	= lwt_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 	.gen_prologue		= tc_cls_act_prologue,
 };
 
-static const struct bpf_verifier_ops cg_sock_ops = {
+const struct bpf_verifier_ops cg_sock_prog_ops = {
 	.get_func_proto		= bpf_base_func_proto,
 	.is_valid_access	= sock_filter_is_valid_access,
 	.convert_ctx_access	= sock_filter_convert_ctx_access,
 };
 
-static struct bpf_prog_type_list sk_filter_type __ro_after_init = {
-	.ops	= &sk_filter_ops,
-	.type	= BPF_PROG_TYPE_SOCKET_FILTER,
-};
-
-static struct bpf_prog_type_list sched_cls_type __ro_after_init = {
-	.ops	= &tc_cls_act_ops,
-	.type	= BPF_PROG_TYPE_SCHED_CLS,
-};
-
-static struct bpf_prog_type_list sched_act_type __ro_after_init = {
-	.ops	= &tc_cls_act_ops,
-	.type	= BPF_PROG_TYPE_SCHED_ACT,
-};
-
-static struct bpf_prog_type_list xdp_type __ro_after_init = {
-	.ops	= &xdp_ops,
-	.type	= BPF_PROG_TYPE_XDP,
-};
-
-static struct bpf_prog_type_list cg_skb_type __ro_after_init = {
-	.ops	= &cg_skb_ops,
-	.type	= BPF_PROG_TYPE_CGROUP_SKB,
-};
-
-static struct bpf_prog_type_list lwt_in_type __ro_after_init = {
-	.ops	= &lwt_inout_ops,
-	.type	= BPF_PROG_TYPE_LWT_IN,
-};
-
-static struct bpf_prog_type_list lwt_out_type __ro_after_init = {
-	.ops	= &lwt_inout_ops,
-	.type	= BPF_PROG_TYPE_LWT_OUT,
-};
-
-static struct bpf_prog_type_list lwt_xmit_type __ro_after_init = {
-	.ops	= &lwt_xmit_ops,
-	.type	= BPF_PROG_TYPE_LWT_XMIT,
-};
-
-static struct bpf_prog_type_list cg_sock_type __ro_after_init = {
-	.ops	= &cg_sock_ops,
-	.type	= BPF_PROG_TYPE_CGROUP_SOCK
-};
-
-static int __init register_sk_filter_ops(void)
-{
-	bpf_register_prog_type(&sk_filter_type);
-	bpf_register_prog_type(&sched_cls_type);
-	bpf_register_prog_type(&sched_act_type);
-	bpf_register_prog_type(&xdp_type);
-	bpf_register_prog_type(&cg_skb_type);
-	bpf_register_prog_type(&cg_sock_type);
-	bpf_register_prog_type(&lwt_in_type);
-	bpf_register_prog_type(&lwt_out_type);
-	bpf_register_prog_type(&lwt_xmit_type);
-
-	return 0;
-}
-late_initcall(register_sk_filter_ops);
-
 int sk_detach_filter(struct sock *sk)
 {
 	int ret = -ENOENT;
-- 
2.11.0

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

* [PATCH v2 2/2] bpf: remove struct bpf_map_type_list
  2017-04-07 19:00 [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list Johannes Berg
@ 2017-04-07 19:00 ` Johannes Berg
  2017-04-07 19:03 ` [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list Johannes Berg
  2017-04-10  1:22 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2017-04-07 19:00 UTC (permalink / raw)
  To: netdev; +Cc: Alexei Starovoitov, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There's no need to have struct bpf_map_type_list since
it just contains a list_head, the type, and the ops
pointer. Since the types are densely packed and not
actually dynamically registered, it's much easier and
smaller to have an array of type->ops pointer. Also
initialize this array statically to remove code needed
to initialize it.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/bpf.h   | 18 +++++++++------
 kernel/bpf/arraymap.c | 64 ++++-----------------------------------------------
 kernel/bpf/hashtab.c  | 38 ++++--------------------------
 kernel/bpf/lpm_trie.c | 14 +----------
 kernel/bpf/stackmap.c | 14 +----------
 kernel/bpf/syscall.c  | 42 ++++++++++++++++++---------------
 6 files changed, 46 insertions(+), 144 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 75cea2890751..4a95a25186e5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -51,12 +51,6 @@ struct bpf_map {
 	atomic_t usercnt;
 };
 
-struct bpf_map_type_list {
-	struct list_head list_node;
-	const struct bpf_map_ops *ops;
-	enum bpf_map_type type;
-};
-
 /* function argument constraints */
 enum bpf_arg_type {
 	ARG_DONTCARE = 0,	/* unused argument in helper function */
@@ -239,7 +233,17 @@ extern const struct bpf_verifier_ops kprobe_prog_ops;
 extern const struct bpf_verifier_ops tracepoint_prog_ops;
 extern const struct bpf_verifier_ops perf_event_prog_ops;
 
-void bpf_register_map_type(struct bpf_map_type_list *tl);
+extern const struct bpf_map_ops array_map_ops;
+extern const struct bpf_map_ops percpu_array_map_ops;
+extern const struct bpf_map_ops prog_array_map_ops;
+extern const struct bpf_map_ops perf_event_array_map_ops;
+extern const struct bpf_map_ops cgroup_array_map_ops;
+extern const struct bpf_map_ops htab_map_ops;
+extern const struct bpf_map_ops htab_lru_map_ops;
+extern const struct bpf_map_ops htab_percpu_map_ops;
+extern const struct bpf_map_ops htab_lru_percpu_map_ops;
+extern const struct bpf_map_ops trie_map_ops;
+extern const struct bpf_map_ops stack_map_ops;
 
 struct bpf_prog *bpf_prog_get(u32 ufd);
 struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type);
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 6b6f41f0b211..376afd14cf32 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -260,7 +260,7 @@ static void array_map_free(struct bpf_map *map)
 	bpf_map_area_free(array);
 }
 
-static const struct bpf_map_ops array_ops = {
+const struct bpf_map_ops array_map_ops = {
 	.map_alloc = array_map_alloc,
 	.map_free = array_map_free,
 	.map_get_next_key = array_map_get_next_key,
@@ -269,12 +269,7 @@ static const struct bpf_map_ops array_ops = {
 	.map_delete_elem = array_map_delete_elem,
 };
 
-static struct bpf_map_type_list array_type __ro_after_init = {
-	.ops = &array_ops,
-	.type = BPF_MAP_TYPE_ARRAY,
-};
-
-static const struct bpf_map_ops percpu_array_ops = {
+const struct bpf_map_ops percpu_array_map_ops = {
 	.map_alloc = array_map_alloc,
 	.map_free = array_map_free,
 	.map_get_next_key = array_map_get_next_key,
@@ -283,19 +278,6 @@ static const struct bpf_map_ops percpu_array_ops = {
 	.map_delete_elem = array_map_delete_elem,
 };
 
-static struct bpf_map_type_list percpu_array_type __ro_after_init = {
-	.ops = &percpu_array_ops,
-	.type = BPF_MAP_TYPE_PERCPU_ARRAY,
-};
-
-static int __init register_array_map(void)
-{
-	bpf_register_map_type(&array_type);
-	bpf_register_map_type(&percpu_array_type);
-	return 0;
-}
-late_initcall(register_array_map);
-
 static struct bpf_map *fd_array_map_alloc(union bpf_attr *attr)
 {
 	/* only file descriptors can be stored in this type of map */
@@ -399,7 +381,7 @@ void bpf_fd_array_map_clear(struct bpf_map *map)
 		fd_array_map_delete_elem(map, &i);
 }
 
-static const struct bpf_map_ops prog_array_ops = {
+const struct bpf_map_ops prog_array_map_ops = {
 	.map_alloc = fd_array_map_alloc,
 	.map_free = fd_array_map_free,
 	.map_get_next_key = array_map_get_next_key,
@@ -409,18 +391,6 @@ static const struct bpf_map_ops prog_array_ops = {
 	.map_fd_put_ptr = prog_fd_array_put_ptr,
 };
 
-static struct bpf_map_type_list prog_array_type __ro_after_init = {
-	.ops = &prog_array_ops,
-	.type = BPF_MAP_TYPE_PROG_ARRAY,
-};
-
-static int __init register_prog_array_map(void)
-{
-	bpf_register_map_type(&prog_array_type);
-	return 0;
-}
-late_initcall(register_prog_array_map);
-
 static struct bpf_event_entry *bpf_event_entry_gen(struct file *perf_file,
 						   struct file *map_file)
 {
@@ -511,7 +481,7 @@ static void perf_event_fd_array_release(struct bpf_map *map,
 	rcu_read_unlock();
 }
 
-static const struct bpf_map_ops perf_event_array_ops = {
+const struct bpf_map_ops perf_event_array_map_ops = {
 	.map_alloc = fd_array_map_alloc,
 	.map_free = fd_array_map_free,
 	.map_get_next_key = array_map_get_next_key,
@@ -522,18 +492,6 @@ static const struct bpf_map_ops perf_event_array_ops = {
 	.map_release = perf_event_fd_array_release,
 };
 
-static struct bpf_map_type_list perf_event_array_type __ro_after_init = {
-	.ops = &perf_event_array_ops,
-	.type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
-};
-
-static int __init register_perf_event_array_map(void)
-{
-	bpf_register_map_type(&perf_event_array_type);
-	return 0;
-}
-late_initcall(register_perf_event_array_map);
-
 #ifdef CONFIG_CGROUPS
 static void *cgroup_fd_array_get_ptr(struct bpf_map *map,
 				     struct file *map_file /* not used */,
@@ -554,7 +512,7 @@ static void cgroup_fd_array_free(struct bpf_map *map)
 	fd_array_map_free(map);
 }
 
-static const struct bpf_map_ops cgroup_array_ops = {
+const struct bpf_map_ops cgroup_array_map_ops = {
 	.map_alloc = fd_array_map_alloc,
 	.map_free = cgroup_fd_array_free,
 	.map_get_next_key = array_map_get_next_key,
@@ -563,16 +521,4 @@ static const struct bpf_map_ops cgroup_array_ops = {
 	.map_fd_get_ptr = cgroup_fd_array_get_ptr,
 	.map_fd_put_ptr = cgroup_fd_array_put_ptr,
 };
-
-static struct bpf_map_type_list cgroup_array_type __ro_after_init = {
-	.ops = &cgroup_array_ops,
-	.type = BPF_MAP_TYPE_CGROUP_ARRAY,
-};
-
-static int __init register_cgroup_array_map(void)
-{
-	bpf_register_map_type(&cgroup_array_type);
-	return 0;
-}
-late_initcall(register_cgroup_array_map);
 #endif
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 3ea87fb19a94..c7913d2aa8ba 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1014,7 +1014,7 @@ static void htab_map_free(struct bpf_map *map)
 	kfree(htab);
 }
 
-static const struct bpf_map_ops htab_ops = {
+const struct bpf_map_ops htab_map_ops = {
 	.map_alloc = htab_map_alloc,
 	.map_free = htab_map_free,
 	.map_get_next_key = htab_map_get_next_key,
@@ -1023,12 +1023,7 @@ static const struct bpf_map_ops htab_ops = {
 	.map_delete_elem = htab_map_delete_elem,
 };
 
-static struct bpf_map_type_list htab_type __ro_after_init = {
-	.ops = &htab_ops,
-	.type = BPF_MAP_TYPE_HASH,
-};
-
-static const struct bpf_map_ops htab_lru_ops = {
+const struct bpf_map_ops htab_lru_map_ops = {
 	.map_alloc = htab_map_alloc,
 	.map_free = htab_map_free,
 	.map_get_next_key = htab_map_get_next_key,
@@ -1037,11 +1032,6 @@ static const struct bpf_map_ops htab_lru_ops = {
 	.map_delete_elem = htab_lru_map_delete_elem,
 };
 
-static struct bpf_map_type_list htab_lru_type __ro_after_init = {
-	.ops = &htab_lru_ops,
-	.type = BPF_MAP_TYPE_LRU_HASH,
-};
-
 /* Called from eBPF program */
 static void *htab_percpu_map_lookup_elem(struct bpf_map *map, void *key)
 {
@@ -1115,7 +1105,7 @@ int bpf_percpu_hash_update(struct bpf_map *map, void *key, void *value,
 	return ret;
 }
 
-static const struct bpf_map_ops htab_percpu_ops = {
+const struct bpf_map_ops htab_percpu_map_ops = {
 	.map_alloc = htab_map_alloc,
 	.map_free = htab_map_free,
 	.map_get_next_key = htab_map_get_next_key,
@@ -1124,12 +1114,7 @@ static const struct bpf_map_ops htab_percpu_ops = {
 	.map_delete_elem = htab_map_delete_elem,
 };
 
-static struct bpf_map_type_list htab_percpu_type __ro_after_init = {
-	.ops = &htab_percpu_ops,
-	.type = BPF_MAP_TYPE_PERCPU_HASH,
-};
-
-static const struct bpf_map_ops htab_lru_percpu_ops = {
+const struct bpf_map_ops htab_lru_percpu_map_ops = {
 	.map_alloc = htab_map_alloc,
 	.map_free = htab_map_free,
 	.map_get_next_key = htab_map_get_next_key,
@@ -1137,18 +1122,3 @@ static const struct bpf_map_ops htab_lru_percpu_ops = {
 	.map_update_elem = htab_lru_percpu_map_update_elem,
 	.map_delete_elem = htab_lru_map_delete_elem,
 };
-
-static struct bpf_map_type_list htab_lru_percpu_type __ro_after_init = {
-	.ops = &htab_lru_percpu_ops,
-	.type = BPF_MAP_TYPE_LRU_PERCPU_HASH,
-};
-
-static int __init register_htab_map(void)
-{
-	bpf_register_map_type(&htab_type);
-	bpf_register_map_type(&htab_percpu_type);
-	bpf_register_map_type(&htab_lru_type);
-	bpf_register_map_type(&htab_lru_percpu_type);
-	return 0;
-}
-late_initcall(register_htab_map);
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
index 8bfe0afaee10..352ae2a2830a 100644
--- a/kernel/bpf/lpm_trie.c
+++ b/kernel/bpf/lpm_trie.c
@@ -500,22 +500,10 @@ static void trie_free(struct bpf_map *map)
 	raw_spin_unlock(&trie->lock);
 }
 
-static const struct bpf_map_ops trie_ops = {
+const struct bpf_map_ops trie_map_ops = {
 	.map_alloc = trie_alloc,
 	.map_free = trie_free,
 	.map_lookup_elem = trie_lookup_elem,
 	.map_update_elem = trie_update_elem,
 	.map_delete_elem = trie_delete_elem,
 };
-
-static struct bpf_map_type_list trie_type __ro_after_init = {
-	.ops = &trie_ops,
-	.type = BPF_MAP_TYPE_LPM_TRIE,
-};
-
-static int __init register_trie_map(void)
-{
-	bpf_register_map_type(&trie_type);
-	return 0;
-}
-late_initcall(register_trie_map);
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 22aa45cd0324..4dfd6f2ec2f9 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -264,7 +264,7 @@ static void stack_map_free(struct bpf_map *map)
 	put_callchain_buffers();
 }
 
-static const struct bpf_map_ops stack_map_ops = {
+const struct bpf_map_ops stack_map_ops = {
 	.map_alloc = stack_map_alloc,
 	.map_free = stack_map_free,
 	.map_get_next_key = stack_map_get_next_key,
@@ -272,15 +272,3 @@ static const struct bpf_map_ops stack_map_ops = {
 	.map_update_elem = stack_map_update_elem,
 	.map_delete_elem = stack_map_delete_elem,
 };
-
-static struct bpf_map_type_list stack_map_type __ro_after_init = {
-	.ops = &stack_map_ops,
-	.type = BPF_MAP_TYPE_STACK_TRACE,
-};
-
-static int __init register_stack_map(void)
-{
-	bpf_register_map_type(&stack_map_type);
-	return 0;
-}
-late_initcall(register_stack_map);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 421aa81ef5d4..c0c02667e641 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -27,30 +27,36 @@ DEFINE_PER_CPU(int, bpf_prog_active);
 
 int sysctl_unprivileged_bpf_disabled __read_mostly;
 
-static LIST_HEAD(bpf_map_types);
+static const struct bpf_map_ops * const bpf_map_types[] = {
+	[BPF_MAP_TYPE_ARRAY] = &array_map_ops,
+	[BPF_MAP_TYPE_PERCPU_ARRAY] = &percpu_array_map_ops,
+	[BPF_MAP_TYPE_PROG_ARRAY] = &prog_array_map_ops,
+	[BPF_MAP_TYPE_PERF_EVENT_ARRAY] = &perf_event_array_map_ops,
+#ifdef CONFIG_CGROUPS
+	[BPF_MAP_TYPE_CGROUP_ARRAY] = &cgroup_array_map_ops,
+#endif
+	[BPF_MAP_TYPE_HASH] = &htab_map_ops,
+	[BPF_MAP_TYPE_PERCPU_HASH] = &htab_percpu_map_ops,
+	[BPF_MAP_TYPE_LRU_HASH] = &htab_lru_map_ops,
+	[BPF_MAP_TYPE_LRU_PERCPU_HASH] = &htab_lru_percpu_map_ops,
+	[BPF_MAP_TYPE_LPM_TRIE] = &trie_map_ops,
+	[BPF_MAP_TYPE_STACK_TRACE] = &stack_map_ops,
+};
 
 static struct bpf_map *find_and_alloc_map(union bpf_attr *attr)
 {
-	struct bpf_map_type_list *tl;
 	struct bpf_map *map;
 
-	list_for_each_entry(tl, &bpf_map_types, list_node) {
-		if (tl->type == attr->map_type) {
-			map = tl->ops->map_alloc(attr);
-			if (IS_ERR(map))
-				return map;
-			map->ops = tl->ops;
-			map->map_type = attr->map_type;
-			return map;
-		}
-	}
-	return ERR_PTR(-EINVAL);
-}
+	if (attr->map_type >= ARRAY_SIZE(bpf_map_types) ||
+	    !bpf_map_types[attr->map_type])
+		return ERR_PTR(-EINVAL);
 
-/* boot time registration of different map implementations */
-void bpf_register_map_type(struct bpf_map_type_list *tl)
-{
-	list_add(&tl->list_node, &bpf_map_types);
+	map = bpf_map_types[attr->map_type]->map_alloc(attr);
+	if (IS_ERR(map))
+		return map;
+	map->ops = bpf_map_types[attr->map_type];
+	map->map_type = attr->map_type;
+	return map;
 }
 
 void *bpf_map_area_alloc(size_t size)
-- 
2.11.0

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

* Re: [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list
  2017-04-07 19:00 [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list Johannes Berg
  2017-04-07 19:00 ` [PATCH v2 2/2] bpf: remove struct bpf_map_type_list Johannes Berg
@ 2017-04-07 19:03 ` Johannes Berg
  2017-04-10  1:22 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2017-04-07 19:03 UTC (permalink / raw)
  To: netdev; +Cc: Alexei Starovoitov

So two things about this, and they apply to the other patch as well:

> +extern const struct bpf_verifier_ops sk_filter_prog_ops;
> +extern const struct bpf_verifier_ops tc_cls_act_prog_ops;
> +extern const struct bpf_verifier_ops xdp_prog_ops;
> +extern const struct bpf_verifier_ops cg_skb_prog_ops;
> +extern const struct bpf_verifier_ops cg_sock_prog_ops;
> +extern const struct bpf_verifier_ops lwt_inout_prog_ops;
> +extern const struct bpf_verifier_ops lwt_xmit_prog_ops;
> +extern const struct bpf_verifier_ops kprobe_prog_ops;
> +extern const struct bpf_verifier_ops tracepoint_prog_ops;
> +extern const struct bpf_verifier_ops perf_event_prog_ops;

I'm not super happy with having to list it here - and maybe I should
add ifdefs here as well.

+static const struct bpf_verifier_ops * const bpf_prog_types[] = {
> +#ifdef CONFIG_NET
> +	[BPF_PROG_TYPE_SOCKET_FILTER] = &sk_filter_prog_ops,
> +	[BPF_PROG_TYPE_SCHED_CLS] = &tc_cls_act_prog_ops,
> +	[BPF_PROG_TYPE_SCHED_ACT] = &tc_cls_act_prog_ops,
> +	[BPF_PROG_TYPE_XDP] = &xdp_prog_ops,
> +	[BPF_PROG_TYPE_CGROUP_SKB] = &cg_skb_prog_ops,
> +	[BPF_PROG_TYPE_CGROUP_SOCK] = &cg_sock_prog_ops,
> +	[BPF_PROG_TYPE_LWT_IN] = &lwt_inout_prog_ops,
> +	[BPF_PROG_TYPE_LWT_OUT] = &lwt_inout_prog_ops,
> +	[BPF_PROG_TYPE_LWT_XMIT] = &lwt_xmit_prog_ops,
> +#endif
> +#ifdef CONFIG_BPF_EVENTS
> +	[BPF_PROG_TYPE_KPROBE] = &kprobe_prog_ops,
> +	[BPF_PROG_TYPE_TRACEPOINT] = &tracepoint_prog_ops,
> +	[BPF_PROG_TYPE_PERF_EVENT] = &perf_event_prog_ops,
> +#endif

And here I list it again.

Something I considered was to add the prog_type to the ops struct, and
use the linker (putting this into a special section) to assemble an
array. But that
 * makes the lwt_inout_prog_ops that are shared not work without
   duplicating
 * requires more complicated search code

This seems optimal for the resulting binary, but it's a bunch more
typing.

johannes

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

* Re: [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list
  2017-04-07 19:00 [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list Johannes Berg
  2017-04-07 19:00 ` [PATCH v2 2/2] bpf: remove struct bpf_map_type_list Johannes Berg
  2017-04-07 19:03 ` [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list Johannes Berg
@ 2017-04-10  1:22 ` David Miller
  2017-04-10  2:23   ` Alexei Starovoitov
  2 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2017-04-10  1:22 UTC (permalink / raw)
  To: johannes; +Cc: netdev, alexei.starovoitov, johannes.berg

From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri,  7 Apr 2017 21:00:07 +0200

> From: Johannes Berg <johannes.berg@intel.com>
> 
> There's no need to have struct bpf_prog_type_list since
> it just contains a list_head, the type, and the ops
> pointer. Since the types are densely packed and not
> actually dynamically registered, it's much easier and
> smaller to have an array of type->ops pointer. Also
> initialize this array statically to remove code needed
> to initialize it.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

If you just don't want to list things multiple times how about:

linux/bpf_verifiers.h:
BPF_VERIFIER(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter_prog_ops)
BPF_VERIFIER(BPF_PROG_TYPE_SCHED_CLS, tc_cls_prog_ops)
 ...

Then in bpf.h:

#define BPF_VERIFIER(TYPE_VAL, SYM) extern const struct bpf_verifier_ops SYM;
#include <linux/bpf_verifiers.h"

and in kernel/bpf/syscall.c:

#define BPF_VERIFIER(TYPE_VAL, SYM) [TYPE_VAL] = &SYM,
#include <linux/bpf_verifiers.h"

or something like that.

Maybe there is some even more straightforward way to do this without
using sections or anything like that, which we can't see right now :)

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

* Re: [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list
  2017-04-10  1:22 ` David Miller
@ 2017-04-10  2:23   ` Alexei Starovoitov
  2017-04-10  5:42     ` Johannes Berg
  0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2017-04-10  2:23 UTC (permalink / raw)
  To: David Miller; +Cc: johannes, netdev, johannes.berg

On Sun, Apr 09, 2017 at 06:22:36PM -0700, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Fri,  7 Apr 2017 21:00:07 +0200
> 
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > There's no need to have struct bpf_prog_type_list since
> > it just contains a list_head, the type, and the ops
> > pointer. Since the types are densely packed and not
> > actually dynamically registered, it's much easier and
> > smaller to have an array of type->ops pointer. Also
> > initialize this array statically to remove code needed
> > to initialize it.
> > 
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> 
> If you just don't want to list things multiple times how about:
> 
> linux/bpf_verifiers.h:
> BPF_VERIFIER(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter_prog_ops)
> BPF_VERIFIER(BPF_PROG_TYPE_SCHED_CLS, tc_cls_prog_ops)
>  ...
> 
> Then in bpf.h:
> 
> #define BPF_VERIFIER(TYPE_VAL, SYM) extern const struct bpf_verifier_ops SYM;
> #include <linux/bpf_verifiers.h"
> 
> and in kernel/bpf/syscall.c:
> 
> #define BPF_VERIFIER(TYPE_VAL, SYM) [TYPE_VAL] = &SYM,
> #include <linux/bpf_verifiers.h"
> 
> or something like that.

That is great suggestion! I like it.

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

* Re: [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list
  2017-04-10  2:23   ` Alexei Starovoitov
@ 2017-04-10  5:42     ` Johannes Berg
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2017-04-10  5:42 UTC (permalink / raw)
  To: Alexei Starovoitov, David Miller; +Cc: netdev


> > If you just don't want to list things multiple times how about:
> > 
> > linux/bpf_verifiers.h:
> > BPF_VERIFIER(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter_prog_ops)
> > BPF_VERIFIER(BPF_PROG_TYPE_SCHED_CLS, tc_cls_prog_ops)
> >  ...
> > 
> > Then in bpf.h:
> > 
> > #define BPF_VERIFIER(TYPE_VAL, SYM) extern const struct
> > bpf_verifier_ops SYM;
> > #include <linux/bpf_verifiers.h"
> > 
> > and in kernel/bpf/syscall.c:
> > 
> > #define BPF_VERIFIER(TYPE_VAL, SYM) [TYPE_VAL] = &SYM,
> > #include <linux/bpf_verifiers.h"
> > 
> > or something like that.
> 
> That is great suggestion! I like it.

It'd doable, but then I need to play with the include guards, like
having "#define __BPF_REINCLUDE" or something like that before
reincluding it, I guess? Unless that file can simply not be included
anywhere?

Maybe I'll play with it - though perhaps it should be named bpf_types.h
or something like that so we don't have to have two files.

johannes

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

end of thread, other threads:[~2017-04-10  5:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-07 19:00 [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list Johannes Berg
2017-04-07 19:00 ` [PATCH v2 2/2] bpf: remove struct bpf_map_type_list Johannes Berg
2017-04-07 19:03 ` [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list Johannes Berg
2017-04-10  1:22 ` David Miller
2017-04-10  2:23   ` Alexei Starovoitov
2017-04-10  5:42     ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).