introduce support for XDP programs in CPUMAP
Subsystems (9)
v4 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].
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 v3:
- fix typo in commit message
- fix access to ctx->ingress_ifindex in cpumap bpf selftest
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/[email protected]/
[2] https://patchwork.ozlabs.org/project/netdev/patch/[email protected]/
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 | 36 +++ 12 files changed, 528 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
Signature · 1 line
2.26.2
PATCH v4 1/9 cpumap: use non-locked version __ptr_ring_consume_batched
From: Jesper Dangaard Brouer <[email protected]>
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.
--- kernel/bpf/cpumap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/bpf/cpumap.c +1 -1
@@ -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];
Signature · 1 line
2.26.2
PATCH v4 2/9 net: Refactor xdp_convert_buff_to_frame
In reply to Lorenzo BianconiFrom: 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
@@ -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;
Signature · 1 line
2.26.2
PATCH v4 3/9 samples/bpf: xdp_redirect_cpu_user: do not update bpf maps in option loop
In reply to Lorenzo BianconiDo 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
@@ -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
PATCH v4 4/9 cpumap: formalize map value as a named struct
In reply to Lorenzo BianconiAs 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.
--- 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(-)
include/uapi/linux/bpf.h +9 -0
@@ -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 ,
kernel/bpf/cpumap.c +13 -12
@@ -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)
tools/include/uapi/linux/bpf.h +9 -0
@@ -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 ,
Signature · 1 line
2.26.2
PATCH v4 5/9 bpf: cpumap: add the possibility to attach an eBPF program to cpumap
In reply to Lorenzo BianconiIntroduce 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
--- 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(-)
include/linux/bpf.h +6 -0
@@ -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) {
include/net/xdp.h +5 -0
@@ -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) {
include/trace/events/xdp.h +10 -4
@@ -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,
include/uapi/linux/bpf.h +5 -0
@@ -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 {
kernel/bpf/cpumap.c +109 -13
200 lines not on this page · raw message
net/core/dev.c +8 -0
@@ -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);
tools/include/uapi/linux/bpf.h +5 -0
@@ -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 {
Signature · 1 line
2.26.2
PATCH v4 6/9 bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
In reply to Lorenzo BianconiIntroduce 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
Attaching a simple XDP program on eth0 to perform XDP_TX gives comparable results:
tx: 288.4 Kpps rx: 288.4 Kpps
--- include/net/xdp.h | 1 + include/trace/events/xdp.h | 6 ++++-- kernel/bpf/cpumap.c | 17 +++++++++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-)
include/net/xdp.h +1 -0
@@ -99,6 +99,7 @@ struct xdp_frame { };struct xdp_cpumap_stats { +unsigned int redirect;unsigned int pass;unsigned int drop; };
include/trace/events/xdp.h +4 -2
@@ -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,
kernel/bpf/cpumap.c +15 -2
@@ -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;
Signature · 1 line
2.26.2
Daniel Borkmann
maintainerRe: [PATCH v4 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
Quoted 91 lines · Lorenzo Bianconi
Introduce
XDP_REDIRECTsupport for eBPF programs attached to cpumap entries.
This patch has been tested on Marvell ESPRESSObin using a modified version ofxdp_redirect_cpusample 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 -exdp_redirect\
-f xdp_redirect_kern.o -mtx_port-r eth0tx: 285.2 Kpps rx: 285.2 Kpps
Attaching a simple XDP program on eth0 to perform
XDP_TXgives comparable results:tx: 288.4 Kpps rx: 288.4 Kpps
Co-developed-by Jesper Dangaard Brouer <[email protected]>Signed-off-by Jesper Dangaard Brouer <[email protected]>Signed-off-by Lorenzo Bianconi <[email protected]>--- include/net/xdp.h | 1 + include/trace/events/xdp.h | 6 ++++-- kernel/bpf/cpumap.c | 17 +++++++++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-)include/net/xdp.h +1
-0@@ -99,6 +99,7 @@ struct xdp_frame { };struct xdp_cpumap_stats { +unsigned int redirect;unsigned int pass;unsigned int drop; };include/trace/events/xdp.h +4
-2@@ -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,kernel/bpf/cpumap.c +1
-1@@ -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++; }
kernel/bpf/cpumap.c static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, · line 277
Quoted 10 lines
kernel/bpf/cpumap.c +9
-0break ; +case XDP_REDIRECT : + err =xdp_do_redirect (xdpf->dev_rx, &xdp, + prog); +if (unlikely (err)) { +xdp_return_frame (xdpf); + stats->drop++; + }else { + stats->redirect++; + }
Could we do better with all the accounting and do this from /inside/ BPF tracing prog instead (otherwise too bad we need to have it here even if the tracepoint is disabled)?
Quoted 5 lines
kernel/bpf/cpumap.c +1
-0+break ;default :bpf_warn_invalid_xdp_action (act);/* fallthrough */
kernel/bpf/cpumap.c static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, · line 293
Quoted 9 lines
kernel/bpf/cpumap.c +4
-1@@ -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 ();
Hm, this looks incorrect. Why do you call the xdp_clear_return_frame_no_direct() /after/ the possibility where there is a rescheduling point for softirq?
Quoted 2 lines
kernel/bpf/cpumap.c +0
-0return nframes;
Re: [PATCH v4 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
Quoted 4 lines · On Thu, 25 Jun 2020 23:28:59 +0200 Daniel Borkmann <[email protected]> wrote:
Quoted 4 lines
(quoted diff) +0
-0@@ -276,7 +286,10 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, } }
Quoted 9 lines
kernel/bpf/cpumap.c static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, · line 293
Quoted 6 lines
kernel/bpf/cpumap.c +4
-1-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 ();Hm, this looks incorrect. Why do you call the
xdp_clear_return_frame_no_direct()/after/ the possibility where there is a rescheduling point for softirq?
Good you caught this! - This needs to be fixed.
Signature · 4 lines
Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [PATCH v4 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
In reply to Daniel BorkmannQuoted 1 line · Lorenzo Bianconi
[...]
Quoted 8 lines
kernel/bpf/cpumap.c static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, · line 293
Quoted 5 lines
kernel/bpf/cpumap.c +4
-0+if (stats->redirect) +xdp_do_flush_map (); + +rcu_read_unlock_bh ();/* resched point, may call do_softirq() */ xdp_clear_return_frame_no_direct ();Hm, this looks incorrect. Why do you call the
xdp_clear_return_frame_no_direct()/after/ the possibility where there is a rescheduling point for softirq?
Hi Daniel,
yes, right. Thx for spotting this, I will fix in v5.
Regards,
Lorenzo
Quoted 4 lines
Quoted 3 lines
return nframes;
Re: [PATCH v4 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
In reply to Daniel BorkmannQuoted 11 lines · On Thu, 25 Jun 2020 23:28:59 +0200 Daniel Borkmann <[email protected]> wrote:
Quoted 11 lines
kernel/bpf/cpumap.c +1
-1@@ -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 ();
Quoted 12 lines
kernel/bpf/cpumap.c static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, · line 274
Quoted 12 lines
kernel/bpf/cpumap.c +6
-0prog =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++;
I consider if this should be a redir_err counter.
Quoted 6 lines
kernel/bpf/cpumap.c static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu, · line 277
Quoted 3 lines
kernel/bpf/cpumap.c +3
-0+ }else { + stats->redirect++; + }Could we do better with all the accounting and do this from /inside/ BPF tracing prog instead (otherwise too bad we need to have it here even if the tracepoint is disabled)?
I'm on-the-fence with this one...
First of all the BPF-prog cannot see the return code of xdp_do_redirect. So, it cannot give the correct/needed stats without this counter. It would basically report the redirects as successful redirects. (This is actually a re-occuring support issue, when end-users misconfigure xdp_redirect sample and think they get good performance, even-though packets are dropped).
Specifically for XDP_REDIRECT we need to update some state anyhow, such that we know to call xdp_do_flush_map(). Thus removing the counter would not gain much performance wise.
Signature · 4 lines
Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [PATCH v4 bpf-next 6/9] bpf: cpumap: implement XDP_REDIRECT for eBPF programs attached to map entries
In reply to Lorenzo BianconiQuoted 25 lines · On Wed, 24 Jun 2020 17:33:55 +0200 Lorenzo Bianconi <[email protected]> wrote:
include/net/xdp.h +1
-0@@ -99,6 +99,7 @@ struct xdp_frame { };struct xdp_cpumap_stats { +unsigned int redirect;unsigned int pass;unsigned int drop; };include/trace/events/xdp.h +1
-0@@ -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(
include/trace/events/xdp.h TRACE_EVENT(xdp_cpumap_kthread, · line 206
Quoted 5 lines
include/trace/events/xdp.h +1
-0@@ -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; ),
Let me stress, that I think can do this in a followup patch (but before a release).
I'm considering that we should store/give a pointer to xdp_stats (struct xdp_cpumap_stats) and let the BPF tracing program do the "decoding"/struct access to get these values. (We will go from storing 12 bytes to 8 bytes (on 64-bit), so I don't expect much gain).
Quoted 14 lines
include/trace/events/xdp.h +2
-2TP_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) );
Signature · 4 lines
Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
PATCH v4 7/9 libbpf: add SEC name for xdp programs attached to CPUMAP
In reply to Lorenzo BianconiAs 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.
--- tools/lib/bpf/libbpf.c | 2 ++ 1 file changed, 2 insertions(+)
tools/lib/bpf/libbpf.c +2 -0
@@ -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),
Signature · 1 line
2.26.2
PATCH v4 8/9 samples/bpf: xdp_redirect_cpu: load a eBPF program on cpumap
In reply to Lorenzo BianconiExtend 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
--- samples/bpf/xdp_redirect_cpu_kern.c | 25 ++-- samples/bpf/xdp_redirect_cpu_user.c | 174 +++++++++++++++++++++++++--- 2 files changed, 177 insertions(+), 22 deletions(-)
samples/bpf/xdp_redirect_cpu_kern.c +17 -8
@@ -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)
samples/bpf/xdp_redirect_cpu_user.c +160 -14
309 lines not on this page · raw message
Signature · 1 line
2.26.2
PATCH v4 9/9 selftest: add tests for XDP programs in CPUMAP entries
In reply to Lorenzo BianconiSimilar 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.
--- .../bpf/prog_tests/xdp_cpumap_attach.c | 70 +++++++++++++++++++ .../bpf/progs/test_xdp_with_cpumap_helpers.c | 36 ++++++++++ 2 files changed, 106 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
tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c new file +70 -0
@@ -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 (); +}
tools/testing/selftests/bpf/progs/test_xdp_with_cpumap_helpers.c new file +36 -0
@@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +#define IFINDEX_LO 1 + +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) +{ +if (ctx->ingress_ifindex ==IFINDEX_LO ) +return XDP_DROP ; + +return XDP_PASS ; +} + +char _license[] SEC("license") = "GPL";
Signature · 1 line
2.26.2