Re: [PATCH HID v3 03/16] HID: bpf: implement HID-BPF through bpf_struct_ops
From: Benjamin Tissoires <bentiss@kernel.org>
Date: 2024-06-14 09:25:30
Also in:
bpf, linux-doc, linux-kselftest, lkml
Subsystem:
hid core layer, the rest · Maintainers:
Jiri Kosina, Benjamin Tissoires, Linus Torvalds
On Jun 10 2024, Alexei Starovoitov wrote:
On Sat, Jun 8, 2024 at 2:01 AM Benjamin Tissoires [off-list ref] wrote:quoted
+ +static int hid_bpf_ops_init_member(const struct btf_type *t, + const struct btf_member *member, + void *kdata, const void *udata) +{ + u32 moff = __btf_member_bit_offset(t, member) / 8; + u32 flags; + + switch (moff) { + case offsetof(struct hid_bpf_ops, hid_id): + /* For hid_id and flags fields, this function has to copy it + * and return 1 to indicate that the data has been handled by + * the struct_ops type, or the verifier will reject the map if + * the value of those fields is not zero. + */ + ((struct hid_bpf_ops *)kdata)->hid_id = ((struct hid_bpf_ops *)udata)->hid_id; + return 1; + case offsetof(struct hid_bpf_ops, flags): + flags = ((struct hid_bpf_ops *)udata)->flags; + if (flags & ~BPF_F_BEFORE) + return -EINVAL; + ((struct hid_bpf_ops *)kdata)->flags = flags;minor nit: I'd cast kdata/udate in the beginning of the function to make the lines shorter and less verbose. Similar to how bpf_tcp_ca_init_member() does it.
The change looks like: ---
diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c
index 9192c66cde20..dde547e734ed 100644
--- a/drivers/hid/bpf/hid_bpf_struct_ops.c
+++ b/drivers/hid/bpf/hid_bpf_struct_ops.c@@ -97,8 +97,14 @@ static int hid_bpf_ops_init_member(const struct btf_type *t, const struct btf_member *member, void *kdata, const void *udata) { - u32 moff = __btf_member_bit_offset(t, member) / 8; - u32 flags; + const struct hid_bpf_ops *uhid_bpf_ops; + struct hid_bpf_ops *khid_bpf_ops; + u32 moff; + + uhid_bpf_ops = (const struct hid_bpf_ops *)udata; + khid_bpf_ops = (struct hid_bpf_ops *)kdata; + + moff = __btf_member_bit_offset(t, member) / 8; switch (moff) { case offsetof(struct hid_bpf_ops, hid_id):
@@ -107,13 +113,12 @@ static int hid_bpf_ops_init_member(const struct btf_type *t, * the struct_ops type, or the verifier will reject the map if * the value of those fields is not zero. */ - ((struct hid_bpf_ops *)kdata)->hid_id = ((struct hid_bpf_ops *)udata)->hid_id; + khid_bpf_ops->hid_id = uhid_bpf_ops->hid_id; return 1; case offsetof(struct hid_bpf_ops, flags): - flags = ((struct hid_bpf_ops *)udata)->flags; - if (flags & ~BPF_F_BEFORE) + if (uhid_bpf_ops->flags & ~BPF_F_BEFORE) return -EINVAL; - ((struct hid_bpf_ops *)kdata)->flags = flags; + khid_bpf_ops->flags = uhid_bpf_ops->flags; return 1; } return 0; ---
Acked-by: Alexei Starovoitov <ast@kernel.org>
And I'm going to apply the series now Cheers, Benjamin