bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/6] introduce support for XDP programs in CPUMAP
@ 2020-05-31 21:46 Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 1/6] net: Refactor xdp_convert_buff_to_frame Lorenzo Bianconi
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-05-31 21:46 UTC (permalink / raw)
  To: bpf, netdev; +Cc: davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern

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]
This series is based on the following series:
- https://patchwork.ozlabs.org/project/netdev/cover/cover.1590698295.git.lorenzo@kernel.org/
- https://patchwork.ozlabs.org/project/netdev/cover/20200529220716.75383-1-dsahern@kernel.org/

[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

Lorenzo Bianconi (5):
  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 a eBPF program to cpumap
  bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map
    entries
  samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap

 include/linux/bpf.h                 |   6 ++
 include/net/xdp.h                   |  35 ++++---
 include/trace/events/xdp.h          |  18 +++-
 include/uapi/linux/bpf.h            |   1 +
 kernel/bpf/cpumap.c                 | 147 +++++++++++++++++++++++-----
 net/core/dev.c                      |   8 ++
 net/core/filter.c                   |   7 ++
 samples/bpf/xdp_redirect_cpu_kern.c |  34 +++++--
 samples/bpf/xdp_redirect_cpu_user.c | 140 ++++++++++++++++++++++----
 tools/include/uapi/linux/bpf.h      |   1 +
 10 files changed, 329 insertions(+), 68 deletions(-)

-- 
2.26.2


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

* [PATCH bpf-next 1/6] net: Refactor xdp_convert_buff_to_frame
  2020-05-31 21:46 [PATCH bpf-next 0/6] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
@ 2020-05-31 21:46 ` Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 2/6] samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop Lorenzo Bianconi
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-05-31 21:46 UTC (permalink / raw)
  To: bpf, netdev
  Cc: davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern, 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..ab1c503808a4 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 -ENOMEM;
 
 	/* 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 -ENOMEM;
 	}
 
-	/* 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] 11+ messages in thread

* [PATCH bpf-next 2/6] samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop
  2020-05-31 21:46 [PATCH bpf-next 0/6] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 1/6] net: Refactor xdp_convert_buff_to_frame Lorenzo Bianconi
@ 2020-05-31 21:46 ` Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 3/6] cpumap: formalize map value as a named struct Lorenzo Bianconi
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-05-31 21:46 UTC (permalink / raw)
  To: bpf, netdev; +Cc: davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern

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

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] 11+ messages in thread

* [PATCH bpf-next 3/6] cpumap: formalize map value as a named struct
  2020-05-31 21:46 [PATCH bpf-next 0/6] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 1/6] net: Refactor xdp_convert_buff_to_frame Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 2/6] samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop Lorenzo Bianconi
@ 2020-05-31 21:46 ` Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-05-31 21:46 UTC (permalink / raw)
  To: bpf, netdev; +Cc: davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern

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>
---
 kernel/bpf/cpumap.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 27595fc6da56..57402276d8af 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -48,11 +48,15 @@ struct xdp_bulk_queue {
 	unsigned int count;
 };
 
+/* CPUMAP value */
+struct bpf_cpumap_val {
+	u32 qsize;	/* queue size */
+};
+
 /* Struct for every remote "destination" CPU in map */
 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 +70,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 +313,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 +344,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 +443,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 +456,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 +529,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.qsize : NULL;
 }
 
 static int cpu_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
-- 
2.26.2


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

* [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap
  2020-05-31 21:46 [PATCH bpf-next 0/6] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (2 preceding siblings ...)
  2020-05-31 21:46 ` [PATCH bpf-next 3/6] cpumap: formalize map value as a named struct Lorenzo Bianconi
@ 2020-05-31 21:46 ` Lorenzo Bianconi
  2020-06-01 22:36   ` Alexei Starovoitov
  2020-06-02  7:53   ` Jesper Dangaard Brouer
  2020-05-31 21:46 ` [PATCH bpf-next 5/6] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 6/6] samples/bpf: xdp_redirect_cpu: load an eBPF program on cpumap Lorenzo Bianconi
  5 siblings, 2 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-05-31 21:46 UTC (permalink / raw)
  To: bpf, netdev; +Cc: davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern

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

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/linux/bpf.h            |   6 ++
 include/trace/events/xdp.h     |  14 +++--
 include/uapi/linux/bpf.h       |   1 +
 kernel/bpf/cpumap.c            | 106 +++++++++++++++++++++++++++++----
 net/core/dev.c                 |   8 +++
 net/core/filter.c              |   7 +++
 tools/include/uapi/linux/bpf.h |   1 +
 7 files changed, 128 insertions(+), 15 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index e042311f991f..d5bbcdcc8321 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1256,6 +1256,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)
