bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 0/2] xsk: XSKMAP lookup improvements
@ 2019-10-25  9:32 Björn Töpel
  2019-10-25  9:32 ` [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP Björn Töpel
  2019-10-25  9:32 ` [PATCH bpf-next v3 2/2] bpf: implement map_gen_lookup() callback for XSKMAP Björn Töpel
  0 siblings, 2 replies; 7+ messages in thread
From: Björn Töpel @ 2019-10-25  9:32 UTC (permalink / raw)
  To: netdev, ast, daniel, jakub.kicinski
  Cc: Björn Töpel, bpf, magnus.karlsson, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, toke

Let's try that again. I managed to mess up Maciej's commit message.

This small set consists of two patches from Maciej and myself which
are optimizing the XSKMAP lookups.  In the first patch, the sockets
are moved to be stored at the tail of the struct xsk_map. The second
patch, Maciej implements map_gen_lookup() for XSKMAP.

Based on the XDP program from tools/lib/bpf/xsk.c where
bpf_map_lookup_elem() is explicitly called, this work yields a 5%
improvement for xdpsock's rxdrop scenario.

Cheers,
Björn and Maciej

v1->v2: * Change size/cost to size_t and use {struct, array}_size
          where appropriate. (Jakub)
v2->v3: * Proper commit message for patch 2.

Björn Töpel (1):
  xsk: store struct xdp_sock as a flexible array member of the XSKMAP

Maciej Fijalkowski (1):
  bpf: implement map_gen_lookup() callback for XSKMAP

 kernel/bpf/xskmap.c | 72 +++++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 32 deletions(-)

-- 
2.20.1


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

