All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP
@ 2020-06-23 21:39 Lorenzo Bianconi
  2020-06-23 21:39 ` [PATCH v3 bpf-next 1/9] cpumap: use non-locked version __ptr_ring_consume_batched Lorenzo Bianconi
                   ` (8 more replies)
  0 siblings, 9 replies; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

Similar to what David Ahern proposed in [1] for DEVMAPs, introduce the
capability to attach and run a XDP program to CPUMAP entries.
The idea behind this feature is to add the possibility to define on which CPU
run the eBPF program if the underlying hw does not support RSS.
I respin patch 1/6 from a previous series sent by David [2].
The functionality has been tested on Marvell Espressobin, i40e and mlx5.
Detailed tests results can be found here:
https://github.com/xdp-project/xdp-project/blob/master/areas/cpumap/cpumap04-map-xdp-prog.org

Changes since v2:
- improved comments
- fix return value in xdp_convert_buff_to_frame
- added patch 1/9: "cpumap: use non-locked version __ptr_ring_consume_batched"
- do not run kmem_cache_alloc_bulk if all frames have been consumed by the XDP
  program attached to the CPUMAP entry
- removed bpf_trace_printk in kselftest

Changes since v1:
- added performance test results
- added kselftest support
- fixed memory accounting with page_pool
- extended xdp_redirect_cpu_user.c to load an external program to perform
  redirect
- reported ifindex to attached eBPF program
- moved bpf_cpumap_val definition to include/uapi/linux/bpf.h

[1] https://patchwork.ozlabs.org/project/netdev/cover/20200529220716.75383-1-dsahern@kernel.org/
[2] https://patchwork.ozlabs.org/project/netdev/patch/20200513014607.40418-2-dsahern@kernel.org/

David Ahern (1):
  net: Refactor xdp_convert_buff_to_frame

Jesper Dangaard Brouer (1):
  cpumap: use non-locked version __ptr_ring_consume_batched

Lorenzo Bianconi (7):
  samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option
    loop
  cpumap: formalize map value as a named struct
  bpf: cpumap: add the possibility to attach an eBPF program to cpumap
  bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map
    entries
  libbpf: add SEC name for xdp programs attached to CPUMAP
  samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap
  selftest: add tests for XDP programs in CPUMAP entries

 include/linux/bpf.h                           |   6 +
 include/net/xdp.h                             |  41 ++--
 include/trace/events/xdp.h                    |  16 +-
 include/uapi/linux/bpf.h                      |  14 ++
 kernel/bpf/cpumap.c                           | 160 +++++++++++---
 net/core/dev.c                                |   8 +
 samples/bpf/xdp_redirect_cpu_kern.c           |  25 ++-
 samples/bpf/xdp_redirect_cpu_user.c           | 208 ++++++++++++++++--
 tools/include/uapi/linux/bpf.h                |  14 ++
 tools/lib/bpf/libbpf.c                        |   2 +
 .../bpf/prog_tests/xdp_cpumap_attach.c        |  70 ++++++
 .../bpf/progs/test_xdp_with_cpumap_helpers.c  |  40 ++++
 12 files changed, 532 insertions(+), 72 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c

-- 
2.26.2


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

* [PATCH v3 bpf-next 1/9] cpumap: use non-locked version __ptr_ring_consume_batched
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-23 21:39 ` [PATCH v3 bpf-next 2/9] net: Refactor xdp_convert_buff_to_frame Lorenzo Bianconi
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

From: Jesper Dangaard Brouer <brouer@redhat.com>

Commit 77361825bb01 ("bpf: cpumap use ptr_ring_consume_batched") changed
away from using single frame ptr_ring dequeue (__ptr_ring_consume) to
consume a batched, but it uses a locked version, which as the comment
explain isn't needed.

Change to use the non-locked version __ptr_ring_consume_batched.

Fixes: 77361825bb01 ("bpf: cpumap use ptr_ring_consume_batched")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 kernel/bpf/cpumap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index bd8658055c16..323c91c4fab0 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -259,7 +259,7 @@ static int cpu_map_kthread_run(void *data)
 		 * kthread CPU pinned. Lockless access to ptr_ring
 		 * consume side valid as no-resize allowed of queue.
 		 */
-		n = ptr_ring_consume_batched(rcpu->queue, frames, CPUMAP_BATCH);
+		n = __ptr_ring_consume_batched(rcpu->queue, frames, CPUMAP_BATCH);
 
 		for (i = 0; i < n; i++) {
 			void *f = frames[i];
-- 
2.26.2


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

* [PATCH v3 bpf-next 2/9] net: Refactor xdp_convert_buff_to_frame
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
  2020-06-23 21:39 ` [PATCH v3 bpf-next 1/9] cpumap: use non-locked version __ptr_ring_consume_batched Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-24  8:06   ` Jesper Dangaard Brouer
  2020-06-23 21:39 ` [PATCH v3 bpf-next 3/9] samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop Lorenzo Bianconi
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko, David Ahern

From: David Ahern <dahern@digitalocean.com>

Move the guts of xdp_convert_buff_to_frame to a new helper,
xdp_update_frame_from_buff so it can be reused removing code duplication

Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 include/net/xdp.h | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 609f819ed08b..5b383c450858 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -121,39 +121,48 @@ void xdp_convert_frame_to_buff(struct xdp_frame *frame, struct xdp_buff *xdp)
 	xdp->frame_sz = frame->frame_sz;
 }
 
-/* Convert xdp_buff to xdp_frame */
 static inline
-struct xdp_frame *xdp_convert_buff_to_frame(struct xdp_buff *xdp)
+int xdp_update_frame_from_buff(struct xdp_buff *xdp,
+			       struct xdp_frame *xdp_frame)
 {
-	struct xdp_frame *xdp_frame;
-	int metasize;
-	int headroom;
-
-	if (xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL)
-		return xdp_convert_zc_to_xdp_frame(xdp);
+	int metasize, headroom;
 
 	/* Assure headroom is available for storing info */
 	headroom = xdp->data - xdp->data_hard_start;
 	metasize = xdp->data - xdp->data_meta;
 	metasize = metasize > 0 ? metasize : 0;
 	if (unlikely((headroom - metasize) < sizeof(*xdp_frame)))
-		return NULL;
+		return -ENOSPC;
 
 	/* Catch if driver didn't reserve tailroom for skb_shared_info */
 	if (unlikely(xdp->data_end > xdp_data_hard_end(xdp))) {
 		XDP_WARN("Driver BUG: missing reserved tailroom");
-		return NULL;
+		return -ENOSPC;
 	}
 
-	/* Store info in top of packet */
-	xdp_frame = xdp->data_hard_start;
-
 	xdp_frame->data = xdp->data;
 	xdp_frame->len  = xdp->data_end - xdp->data;
 	xdp_frame->headroom = headroom - sizeof(*xdp_frame);
 	xdp_frame->metasize = metasize;
 	xdp_frame->frame_sz = xdp->frame_sz;
 
+	return 0;
+}
+
+/* Convert xdp_buff to xdp_frame */
+static inline
+struct xdp_frame *xdp_convert_buff_to_frame(struct xdp_buff *xdp)
+{
+	struct xdp_frame *xdp_frame;
+
+	if (xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL)
+		return xdp_convert_zc_to_xdp_frame(xdp);
+
+	/* Store info in top of packet */
+	xdp_frame = xdp->data_hard_start;
+	if (unlikely(xdp_update_frame_from_buff(xdp, xdp_frame) < 0))
+		return NULL;
+
 	/* rxq only valid until napi_schedule ends, convert to xdp_mem_info */
 	xdp_frame->mem = xdp->rxq->mem;
 
-- 
2.26.2


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

* [PATCH v3 bpf-next 3/9] samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
  2020-06-23 21:39 ` [PATCH v3 bpf-next 1/9] cpumap: use non-locked version __ptr_ring_consume_batched Lorenzo Bianconi
  2020-06-23 21:39 ` [PATCH v3 bpf-next 2/9] net: Refactor xdp_convert_buff_to_frame Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-23 21:39 ` [PATCH v3 bpf-next 4/9] cpumap: formalize map value as a named struct Lorenzo Bianconi
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

Do not update xdp_redirect_cpu maps running while option loop but
defer it after all available options have been parsed. This is a
preliminary patch to pass the program name we want to attach to the
map entries as a user option

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 samples/bpf/xdp_redirect_cpu_user.c | 36 +++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index f3468168982e..1a054737c35a 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -684,6 +684,7 @@ int main(int argc, char **argv)
 	int add_cpu = -1;
 	int opt, err;
 	int prog_fd;
+	int *cpu, i;
 	__u32 qsize;
 
 	n_cpus = get_nprocs_conf();
@@ -719,6 +720,13 @@ int main(int argc, char **argv)
 	}
 	mark_cpus_unavailable();
 
+	cpu = malloc(n_cpus * sizeof(int));
+	if (!cpu) {
+		fprintf(stderr, "failed to allocate cpu array\n");
+		return EXIT_FAIL;
+	}
+	memset(cpu, 0, n_cpus * sizeof(int));
+
 	/* Parse commands line args */
 	while ((opt = getopt_long(argc, argv, "hSd:s:p:q:c:xzF",
 				  long_options, &longindex)) != -1) {
@@ -763,8 +771,7 @@ int main(int argc, char **argv)
 					errno, strerror(errno));
 				goto error;
 			}
-			create_cpu_entry(add_cpu, qsize, added_cpus, true);
-			added_cpus++;
+			cpu[added_cpus++] = add_cpu;
 			break;
 		case 'q':
 			qsize = atoi(optarg);
@@ -775,6 +782,7 @@ int main(int argc, char **argv)
 		case 'h':
 		error:
 		default:
+			free(cpu);
 			usage(argv, obj);
 			return EXIT_FAIL_OPTION;
 		}
@@ -787,16 +795,21 @@ int main(int argc, char **argv)
 	if (ifindex == -1) {
 		fprintf(stderr, "ERR: required option --dev missing\n");
 		usage(argv, obj);
-		return EXIT_FAIL_OPTION;
+		err = EXIT_FAIL_OPTION;
+		goto out;
 	}
 	/* Required option */
 	if (add_cpu == -1) {
 		fprintf(stderr, "ERR: required option --cpu missing\n");
 		fprintf(stderr, " Specify multiple --cpu option to add more\n");
 		usage(argv, obj);
-		return EXIT_FAIL_OPTION;
+		err = EXIT_FAIL_OPTION;
+		goto out;
 	}
 
+	for (i = 0; i < added_cpus; i++)
+		create_cpu_entry(cpu[i], qsize, i, true);
+
 	/* Remove XDP program when program is interrupted or killed */
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
@@ -804,27 +817,32 @@ int main(int argc, char **argv)
 	prog = bpf_object__find_program_by_title(obj, prog_name);
 	if (!prog) {
 		fprintf(stderr, "bpf_object__find_program_by_title failed\n");
-		return EXIT_FAIL;
+		err = EXIT_FAIL;
+		goto out;
 	}
 
 	prog_fd = bpf_program__fd(prog);
 	if (prog_fd < 0) {
 		fprintf(stderr, "bpf_program__fd failed\n");
-		return EXIT_FAIL;
+		err = EXIT_FAIL;
+		goto out;
 	}
 
 	if (bpf_set_link_xdp_fd(ifindex, prog_fd, xdp_flags) < 0) {
 		fprintf(stderr, "link set xdp fd failed\n");
-		return EXIT_FAIL_XDP;
+		err = EXIT_FAIL_XDP;
+		goto out;
 	}
 
 	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
 	if (err) {
 		printf("can't get prog info - %s\n", strerror(errno));
-		return err;
+		goto out;
 	}
 	prog_id = info.id;
 
 	stats_poll(interval, use_separators, prog_name, stress_mode);
-	return EXIT_OK;
+out:
+	free(cpu);
+	return err;
 }
-- 
2.26.2


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

* [PATCH v3 bpf-next 4/9] cpumap: formalize map value as a named struct
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (2 preceding siblings ...)
  2020-06-23 21:39 ` [PATCH v3 bpf-next 3/9] samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-24  8:08   ` Jesper Dangaard Brouer
  2020-06-23 21:39 ` [PATCH v3 bpf-next 5/9] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

As it has been already done for devmap, introduce 'struct bpf_cpumap_val'
to formalize the expected values that can be passed in for a CPUMAP.
Update cpumap code to use the struct.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/uapi/linux/bpf.h       |  9 +++++++++
 kernel/bpf/cpumap.c            | 25 +++++++++++++------------
 tools/include/uapi/linux/bpf.h |  9 +++++++++
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 19684813faae..fa3b342985f9 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3774,6 +3774,15 @@ struct bpf_devmap_val {
 	} bpf_prog;
 };
 