@@ -1416,6 +1417,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/trace/events/xdp.h b/include/trace/events/xdp.h
index b73d3e141323..06ec557c6bf5 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, unsigned int xdp_pass, unsigned int xdp_drop),
 
-	TP_ARGS(map_id, processed, drops, sched),
+	TP_ARGS(map_id, processed, drops, sched, xdp_pass, xdp_drop),
 
 	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_pass;
+		__entry->xdp_drop	= xdp_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 f74bc4a2385e..10158943c0db 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
 };
 
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 57402276d8af..24ab0a6b9772 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -51,6 +51,10 @@ struct xdp_bulk_queue {
 /* CPUMAP value */
 struct bpf_cpumap_val {
 	u32 qsize;	/* queue size */
+	union {
+		int fd;	/* program file descriptor */
+		u32 id;	/* program id */
+	} prog;
 };
 
 /* Struct for every remote "destination" CPU in map */
@@ -72,6 +76,7 @@ struct bpf_cpu_map_entry {
 	struct rcu_head rcu;
 
 	struct bpf_cpumap_val value;
+	struct bpf_prog *prog;
 };
 
 struct bpf_cpu_map {
@@ -86,6 +91,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;
@@ -96,7 +102,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, prog.fd)) ||
+	    attr->map_flags & ~BPF_F_NUMA_NODE)
 		return ERR_PTR(-EINVAL);
 
 	cmap = kzalloc(sizeof(*cmap), GFP_USER);
@@ -240,11 +248,14 @@ static int cpu_map_kthread_run(void *data)
 	 * kthread_stop signal until queue is empty.
 	 */
 	while (!kthread_should_stop() || !__ptr_ring_empty(rcpu->queue)) {
+		unsigned int xdp_pass = 0, xdp_drop = 0;
+		gfp_t gfp = __GFP_ZERO | GFP_ATOMIC;
 		unsigned int drops = 0, sched = 0;
+		void *xdp_frames[CPUMAP_BATCH];
 		void *frames[CPUMAP_BATCH];
 		void *skbs[CPUMAP_BATCH];
-		gfp_t gfp = __GFP_ZERO | GFP_ATOMIC;
-		int i, n, m;
+		int i, n, m, nframes = 0;
+		struct bpf_prog *prog;
 
 		/* Release CPU reschedule checks */
 		if (__ptr_ring_empty(rcpu->queue)) {
@@ -265,28 +276,67 @@ 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, xdp_frames,
+					     CPUMAP_BATCH);
 
+		rcu_read_lock();
+
+		prog = READ_ONCE(rcpu->prog);
 		for (i = 0; i < n; i++) {
-			void *f = frames[i];
+			void *f = xdp_frames[i];
 			struct page *page = virt_to_page(f);
+			struct xdp_frame *xdpf;
+			struct xdp_buff xdp;
+			u32 act;
+			int err;
 
 			/* Bring struct page memory area to curr CPU. Read by
 			 * build_skb_around via page_is_pfmemalloc(), and when
 			 * freed written by page_frag_free call.
 			 */
 			prefetchw(page);
+			if (!prog) {
+				frames[nframes++] = xdp_frames[i];
+				continue;
+			}
+
+			xdpf = f;
+			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);
+					drops++;
+				} else {
+					frames[nframes++] = xdpf;
+					xdp_pass++;
+				}
+				break;
+			default:
+				bpf_warn_invalid_xdp_action(act);
+				/* fallthrough */
+			case XDP_DROP:
+				xdp_return_frame(xdpf);
+				xdp_drop++;
+				break;
+			}
 		}
 