* [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP
  2019-10-25  9:32 [PATCH bpf-next v3 0/2] xsk: XSKMAP lookup improvements Björn Töpel
@ 2019-10-25  9:32 ` Björn Töpel
  2019-10-25 18:18   ` Jonathan Lemon
  2019-10-28 18:00   ` Jakub Kicinski
  2019-10-25  9:32 ` [PATCH bpf-next v3 2/2] bpf: implement map_gen_lookup() callback for XSKMAP Björn Töpel
  1 sibling, 2 replies; 7+ messages in thread
From: Björn Töpel @ 2019-10-25  9:32 UTC (permalink / raw)
  To: netdev, ast, daniel, jakub.kicinski
  Cc: Björn Töpel, bpf, magnus.karlsson, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, toke

From: Björn Töpel <bjorn.topel@intel.com>

Prior this commit, the array storing XDP socket instances were stored
in a separate allocated array of the XSKMAP. Now, we store the sockets
as a flexible array member in a similar fashion as the arraymap. Doing
so, we do less pointer chasing in the lookup.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 kernel/bpf/xskmap.c | 55 +++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
index 82a1ffe15dfa..a83e92fe2971 100644
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c
@@ -11,9 +11,9 @@
 
 struct xsk_map {
 	struct bpf_map map;
-	struct xdp_sock **xsk_map;
 	struct list_head __percpu *flush_list;
 	spinlock_t lock; /* Synchronize map updates */
+	struct xdp_sock *xsk_map[];
 };
 
 int xsk_map_inc(struct xsk_map *map)
@@ -80,9 +80,10 @@ static void xsk_map_sock_delete(struct xdp_sock *xs,
 
 static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
 {
+	struct bpf_map_memory mem;
+	int cpu, err, numa_node;
 	struct xsk_map *m;
-	int cpu, err;
-	u64 cost;
+	size_t cost, size;
 
 	if (!capable(CAP_NET_ADMIN))
 		return ERR_PTR(-EPERM);
@@ -92,44 +93,35 @@ static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
 	    attr->map_flags & ~(BPF_F_NUMA_NODE | BPF_F_RDONLY | BPF_F_WRONLY))
 		return ERR_PTR(-EINVAL);
 
-	m = kzalloc(sizeof(*m), GFP_USER);
-	if (!m)
+	numa_node = bpf_map_attr_numa_node(attr);
+	size = struct_size(m, xsk_map, attr->max_entries);
+	cost = size + array_size(sizeof(*m->flush_list), num_possible_cpus());
+
+	err = bpf_map_charge_init(&mem, cost);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	m = bpf_map_area_alloc(size, numa_node);
+	if (!m) {
+		bpf_map_charge_finish(&mem);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	bpf_map_init_from_attr(&m->map, attr);
+	bpf_map_charge_move(&m->map.memory, &mem);
 	spin_lock_init(&m->lock);
 
-	cost = (u64)m->map.max_entries * sizeof(struct xdp_sock *);
-	cost += sizeof(struct list_head) * num_possible_cpus();
-
-	/* Notice returns -EPERM on if map size is larger than memlock limit */
-	err = bpf_map_charge_init(&m->map.memory, cost);
-	if (err)
-		goto free_m;
-
-	err = -ENOMEM;
-
 	m->flush_list = alloc_percpu(struct list_head);
-	if (!m->flush_list)
-		goto free_charge;
+	if (!m->flush_list) {
+		bpf_map_charge_finish(&m->map.memory);
+		bpf_map_area_free(m);
+		return ERR_PTR(-ENOMEM);
+	}
 
 	for_each_possible_cpu(cpu)
 		INIT_LIST_HEAD(per_cpu_ptr(m->flush_list, cpu));
 
-	m->xsk_map = bpf_map_area_alloc(m->map.max_entries *
-					sizeof(struct xdp_sock *),
-					m->map.numa_node);
-	if (!m->xsk_map)
-		goto free_percpu;
 	return &m->map;
-
-free_percpu:
-	free_percpu(m->flush_list);
-free_charge:
-	bpf_map_charge_finish(&m->map.memory);
-free_m:
-	kfree(m);
-	return ERR_PTR(err);
 }
 
 static void xsk_map_free(struct bpf_map *map)
@@ -139,8 +131,7 @@ static void xsk_map_free(struct bpf_map *map)
 	bpf_clear_redirect_map(map);
 	synchronize_net();
 	free_percpu(m->flush_list);
-	bpf_map_area_free(m->xsk_map);
-	kfree(m);
+	bpf_map_area_free(m);
 }
 
 static int xsk_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
-- 
2.20.1


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

* [PATCH bpf-next v3 2/2] bpf: implement map_gen_lookup() callback for XSKMAP
  2019-10-25  9:32 [PATCH bpf-next v3 0/2] xsk: XSKMAP lookup improvements Björn Töpel
  2019-10-25  9:32 ` [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP Björn Töpel
@ 2019-10-25  9:32 ` Björn Töpel
  2019-10-25 18:19   ` Jonathan Lemon
  1 sibling, 1 reply; 7+ messages in thread
From: Björn Töpel @ 2019-10-25  9:32 UTC (permalink / raw)
  To: netdev, ast, daniel, jakub.kicinski
  Cc: Maciej Fijalkowski, bpf, magnus.karlsson, magnus.karlsson,
	jonathan.lemon, toke

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Inline the xsk_map_lookup_elem() via implementing the map_gen_lookup()
callback. This results in emitting the bpf instructions in place of
bpf_map_lookup_elem() helper call and better performance of bpf
programs.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 kernel/bpf/xskmap.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
index a83e92fe2971..62ce88e57d98 100644
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c
@@ -163,6 +163,22 @@ struct xdp_sock *__xsk_map_lookup_elem(struct bpf_map *map, u32 key)
 	return xs;
 }
 
+static u32 xsk_map_gen_lookup(struct bpf_map *map, struct bpf_insn *insn_buf)
+{
+	const int ret = BPF_REG_0, mp = BPF_REG_1, index = BPF_REG_2;
+	struct bpf_insn *insn = insn_buf;
+
+	*insn++ = BPF_LDX_MEM(BPF_W, ret, index, 0);
+	*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 5);
+	*insn++ = BPF_ALU64_IMM(BPF_LSH, ret, ilog2(sizeof(struct xsk_sock *)));
+	*insn++ = BPF_ALU64_IMM(BPF_ADD, mp, offsetof(struct xsk_map, xsk_map));
+	*insn++ = BPF_ALU64_REG(BPF_ADD, ret, mp);
+	*insn++ = BPF_LDX_MEM(BPF_DW, ret, ret, 0);
+	*insn++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1);
+	*insn++ = BPF_MOV64_IMM(ret, 0);
+	return insn - insn_buf;
+}
+
 int __xsk_map_redirect(struct bpf_map *map, struct xdp_buff *xdp,
 		       struct xdp_sock *xs)
 {
@@ -303,6 +319,7 @@ const struct bpf_map_ops xsk_map_ops = {
 	.map_free = xsk_map_free,
 	.map_get_next_key = xsk_map_get_next_key,
 	.map_lookup_elem = xsk_map_lookup_elem,
+	.map_gen_lookup = xsk_map_gen_lookup,
 	.map_lookup_elem_sys_only = xsk_map_lookup_elem_sys_only,
 	.map_update_elem = xsk_map_update_elem,
 	.map_delete_elem = xsk_map_delete_elem,
-- 
2.20.1


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

* Re: [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP
  2019-10-25  9:32 ` [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP Björn Töpel
@ 2019-10-25 18:18   ` Jonathan Lemon
  2019-10-28 18:00   ` Jakub Kicinski
  1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Lemon @ 2019-10-25 18:18 UTC (permalink / raw)
  To: Björn Töpel
  Cc: netdev, ast, daniel, jakub.kicinski, Björn Töpel, bpf,
	magnus.karlsson, magnus.karlsson, maciej.fijalkowski, toke



On 25 Oct 2019, at 2:32, Björn Töpel wrote:

> From: Björn Töpel <bjorn.topel@intel.com>
>
> Prior this commit, the array storing XDP socket instances were stored
> in a separate allocated array of the XSKMAP. Now, we store the sockets
> as a flexible array member in a similar fashion as the arraymap. Doing
> so, we do less pointer chasing in the lookup.
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>

Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>

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

* Re: [PATCH bpf-next v3 2/2] bpf: implement map_gen_lookup() callback for XSKMAP
  2019-10-25  9:32 ` [PATCH bpf-next v3 2/2] bpf: implement map_gen_lookup() callback for XSKMAP Björn Töpel
@ 2019-10-25 18:19   ` Jonathan Lemon
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Lemon @ 2019-10-25 18:19 UTC (permalink / raw)
  To: Björn Töpel
  Cc: netdev, ast, daniel, jakub.kicinski, Maciej Fijalkowski, bpf,
	magnus.karlsson, magnus.karlsson, toke



On 25 Oct 2019, at 2:32, Björn Töpel wrote:

> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> Inline the xsk_map_lookup_elem() via implementing the map_gen_lookup()
> callback. This results in emitting the bpf instructions in place of
> bpf_map_lookup_elem() helper call and better performance of bpf
> programs.
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>

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

* Re: [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP
  2019-10-25  9:32 ` [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP Björn Töpel
  2019-10-25 18:18   ` Jonathan Lemon
@ 2019-10-28 18:00   ` Jakub Kicinski
  2019-10-28 20:54     ` Björn Töpel
  1 sibling, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2019-10-28 18:00 UTC (permalink / raw)
  To: Björn Töpel
  Cc: netdev, ast, daniel, Björn Töpel, bpf, magnus.karlsson,
	magnus.karlsson, maciej.fijalkowski, jonathan.lemon, toke

On Fri, 25 Oct 2019 11:32:18 +0200, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> Prior this commit, the array storing XDP socket instances were stored
> in a separate allocated array of the XSKMAP. Now, we store the sockets
> as a flexible array member in a similar fashion as the arraymap. Doing
> so, we do less pointer chasing in the lookup.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>

Damn, looks like I managed to reply to v2. 

I think the size maths may overflow on 32bit machines on the addition.

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

* Re: [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP
  2019-10-28 18:00   ` Jakub Kicinski
@ 2019-10-28 20:54     ` Björn Töpel
  0 siblings, 0 replies; 7+ messages in thread
From: Björn Töpel @ 2019-10-28 20:54 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Netdev, Alexei Starovoitov, Daniel Borkmann,
	Björn Töpel, bpf, Magnus Karlsson, Karlsson, Magnus,
	Fijalkowski, Maciej, Jonathan Lemon,
	Toke Høiland-Jørgensen

On Mon, 28 Oct 2019 at 19:01, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Fri, 25 Oct 2019 11:32:18 +0200, Björn Töpel wrote:
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > Prior this commit, the array storing XDP socket instances were stored
> > in a separate allocated array of the XSKMAP. Now, we store the sockets
> > as a flexible array member in a similar fashion as the arraymap. Doing
> > so, we do less pointer chasing in the lookup.
> >
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
>
> Damn, looks like I managed to reply to v2.
>

No worries! Thanks for taking a second look.

> I think the size maths may overflow on 32bit machines on the addition.

Ugh, right. Re-reading the older threads. Thanks for pointing this
out!I'll get back with a v4.


Björn

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

end of thread, other threads:[~2019-10-28 20:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-25  9:32 [PATCH bpf-next v3 0/2] xsk: XSKMAP lookup improvements Björn Töpel
2019-10-25  9:32 ` [PATCH bpf-next v3 1/2] xsk: store struct xdp_sock as a flexible array member of the XSKMAP Björn Töpel
2019-10-25 18:18   ` Jonathan Lemon
2019-10-28 18:00   ` Jakub Kicinski
2019-10-28 20:54     ` Björn Töpel
2019-10-25  9:32 ` [PATCH bpf-next v3 2/2] bpf: implement map_gen_lookup() callback for XSKMAP Björn Töpel
2019-10-25 18:19   ` Jonathan Lemon

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).