| Message ID | 20220908000254.3079129-3-joannelkoong@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Dynptr convenience helpers | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for bpf-next, async |
| netdev/apply | fail | Patch does not apply to bpf-next |
| bpf/vmtest-bpf-next-PR | fail | merge-conflict |
On Thu, Sep 8, 2022 at 1:10 AM Joanne Koong <joannelkoong@gmail.com> wrote: > > Add two new helper functions: bpf_dynptr_trim and bpf_dynptr_advance. > > bpf_dynptr_trim decreases the size of a dynptr by the specified > number of bytes (offset remains the same). bpf_dynptr_advance advances > the offset of the dynptr by the specified number of bytes (size > decreases correspondingly). > > One example where trimming / advancing the dynptr may useful is for > hashing. If the dynptr points to a larger struct, it is possible to hash > an individual field within the struct through dynptrs by using > bpf_dynptr_advance+trim. > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> Acked-by: Song Liu <song@kernel.org>
On Wed, 7 Sep 2022 17:02:48 -0700 Joanne Koong <joannelkoong@gmail.com> wrote: > Add two new helper functions: bpf_dynptr_trim and bpf_dynptr_advance. > > bpf_dynptr_trim decreases the size of a dynptr by the specified > number of bytes (offset remains the same). bpf_dynptr_advance advances > the offset of the dynptr by the specified number of bytes (size > decreases correspondingly). > > One example where trimming / advancing the dynptr may useful is for > hashing. If the dynptr points to a larger struct, it is possible to hash > an individual field within the struct through dynptrs by using > bpf_dynptr_advance+trim. > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
On Wed, Sep 7, 2022 at 5:10 PM Joanne Koong <joannelkoong@gmail.com> wrote: > > Add two new helper functions: bpf_dynptr_trim and bpf_dynptr_advance. > > bpf_dynptr_trim decreases the size of a dynptr by the specified > number of bytes (offset remains the same). bpf_dynptr_advance advances > the offset of the dynptr by the specified number of bytes (size > decreases correspondingly). > > One example where trimming / advancing the dynptr may useful is for > hashing. If the dynptr points to a larger struct, it is possible to hash > an individual field within the struct through dynptrs by using > bpf_dynptr_advance+trim. > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > --- > include/uapi/linux/bpf.h | 16 +++++++++ > kernel/bpf/helpers.c | 63 ++++++++++++++++++++++++++++++++++ > tools/include/uapi/linux/bpf.h | 16 +++++++++ > 3 files changed, 95 insertions(+) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index cce3356765fc..3b054553be30 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -5453,6 +5453,20 @@ union bpf_attr { > * dynptr is invalid or if the offset and length is out of bounds > * or in a paged buffer for skb-type dynptrs or across fragments > * for xdp-type dynptrs. > + * > + * long bpf_dynptr_advance(struct bpf_dynptr *ptr, u32 len) > + * Description > + * Advance a dynptr by *len*. This might be a bit to succinct a description. "Advance dynptr's internal offset by *len* bytes."? > + * Return > + * 0 on success, -EINVAL if the dynptr is invalid, -ERANGE if *len* > + * exceeds the bounds of the dynptr. > + * > + * long bpf_dynptr_trim(struct bpf_dynptr *ptr, u32 len) > + * Description > + * Trim a dynptr by *len*. Similar as above, something like "Reduce the size of memory pointed to by dynptr by *len* without changing offset"? > + * Return > + * 0 on success, -EINVAL if the dynptr is invalid, -ERANGE if > + * trying to trim more bytes than the size of the dynptr. > */ > #define __BPF_FUNC_MAPPER(FN) \ > FN(unspec), \ > @@ -5667,6 +5681,8 @@ union bpf_attr { > FN(dynptr_from_skb), \ > FN(dynptr_from_xdp), \ > FN(dynptr_data_rdonly), \ > + FN(dynptr_advance), \ > + FN(dynptr_trim), \ > /* */ > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index 30a59c9e5df3..9f356105ab49 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -1423,6 +1423,13 @@ static u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr) > return ptr->size & DYNPTR_SIZE_MASK; > } > > +static void bpf_dynptr_set_size(struct bpf_dynptr_kern *ptr, u32 new_size) > +{ > + u32 metadata = ptr->size & ~DYNPTR_SIZE_MASK; > + > + ptr->size = new_size | metadata; > +} > + > int bpf_dynptr_check_size(u32 size) > { > return size > DYNPTR_MAX_SIZE ? -E2BIG : 0; > @@ -1646,6 +1653,58 @@ static const struct bpf_func_proto bpf_dynptr_data_proto = { > .arg3_type = ARG_CONST_ALLOC_SIZE_OR_ZERO, > }; > > +BPF_CALL_2(bpf_dynptr_advance, struct bpf_dynptr_kern *, ptr, u32, len) > +{ > + u32 size; > + > + if (!ptr->data) > + return -EINVAL; > + > + size = bpf_dynptr_get_size(ptr); > + > + if (len > size) > + return -ERANGE; > + > + ptr->offset += len; > + > + bpf_dynptr_set_size(ptr, size - len); Kind of weird that offset is adjusted directly through field, but size is set through helper. One idea is to have (internal) helper that *increments* offset and *decrements* size as one step. You can move all the error checking inside it, instead of duplicating. E.g., something like this signature static int bpf_dynptr_adjust(struct btf_dynptr_kern *ptr, u32 off_inc, u32 sz_dec); This increment/decrement is a bit surprising, but we don't allow size to grow and offset to decrement, so this makes sense from that point of view. With that, we'll just do return bpf_dynptr_adjust(ptr, len, len); > + > + return 0; > +} > + > +static const struct bpf_func_proto bpf_dynptr_advance_proto = { > + .func = bpf_dynptr_advance, > + .gpl_only = false, > + .ret_type = RET_INTEGER, > + .arg1_type = ARG_PTR_TO_DYNPTR, > + .arg2_type = ARG_ANYTHING, > +}; > + > +BPF_CALL_2(bpf_dynptr_trim, struct bpf_dynptr_kern *, ptr, u32, len) > +{ > + u32 size; > + > + if (!ptr->data) > + return -EINVAL; > + > + size = bpf_dynptr_get_size(ptr); > + > + if (len > size) > + return -ERANGE; > + > + bpf_dynptr_set_size(ptr, size - len); and here we can just do return bpf_dynptr_adjust(ptr, 0, len); > + > + return 0; > +} > + > +static const struct bpf_func_proto bpf_dynptr_trim_proto = { > + .func = bpf_dynptr_trim, > + .gpl_only = false, > + .ret_type = RET_INTEGER, > + .arg1_type = ARG_PTR_TO_DYNPTR, > + .arg2_type = ARG_ANYTHING, > +}; [...]
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index cce3356765fc..3b054553be30 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5453,6 +5453,20 @@ union bpf_attr { * dynptr is invalid or if the offset and length is out of bounds * or in a paged buffer for skb-type dynptrs or across fragments * for xdp-type dynptrs. + * + * long bpf_dynptr_advance(struct bpf_dynptr *ptr, u32 len) + * Description + * Advance a dynptr by *len*. + * Return + * 0 on success, -EINVAL if the dynptr is invalid, -ERANGE if *len* + * exceeds the bounds of the dynptr. + * + * long bpf_dynptr_trim(struct bpf_dynptr *ptr, u32 len) + * Description + * Trim a dynptr by *len*. + * Return + * 0 on success, -EINVAL if the dynptr is invalid, -ERANGE if + * trying to trim more bytes than the size of the dynptr. */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -5667,6 +5681,8 @@ union bpf_attr { FN(dynptr_from_skb), \ FN(dynptr_from_xdp), \ FN(dynptr_data_rdonly), \ + FN(dynptr_advance), \ + FN(dynptr_trim), \ /* */ /* integer value in 'imm' field of BPF_CALL instruction selects which helper diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 30a59c9e5df3..9f356105ab49 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1423,6 +1423,13 @@ static u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr) return ptr->size & DYNPTR_SIZE_MASK; } +static void bpf_dynptr_set_size(struct bpf_dynptr_kern *ptr, u32 new_size) +{ + u32 metadata = ptr->size & ~DYNPTR_SIZE_MASK; + + ptr->size = new_size | metadata; +} + int bpf_dynptr_check_size(u32 size) { return size > DYNPTR_MAX_SIZE ? -E2BIG : 0; @@ -1646,6 +1653,58 @@ static const struct bpf_func_proto bpf_dynptr_data_proto = { .arg3_type = ARG_CONST_ALLOC_SIZE_OR_ZERO, }; +BPF_CALL_2(bpf_dynptr_advance, struct bpf_dynptr_kern *, ptr, u32, len) +{ + u32 size; + + if (!ptr->data) + return -EINVAL; + + size = bpf_dynptr_get_size(ptr); + + if (len > size) + return -ERANGE; + + ptr->offset += len; + + bpf_dynptr_set_size(ptr, size - len); + + return 0; +} + +static const struct bpf_func_proto bpf_dynptr_advance_proto = { + .func = bpf_dynptr_advance, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_DYNPTR, + .arg2_type = ARG_ANYTHING, +}; + +BPF_CALL_2(bpf_dynptr_trim, struct bpf_dynptr_kern *, ptr, u32, len) +{ + u32 size; + + if (!ptr->data) + return -EINVAL; + + size = bpf_dynptr_get_size(ptr); + + if (len > size) + return -ERANGE; + + bpf_dynptr_set_size(ptr, size - len); + + return 0; +} + +static const struct bpf_func_proto bpf_dynptr_trim_proto = { + .func = bpf_dynptr_trim, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_DYNPTR, + .arg2_type = ARG_ANYTHING, +}; + const struct bpf_func_proto bpf_get_current_task_proto __weak; const struct bpf_func_proto bpf_get_current_task_btf_proto __weak; const struct bpf_func_proto bpf_probe_read_user_proto __weak; @@ -1718,6 +1777,10 @@ bpf_base_func_proto(enum bpf_func_id func_id) return &bpf_dynptr_data_proto; case BPF_FUNC_dynptr_data_rdonly: return &bpf_dynptr_data_rdonly_proto; + case BPF_FUNC_dynptr_advance: + return &bpf_dynptr_advance_proto; + case BPF_FUNC_dynptr_trim: + return &bpf_dynptr_trim_proto; default: break; } diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index cce3356765fc..3b054553be30 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -5453,6 +5453,20 @@ union bpf_attr { * dynptr is invalid or if the offset and length is out of bounds * or in a paged buffer for skb-type dynptrs or across fragments * for xdp-type dynptrs. + * + * long bpf_dynptr_advance(struct bpf_dynptr *ptr, u32 len) + * Description + * Advance a dynptr by *len*. + * Return + * 0 on success, -EINVAL if the dynptr is invalid, -ERANGE if *len* + * exceeds the bounds of the dynptr. + * + * long bpf_dynptr_trim(struct bpf_dynptr *ptr, u32 len) + * Description + * Trim a dynptr by *len*. + * Return + * 0 on success, -EINVAL if the dynptr is invalid, -ERANGE if + * trying to trim more bytes than the size of the dynptr. */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -5667,6 +5681,8 @@ union bpf_attr { FN(dynptr_from_skb), \ FN(dynptr_from_xdp), \ FN(dynptr_data_rdonly), \ + FN(dynptr_advance), \ + FN(dynptr_trim), \ /* */ /* integer value in 'imm' field of BPF_CALL instruction selects which helper
Add two new helper functions: bpf_dynptr_trim and bpf_dynptr_advance. bpf_dynptr_trim decreases the size of a dynptr by the specified number of bytes (offset remains the same). bpf_dynptr_advance advances the offset of the dynptr by the specified number of bytes (size decreases correspondingly). One example where trimming / advancing the dynptr may useful is for hashing. If the dynptr points to a larger struct, it is possible to hash an individual field within the struct through dynptrs by using bpf_dynptr_advance+trim. Signed-off-by: Joanne Koong <joannelkoong@gmail.com> --- include/uapi/linux/bpf.h | 16 +++++++++ kernel/bpf/helpers.c | 63 ++++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 16 +++++++++ 3 files changed, 95 insertions(+)