| Message ID | 20191211223344.165549-4-brianvv@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | add bpf batch ops to process more than 1 elem | expand |
On 12/11/19 2:33 PM, Brian Vazquez wrote: > This commit adds generic support for update and delete batch ops that > can be used for almost all the bpf maps. These commands share the same > UAPI attr that lookup and lookup_and_delete batch ops use and the > syscall commands are: > > BPF_MAP_UPDATE_BATCH > BPF_MAP_DELETE_BATCH > > The main difference between update/delete and lookup/lookup_and_delete > batch ops is that for update/delete keys/values must be specified for > userspace and because of that, neither in_batch nor out_batch are used. > > Suggested-by: Stanislav Fomichev <sdf@google.com> > Signed-off-by: Brian Vazquez <brianvv@google.com> > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > include/linux/bpf.h | 10 ++++ > include/uapi/linux/bpf.h | 2 + > kernel/bpf/syscall.c | 117 ++++++++++++++++++++++++++++++++++++++- > 3 files changed, 128 insertions(+), 1 deletion(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index a16f209255a59..851fb3ff084b0 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -48,6 +48,10 @@ struct bpf_map_ops { > int (*map_lookup_and_delete_batch)(struct bpf_map *map, > const union bpf_attr *attr, > union bpf_attr __user *uattr); > + int (*map_update_batch)(struct bpf_map *map, const union bpf_attr *attr, > + union bpf_attr __user *uattr); > + int (*map_delete_batch)(struct bpf_map *map, const union bpf_attr *attr, > + union bpf_attr __user *uattr); > > /* funcs callable from userspace and from eBPF programs */ > void *(*map_lookup_elem)(struct bpf_map *map, void *key); > @@ -849,6 +853,12 @@ int generic_map_lookup_batch(struct bpf_map *map, > int generic_map_lookup_and_delete_batch(struct bpf_map *map, > const union bpf_attr *attr, > union bpf_attr __user *uattr); > +int generic_map_update_batch(struct bpf_map *map, > + const union bpf_attr *attr, > + union bpf_attr __user *uattr); > +int generic_map_delete_batch(struct bpf_map *map, > + const union bpf_attr *attr, > + union bpf_attr __user *uattr); > > extern int sysctl_unprivileged_bpf_disabled; > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 36d3b885ddedd..dab24a763e4bb 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -109,6 +109,8 @@ enum bpf_cmd { > BPF_BTF_GET_NEXT_ID, > BPF_MAP_LOOKUP_BATCH, > BPF_MAP_LOOKUP_AND_DELETE_BATCH, > + BPF_MAP_UPDATE_BATCH, > + BPF_MAP_DELETE_BATCH, > }; > > enum bpf_map_type { > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 708aa89fe2308..8272e76183068 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -1206,6 +1206,111 @@ static int map_get_next_key(union bpf_attr *attr) > return err; > } > > +int generic_map_delete_batch(struct bpf_map *map, > + const union bpf_attr *attr, > + union bpf_attr __user *uattr) > +{ > + void __user *keys = u64_to_user_ptr(attr->batch.keys); > + u32 cp, max_count; > + int err = 0; > + void *key; > + > + if (attr->batch.elem_flags & ~BPF_F_LOCK) > + return -EINVAL; > + > + if ((attr->batch.elem_flags & BPF_F_LOCK) && > + !map_value_has_spin_lock(map)) { > + return -EINVAL; > + } > + > + max_count = attr->batch.count; > + if (!max_count) > + return -EINVAL; To be consistent with lookup and lookup_and_delete, if max_count = 0, we can just return 0 instead of -EINVAL. > + > + for (cp = 0; cp < max_count; cp++) { > + key = __bpf_copy_key(keys + cp * map->key_size, map->key_size); > + if (IS_ERR(key)) { > + err = PTR_ERR(key); > + break; > + } > + > + if (bpf_map_is_dev_bound(map)) { > + err = bpf_map_offload_delete_elem(map, key); > + break; > + } > + > + preempt_disable(); > + __this_cpu_inc(bpf_prog_active); > + rcu_read_lock(); > + err = map->ops->map_delete_elem(map, key); > + rcu_read_unlock(); > + __this_cpu_dec(bpf_prog_active); > + preempt_enable(); > + maybe_wait_bpf_programs(map); > + if (err) > + break; > + } > + if (copy_to_user(&uattr->batch.count, &cp, sizeof(cp))) > + err = -EFAULT; > + return err; > +} > + > +int generic_map_update_batch(struct bpf_map *map, > + const union bpf_attr *attr, > + union bpf_attr __user *uattr) > +{ > + void __user *values = u64_to_user_ptr(attr->batch.values); > + void __user *keys = u64_to_user_ptr(attr->batch.keys); > + u32 value_size, cp, max_count; > + int ufd = attr->map_fd; > + void *key, *value; > + struct fd f; > + int err = 0; > + > + f = fdget(ufd); I did not find the pairing fdput(). Also, the variable 'f' is used way down in the loop, so you can do fdget() later. > + if (attr->batch.elem_flags & ~BPF_F_LOCK) > + return -EINVAL; > + > + if ((attr->batch.elem_flags & BPF_F_LOCK) && > + !map_value_has_spin_lock(map)) { > + return -EINVAL; > + } > + > + value_size = bpf_map_value_size(map); > + > + max_count = attr->batch.count; > + if (!max_count) > + return 0; > + > + value = kmalloc(value_size, GFP_USER | __GFP_NOWARN); > + if (!value) > + return -ENOMEM; > + > + for (cp = 0; cp < max_count; cp++) { > + key = __bpf_copy_key(keys + cp * map->key_size, map->key_size); > + if (IS_ERR(key)) { > + err = PTR_ERR(key); > + break; > + } > + err = -EFAULT; > + if (copy_from_user(value, values + cp * value_size, value_size)) > + break; > + > + err = bpf_map_update_value(map, f, key, value, > + attr->batch.elem_flags); > + > + if (err) > + break; > + } > + > + if (copy_to_user(&uattr->batch.count, &cp, sizeof(cp))) > + err = -EFAULT; > + > + kfree(value); > + kfree(key); > + return err; > +} > + [...]
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index a16f209255a59..851fb3ff084b0 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -48,6 +48,10 @@ struct bpf_map_ops { int (*map_lookup_and_delete_batch)(struct bpf_map *map, const union bpf_attr *attr, union bpf_attr __user *uattr); + int (*map_update_batch)(struct bpf_map *map, const union bpf_attr *attr, + union bpf_attr __user *uattr); + int (*map_delete_batch)(struct bpf_map *map, const union bpf_attr *attr, + union bpf_attr __user *uattr); /* funcs callable from userspace and from eBPF programs */ void *(*map_lookup_elem)(struct bpf_map *map, void *key); @@ -849,6 +853,12 @@ int generic_map_lookup_batch(struct bpf_map *map, int generic_map_lookup_and_delete_batch(struct bpf_map *map, const union bpf_attr *attr, union bpf_attr __user *uattr); +int generic_map_update_batch(struct bpf_map *map, + const union bpf_attr *attr, + union bpf_attr __user *uattr); +int generic_map_delete_batch(struct bpf_map *map, + const union bpf_attr *attr, + union bpf_attr __user *uattr); extern int sysctl_unprivileged_bpf_disabled; diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 36d3b885ddedd..dab24a763e4bb 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -109,6 +109,8 @@ enum bpf_cmd { BPF_BTF_GET_NEXT_ID, BPF_MAP_LOOKUP_BATCH, BPF_MAP_LOOKUP_AND_DELETE_BATCH, + BPF_MAP_UPDATE_BATCH, + BPF_MAP_DELETE_BATCH, }; enum bpf_map_type { diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 708aa89fe2308..8272e76183068 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1206,6 +1206,111 @@ static int map_get_next_key(union bpf_attr *attr) return err; } +int generic_map_delete_batch(struct bpf_map *map, + const union bpf_attr *attr, + union bpf_attr __user *uattr) +{ + void __user *keys = u64_to_user_ptr(attr->batch.keys); + u32 cp, max_count; + int err = 0; + void *key; + + if (attr->batch.elem_flags & ~BPF_F_LOCK) + return -EINVAL; + + if ((attr->batch.elem_flags & BPF_F_LOCK) && + !map_value_has_spin_lock(map)) { + return -EINVAL; + } + + max_count = attr->batch.count; + if (!max_count) + return -EINVAL; + + for (cp = 0; cp < max_count; cp++) { + key = __bpf_copy_key(keys + cp * map->key_size, map->key_size); + if (IS_ERR(key)) { + err = PTR_ERR(key); + break; + } + + if (bpf_map_is_dev_bound(map)) { + err = bpf_map_offload_delete_elem(map, key); + break; + } + + preempt_disable(); + __this_cpu_inc(bpf_prog_active); + rcu_read_lock(); + err = map->ops->map_delete_elem(map, key); + rcu_read_unlock(); + __this_cpu_dec(bpf_prog_active); + preempt_enable(); + maybe_wait_bpf_programs(map); + if (err) + break; + } + if (copy_to_user(&uattr->batch.count, &cp, sizeof(cp))) + err = -EFAULT; + return err; +} + +int generic_map_update_batch(struct bpf_map *map, + const union bpf_attr *attr, + union bpf_attr __user *uattr) +{ + void __user *values = u64_to_user_ptr(attr->batch.values); + void __user *keys = u64_to_user_ptr(attr->batch.keys); + u32 value_size, cp, max_count; + int ufd = attr->map_fd; + void *key, *value; + struct fd f; + int err = 0; + + f = fdget(ufd); + if (attr->batch.elem_flags & ~BPF_F_LOCK) + return -EINVAL; + + if ((attr->batch.elem_flags & BPF_F_LOCK) && + !map_value_has_spin_lock(map)) { + return -EINVAL; + } + + value_size = bpf_map_value_size(map); + + max_count = attr->batch.count; + if (!max_count) + return 0; + + value = kmalloc(value_size, GFP_USER | __GFP_NOWARN); + if (!value) + return -ENOMEM; + + for (cp = 0; cp < max_count; cp++) { + key = __bpf_copy_key(keys + cp * map->key_size, map->key_size); + if (IS_ERR(key)) { + err = PTR_ERR(key); + break; + } + err = -EFAULT; + if (copy_from_user(value, values + cp * value_size, value_size)) + break; + + err = bpf_map_update_value(map, f, key, value, + attr->batch.elem_flags); + + if (err) + break; + } + + if (copy_to_user(&uattr->batch.count, &cp, sizeof(cp))) + err = -EFAULT; + + kfree(value); + kfree(key); + return err; +} + #define MAP_LOOKUP_RETRIES 3 static int __generic_map_lookup_batch(struct bpf_map *map, @@ -3203,8 +3308,12 @@ static int bpf_map_do_batch(const union bpf_attr *attr, if (cmd == BPF_MAP_LOOKUP_BATCH) BPF_DO_BATCH(map->ops->map_lookup_batch); - else + else if (cmd == BPF_MAP_LOOKUP_AND_DELETE_BATCH) BPF_DO_BATCH(map->ops->map_lookup_and_delete_batch); + else if (cmd == BPF_MAP_UPDATE_BATCH) + BPF_DO_BATCH(map->ops->map_update_batch); + else + BPF_DO_BATCH(map->ops->map_delete_batch); err_put: fdput(f); @@ -3315,6 +3424,12 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz err = bpf_map_do_batch(&attr, uattr, BPF_MAP_LOOKUP_AND_DELETE_BATCH); break; + case BPF_MAP_UPDATE_BATCH: + err = bpf_map_do_batch(&attr, uattr, BPF_MAP_UPDATE_BATCH); + break; + case BPF_MAP_DELETE_BATCH: + err = bpf_map_do_batch(&attr, uattr, BPF_MAP_DELETE_BATCH); + break; default: err = -EINVAL; break;