-		m = kmem_cache_alloc_bulk(skbuff_head_cache, gfp, n, skbs);
+		rcu_read_unlock();
+
+		m = kmem_cache_alloc_bulk(skbuff_head_cache, gfp,
+					  nframes, skbs);
 		if (unlikely(m == 0)) {
-			for (i = 0; i < n; i++)
+			for (i = 0; i < nframes; i++)
 				skbs[i] = NULL; /* effect: xdp_return_frame */
-			drops = n;
+			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;
@@ -303,7 +353,8 @@ 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,
+					 xdp_pass, xdp_drop);
 
 		local_bh_enable(); /* resched point, may call do_softirq() */
 	}
@@ -313,11 +364,37 @@ 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.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)
 {
 	gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
 	struct bpf_cpu_map_entry *rcpu;
+	int prog_fd = value->prog.fd;
 	struct xdp_bulk_queue *bq;
 	int numa, err, i;
 
@@ -361,6 +438,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 (prog_fd >= 0 && __cpu_map_load_bpf_program(rcpu, prog_fd))
+		goto free_ptr_ring;
+
 	/* Make sure kthread runs on a single CPU */
 	kthread_bind(rcpu->kthread, cpu);
 	wake_up_process(rcpu->kthread);
@@ -420,6 +500,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);
@@ -443,7 +525,9 @@ 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_cpumap_val cpumap_value = {
+		.prog.fd = -1,
+	};
 	struct bpf_cpu_map_entry *rcpu;
 	/* Array index key correspond to CPU number */
 	u32 key_cpu = *(u32 *)key;
diff --git a/net/core/dev.c b/net/core/dev.c
index 10684833f864..4b95adaa4641 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5429,6 +5429,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;
 		}
 	}
 
@@ -8853,6 +8855,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/net/core/filter.c b/net/core/filter.c
index 2e9dbfd8e60c..0c748c95179e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7021,6 +7021,13 @@ static bool xdp_is_valid_access(int off, int size,
 		}
 	}
 