+/* CPUMAP map-value layout
+ *
+ * The struct data-layout of map-value is a configuration interface.
+ * New members can only be added to the end of this structure.
+ */
+struct bpf_cpumap_val {
+	__u32 qsize;	/* queue size to remote target CPU */
+};
+
 enum sk_action {
 	SK_DROP = 0,
 	SK_PASS,
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 323c91c4fab0..7e8eec4f7089 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -52,7 +52,6 @@ struct xdp_bulk_queue {
 struct bpf_cpu_map_entry {
 	u32 cpu;    /* kthread CPU and map index */
 	int map_id; /* Back reference to map */
-	u32 qsize;  /* Queue size placeholder for map lookup */
 
 	/* XDP can run multiple RX-ring queues, need __percpu enqueue store */
 	struct xdp_bulk_queue __percpu *bulkq;
@@ -66,6 +65,8 @@ struct bpf_cpu_map_entry {
 
 	atomic_t refcnt; /* Control when this struct can be free'ed */
 	struct rcu_head rcu;
+
+	struct bpf_cpumap_val value;
 };
 
 struct bpf_cpu_map {
@@ -307,8 +308,8 @@ static int cpu_map_kthread_run(void *data)
 	return 0;
 }
 
-static struct bpf_cpu_map_entry *__cpu_map_entry_alloc(u32 qsize, u32 cpu,
-						       int map_id)
+static struct bpf_cpu_map_entry *
+__cpu_map_entry_alloc(struct bpf_cpumap_val *value, u32 cpu, int map_id)
 {
 	gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
 	struct bpf_cpu_map_entry *rcpu;
@@ -338,13 +339,13 @@ static struct bpf_cpu_map_entry *__cpu_map_entry_alloc(u32 qsize, u32 cpu,
 	if (!rcpu->queue)
 		goto free_bulkq;
 
-	err = ptr_ring_init(rcpu->queue, qsize, gfp);
+	err = ptr_ring_init(rcpu->queue, value->qsize, gfp);
 	if (err)
 		goto free_queue;
 
 	rcpu->cpu    = cpu;
 	rcpu->map_id = map_id;
-	rcpu->qsize  = qsize;
+	rcpu->value.qsize  = value->qsize;
 
 	/* Setup kthread */
 	rcpu->kthread = kthread_create_on_node(cpu_map_kthread_run, rcpu, numa,
@@ -437,12 +438,12 @@ static int cpu_map_update_elem(struct bpf_map *map, void *key, void *value,
 			       u64 map_flags)
 {
 	struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
+	struct bpf_cpumap_val cpumap_value = {};
 	struct bpf_cpu_map_entry *rcpu;
-
 	/* Array index key correspond to CPU number */
 	u32 key_cpu = *(u32 *)key;
-	/* Value is the queue size */
-	u32 qsize = *(u32 *)value;
+
+	memcpy(&cpumap_value, value, map->value_size);
 
 	if (unlikely(map_flags > BPF_EXIST))
 		return -EINVAL;
@@ -450,18 +451,18 @@ static int cpu_map_update_elem(struct bpf_map *map, void *key, void *value,
 		return -E2BIG;
 	if (unlikely(map_flags == BPF_NOEXIST))
 		return -EEXIST;
-	if (unlikely(qsize > 16384)) /* sanity limit on qsize */
+	if (unlikely(cpumap_value.qsize > 16384)) /* sanity limit on qsize */
 		return -EOVERFLOW;
 
 	/* Make sure CPU is a valid possible cpu */
 	if (key_cpu >= nr_cpumask_bits || !cpu_possible(key_cpu))
 		return -ENODEV;
 
-	if (qsize == 0) {
+	if (cpumap_value.qsize == 0) {
 		rcpu = NULL; /* Same as deleting */
 	} else {
 		/* Updating qsize cause re-allocation of bpf_cpu_map_entry */
-		rcpu = __cpu_map_entry_alloc(qsize, key_cpu, map->id);
+		rcpu = __cpu_map_entry_alloc(&cpumap_value, key_cpu, map->id);
 		if (!rcpu)
 			return -ENOMEM;
 		rcpu->cmap = cmap;
@@ -523,7 +524,7 @@ static void *cpu_map_lookup_elem(struct bpf_map *map, void *key)
 	struct bpf_cpu_map_entry *rcpu =
 		__cpu_map_lookup_elem(map, *(u32 *)key);
 
-	return rcpu ? &rcpu->qsize : NULL;
+	return rcpu ? &rcpu->value : NULL;
 }
 
 static int cpu_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 19684813faae..fa3b342985f9 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3774,6 +3774,15 @@ struct bpf_devmap_val {
 	} bpf_prog;
 };
 
+/* CPUMAP map-value layout
+ *
+ * The struct data-layout of map-value is a configuration interface.
+ * New members can only be added to the end of this structure.
+ */
+struct bpf_cpumap_val {
+	__u32 qsize;	/* queue size to remote target CPU */
+};
+
 enum sk_action {
 	SK_DROP = 0,
 	SK_PASS,
-- 
2.26.2


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

* [PATCH v3 bpf-next 5/9] bpf: cpumap: add the possibility to attach an eBPF program to cpumap
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (3 preceding siblings ...)
  2020-06-23 21:39 ` [PATCH v3 bpf-next 4/9] cpumap: formalize map value as a named struct Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-24  8:12   ` Jesper Dangaard Brouer
  2020-06-23 21:39 ` [PATCH v3 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries Lorenzo Bianconi
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

Introduce the capability to attach an eBPF program to cpumap entries.
The idea behind this feature is to add the possibility to define on
which CPU run the eBPF program if the underlying hw does not support
RSS. Current supported verdicts are XDP_DROP and XDP_PASS.

This patch has been tested on Marvell ESPRESSObin using xdp_redirect_cpu
sample available in the kernel tree to identify possible performance
regressions. Results show there are no observable differences in
packet-per-second:

$./xdp_redirect_cpu --progname xdp_cpu_map0 --dev eth0 --cpu 1
rx: 354.8 Kpps
rx: 356.0 Kpps
rx: 356.8 Kpps
rx: 356.3 Kpps
rx: 356.6 Kpps
rx: 356.6 Kpps
rx: 356.7 Kpps
rx: 355.8 Kpps
rx: 356.8 Kpps
rx: 356.8 Kpps

Co-developed-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/linux/bpf.h            |   6 ++
 include/net/xdp.h              |   5 ++
 include/trace/events/xdp.h     |  14 ++--
 include/uapi/linux/bpf.h       |   5 ++
 kernel/bpf/cpumap.c            | 122 +++++++++++++++++++++++++++++----
 net/core/dev.c                 |   8 +++
 tools/include/uapi/linux/bpf.h |   5 ++
 7 files changed, 148 insertions(+), 17 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1e1501ee53ce..759f280265d0 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1265,6 +1265,7 @@ struct bpf_cpu_map_entry *__cpu_map_lookup_elem(struct bpf_map *map, u32 key);
 void __cpu_map_flush(void);
 int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu, struct xdp_buff *xdp,
 		    struct net_device *dev_rx);
+bool cpu_map_prog_allowed(struct bpf_map *map);
 
 /* Return map's numa specified by userspace */
 static inline int bpf_map_attr_numa_node(const union bpf_attr *attr)
@@ -1425,6 +1426,11 @@ static inline int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu,
 	return 0;
 }
 
+static inline bool cpu_map_prog_allowed(struct bpf_map *map)
+{
+	return false;
+}
+
 static inline struct bpf_prog *bpf_prog_get_type_path(const char *name,
 				enum bpf_prog_type type)
 {
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 5b383c450858..83b9e0142b52 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -98,6 +98,11 @@ struct xdp_frame {
 	struct net_device *dev_rx; /* used by cpumap */
 };
 
+struct xdp_cpumap_stats {
+	unsigned int pass;
+	unsigned int drop;
+};
+
 /* Clear kernel pointers in xdp_frame */
 static inline void xdp_scrub_frame(struct xdp_frame *frame)
 {
diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index b73d3e141323..e2c99f5bee39 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -177,9 +177,9 @@ DEFINE_EVENT(xdp_redirect_template, xdp_redirect_map_err,
 TRACE_EVENT(xdp_cpumap_kthread,
 
 	TP_PROTO(int map_id, unsigned int processed,  unsigned int drops,
-		 int sched),
+		 int sched, struct xdp_cpumap_stats *xdp_stats),
 
-	TP_ARGS(map_id, processed, drops, sched),
+	TP_ARGS(map_id, processed, drops, sched, xdp_stats),
 
 	TP_STRUCT__entry(
 		__field(int, map_id)
@@ -188,6 +188,8 @@ TRACE_EVENT(xdp_cpumap_kthread,
 		__field(unsigned int, drops)
 		__field(unsigned int, processed)
 		__field(int, sched)
+		__field(unsigned int, xdp_pass)
+		__field(unsigned int, xdp_drop)
 	),
 
 	TP_fast_assign(
@@ -197,16 +199,20 @@ TRACE_EVENT(xdp_cpumap_kthread,
 		__entry->drops		= drops;
 		__entry->processed	= processed;
 		__entry->sched	= sched;
+		__entry->xdp_pass	= xdp_stats->pass;
+		__entry->xdp_drop	= xdp_stats->drop;
 	),
 
 	TP_printk("kthread"
 		  " cpu=%d map_id=%d action=%s"
 		  " processed=%u drops=%u"
-		  " sched=%d",
+		  " sched=%d"
+		  " xdp_pass=%u xdp_drop=%u",
 		  __entry->cpu, __entry->map_id,
 		  __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB),
 		  __entry->processed, __entry->drops,
-		  __entry->sched)
+		  __entry->sched,
+		  __entry->xdp_pass, __entry->xdp_drop)
 );
 
 TRACE_EVENT(xdp_cpumap_enqueue,
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index fa3b342985f9..856c8db22517 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -226,6 +226,7 @@ enum bpf_attach_type {
 	BPF_CGROUP_INET4_GETSOCKNAME,
 	BPF_CGROUP_INET6_GETSOCKNAME,
 	BPF_XDP_DEVMAP,
+	BPF_XDP_CPUMAP,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -3781,6 +3782,10 @@ struct bpf_devmap_val {
  */
 struct bpf_cpumap_val {
 	__u32 qsize;	/* queue size to remote target CPU */
+	union {
+		int   fd;	/* prog fd on map write */
+		__u32 id;	/* prog id on map read */
+	} bpf_prog;
 };
 
 enum sk_action {
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 7e8eec4f7089..4e4cd240f07b 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -67,6 +67,7 @@ struct bpf_cpu_map_entry {
 	struct rcu_head rcu;
 
 	struct bpf_cpumap_val value;
+	struct bpf_prog *prog;
 };
 
 struct bpf_cpu_map {
@@ -81,6 +82,7 @@ static int bq_flush_to_queue(struct xdp_bulk_queue *bq);
 
 static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
 {
+	u32 value_size = attr->value_size;
 	struct bpf_cpu_map *cmap;
 	int err = -ENOMEM;
 	u64 cost;
@@ -91,7 +93,9 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
 
 	/* check sanity of attributes */
 	if (attr->max_entries == 0 || attr->key_size != 4 ||
-	    attr->value_size != 4 || attr->map_flags & ~BPF_F_NUMA_NODE)
+	    (value_size != offsetofend(struct bpf_cpumap_val, qsize) &&
+	     value_size != offsetofend(struct bpf_cpumap_val, bpf_prog.fd)) ||
+	    attr->map_flags & ~BPF_F_NUMA_NODE)
 		return ERR_PTR(-EINVAL);
 
 	cmap = kzalloc(sizeof(*cmap), GFP_USER);
@@ -221,6 +225,63 @@ static void put_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
 	}
 }
 
+static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu,
+				    void **frames, int n,
+				    struct xdp_cpumap_stats *stats)
+{
+	struct xdp_rxq_info rxq;
+	struct bpf_prog *prog;
+	struct xdp_buff xdp;
+	int i, nframes = 0;
+
+	if (!rcpu->prog)
+		return n;
+
+	xdp_set_return_frame_no_direct();
+	xdp.rxq = &rxq;
+
+	rcu_read_lock();
+
+	prog = READ_ONCE(rcpu->prog);
+	for (i = 0; i < n; i++) {
+		struct xdp_frame *xdpf = frames[i];
+		u32 act;
+		int err;
+
+		rxq.dev = xdpf->dev_rx;
+		rxq.mem = xdpf->mem;
+		/* TODO: report queue_index to xdp_rxq_info */
+
+		xdp_convert_frame_to_buff(xdpf, &xdp);
+
+		act = bpf_prog_run_xdp(prog, &xdp);
+		switch (act) {
+		case XDP_PASS:
+			err = xdp_update_frame_from_buff(&xdp, xdpf);
+			if (err < 0) {
+				xdp_return_frame(xdpf);
+				stats->drop++;
+			} else {
+				frames[nframes++] = xdpf;
+				stats->pass++;
+			}
+			break;
+		default:
+			bpf_warn_invalid_xdp_action(act);
+			/* fallthrough */
+		case XDP_DROP:
+			xdp_return_frame(xdpf);
+			stats->drop++;
+			break;
+		}
+	}
+
+	rcu_read_unlock();
+	xdp_clear_return_frame_no_direct();
+
+	return nframes;
+}
+
 #define CPUMAP_BATCH 8
 
 static int cpu_map_kthread_run(void *data)
@@ -235,11 +296,12 @@ static int cpu_map_kthread_run(void *data)
 	 * kthread_stop signal until queue is empty.
 	 */
 	while (!kthread_should_stop() || !__ptr_ring_empty(rcpu->queue)) {
+		struct xdp_cpumap_stats stats = {}; /* zero stats */
+		gfp_t gfp = __GFP_ZERO | GFP_ATOMIC;
 		unsigned int drops = 0, sched = 0;
 		void *frames[CPUMAP_BATCH];
 		void *skbs[CPUMAP_BATCH];
-		gfp_t gfp = __GFP_ZERO | GFP_ATOMIC;
-		int i, n, m;
+		int i, n, m, nframes;
 
 		/* Release CPU reschedule checks */
 		if (__ptr_ring_empty(rcpu->queue)) {
@@ -260,8 +322,8 @@ static int cpu_map_kthread_run(void *data)
 		 * kthread CPU pinned. Lockless access to ptr_ring
 		 * consume side valid as no-resize allowed of queue.
 		 */
-		n = __ptr_ring_consume_batched(rcpu->queue, frames, CPUMAP_BATCH);
-
+		n = __ptr_ring_consume_batched(rcpu->queue, frames,
+					       CPUMAP_BATCH);
 		for (i = 0; i < n; i++) {
 			void *f = frames[i];
 			struct page *page = virt_to_page(f);
@@ -273,15 +335,19 @@ static int cpu_map_kthread_run(void *data)
 			prefetchw(page);
 		}
 
-		m = kmem_cache_alloc_bulk(skbuff_head_cache, gfp, n, skbs);
-		if (unlikely(m == 0)) {
-			for (i = 0; i < n; i++)
-				skbs[i] = NULL; /* effect: xdp_return_frame */
-			drops = n;
+		/* Support running another XDP prog on this CPU */
+		nframes = cpu_map_bpf_prog_run_xdp(rcpu, frames, n, &stats);
+		if (nframes) {
+			m = kmem_cache_alloc_bulk(skbuff_head_cache, gfp, nframes, skbs);
+			if (unlikely(m == 0)) {
+				for (i = 0; i < nframes; i++)
+					skbs[i] = NULL; /* effect: xdp_return_frame */
+				drops += nframes;
+			}
 		}
 
 		local_bh_disable();
-		for (i = 0; i < n; i++) {
+		for (i = 0; i < nframes; i++) {
 			struct xdp_frame *xdpf = frames[i];
 			struct sk_buff *skb = skbs[i];
 			int ret;
@@ -298,7 +364,7 @@ static int cpu_map_kthread_run(void *data)
 				drops++;
 		}
 		/* Feedback loop via tracepoint */
-		trace_xdp_cpumap_kthread(rcpu->map_id, n, drops, sched);
+		trace_xdp_cpumap_kthread(rcpu->map_id, n, drops, sched, &stats);
 
 		local_bh_enable(); /* resched point, may call do_softirq() */
 	}
@@ -308,13 +374,38 @@ static int cpu_map_kthread_run(void *data)
 	return 0;
 }
 
+bool cpu_map_prog_allowed(struct bpf_map *map)
+{
+	return map->map_type == BPF_MAP_TYPE_CPUMAP &&
+	       map->value_size != offsetofend(struct bpf_cpumap_val, qsize);
+}
+
+static int __cpu_map_load_bpf_program(struct bpf_cpu_map_entry *rcpu, int fd)
+{
+	struct bpf_prog *prog;
+
+	prog = bpf_prog_get_type_dev(fd, BPF_PROG_TYPE_XDP, false);
+	if (IS_ERR(prog))
+		return PTR_ERR(prog);
+
+	if (prog->expected_attach_type != BPF_XDP_CPUMAP) {
+		bpf_prog_put(prog);
+		return -EINVAL;
+	}
+
+	rcpu->value.bpf_prog.id = prog->aux->id;
+	rcpu->prog = prog;
+
+	return 0;
+}
+
 static struct bpf_cpu_map_entry *
 __cpu_map_entry_alloc(struct bpf_cpumap_val *value, u32 cpu, int map_id)
 {
+	int numa, err, i, fd = value->bpf_prog.fd;
 	gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
 	struct bpf_cpu_map_entry *rcpu;
 	struct xdp_bulk_queue *bq;
-	int numa, err, i;
 
 	/* Have map->numa_node, but choose node of redirect target CPU */
 	numa = cpu_to_node(cpu);
@@ -356,6 +447,9 @@ __cpu_map_entry_alloc(struct bpf_cpumap_val *value, u32 cpu, int map_id)
 	get_cpu_map_entry(rcpu); /* 1-refcnt for being in cmap->cpu_map[] */
 	get_cpu_map_entry(rcpu); /* 1-refcnt for kthread */
 
+	if (fd > 0 && __cpu_map_load_bpf_program(rcpu, fd))
+		goto free_ptr_ring;
+
 	/* Make sure kthread runs on a single CPU */
 	kthread_bind(rcpu->kthread, cpu);
 	wake_up_process(rcpu->kthread);
@@ -415,6 +509,8 @@ static void __cpu_map_entry_replace(struct bpf_cpu_map *cmap,
 
 	old_rcpu = xchg(&cmap->cpu_map[key_cpu], rcpu);
 	if (old_rcpu) {
+		if (old_rcpu->prog)
+			bpf_prog_put(old_rcpu->prog);
 		call_rcu(&old_rcpu->rcu, __cpu_map_entry_free);
 		INIT_WORK(&old_rcpu->kthread_stop_wq, cpu_map_kthread_stop);
 		schedule_work(&old_rcpu->kthread_stop_wq);
diff --git a/net/core/dev.c b/net/core/dev.c
index 6bc2388141f6..2867df05cf82 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5440,6 +5440,8 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 		for (i = 0; i < new->aux->used_map_cnt; i++) {
 			if (dev_map_can_have_prog(new->aux->used_maps[i]))
 				return -EINVAL;
+			if (cpu_map_prog_allowed(new->aux->used_maps[i]))
+				return -EINVAL;
 		}
 	}
 
@@ -8864,6 +8866,12 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 			return -EINVAL;
 		}
 
+		if (prog->expected_attach_type == BPF_XDP_CPUMAP) {
+			NL_SET_ERR_MSG(extack, "BPF_XDP_CPUMAP programs can not be attached to a device");
+			bpf_prog_put(prog);
+			return -EINVAL;
+		}
+
 		/* prog->aux->id may be 0 for orphaned device-bound progs */
 		if (prog->aux->id && prog->aux->id == prog_id) {
 			bpf_prog_put(prog);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index fa3b342985f9..856c8db22517 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -226,6 +226,7 @@ enum bpf_attach_type {
 	BPF_CGROUP_INET4_GETSOCKNAME,
 	BPF_CGROUP_INET6_GETSOCKNAME,
 	BPF_XDP_DEVMAP,
+	BPF_XDP_CPUMAP,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -3781,6 +3782,10 @@ struct bpf_devmap_val {
  */
 struct bpf_cpumap_val {
 	__u32 qsize;	/* queue size to remote target CPU */
+	union {
+		int   fd;	/* prog fd on map write */
+		__u32 id;	/* prog id on map read */
+	} bpf_prog;
 };
 
 enum sk_action {
-- 
2.26.2


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

* [PATCH v3 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (4 preceding siblings ...)
  2020-06-23 21:39 ` [PATCH v3 bpf-next 5/9] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-24  8:32   ` Jesper Dangaard Brouer
  2020-06-23 21:39 ` [PATCH v3 bpf-next 7/9] libbpf: add SEC name for xdp programs attached to CPUMAP Lorenzo Bianconi
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

Introduce XDP_REDIRECT support for eBPF programs attached to cpumap
entries.
This patch has been tested on Marvell ESPRESSObin using a modified
version of xdp_redirect_cpu sample in order to attach a XDP program
to CPUMAP entries to perform a redirect on the mvneta interface.
In particular the following scenario has been tested:

rq (cpu0) --> mvneta - XDP_REDIRECT (cpu0) --> CPUMAP - XDP_REDIRECT (cpu1) --> mvneta

$./xdp_redirect_cpu -p xdp_cpu_map0 -d eth0 -c 1 -e xdp_redirect \
	-f xdp_redirect_kern.o -m tx_port -r eth0

tx: 285.2 Kpps rx: 285.2 Kpps

Attacching a simple XDP program on eth0 to perform XDP_TX gives
comparable results:

tx: 288.4 Kpps rx: 288.4 Kpps

Co-developed-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/net/xdp.h          |  1 +
 include/trace/events/xdp.h |  6 ++++--
 kernel/bpf/cpumap.c        | 17 +++++++++++++++--
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 83b9e0142b52..5be0d4d65b94 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -99,6 +99,7 @@ struct xdp_frame {
 };
 
 struct xdp_cpumap_stats {
+	unsigned int redirect;
 	unsigned int pass;
 	unsigned int drop;
 };
diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index e2c99f5bee39..cd24e8a59529 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -190,6 +190,7 @@ TRACE_EVENT(xdp_cpumap_kthread,
 		__field(int, sched)
 		__field(unsigned int, xdp_pass)
 		__field(unsigned int, xdp_drop)
+		__field(unsigned int, xdp_redirect)
 	),
 
 	TP_fast_assign(
@@ -201,18 +202,19 @@ TRACE_EVENT(xdp_cpumap_kthread,
 		__entry->sched	= sched;
 		__entry->xdp_pass	= xdp_stats->pass;
 		__entry->xdp_drop	= xdp_stats->drop;
+		__entry->xdp_redirect	= xdp_stats->redirect;
 	),
 
 	TP_printk("kthread"
 		  " cpu=%d map_id=%d action=%s"
 		  " processed=%u drops=%u"
 		  " sched=%d"
-		  " xdp_pass=%u xdp_drop=%u",
+		  " xdp_pass=%u xdp_drop=%u xdp_redirect=%u",
 		  __entry->cpu, __entry->map_id,
 		  __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB),
 		  __entry->processed, __entry->drops,
 		  __entry->sched,
-		  __entry->xdp_pass, __entry->xdp_drop)
+		  __entry->xdp_pass, __entry->xdp_drop, __entry->xdp_redirect)
 );
 
 TRACE_EVENT(xdp_cpumap_enqueue,
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 4e4cd240f07b..c0b2f265ccb2 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -240,7 +240,7 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu,
 	xdp_set_return_frame_no_direct();
 	xdp.rxq = &rxq;
 
-	rcu_read_lock();
+	rcu_read_lock_bh();
 
 	prog = READ_ONCE(rcpu->prog);
 	for (i = 0; i < n; i++) {
@@ -266,6 +266,16 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu,
 				stats->pass++;
 			}
 			break;
+		case XDP_REDIRECT:
+			err = xdp_do_redirect(xdpf->dev_rx, &xdp,
+					      prog);
+			if (unlikely(err)) {
+				xdp_return_frame(xdpf);
+				stats->drop++;
+			} else {
+				stats->redirect++;
+			}
+			break;
 		default:
 			bpf_warn_invalid_xdp_action(act);
 			/* fallthrough */
@@ -276,7 +286,10 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu,
 		}
 	}
 
-	rcu_read_unlock();
+	if (stats->redirect)
+		xdp_do_flush_map();
+
+	rcu_read_unlock_bh(); /* resched point, may call do_softirq() */
 	xdp_clear_return_frame_no_direct();
 
 	return nframes;
-- 
2.26.2


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

* [PATCH v3 bpf-next 7/9] libbpf: add SEC name for xdp programs attached to CPUMAP
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (5 preceding siblings ...)
  2020-06-23 21:39 ` [PATCH v3 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-24  5:49   ` Andrii Nakryiko
  2020-06-23 21:39 ` [PATCH v3 bpf-next 8/9] samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap Lorenzo Bianconi
  2020-06-23 21:39 ` [PATCH v3 bpf-next 9/9] selftest: add tests for XDP programs in CPUMAP entries Lorenzo Bianconi
  8 siblings, 1 reply; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

As for DEVMAP, support SEC("xdp_cpumap*") as a short cut for loading
the program with type BPF_PROG_TYPE_XDP and expected attach type
BPF_XDP_CPUMAP.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 tools/lib/bpf/libbpf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 18461deb1b19..16fa3b84ac38 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6866,6 +6866,8 @@ static const struct bpf_sec_def section_defs[] = {
 		.attach_fn = attach_iter),
 	BPF_EAPROG_SEC("xdp_devmap",		BPF_PROG_TYPE_XDP,
 						BPF_XDP_DEVMAP),
+	BPF_EAPROG_SEC("xdp_cpumap/",		BPF_PROG_TYPE_XDP,
+						BPF_XDP_CPUMAP),
 	BPF_PROG_SEC("xdp",			BPF_PROG_TYPE_XDP),
 	BPF_PROG_SEC("perf_event",		BPF_PROG_TYPE_PERF_EVENT),
 	BPF_PROG_SEC("lwt_in",			BPF_PROG_TYPE_LWT_IN),
-- 
2.26.2


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

* [PATCH v3 bpf-next 8/9] samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (6 preceding siblings ...)
  2020-06-23 21:39 ` [PATCH v3 bpf-next 7/9] libbpf: add SEC name for xdp programs attached to CPUMAP Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-24  9:00   ` Jesper Dangaard Brouer
  2020-06-23 21:39 ` [PATCH v3 bpf-next 9/9] selftest: add tests for XDP programs in CPUMAP entries Lorenzo Bianconi
  8 siblings, 1 reply; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

Extend xdp_redirect_cpu_{usr,kern}.c adding the possibility to load
a XDP program on cpumap entries. The following options have been added:
- mprog-name: cpumap entry program name
- mprog-filename: cpumap entry program filename
- redirect-device: output interface if the cpumap program performs a
  XDP_REDIRECT to an egress interface
- redirect-map: bpf map used to perform XDP_REDIRECT to an egress
  interface
- mprog-disable: disable loading XDP program on cpumap entries

Add xdp_pass, xdp_drop, xdp_redirect stats accounting

Co-developed-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 samples/bpf/xdp_redirect_cpu_kern.c |  25 ++--
 samples/bpf/xdp_redirect_cpu_user.c | 174 +++++++++++++++++++++++++---
 2 files changed, 177 insertions(+), 22 deletions(-)

diff --git a/samples/bpf/xdp_redirect_cpu_kern.c b/samples/bpf/xdp_redirect_cpu_kern.c
index 2baf8db1f7e7..8255025dea97 100644
--- a/samples/bpf/xdp_redirect_cpu_kern.c
+++ b/samples/bpf/xdp_redirect_cpu_kern.c
@@ -21,7 +21,7 @@
 struct {
 	__uint(type, BPF_MAP_TYPE_CPUMAP);
 	__uint(key_size, sizeof(u32));
-	__uint(value_size, sizeof(u32));
+	__uint(value_size, sizeof(struct bpf_cpumap_val));
 	__uint(max_entries, MAX_CPUS);
 } cpu_map SEC(".maps");
 
@@ -30,6 +30,9 @@ struct datarec {
 	__u64 processed;
 	__u64 dropped;
 	__u64 issue;
+	__u64 xdp_pass;
+	__u64 xdp_drop;
+	__u64 xdp_redirect;
 };
 
 /* Count RX packets, as XDP bpf_prog doesn't get direct TX-success
@@ -692,13 +695,16 @@ int trace_xdp_cpumap_enqueue(struct cpumap_enqueue_ctx *ctx)
  * Code in:         kernel/include/trace/events/xdp.h
  */
 struct cpumap_kthread_ctx {
-	u64 __pad;		// First 8 bytes are not accessible by bpf code
-	int map_id;		//	offset:8;  size:4; signed:1;
-	u32 act;		//	offset:12; size:4; signed:0;
-	int cpu;		//	offset:16; size:4; signed:1;
-	unsigned int drops;	//	offset:20; size:4; signed:0;
-	unsigned int processed;	//	offset:24; size:4; signed:0;
-	int sched;		//	offset:28; size:4; signed:1;
+	u64 __pad;			// First 8 bytes are not accessible
+	int map_id;			//	offset:8;  size:4; signed:1;
+	u32 act;			//	offset:12; size:4; signed:0;
+	int cpu;			//	offset:16; size:4; signed:1;
+	unsigned int drops;		//	offset:20; size:4; signed:0;
+	unsigned int processed;		//	offset:24; size:4; signed:0;
+	int sched;			//	offset:28; size:4; signed:1;
+	unsigned int xdp_pass;		//	offset:32; size:4; signed:0;
+	unsigned int xdp_drop;		//	offset:36; size:4; signed:0;
+	unsigned int xdp_redirect;	//	offset:40; size:4; signed:0;
 };
 
 SEC("tracepoint/xdp/xdp_cpumap_kthread")
@@ -712,6 +718,9 @@ int trace_xdp_cpumap_kthread(struct cpumap_kthread_ctx *ctx)
 		return 0;
 	rec->processed += ctx->processed;
 	rec->dropped   += ctx->drops;
+	rec->xdp_pass  += ctx->xdp_pass;
+	rec->xdp_drop  += ctx->xdp_drop;
+	rec->xdp_redirect  += ctx->xdp_redirect;
 
 	/* Count times kthread yielded CPU via schedule call */
 	if (ctx->sched)
diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 1a054737c35a..4b1264ca7ab7 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -70,6 +70,11 @@ static const struct option long_options[] = {
 	{"stress-mode", no_argument,		NULL, 'x' },
 	{"no-separators", no_argument,		NULL, 'z' },
 	{"force",	no_argument,		NULL, 'F' },
+	{"mprog-disable", no_argument,		NULL, 'n' },
+	{"mprog-name",	required_argument,	NULL, 'e' },
+	{"mprog-filename", required_argument,	NULL, 'f' },
+	{"redirect-device", required_argument,	NULL, 'r' },
+	{"redirect-map", required_argument,	NULL, 'm' },
 	{0, 0, NULL,  0 }
 };
 
@@ -156,6 +161,9 @@ struct datarec {
 	__u64 processed;
 	__u64 dropped;
 	__u64 issue;
+	__u64 xdp_pass;
+	__u64 xdp_drop;
+	__u64 xdp_redirect;
 };
 struct record {
 	__u64 timestamp;
@@ -175,6 +183,9 @@ static bool map_collect_percpu(int fd, __u32 key, struct record *rec)
 	/* For percpu maps, userspace gets a value per possible CPU */
 	unsigned int nr_cpus = bpf_num_possible_cpus();
 	struct datarec values[nr_cpus];
+	__u64 sum_xdp_redirect = 0;
+	__u64 sum_xdp_pass = 0;
+	__u64 sum_xdp_drop = 0;
 	__u64 sum_processed = 0;
 	__u64 sum_dropped = 0;
 	__u64 sum_issue = 0;
@@ -196,10 +207,19 @@ static bool map_collect_percpu(int fd, __u32 key, struct record *rec)
 		sum_dropped        += values[i].dropped;
 		rec->cpu[i].issue = values[i].issue;
 		sum_issue        += values[i].issue;
+		rec->cpu[i].xdp_pass = values[i].xdp_pass;
+		sum_xdp_pass += values[i].xdp_pass;
+		rec->cpu[i].xdp_drop = values[i].xdp_drop;
+		sum_xdp_drop += values[i].xdp_drop;
+		rec->cpu[i].xdp_redirect = values[i].xdp_redirect;
+		sum_xdp_redirect += values[i].xdp_redirect;
 	}
 	rec->total.processed = sum_processed;
 	rec->total.dropped   = sum_dropped;
 	rec->total.issue     = sum_issue;
+	rec->total.xdp_pass  = sum_xdp_pass;
+	rec->total.xdp_drop  = sum_xdp_drop;
+	rec->total.xdp_redirect = sum_xdp_redirect;
 	return true;
 }
 
@@ -303,17 +323,33 @@ static __u64 calc_errs_pps(struct datarec *r,
 	return pps;
 }
 
+static void calc_xdp_pps(struct datarec *r, struct datarec *p,
+			 double *xdp_pass, double *xdp_drop,
+			 double *xdp_redirect, double period_)
+{
+	*xdp_pass = 0, *xdp_drop = 0, *xdp_redirect = 0;
+	if (period_ > 0) {
+		*xdp_redirect = (r->xdp_redirect - p->xdp_redirect) / period_;
+		*xdp_pass = (r->xdp_pass - p->xdp_pass) / period_;
+		*xdp_drop = (r->xdp_drop - p->xdp_drop) / period_;
+	}
+}
+
 static void stats_print(struct stats_record *stats_rec,
 			struct stats_record *stats_prev,
-			char *prog_name)
+			char *prog_name, char *mprog_name, int mprog_fd)
 {
 	unsigned int nr_cpus = bpf_num_possible_cpus();
 	double pps = 0, drop = 0, err = 0;
+	bool mprog_enabled = false;
 	struct record *rec, *prev;
 	int to_cpu;
 	double t;
 	int i;
 
+	if (mprog_fd > 0)
+		mprog_enabled = true;
+
 	/* Header */
 	printf("Running XDP/eBPF prog_name:%s\n", prog_name);
 	printf("%-15s %-7s %-14s %-11s %-9s\n",
@@ -458,6 +494,33 @@ static void stats_print(struct stats_record *stats_rec,
 		printf(fm2_err, "xdp_exception", "total", pps, drop);
 	}
 
+	/* CPUMAP attached XDP program that runs on remote/destination CPU */
+	if (mprog_enabled) {
+		char *fmt_k = "%-15s %-7d %'-14.0f %'-11.0f %'-10.0f\n";
+		char *fm2_k = "%-15s %-7s %'-14.0f %'-11.0f %'-10.0f\n";
+		double xdp_pass, xdp_drop, xdp_redirect;
+
+		printf("\n2nd remote XDP/eBPF prog_name: %s\n", mprog_name);
+		printf("%-15s %-7s %-14s %-11s %-9s\n",
+		       "XDP-cpumap", "CPU:to", "xdp-pass", "xdp-drop", "xdp-redir");
+
+		rec  = &stats_rec->kthread;
+		prev = &stats_prev->kthread;
+		t = calc_period(rec, prev);
+		for (i = 0; i < nr_cpus; i++) {
+			struct datarec *r = &rec->cpu[i];
+			struct datarec *p = &prev->cpu[i];
+
+			calc_xdp_pps(r, p, &xdp_pass, &xdp_drop,
+				     &xdp_redirect, t);
+			if (xdp_pass > 0 || xdp_drop > 0 || xdp_redirect > 0)
+				printf(fmt_k, "xdp-in-kthread", i, xdp_pass, xdp_drop, xdp_redirect);
+		}
+		calc_xdp_pps(&rec->total, &prev->total, &xdp_pass, &xdp_drop,
+			     &xdp_redirect, t);
+		printf(fm2_k, "xdp-in-kthread", "total", xdp_pass, xdp_drop, xdp_redirect);
+	}
+
 	printf("\n");
 	fflush(stdout);
 }
@@ -494,7 +557,7 @@ static inline void swap(struct stats_record **a, struct stats_record **b)
 	*b = tmp;
 }
 
-static int create_cpu_entry(__u32 cpu, __u32 queue_size,
+static int create_cpu_entry(__u32 cpu, struct bpf_cpumap_val *value,
 			    __u32 avail_idx, bool new)
 {
 	__u32 curr_cpus_count = 0;
@@ -504,7 +567,7 @@ static int create_cpu_entry(__u32 cpu, __u32 queue_size,
 	/* Add a CPU entry to cpumap, as this allocate a cpu entry in
 	 * the kernel for the cpu.
 	 */
-	ret = bpf_map_update_elem(cpu_map_fd, &cpu, &queue_size, 0);
+	ret = bpf_map_update_elem(cpu_map_fd, &cpu, value, 0);
 	if (ret) {
 		fprintf(stderr, "Create CPU entry failed (err:%d)\n", ret);
 		exit(EXIT_FAIL_BPF);
@@ -535,9 +598,9 @@ static int create_cpu_entry(__u32 cpu, __u32 queue_size,
 		}
 	}
 	/* map_fd[7] = cpus_iterator */
-	printf("%s CPU:%u as idx:%u queue_size:%d (total cpus_count:%u)\n",
+	printf("%s CPU:%u as idx:%u qsize:%d prog_fd: %d (cpus_count:%u)\n",
 	       new ? "Add-new":"Replace", cpu, avail_idx,
-	       queue_size, curr_cpus_count);
+	       value->qsize, value->bpf_prog.fd, curr_cpus_count);
 
 	return 0;
 }
@@ -561,21 +624,26 @@ static void mark_cpus_unavailable(void)
 }
 
 /* Stress cpumap management code by concurrently changing underlying cpumap */
-static void stress_cpumap(void)
+static void stress_cpumap(struct bpf_cpumap_val *value)
 {
 	/* Changing qsize will cause kernel to free and alloc a new
 	 * bpf_cpu_map_entry, with an associated/complicated tear-down
 	 * procedure.
 	 */
-	create_cpu_entry(1,  1024, 0, false);
-	create_cpu_entry(1,     8, 0, false);
-	create_cpu_entry(1, 16000, 0, false);
+	value->qsize = 1024;
+	create_cpu_entry(1, value, 0, false);
+	value->qsize = 8;
+	create_cpu_entry(1, value, 0, false);
+	value->qsize = 16000;
+	create_cpu_entry(1, value, 0, false);
 }
 
 static void stats_poll(int interval, bool use_separators, char *prog_name,
+		       char *mprog_name, struct bpf_cpumap_val *value,
 		       bool stress_mode)
 {
 	struct stats_record *record, *prev;
+	int mprog_fd;
 
 	record = alloc_stats_record();
 	prev   = alloc_stats_record();
@@ -587,11 +655,12 @@ static void stats_poll(int interval, bool use_separators, char *prog_name,
 
 	while (1) {
 		swap(&prev, &record);
+		mprog_fd = value->bpf_prog.fd;
 		stats_collect(record);
-		stats_print(record, prev, prog_name);
+		stats_print(record, prev, prog_name, mprog_name, mprog_fd);
 		sleep(interval);
 		if (stress_mode)
-			stress_cpumap();
+			stress_cpumap(value);
 	}
 
 	free_stats_record(record);
@@ -664,15 +733,66 @@ static int init_map_fds(struct bpf_object *obj)
 	return 0;
 }
 
+static int load_cpumap_prog(char *file_name, char *prog_name,
+			    char *redir_interface, char *redir_map)
+{
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type		= BPF_PROG_TYPE_XDP,
+		.expected_attach_type	= BPF_XDP_CPUMAP,
+		.file = file_name,
+	};
+	struct bpf_program *prog;
+	struct bpf_object *obj;
+	int fd;
+
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &fd))
+		return -1;
+
+	if (fd < 0) {
+		fprintf(stderr, "ERR: bpf_prog_load_xattr: %s\n",
+			strerror(errno));
+		return fd;
+	}
+
+	if (redir_interface && redir_map) {
+		int err, map_fd, ifindex_out, key = 0;
+
+		map_fd = bpf_object__find_map_fd_by_name(obj, redir_map);
+		if (map_fd < 0)
+			return map_fd;
+
+		ifindex_out = if_nametoindex(redir_interface);
+		if (!ifindex_out)
+			return -1;
+
+		err = bpf_map_update_elem(map_fd, &key, &ifindex_out, 0);
+		if (err < 0)
+			return err;
+	}
+
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (!prog) {
+		fprintf(stderr, "bpf_object__find_program_by_title failed\n");
+		return EXIT_FAIL;
+	}
+
+	return bpf_program__fd(prog);
+}
+
 int main(int argc, char **argv)
 {
 	struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
 	char *prog_name = "xdp_cpu_map5_lb_hash_ip_pairs";
+	char *mprog_filename = "xdp_redirect_kern.o";
+	char *redir_interface = NULL, *redir_map = NULL;
+	char *mprog_name = "xdp_redirect_dummy";
+	bool mprog_disable = false;
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_UNSPEC,
 	};
 	struct bpf_prog_info info = {};
 	__u32 info_len = sizeof(info);
+	struct bpf_cpumap_val value;
 	bool use_separators = true;
 	bool stress_mode = false;
 	struct bpf_program *prog;
@@ -728,7 +848,7 @@ int main(int argc, char **argv)
 	memset(cpu, 0, n_cpus * sizeof(int));
 
 	/* Parse commands line args */
-	while ((opt = getopt_long(argc, argv, "hSd:s:p:q:c:xzF",
+	while ((opt = getopt_long(argc, argv, "hSd:s:p:q:c:xzFf:e:r:m:",
 				  long_options, &longindex)) != -1) {
 		switch (opt) {
 		case 'd':
@@ -762,6 +882,21 @@ int main(int argc, char **argv)
 			/* Selecting eBPF prog to load */
 			prog_name = optarg;
 			break;
+		case 'n':
+			mprog_disable = true;
+			break;
+		case 'f':
+			mprog_filename = optarg;
+			break;
+		case 'e':
+			mprog_name = optarg;
+			break;
+		case 'r':
+			redir_interface = optarg;
+			break;
+		case 'm':
+			redir_map = optarg;
+			break;
 		case 'c':
 			/* Add multiple CPUs */
 			add_cpu = strtoul(optarg, NULL, 0);
@@ -807,8 +942,18 @@ int main(int argc, char **argv)
 		goto out;
 	}
 
+	value.bpf_prog.fd = 0;
+	if (!mprog_disable)
+		value.bpf_prog.fd = load_cpumap_prog(mprog_filename, mprog_name,
+						     redir_interface, redir_map);
+	if (value.bpf_prog.fd < 0) {
+		err = value.bpf_prog.fd;
+		goto out;
+	}
+	value.qsize = qsize;
+
 	for (i = 0; i < added_cpus; i++)
-		create_cpu_entry(cpu[i], qsize, i, true);
+		create_cpu_entry(cpu[i], &value, i, true);
 
 	/* Remove XDP program when program is interrupted or killed */
 	signal(SIGINT, int_exit);
@@ -841,7 +986,8 @@ int main(int argc, char **argv)
 	}
 	prog_id = info.id;
 
-	stats_poll(interval, use_separators, prog_name, stress_mode);
+	stats_poll(interval, use_separators, prog_name, mprog_name,
+		   &value, stress_mode);
 out:
 	free(cpu);
 	return err;
-- 
2.26.2


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

* [PATCH v3 bpf-next 9/9] selftest: add tests for XDP programs in CPUMAP entries
  2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (7 preceding siblings ...)
  2020-06-23 21:39 ` [PATCH v3 bpf-next 8/9] samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap Lorenzo Bianconi
@ 2020-06-23 21:39 ` Lorenzo Bianconi
  2020-06-24  5:51   ` Andrii Nakryiko
  8 siblings, 1 reply; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-23 21:39 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, ast, brouer, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko

Similar to what have been done for DEVMAP, introduce tests to verify
ability to add a XDP program to an entry in a CPUMAP.
Verify CPUMAP programs can not be attached to devices as a normal
XDP program, and only programs with BPF_XDP_CPUMAP attach type can
be loaded in a CPUMAP.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 .../bpf/prog_tests/xdp_cpumap_attach.c        | 70 +++++++++++++++++++
 .../bpf/progs/test_xdp_with_cpumap_helpers.c  | 40 +++++++++++
 2 files changed, 110 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
new file mode 100644
index 000000000000..0176573fe4e7
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <uapi/linux/bpf.h>
+#include <linux/if_link.h>
+#include <test_progs.h>
+
+#include "test_xdp_with_cpumap_helpers.skel.h"
+
+#define IFINDEX_LO	1
+
+void test_xdp_with_cpumap_helpers(void)
+{
+	struct test_xdp_with_cpumap_helpers *skel;
+	struct bpf_prog_info info = {};
+	struct bpf_cpumap_val val = {
+		.qsize = 192,
+	};
+	__u32 duration = 0, idx = 0;
+	__u32 len = sizeof(info);
+	int err, prog_fd, map_fd;
+
+	skel = test_xdp_with_cpumap_helpers__open_and_load();
+	if (CHECK_FAIL(!skel)) {
+		perror("test_xdp_with_cpumap_helpers__open_and_load");
+		return;
+	}
+
+	/* can not attach program with cpumaps that allow programs
+	 * as xdp generic
+	 */
+	prog_fd = bpf_program__fd(skel->progs.xdp_redir_prog);
+	err = bpf_set_link_xdp_fd(IFINDEX_LO, prog_fd, XDP_FLAGS_SKB_MODE);
+	CHECK(err == 0, "Generic attach of program with 8-byte CPUMAP",
+	      "should have failed\n");
+
+	prog_fd = bpf_program__fd(skel->progs.xdp_dummy_cm);
+	map_fd = bpf_map__fd(skel->maps.cpu_map);
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &len);
+	if (CHECK_FAIL(err))
+		goto out_close;
+
+	val.bpf_prog.fd = prog_fd;
+	err = bpf_map_update_elem(map_fd, &idx, &val, 0);
+	CHECK(err, "Add program to cpumap entry", "err %d errno %d\n",
+	      err, errno);
+
+	err = bpf_map_lookup_elem(map_fd, &idx, &val);
+	CHECK(err, "Read cpumap entry", "err %d errno %d\n", err, errno);
+	CHECK(info.id != val.bpf_prog.id, "Expected program id in cpumap entry",
+	      "expected %u read %u\n", info.id, val.bpf_prog.id);
+
+	/* can not attach BPF_XDP_CPUMAP program to a device */
+	err = bpf_set_link_xdp_fd(IFINDEX_LO, prog_fd, XDP_FLAGS_SKB_MODE);
+	CHECK(err == 0, "Attach of BPF_XDP_CPUMAP program",
+	      "should have failed\n");
+
+	val.qsize = 192;
+	val.bpf_prog.fd = bpf_program__fd(skel->progs.xdp_dummy_prog);
+	err = bpf_map_update_elem(map_fd, &idx, &val, 0);
+	CHECK(err == 0, "Add non-BPF_XDP_CPUMAP program to cpumap entry",
+	      "should have failed\n");
+
+out_close:
+	test_xdp_with_cpumap_helpers__destroy(skel);
+}
+
+void test_xdp_cpumap_attach(void)
+{
+	if (test__start_subtest("cpumap_with_progs"))
+		test_xdp_with_cpumap_helpers();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c
new file mode 100644
index 000000000000..861848d0c353
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+static __u32 ingress_ifindex;
+
+struct {
+	__uint(type, BPF_MAP_TYPE_CPUMAP);
+	__uint(key_size, sizeof(__u32));
+	__uint(value_size, sizeof(struct bpf_cpumap_val));
+	__uint(max_entries, 4);
+} cpu_map SEC(".maps");
+
+SEC("xdp_redir")
+int xdp_redir_prog(struct xdp_md *ctx)
+{
+	return bpf_redirect_map(&cpu_map, 1, 0);
+}
+
+SEC("xdp_dummy")
+int xdp_dummy_prog(struct xdp_md *ctx)
+{
+	return XDP_PASS;
+}
+
+SEC("xdp_cpumap/dummy_cm")
+int xdp_dummy_cm(struct xdp_md *ctx)
+{
+	char fmt[] = "devmap redirect: dev %u len %u\n";
+	void *data_end = (void *)(long)ctx->data_end;
+	void *data = (void *)(long)ctx->data;
+	unsigned int len = data_end - data;
+
+	ingress_ifindex = ctx->ingress_ifindex;
+
+	return XDP_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.26.2


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

* Re: [PATCH v3 bpf-next 7/9] libbpf: add SEC name for xdp programs attached to CPUMAP
  2020-06-23 21:39 ` [PATCH v3 bpf-next 7/9] libbpf: add SEC name for xdp programs attached to CPUMAP Lorenzo Bianconi
@ 2020-06-24  5:49   ` Andrii Nakryiko
  2020-06-24  8:36     ` Jesper Dangaard Brouer
  0 siblings, 1 reply; 23+ messages in thread
From: Andrii Nakryiko @ 2020-06-24  5:49 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Networking, bpf, David S. Miller, Alexei Starovoitov,
	Jesper Dangaard Brouer, Daniel Borkmann,
	Toke Høiland-Jørgensen, lorenzo.bianconi, David Ahern

On Tue, Jun 23, 2020 at 2:40 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> As for DEVMAP, support SEC("xdp_cpumap*") as a short cut for loading
> the program with type BPF_PROG_TYPE_XDP and expected attach type
> BPF_XDP_CPUMAP.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---

Thanks!

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  tools/lib/bpf/libbpf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 18461deb1b19..16fa3b84ac38 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -6866,6 +6866,8 @@ static const struct bpf_sec_def section_defs[] = {
>                 .attach_fn = attach_iter),
>         BPF_EAPROG_SEC("xdp_devmap",            BPF_PROG_TYPE_XDP,
>                                                 BPF_XDP_DEVMAP),
> +       BPF_EAPROG_SEC("xdp_cpumap/",           BPF_PROG_TYPE_XDP,
> +                                               BPF_XDP_CPUMAP),
>         BPF_PROG_SEC("xdp",                     BPF_PROG_TYPE_XDP),
>         BPF_PROG_SEC("perf_event",              BPF_PROG_TYPE_PERF_EVENT),
>         BPF_PROG_SEC("lwt_in",                  BPF_PROG_TYPE_LWT_IN),
> --
> 2.26.2
>

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

* Re: [PATCH v3 bpf-next 9/9] selftest: add tests for XDP programs in CPUMAP entries
  2020-06-23 21:39 ` [PATCH v3 bpf-next 9/9] selftest: add tests for XDP programs in CPUMAP entries Lorenzo Bianconi
@ 2020-06-24  5:51   ` Andrii Nakryiko
  2020-06-24 15:37     ` Lorenzo Bianconi
  0 siblings, 1 reply; 23+ messages in thread
From: Andrii Nakryiko @ 2020-06-24  5:51 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Networking, bpf, David S. Miller, Alexei Starovoitov,
	Jesper Dangaard Brouer, Daniel Borkmann,
	Toke Høiland-Jørgensen, lorenzo.bianconi, David Ahern

On Tue, Jun 23, 2020 at 2:40 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> Similar to what have been done for DEVMAP, introduce tests to verify
> ability to add a XDP program to an entry in a CPUMAP.
> Verify CPUMAP programs can not be attached to devices as a normal
> XDP program, and only programs with BPF_XDP_CPUMAP attach type can
> be loaded in a CPUMAP.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  .../bpf/prog_tests/xdp_cpumap_attach.c        | 70 +++++++++++++++++++
>  .../bpf/progs/test_xdp_with_cpumap_helpers.c  | 40 +++++++++++
>  2 files changed, 110 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
> new file mode 100644
> index 000000000000..0176573fe4e7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <uapi/linux/bpf.h>
> +#include <linux/if_link.h>
> +#include <test_progs.h>
> +
> +#include "test_xdp_with_cpumap_helpers.skel.h"
> +
> +#define IFINDEX_LO     1
> +
> +void test_xdp_with_cpumap_helpers(void)
> +{
> +       struct test_xdp_with_cpumap_helpers *skel;
> +       struct bpf_prog_info info = {};
> +       struct bpf_cpumap_val val = {
> +               .qsize = 192,
> +       };
> +       __u32 duration = 0, idx = 0;
> +       __u32 len = sizeof(info);
> +       int err, prog_fd, map_fd;
> +
> +       skel = test_xdp_with_cpumap_helpers__open_and_load();
> +       if (CHECK_FAIL(!skel)) {
> +               perror("test_xdp_with_cpumap_helpers__open_and_load");
> +               return;
> +       }
> +
> +       /* can not attach program with cpumaps that allow programs
> +        * as xdp generic
> +        */
> +       prog_fd = bpf_program__fd(skel->progs.xdp_redir_prog);
> +       err = bpf_set_link_xdp_fd(IFINDEX_LO, prog_fd, XDP_FLAGS_SKB_MODE);
> +       CHECK(err == 0, "Generic attach of program with 8-byte CPUMAP",
> +             "should have failed\n");
> +
> +       prog_fd = bpf_program__fd(skel->progs.xdp_dummy_cm);
> +       map_fd = bpf_map__fd(skel->maps.cpu_map);
> +       err = bpf_obj_get_info_by_fd(prog_fd, &info, &len);
> +       if (CHECK_FAIL(err))
> +               goto out_close;
> +
> +       val.bpf_prog.fd = prog_fd;
> +       err = bpf_map_update_elem(map_fd, &idx, &val, 0);
> +       CHECK(err, "Add program to cpumap entry", "err %d errno %d\n",
> +             err, errno);
> +
> +       err = bpf_map_lookup_elem(map_fd, &idx, &val);
> +       CHECK(err, "Read cpumap entry", "err %d errno %d\n", err, errno);
> +       CHECK(info.id != val.bpf_prog.id, "Expected program id in cpumap entry",
> +             "expected %u read %u\n", info.id, val.bpf_prog.id);
> +
> +       /* can not attach BPF_XDP_CPUMAP program to a device */
> +       err = bpf_set_link_xdp_fd(IFINDEX_LO, prog_fd, XDP_FLAGS_SKB_MODE);
> +       CHECK(err == 0, "Attach of BPF_XDP_CPUMAP program",
> +             "should have failed\n");
> +
> +       val.qsize = 192;
> +       val.bpf_prog.fd = bpf_program__fd(skel->progs.xdp_dummy_prog);
> +       err = bpf_map_update_elem(map_fd, &idx, &val, 0);
> +       CHECK(err == 0, "Add non-BPF_XDP_CPUMAP program to cpumap entry",
> +             "should have failed\n");
> +
> +out_close:
> +       test_xdp_with_cpumap_helpers__destroy(skel);
> +}
> +
> +void test_xdp_cpumap_attach(void)
> +{
> +       if (test__start_subtest("cpumap_with_progs"))
> +               test_xdp_with_cpumap_helpers();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c
> new file mode 100644
> index 000000000000..861848d0c353
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +static __u32 ingress_ifindex;
> +
> +struct {
> +       __uint(type, BPF_MAP_TYPE_CPUMAP);
> +       __uint(key_size, sizeof(__u32));
> +       __uint(value_size, sizeof(struct bpf_cpumap_val));
> +       __uint(max_entries, 4);
> +} cpu_map SEC(".maps");
> +
> +SEC("xdp_redir")
> +int xdp_redir_prog(struct xdp_md *ctx)
> +{
> +       return bpf_redirect_map(&cpu_map, 1, 0);
> +}
> +
> +SEC("xdp_dummy")
> +int xdp_dummy_prog(struct xdp_md *ctx)
> +{
> +       return XDP_PASS;
> +}
> +
> +SEC("xdp_cpumap/dummy_cm")
> +int xdp_dummy_cm(struct xdp_md *ctx)
> +{
> +       char fmt[] = "devmap redirect: dev %u len %u\n";
> +       void *data_end = (void *)(long)ctx->data_end;
> +       void *data = (void *)(long)ctx->data;
> +       unsigned int len = data_end - data;
> +
> +       ingress_ifindex = ctx->ingress_ifindex;

Have you checked the generated BPF assembly to verify
ctx->ingress_ifindex is actually read? ingress_ifindex variable is
declared static, so I'm guessing Clang just optimized it away. If you
want to be sure this actually gets executed, make ingress_ifindex
global var.

> +
> +       return XDP_PASS;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.26.2
>

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

* Re: [PATCH v3 bpf-next 2/9] net: Refactor xdp_convert_buff_to_frame
  2020-06-23 21:39 ` [PATCH v3 bpf-next 2/9] net: Refactor xdp_convert_buff_to_frame Lorenzo Bianconi
@ 2020-06-24  8:06   ` Jesper Dangaard Brouer
  0 siblings, 0 replies; 23+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-24  8:06 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev, bpf, davem, ast, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko, David Ahern, brouer

On Tue, 23 Jun 2020 23:39:27 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> From: David Ahern <dahern@digitalocean.com>
> 
> Move the guts of xdp_convert_buff_to_frame to a new helper,
> xdp_update_frame_from_buff so it can be reused removing code duplication
> 
> Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> Signed-off-by: David Ahern <dahern@digitalocean.com>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


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

* Re: [PATCH v3 bpf-next 4/9] cpumap: formalize map value as a named struct
  2020-06-23 21:39 ` [PATCH v3 bpf-next 4/9] cpumap: formalize map value as a named struct Lorenzo Bianconi
@ 2020-06-24  8:08   ` Jesper Dangaard Brouer
  0 siblings, 0 replies; 23+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-24  8:08 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev, bpf, davem, ast, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko, brouer

On Tue, 23 Jun 2020 23:39:29 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> As it has been already done for devmap, introduce 'struct bpf_cpumap_val'
> to formalize the expected values that can be passed in for a CPUMAP.
> Update cpumap code to use the struct.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


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

* Re: [PATCH v3 bpf-next 5/9] bpf: cpumap: add the possibility to attach an eBPF program to cpumap
  2020-06-23 21:39 ` [PATCH v3 bpf-next 5/9] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
@ 2020-06-24  8:12   ` Jesper Dangaard Brouer
  0 siblings, 0 replies; 23+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-24  8:12 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev, bpf, davem, ast, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko, brouer

On Tue, 23 Jun 2020 23:39:30 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> Introduce the capability to attach an eBPF program to cpumap entries.
> The idea behind this feature is to add the possibility to define on
> which CPU run the eBPF program if the underlying hw does not support
> RSS. Current supported verdicts are XDP_DROP and XDP_PASS.
> 
> This patch has been tested on Marvell ESPRESSObin using xdp_redirect_cpu
> sample available in the kernel tree to identify possible performance
> regressions. Results show there are no observable differences in
> packet-per-second:
> 
> $./xdp_redirect_cpu --progname xdp_cpu_map0 --dev eth0 --cpu 1
> rx: 354.8 Kpps
> rx: 356.0 Kpps
> rx: 356.8 Kpps
> rx: 356.3 Kpps
> rx: 356.6 Kpps
> rx: 356.6 Kpps
> rx: 356.7 Kpps
> rx: 355.8 Kpps
> rx: 356.8 Kpps
> rx: 356.8 Kpps
> 
> Co-developed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


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

* Re: [PATCH v3 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
  2020-06-23 21:39 ` [PATCH v3 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries Lorenzo Bianconi
@ 2020-06-24  8:32   ` Jesper Dangaard Brouer
  0 siblings, 0 replies; 23+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-24  8:32 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev, bpf, davem, ast, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko, brouer

On Tue, 23 Jun 2020 23:39:31 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> Introduce XDP_REDIRECT support for eBPF programs attached to cpumap
> entries.
> This patch has been tested on Marvell ESPRESSObin using a modified
> version of xdp_redirect_cpu sample in order to attach a XDP program
> to CPUMAP entries to perform a redirect on the mvneta interface.
> In particular the following scenario has been tested:
> 
> rq (cpu0) --> mvneta - XDP_REDIRECT (cpu0) --> CPUMAP - XDP_REDIRECT (cpu1) --> mvneta
> 
> $./xdp_redirect_cpu -p xdp_cpu_map0 -d eth0 -c 1 -e xdp_redirect \
> 	-f xdp_redirect_kern.o -m tx_port -r eth0
> 
> tx: 285.2 Kpps rx: 285.2 Kpps
> 
> Attacching a simple XDP program on eth0 to perform XDP_TX gives
  ^^^^^^^^^^

Spelling/typo.

> comparable results:
> 
> tx: 288.4 Kpps rx: 288.4 Kpps
> 
> Co-developed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

> ---
>  include/net/xdp.h          |  1 +
>  include/trace/events/xdp.h |  6 ++++--
>  kernel/bpf/cpumap.c        | 17 +++++++++++++++--
>  3 files changed, 20 insertions(+), 4 deletions(-)
[...]

> @@ -276,7 +286,10 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu,
>  		}
>  	}
>  
> -	rcu_read_unlock();
> +	if (stats->redirect)
> +		xdp_do_flush_map();
> +
> +	rcu_read_unlock_bh(); /* resched point, may call do_softirq() */

I've tested (on x86) that this extra resched point does not cause sched issues.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


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

* Re: [PATCH v3 bpf-next 7/9] libbpf: add SEC name for xdp programs attached to CPUMAP
  2020-06-24  5:49   ` Andrii Nakryiko
@ 2020-06-24  8:36     ` Jesper Dangaard Brouer
  2020-06-24 14:29       ` Alexei Starovoitov
  0 siblings, 1 reply; 23+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-24  8:36 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Lorenzo Bianconi, Networking, bpf, David S. Miller,
	Alexei Starovoitov, Daniel Borkmann,
	Toke Høiland-Jørgensen, lorenzo.bianconi, David Ahern,
	brouer

On Tue, 23 Jun 2020 22:49:02 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Tue, Jun 23, 2020 at 2:40 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > As for DEVMAP, support SEC("xdp_cpumap*") as a short cut for loading
                                ^^^^^^^^^^^

Maybe update desc to include the "/" ?
  
> > the program with type BPF_PROG_TYPE_XDP and expected attach type
> > BPF_XDP_CPUMAP.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---  
> 
> Thanks!
> 
> Acked-by: Andrii Nakryiko <andriin@fb.com>

I like this extra "/".

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

If we agree, I hope we can also adjust this for devmap in bpf-tree ?


> >  tools/lib/bpf/libbpf.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 18461deb1b19..16fa3b84ac38 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -6866,6 +6866,8 @@ static const struct bpf_sec_def section_defs[] = {
> >                 .attach_fn = attach_iter),
> >         BPF_EAPROG_SEC("xdp_devmap",            BPF_PROG_TYPE_XDP,
> >                                                 BPF_XDP_DEVMAP),
> > +       BPF_EAPROG_SEC("xdp_cpumap/",           BPF_PROG_TYPE_XDP,
> > +                                               BPF_XDP_CPUMAP),
> >         BPF_PROG_SEC("xdp",                     BPF_PROG_TYPE_XDP),
> >         BPF_PROG_SEC("perf_event",              BPF_PROG_TYPE_PERF_EVENT),
> >         BPF_PROG_SEC("lwt_in",                  BPF_PROG_TYPE_LWT_IN),
> > --

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


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

* Re: [PATCH v3 bpf-next 8/9] samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap
  2020-06-23 21:39 ` [PATCH v3 bpf-next 8/9] samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap Lorenzo Bianconi
@ 2020-06-24  9:00   ` Jesper Dangaard Brouer
  0 siblings, 0 replies; 23+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-24  9:00 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev, bpf, davem, ast, daniel, toke, lorenzo.bianconi, dsahern,
	andrii.nakryiko, brouer

On Tue, 23 Jun 2020 23:39:33 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> Extend xdp_redirect_cpu_{usr,kern}.c adding the possibility to load
> a XDP program on cpumap entries. The following options have been added:
> - mprog-name: cpumap entry program name
> - mprog-filename: cpumap entry program filename
> - redirect-device: output interface if the cpumap program performs a
>   XDP_REDIRECT to an egress interface
> - redirect-map: bpf map used to perform XDP_REDIRECT to an egress
>   interface
> - mprog-disable: disable loading XDP program on cpumap entries
> 
> Add xdp_pass, xdp_drop, xdp_redirect stats accounting
> 
> Co-developed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>


Example command and output:

sudo ./xdp_redirect_cpu --dev i40e2 --qsize 64 --cpu 2 --prog xdp_cpu_map0 \
  --mprog-filename xdp1_kern.o   --mprog-name xdp1

Running XDP/eBPF prog_name:xdp_cpu_map0
XDP-cpumap      CPU:to  pps            drop-pps    extra-info
XDP-RX          4       17,904,077     0           0          
XDP-RX          total   17,904,077     0          
cpumap-enqueue    4:2   17,904,076     418,122     8.00       bulk-average
cpumap-enqueue  sum:2   17,904,076     418,122     8.00       bulk-average
cpumap_kthread  2       17,485,954     0           35,139     sched
cpumap_kthread  total   17,485,954     0           35,139     sched-sum
redirect_err    total   0              0          
xdp_exception   total   0              0          

2nd remote XDP/eBPF prog_name: xdp1
XDP-cpumap      CPU:to  xdp-pass       xdp-drop    xdp-redir
xdp-in-kthread  2       0              17,485,955  0         
xdp-in-kthread  total   0              17,485,955  0         



Another example, where RSS is deliberately setup to only hit CPU-0.

 $ sudo ./xdp_redirect_cpu --dev i40e2 --qsize 192 --cpu 2  --cpu 3 --cpu 4 \
    --prog xdp_cpu_map5_lb_hash_ip_pairs \
    --mprog-name xdp_redirect --redirect-device mlx5p1  --redirect-map tx_port

Running XDP/eBPF prog_name:xdp_cpu_map5_lb_hash_ip_pairs
XDP-cpumap      CPU:to  pps            drop-pps    extra-info
XDP-RX          0       10,603,984     0           0          
XDP-RX          total   10,603,984     0          
cpumap-enqueue    0:2   3,539,815      0           6.88       bulk-average
cpumap-enqueue  sum:2   3,539,815      0           6.88       bulk-average
cpumap-enqueue    0:3   3,554,091      0           6.88       bulk-average
cpumap-enqueue  sum:3   3,554,091      0           6.88       bulk-average
cpumap-enqueue    0:4   3,510,076      84          6.87       bulk-average
cpumap-enqueue  sum:4   3,510,076      84          6.87       bulk-average
cpumap_kthread  2       3,539,817      0           91,310     sched
cpumap_kthread  3       3,554,101      0           93,604     sched
cpumap_kthread  4       3,509,991      0           92,893     sched
cpumap_kthread  total   10,603,910     0           277,809    sched-sum
redirect_err    total   0              0          
xdp_exception   total   0              0          

2nd remote XDP/eBPF prog_name: xdp_redirect
XDP-cpumap      CPU:to  xdp-pass       xdp-drop    xdp-redir
xdp-in-kthread  2       0              0           3,539,818 
xdp-in-kthread  3       0              0           3,554,101 
xdp-in-kthread  4       0              0           3,509,991 
xdp-in-kthread  total   0              0           10,603,910

More performance test examples can be found here:
 https://github.com/xdp-project/xdp-project/blob/master/areas/cpumap/cpumap04-map-xdp-prog.org

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


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

* Re: [PATCH v3 bpf-next 7/9] libbpf: add SEC name for xdp programs attached to CPUMAP
  2020-06-24  8:36     ` Jesper Dangaard Brouer
@ 2020-06-24 14:29       ` Alexei Starovoitov
  2020-06-25 14:26         ` [PATCH bpf] libbpf: adjust SEC short cut for expected attach type BPF_XDP_DEVMAP Jesper Dangaard Brouer
  0 siblings, 1 reply; 23+ messages in thread
From: Alexei Starovoitov @ 2020-06-24 14:29 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Andrii Nakryiko, Lorenzo Bianconi, Networking, bpf,
	David S. Miller, Alexei Starovoitov, Daniel Borkmann,
	Toke Høiland-Jørgensen, lorenzo.bianconi, David Ahern

On Wed, Jun 24, 2020 at 1:36 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> On Tue, 23 Jun 2020 22:49:02 -0700
> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> > On Tue, Jun 23, 2020 at 2:40 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > >
> > > As for DEVMAP, support SEC("xdp_cpumap*") as a short cut for loading
>                                 ^^^^^^^^^^^
>
> Maybe update desc to include the "/" ?

+1

>
> > > the program with type BPF_PROG_TYPE_XDP and expected attach type
> > > BPF_XDP_CPUMAP.
> > >
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> >
> > Thanks!
> >
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
>
> I like this extra "/".
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> If we agree, I hope we can also adjust this for devmap in bpf-tree ?

Yes. Please.
It's not in an official release. There is time to adjust things.

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

* Re: [PATCH v3 bpf-next 9/9] selftest: add tests for XDP programs in CPUMAP entries
  2020-06-24  5:51   ` Andrii Nakryiko
@ 2020-06-24 15:37     ` Lorenzo Bianconi
  0 siblings, 0 replies; 23+ messages in thread
From: Lorenzo Bianconi @ 2020-06-24 15:37 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Networking, bpf, David S. Miller, Alexei Starovoitov,
	Jesper Dangaard Brouer, Daniel Borkmann,
	Toke Høiland-Jørgensen, lorenzo.bianconi, David Ahern

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

> On Tue, Jun 23, 2020 at 2:40 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > Similar to what have been done for DEVMAP, introduce tests to verify

[...]

> > +SEC("xdp_cpumap/dummy_cm")
> > +int xdp_dummy_cm(struct xdp_md *ctx)
> > +{
> > +       char fmt[] = "devmap redirect: dev %u len %u\n";
> > +       void *data_end = (void *)(long)ctx->data_end;
> > +       void *data = (void *)(long)ctx->data;
> > +       unsigned int len = data_end - data;
> > +
> > +       ingress_ifindex = ctx->ingress_ifindex;
> 
> Have you checked the generated BPF assembly to verify
> ctx->ingress_ifindex is actually read? ingress_ifindex variable is
> declared static, so I'm guessing Clang just optimized it away. If you
> want to be sure this actually gets executed, make ingress_ifindex
> global var.

ack, thx for the review. I will fix it in v4.

Regards,
Lorenzo

> 
> > +
> > +       return XDP_PASS;
> > +}
> > +
> > +char _license[] SEC("license") = "GPL";
> > --
> > 2.26.2
> >

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* [PATCH bpf] libbpf: adjust SEC short cut for expected attach type BPF_XDP_DEVMAP
  2020-06-24 14:29       ` Alexei Starovoitov
@ 2020-06-25 14:26         ` Jesper Dangaard Brouer
  2020-06-25 16:58           ` Andrii Nakryiko
  2020-06-25 21:12           ` Daniel Borkmann
  0 siblings, 2 replies; 23+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-25 14:26 UTC (permalink / raw)
  To: bpf, andriin
  Cc: Jesper Dangaard Brouer, netdev, Alexei Starovoitov,
	Daniel Borkmann, David Ahern

Adjust the SEC("xdp_devmap/") prog type prefix to contain a
slash "/" for expected attach type BPF_XDP_DEVMAP.  This is consistent
with other prog types like tracing.

Fixes: 2778797037a6 ("libbpf: Add SEC name for xdp programs attached to device map")
Suggested-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 tools/lib/bpf/libbpf.c                             |    2 +-
 .../bpf/progs/test_xdp_with_devmap_helpers.c       |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f17151d866e6..11e4725b8b1c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6659,7 +6659,7 @@ static const struct bpf_sec_def section_defs[] = {
 		.expected_attach_type = BPF_TRACE_ITER,
 		.is_attach_btf = true,
 		.attach_fn = attach_iter),
-	BPF_EAPROG_SEC("xdp_devmap",		BPF_PROG_TYPE_XDP,
+	BPF_EAPROG_SEC("xdp_devmap/",		BPF_PROG_TYPE_XDP,
 						BPF_XDP_DEVMAP),
 	BPF_PROG_SEC("xdp",			BPF_PROG_TYPE_XDP),
 	BPF_PROG_SEC("perf_event",		BPF_PROG_TYPE_PERF_EVENT),
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c
index 330811260123..0ac086497722 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c
@@ -27,7 +27,7 @@ int xdp_dummy_prog(struct xdp_md *ctx)
 /* valid program on DEVMAP entry via SEC name;
  * has access to egress and ingress ifindex
  */
-SEC("xdp_devmap")
+SEC("xdp_devmap/map_prog")
 int xdp_dummy_dm(struct xdp_md *ctx)
 {
 	char fmt[] = "devmap redirect: dev %u -> dev %u len %u\n";



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

* Re: [PATCH bpf] libbpf: adjust SEC short cut for expected attach type BPF_XDP_DEVMAP
  2020-06-25 14:26         ` [PATCH bpf] libbpf: adjust SEC short cut for expected attach type BPF_XDP_DEVMAP Jesper Dangaard Brouer
@ 2020-06-25 16:58           ` Andrii Nakryiko
  2020-06-25 21:12           ` Daniel Borkmann
  1 sibling, 0 replies; 23+ messages in thread
From: Andrii Nakryiko @ 2020-06-25 16:58 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: bpf, Andrii Nakryiko, Networking, Alexei Starovoitov,
	Daniel Borkmann, David Ahern

On Thu, Jun 25, 2020 at 7:28 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> Adjust the SEC("xdp_devmap/") prog type prefix to contain a
> slash "/" for expected attach type BPF_XDP_DEVMAP.  This is consistent
> with other prog types like tracing.
>
> Fixes: 2778797037a6 ("libbpf: Add SEC name for xdp programs attached to device map")
> Suggested-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---

Thanks!

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  tools/lib/bpf/libbpf.c                             |    2 +-
>  .../bpf/progs/test_xdp_with_devmap_helpers.c       |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

[...]

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

* Re: [PATCH bpf] libbpf: adjust SEC short cut for expected attach type BPF_XDP_DEVMAP
  2020-06-25 14:26         ` [PATCH bpf] libbpf: adjust SEC short cut for expected attach type BPF_XDP_DEVMAP Jesper Dangaard Brouer
  2020-06-25 16:58           ` Andrii Nakryiko
@ 2020-06-25 21:12           ` Daniel Borkmann
  1 sibling, 0 replies; 23+ messages in thread
From: Daniel Borkmann @ 2020-06-25 21:12 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, bpf, andriin
  Cc: netdev, Alexei Starovoitov, Daniel Borkmann, David Ahern

On 6/25/20 4:26 PM, Jesper Dangaard Brouer wrote:
> Adjust the SEC("xdp_devmap/") prog type prefix to contain a
> slash "/" for expected attach type BPF_XDP_DEVMAP.  This is consistent
> with other prog types like tracing.
> 
> Fixes: 2778797037a6 ("libbpf: Add SEC name for xdp programs attached to device map")
> Suggested-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

Given it's not released yet, lgtm, applied, thanks!

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

end of thread, other threads:[~2020-06-25 21:12 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-23 21:39 [PATCH v3 bpf-next 0/9] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
2020-06-23 21:39 ` [PATCH v3 bpf-next 1/9] cpumap: use non-locked version __ptr_ring_consume_batched Lorenzo Bianconi
2020-06-23 21:39 ` [PATCH v3 bpf-next 2/9] net: Refactor xdp_convert_buff_to_frame Lorenzo Bianconi
2020-06-24  8:06   ` Jesper Dangaard Brouer
2020-06-23 21:39 ` [PATCH v3 bpf-next 3/9] samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop Lorenzo Bianconi
2020-06-23 21:39 ` [PATCH v3 bpf-next 4/9] cpumap: formalize map value as a named struct Lorenzo Bianconi
2020-06-24  8:08   ` Jesper Dangaard Brouer
2020-06-23 21:39 ` [PATCH v3 bpf-next 5/9] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
2020-06-24  8:12   ` Jesper Dangaard Brouer
2020-06-23 21:39 ` [PATCH v3 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries Lorenzo Bianconi
2020-06-24  8:32   ` Jesper Dangaard Brouer
2020-06-23 21:39 ` [PATCH v3 bpf-next 7/9] libbpf: add SEC name for xdp programs attached to CPUMAP Lorenzo Bianconi
2020-06-24  5:49   ` Andrii Nakryiko
2020-06-24  8:36     ` Jesper Dangaard Brouer
2020-06-24 14:29       ` Alexei Starovoitov
2020-06-25 14:26         ` [PATCH bpf] libbpf: adjust SEC short cut for expected attach type BPF_XDP_DEVMAP Jesper Dangaard Brouer
2020-06-25 16:58           ` Andrii Nakryiko
2020-06-25 21:12           ` Daniel Borkmann
2020-06-23 21:39 ` [PATCH v3 bpf-next 8/9] samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap Lorenzo Bianconi
2020-06-24  9:00   ` Jesper Dangaard Brouer
2020-06-23 21:39 ` [PATCH v3 bpf-next 9/9] selftest: add tests for XDP programs in CPUMAP entries Lorenzo Bianconi
2020-06-24  5:51   ` Andrii Nakryiko
2020-06-24 15:37     ` Lorenzo Bianconi

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.