Skip to contentSkip to conversation

introduce support for XDP programs in CPUMAP

PATCH Superseded Lorenzo Bianconi 3 participants 11 messages May 31, 2020 – Jun 2, 2020

Subsystems (7)

v1 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

cover introduce support for XDP programs in CPUMAP

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/[email protected]/
- https://patchwork.ozlabs.org/project/netdev/cover/[email protected]/

[1] https://patchwork.ozlabs.org/project/netdev/cover/[email protected]/
[2] https://patchwork.ozlabs.org/project/netdev/patch/[email protected]/

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(-)
Signature · 1 line
2.26.2
#1 lore raw

PATCH 1/6 net: Refactor xdp_convert_buff_to_frame

From: David Ahern <[email protected]>

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

---
 include/net/xdp.h | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)
include/net/xdp.h +22 -13
index 609f819ed08b..ab1c503808a4 100644
@@ -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;
 
Signature · 1 line
2.26.2
#2 lore raw

PATCH 2/6 samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop

In reply to Lorenzo Bianconi

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

---
 samples/bpf/xdp_redirect_cpu_user.c | 36 +++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 9 deletions(-)
samples/bpf/xdp_redirect_cpu_user.c +27 -9
index f3468168982e..1a054737c35a 100644
@@ -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;
 }
Signature · 1 line
2.26.2
#3 lore raw

PATCH 3/6 cpumap: formalize map value as a named struct

In reply to Lorenzo Bianconi

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.

---
 kernel/bpf/cpumap.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)
kernel/bpf/cpumap.c +18 -12
index 27595fc6da56..57402276d8af 100644
@@ -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)
Signature · 1 line
2.26.2
#4 lore raw

PATCH 4/6 bpf: cpumap: add the possibility to attach an eBPF program to cpumap

In reply to Lorenzo Bianconi

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

---
 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(-)
include/linux/bpf.h +6 -0
index e042311f991f..d5bbcdcc8321 100644
@@ -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)
 {
include/trace/events/xdp.h +10 -4
index b73d3e141323..06ec557c6bf5 100644
@@ -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,
include/uapi/linux/bpf.h +1 -0
index f74bc4a2385e..10158943c0db 100644
@@ -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
 };
 
kernel/bpf/cpumap.c +95 -11

196 lines not on this page · raw message

net/core/dev.c +8 -0
index 10684833f864..4b95adaa4641 100644
@@ -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);
net/core/filter.c +7 -0
index 2e9dbfd8e60c..0c748c95179e 100644
@@ -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) {
tools/include/uapi/linux/bpf.h +1 -0
index f74bc4a2385e..10158943c0db 100644
@@ -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
 };
 
Signature · 1 line
2.26.2
#5 lore raw

Re: [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap

Quoted 10 lines · Lorenzo Bianconi
kernel/bpf/cpumap.c +7 -1
+
+		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;

kernel/bpf/cpumap.c static int cpu_map_kthread_run(void *data) · line 301

Quoted 10 lines
kernel/bpf/cpumap.c +4 -0
 
 			/* 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.

#6 lore raw

Re: [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap

Quoted 12 lines · Lorenzo Bianconi
Quoted 11 lines
kernel/bpf/cpumap.c +7 -1
+
+		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;
Quoted 15 lines

kernel/bpf/cpumap.c static int cpu_map_kthread_run(void *data) · line 301

Quoted 9 lines
kernel/bpf/cpumap.c +4 -0
 			/* 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

Quoted 2 lines

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

#7 lore raw

Re: [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap

In reply to Lorenzo Bianconi

kernel/bpf/cpumap.c struct xdp_bulk_queue { · line 58

Quoted 13 lines · On Sun, 31 May 2020 23:46:49 +0200 Lorenzo Bianconi <[email protected]> wrote:
kernel/bpf/cpumap.c +4 -0
index 57402276d8af..24ab0a6b9772 100644
@@ -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 */
};
Signature · 17 lines
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;
};
#8 lore raw

Re: [PATCH bpf-next 4/6] bpf: cpumap: add the possibility to attach an eBPF program to cpumap

Quoted 19 lines

kernel/bpf/cpumap.c struct xdp_bulk_queue { · line 58

Quoted 13 lines · On Sun, 31 May 2020 23:46:49 +0200 Lorenzo Bianconi <[email protected]> wrote:
kernel/bpf/cpumap.c +4 -0
index 57402276d8af..24ab0a6b9772 100644
@@ -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

Quoted 44 lines

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 */
};
Signature · 17 lines
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;
};
#9 lore raw

PATCH 5/6 bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries

In reply to Lorenzo Bianconi

Add XDP_REDIRECT support for eBPF programs attached to cpumap entries

---
 include/trace/events/xdp.h | 12 ++++++++----
 kernel/bpf/cpumap.c        | 21 +++++++++++++++++----
 2 files changed, 25 insertions(+), 8 deletions(-)
include/trace/events/xdp.h +8 -4
index 06ec557c6bf5..162ce06c6da0 100644
@@ -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,
kernel/bpf/cpumap.c +17 -4
index 24ab0a6b9772..a45157627fbc 100644
@@ -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() */
 	}
Signature · 1 line
2.26.2
#10 lore raw

PATCH 6/6 samples/bpf: xdp_redirect_cpu: load an eBPF program on cpumap

In reply to Lorenzo Bianconi

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

---
 samples/bpf/xdp_redirect_cpu_kern.c |  34 ++++++---
 samples/bpf/xdp_redirect_cpu_user.c | 106 ++++++++++++++++++++++++----
 2 files changed, 119 insertions(+), 21 deletions(-)
samples/bpf/xdp_redirect_cpu_kern.c +26 -8
index 2baf8db1f7e7..30f500237753 100644
@@ -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)
samples/bpf/xdp_redirect_cpu_user.c +93 -13

231 lines not on this page · raw message

Signature · 1 line
2.26.2