+	if (prog->expected_attach_type == BPF_XDP_CPUMAP) {
+		switch (off) {
+		case offsetof(struct xdp_md, ingress_ifindex):
+			return false;
+		}
+	}
+
 	if (type == BPF_WRITE) {
 		if (bpf_prog_is_dev_bound(prog->aux)) {
 			switch (off) {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index f74bc4a2385e..10158943c0db 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
 };
 
-- 
2.26.2


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

* [PATCH bpf-next 5/6] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
  2020-05-31 21:46 [PATCH bpf-next 0/6] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (3 preceding siblings ...)
  2020-05-31 21:46 ` [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
@ 2020-05-31 21:46 ` Lorenzo Bianconi
  2020-05-31 21:46 ` [PATCH bpf-next 6/6] samples/bpf: xdp_redirect_cpu: load an eBPF program on cpumap Lorenzo Bianconi
  5 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-05-31 21:46 UTC (permalink / raw)
  To: bpf, netdev; +Cc: davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern

Add XDP_REDIRECT support for eBPF programs attached to cpumap entries

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/trace/events/xdp.h | 12 ++++++++----
 kernel/bpf/cpumap.c        | 21 +++++++++++++++++----
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index 06ec557c6bf5..162ce06c6da0 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -177,9 +177,11 @@ 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, unsigned int xdp_pass, unsigned int xdp_drop),
+		 int sched, unsigned int xdp_pass, unsigned int xdp_drop,
+		 unsigned int xdp_redirect),
 
-	TP_ARGS(map_id, processed, drops, sched, xdp_pass, xdp_drop),
+	TP_ARGS(map_id, processed, drops, sched, xdp_pass, xdp_drop,
+		xdp_redirect),
 
 	TP_STRUCT__entry(
 		__field(int, map_id)
@@ -190,6 +192,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 +204,19 @@ TRACE_EVENT(xdp_cpumap_kthread,
 		__entry->sched	= sched;
 		__entry->xdp_pass	= xdp_pass;
 		__entry->xdp_drop	= xdp_drop;
+		__entry->xdp_redirect	= xdp_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 24ab0a6b9772..a45157627fbc 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -248,7 +248,7 @@ static int cpu_map_kthread_run(void *data)
 	 * kthread_stop signal until queue is empty.
 	 */
 	while (!kthread_should_stop() || !__ptr_ring_empty(rcpu->queue)) {
-		unsigned int xdp_pass = 0, xdp_drop = 0;
+		unsigned int xdp_pass = 0, xdp_drop = 0, xdp_redirect = 0;
 		gfp_t gfp = __GFP_ZERO | GFP_ATOMIC;
 		unsigned int drops = 0, sched = 0;
 		void *xdp_frames[CPUMAP_BATCH];
@@ -279,7 +279,7 @@ static int cpu_map_kthread_run(void *data)
 		n = ptr_ring_consume_batched(rcpu->queue, xdp_frames,
 					     CPUMAP_BATCH);
 
-		rcu_read_lock();
+		rcu_read_lock_bh();
 
 		prog = READ_ONCE(rcpu->prog);
 		for (i = 0; i < n; i++) {
@@ -315,6 +315,16 @@ static int cpu_map_kthread_run(void *data)
 					xdp_pass++;
 				}
 				break;
+			case XDP_REDIRECT:
+				err = xdp_do_redirect(xdpf->dev_rx, &xdp,
+						      prog);
+				if (unlikely(err)) {
+					xdp_return_frame(xdpf);
+					drops++;
+				} else {
+					xdp_redirect++;
+				}
+				break;
 			default:
 				bpf_warn_invalid_xdp_action(act);
 				/* fallthrough */
@@ -325,7 +335,10 @@ static int cpu_map_kthread_run(void *data)
 			}
 		}
 
-		rcu_read_unlock();
+		if (xdp_redirect)
+			xdp_do_flush_map();
+
+		rcu_read_unlock_bh();
 
 		m = kmem_cache_alloc_bulk(skbuff_head_cache, gfp,
 					  nframes, skbs);
@@ -354,7 +367,7 @@ static int cpu_map_kthread_run(void *data)
 		}
 		/* Feedback loop via tracepoint */
 		trace_xdp_cpumap_kthread(rcpu->map_id, n, drops, sched,
-					 xdp_pass, xdp_drop);
+					 xdp_pass, xdp_drop, xdp_redirect);
 
 		local_bh_enable(); /* resched point, may call do_softirq() */
 	}
-- 
2.26.2


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

* [PATCH bpf-next 6/6] samples/bpf: xdp_redirect_cpu: load an eBPF program on cpumap
  2020-05-31 21:46 [PATCH bpf-next 0/6] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
                   ` (4 preceding siblings ...)
  2020-05-31 21:46 ` [PATCH bpf-next 5/6] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries Lorenzo Bianconi
@ 2020-05-31 21:46 ` Lorenzo Bianconi
  5 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-05-31 21:46 UTC (permalink / raw)
  To: bpf, netdev; +Cc: davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern

Extend xdp_redirect_cpu_{usr,kern}.c adding the possibility to load
a XDP program on cpumap entries

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 samples/bpf/xdp_redirect_cpu_kern.c |  34 ++++++---
 samples/bpf/xdp_redirect_cpu_user.c | 106 ++++++++++++++++++++++++----
 2 files changed, 119 insertions(+), 21 deletions(-)

diff --git a/samples/bpf/xdp_redirect_cpu_kern.c b/samples/bpf/xdp_redirect_cpu_kern.c
index 2baf8db1f7e7..30f500237753 100644
--- a/samples/bpf/xdp_redirect_cpu_kern.c
+++ b/samples/bpf/xdp_redirect_cpu_kern.c
@@ -17,11 +17,20 @@
 
 #define MAX_CPUS NR_CPUS
 
+/* CPUMAP value */
+struct bpf_cpumap_val {
+	u32 qsize;
+	union {
+		int fd;
+		u32 id;
+	} prog;
+};
+
 /* Special map type that can XDP_REDIRECT frames to another CPU */
 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 +39,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 +704,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 +727,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..13f598a24759 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -30,6 +30,15 @@ static const char *__doc__ =
 
 #include "bpf_util.h"
 
+/* CPUMAP value */
+struct bpf_cpumap_val {
+	__u32 qsize;
+	union {
+		int fd;
+		__u32 id;
+	} prog;
+};
+
 static int ifindex = -1;
 static char ifname_buf[IF_NAMESIZE];
 static char *ifname;
@@ -70,6 +79,8 @@ static const struct option long_options[] = {
 	{"stress-mode", no_argument,		NULL, 'x' },
 	{"no-separators", no_argument,		NULL, 'z' },
 	{"force",	no_argument,		NULL, 'F' },
+	{"map-prog-filename", required_argument, NULL, 'f' },
+	{"map-prog-name", required_argument,	NULL, 'P' },
 	{0, 0, NULL,  0 }
 };
 
@@ -156,6 +167,9 @@ struct datarec {
 	__u64 processed;
 	__u64 dropped;
 	__u64 issue;
+	__u64 xdp_pass;
+	__u64 xdp_drop;
+	__u64 xdp_redirect;
 };
 struct record {
 	__u64 timestamp;
@@ -175,6 +189,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 +213,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;
 }
 
@@ -405,6 +431,9 @@ static void stats_print(struct stats_record *stats_rec,
 			if (pps > 0)
 				printf(fmt_k, "cpumap_kthread",
 				       i, pps, drop, err, e_str);
+			printf("%d xdp-pass %llu xdp-drop %llu "
+			       "xdp-redirect %llu\n", i, r->xdp_pass,
+			       r->xdp_drop, r->xdp_redirect);
 		}
 		pps = calc_pps(&rec->total, &prev->total, t);
 		drop = calc_drop_pps(&rec->total, &prev->total, t);
@@ -412,6 +441,9 @@ static void stats_print(struct stats_record *stats_rec,
 		if (err > 0)
 			e_str = "sched-sum";
 		printf(fm2_k, "cpumap_kthread", "total", pps, drop, err, e_str);
+		printf("xdp-pass %llu xdp-drop %llu xdp-redirect %llu\n",
+		       rec->total.xdp_pass, rec->total.xdp_drop,
+		       rec->total.xdp_redirect);
 	}
 
 	/* XDP redirect err tracepoints (very unlikely) */
@@ -494,7 +526,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 +536,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 +567,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->prog.fd, curr_cpus_count);
 
 	return 0;
 }
@@ -561,19 +593,22 @@ 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,
-		       bool stress_mode)
+		       struct bpf_cpumap_val *value, bool stress_mode)
 {
 	struct stats_record *record, *prev;
 
@@ -591,7 +626,7 @@ static void stats_poll(int interval, bool use_separators, char *prog_name,
 		stats_print(record, prev, prog_name);
 		sleep(interval);
 		if (stress_mode)
-			stress_cpumap();
+			stress_cpumap(value);
 	}
 
 	free_stats_record(record);
@@ -664,15 +699,47 @@ static int init_map_fds(struct bpf_object *obj)
 	return 0;
 }
 
+static int load_cpumap_prog(char *file_name, char *prog_name)
+{
+	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;
+	}
+
+	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 *map_prog_filename = "xdp_redirect_kern.o";
+	char *map_prog_name = "xdp_redirect_dummy";
 	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 +795,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:P:",
 				  long_options, &longindex)) != -1) {
 		switch (opt) {
 		case 'd':
@@ -762,6 +829,12 @@ int main(int argc, char **argv)
 			/* Selecting eBPF prog to load */
 			prog_name = optarg;
 			break;
+		case 'f':
+			map_prog_filename = optarg;
+			break;
+		case 'P':
+			map_prog_name = optarg;
+			break;
 		case 'c':
 			/* Add multiple CPUs */
 			add_cpu = strtoul(optarg, NULL, 0);
@@ -807,8 +880,15 @@ int main(int argc, char **argv)
 		goto out;
 	}
 
+	value.prog.fd = load_cpumap_prog(map_prog_filename, map_prog_name);
+	if (value.prog.fd < 0) {
+		err = value.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 +921,7 @@ 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, &value, stress_mode);
 out:
 	free(cpu);
 	return err;
-- 
2.26.2


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

* Re: [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap
  2020-05-31 21:46 ` [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
@ 2020-06-01 22:36   ` Alexei Starovoitov
  2020-06-02  9:27     ` Lorenzo Bianconi
  2020-06-02  7:53   ` Jesper Dangaard Brouer
  1 sibling, 1 reply; 11+ messages in thread
From: Alexei Starovoitov @ 2020-06-01 22:36 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: bpf, netdev, davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern

On Sun, May 31, 2020 at 11:46:49PM +0200, Lorenzo Bianconi wrote:
> +
> +		prog = READ_ONCE(rcpu->prog);
>  		for (i = 0; i < n; i++) {
> -			void *f = frames[i];
> +			void *f = xdp_frames[i];
>  			struct page *page = virt_to_page(f);
> +			struct xdp_frame *xdpf;
> +			struct xdp_buff xdp;
> +			u32 act;
> +			int err;
>  
>  			/* Bring struct page memory area to curr CPU. Read by
>  			 * build_skb_around via page_is_pfmemalloc(), and when
>  			 * freed written by page_frag_free call.
>  			 */
>  			prefetchw(page);
> +			if (!prog) {
> +				frames[nframes++] = xdp_frames[i];
> +				continue;
> +			}

I'm not sure compiler will be smart enough to hoist !prog check out of the loop.
Otherwise default cpumap case will be a bit slower.
I'd like to see performance numbers before/after and acks from folks
who are using cpumap before applying.
Also please add selftest for it. samples/bpf/ in patch 6 is not enough.

Other than the above the feature looks good to me. It nicely complements devmap.

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

* Re: [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap
  2020-05-31 21:46 ` [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
  2020-06-01 22:36   ` Alexei Starovoitov
@ 2020-06-02  7:53   ` Jesper Dangaard Brouer
  2020-06-02  9:29     ` Lorenzo Bianconi
  1 sibling, 1 reply; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-02  7:53 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: bpf, netdev, davem, ast, toke, daniel, lorenzo.bianconi, dsahern, brouer

On Sun, 31 May 2020 23:46:49 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index 57402276d8af..24ab0a6b9772 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -51,6 +51,10 @@ struct xdp_bulk_queue {
>  /* CPUMAP value */
>  struct bpf_cpumap_val {
>  	u32 qsize;	/* queue size */
> +	union {
> +		int fd;	/* program file descriptor */
> +		u32 id;	/* program id */
> +	} prog;
>  };
  
Please name the union 'bpf_prog' and not 'prog'.
We should match what David Ahern did for devmap.

Even-though we are NOT exposing this in the UAPI header-file, this still
becomes a UAPI interface (actually kABI).  The struct member names are
still important, even-though this is a binary layout, because the BTF
info is basically documenting this API.

Notice when kernel is compiled with BTF info, you (or end-user) can use
pahole to "reverse" the struct layout (comments don't survive, so we
need descriptive member names):

$ pahole bpf_devmap_val
struct bpf_devmap_val {
	__u32                      ifindex;              /*     0     4 */
	union {
		int                fd;                   /*     4     4 */
		__u32              id;                   /*     4     4 */
	} bpf_prog;                                      /*     4     4 */
	struct {
		unsigned char      data[24];             /*     8    24 */
	} storage;                                       /*     8    24 */

	/* size: 32, cachelines: 1, members: 3 */
	/* last cacheline: 32 bytes */
};

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


$ bpftool btf dump file /sys/kernel/btf/vmlinux format c | grep -A10 'struct bpf_devmap_val {'
struct bpf_devmap_val {
	__u32 ifindex;
	union {
		int fd;
		__u32 id;
	} bpf_prog;
	struct {
		unsigned char data[24];
	} storage;
};


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

* Re: [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap
  2020-06-01 22:36   ` Alexei Starovoitov
@ 2020-06-02  9:27     ` Lorenzo Bianconi
  0 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-06-02  9:27 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: bpf, netdev, davem, ast, brouer, toke, daniel, lorenzo.bianconi, dsahern

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

> On Sun, May 31, 2020 at 11:46:49PM +0200, Lorenzo Bianconi wrote:
> > +
> > +		prog = READ_ONCE(rcpu->prog);
> >  		for (i = 0; i < n; i++) {
> > -			void *f = frames[i];
> > +			void *f = xdp_frames[i];
> >  			struct page *page = virt_to_page(f);
> > +			struct xdp_frame *xdpf;
> > +			struct xdp_buff xdp;
> > +			u32 act;
> > +			int err;
> >  
> >  			/* Bring struct page memory area to curr CPU. Read by
> >  			 * build_skb_around via page_is_pfmemalloc(), and when
> >  			 * freed written by page_frag_free call.
> >  			 */
> >  			prefetchw(page);
> > +			if (!prog) {
> > +				frames[nframes++] = xdp_frames[i];
> > +				continue;
> > +			}
> 
> I'm not sure compiler will be smart enough to hoist !prog check out of the loop.
> Otherwise default cpumap case will be a bit slower.
> I'd like to see performance numbers before/after and acks from folks
> who are using cpumap before applying.
> Also please add selftest for it. samples/bpf/ in patch 6 is not enough.

Hi Alexei,

thx for the review. I will add a selftest and some performance numbers in v2.

Regards,
Lorenzo

> 
> Other than the above the feature looks good to me. It nicely complements devmap.

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

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

* Re: [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap
  2020-06-02  7:53   ` Jesper Dangaard Brouer
@ 2020-06-02  9:29     ` Lorenzo Bianconi
  0 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2020-06-02  9:29 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: bpf, netdev, davem, ast, toke, daniel, lorenzo.bianconi, dsahern

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

> On Sun, 31 May 2020 23:46:49 +0200
> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> 
> > diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> > index 57402276d8af..24ab0a6b9772 100644
> > --- a/kernel/bpf/cpumap.c
> > +++ b/kernel/bpf/cpumap.c
> > @@ -51,6 +51,10 @@ struct xdp_bulk_queue {
> >  /* CPUMAP value */
> >  struct bpf_cpumap_val {
> >  	u32 qsize;	/* queue size */
> > +	union {
> > +		int fd;	/* program file descriptor */
> > +		u32 id;	/* program id */
> > +	} prog;
> >  };
>   
> Please name the union 'bpf_prog' and not 'prog'.
> We should match what David Ahern did for devmap.

Hi Jesper,

ack, I will align the struct to David's one in v2.

Regards,
Lorenzo

> 
> Even-though we are NOT exposing this in the UAPI header-file, this still
> becomes a UAPI interface (actually kABI).  The struct member names are
> still important, even-though this is a binary layout, because the BTF
> info is basically documenting this API.
> 
> Notice when kernel is compiled with BTF info, you (or end-user) can use
> pahole to "reverse" the struct layout (comments don't survive, so we
> need descriptive member names):
> 
> $ pahole bpf_devmap_val
> struct bpf_devmap_val {
> 	__u32                      ifindex;              /*     0     4 */
> 	union {
> 		int                fd;                   /*     4     4 */
> 		__u32              id;                   /*     4     4 */
> 	} bpf_prog;                                      /*     4     4 */
> 	struct {
> 		unsigned char      data[24];             /*     8    24 */
> 	} storage;                                       /*     8    24 */
> 
> 	/* size: 32, cachelines: 1, members: 3 */
> 	/* last cacheline: 32 bytes */
> };
> 
> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
> 
> 
> $ bpftool btf dump file /sys/kernel/btf/vmlinux format c | grep -A10 'struct bpf_devmap_val {'
> struct bpf_devmap_val {
> 	__u32 ifindex;
> 	union {
> 		int fd;
> 		__u32 id;
> 	} bpf_prog;
> 	struct {
> 		unsigned char data[24];
> 	} storage;
> };
> 

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

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

end of thread, other threads:[~2020-06-02  9:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-31 21:46 [PATCH bpf-next 0/6] introduce support for XDP programs in CPUMAP Lorenzo Bianconi
2020-05-31 21:46 ` [PATCH bpf-next 1/6] net: Refactor xdp_convert_buff_to_frame Lorenzo Bianconi
2020-05-31 21:46 ` [PATCH bpf-next 2/6] samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop Lorenzo Bianconi
2020-05-31 21:46 ` [PATCH bpf-next 3/6] cpumap: formalize map value as a named struct Lorenzo Bianconi
2020-05-31 21:46 ` [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap Lorenzo Bianconi
2020-06-01 22:36   ` Alexei Starovoitov
2020-06-02  9:27     ` Lorenzo Bianconi
2020-06-02  7:53   ` Jesper Dangaard Brouer
2020-06-02  9:29     ` Lorenzo Bianconi
2020-05-31 21:46 ` [PATCH bpf-next 5/6] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries Lorenzo Bianconi
2020-05-31 21:46 ` [PATCH bpf-next 6/6] samples/bpf: xdp_redirect_cpu: load an eBPF program on cpumap Lorenzo Bianconi

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