On 3/17/23 6:11 PM, Kui-Feng Lee wrote:
--- a/net/bpf/bpf_dummy_struct_ops.c
+++ b/net/bpf/bpf_dummy_struct_ops.c
@@ -222,12 +222,18 @@ static void bpf_dummy_unreg(void *kdata)
{
}
+static int bpf_dummy_update(void *kdata, void *old_kdata)
+{
+ return -EOPNOTSUPP;
+}
+
struct bpf_struct_ops bpf_bpf_dummy_ops = {
.verifier_ops = &bpf_dummy_verifier_ops,
.init = bpf_dummy_init,
.check_member = bpf_dummy_ops_check_member,
.init_member = bpf_dummy_init_member,
.reg = bpf_dummy_reg,
+ .update = bpf_dummy_update,
When looking at this together in patch 5, the changes in
bpf_dummy_struct_ops.c should not be needed.
I don't follow you.
If we don't assign a function to .update, it will fail in
bpf_struct_ops_map_link_update(). Of course, I can add a check
in bpf_struct_ops_map_link_update() to return an error if .update
is NULL.
Yes, test ->update in bpf_struct_ops.c is better but not in
bpf_struct_ops_map_link_update (more on this later). It does not need the dummy
struct_ops to test the link. The dummy struct_ops was created to catch the
trampoline img issue with ops/func having different args and return value.
It is better to enforce the BPF_F_LINK struct_ops must support both ->validate
and ->update at the beginning and it can be revisited later. The current
'->validate' testing in bpf_struct_ops_map_update_elem() in patch 3 is too late.
Being able to '->validate' is particularly important for BPF_F_LINK struct_ops.
Testing '->validate' and '->update' in bpf_struct_ops_init() will be too strict
for now though when considering other on-going efforts to support struct_ops in
other subsystems. A better place to test both '->validate' and '->update' should
be in bpf_struct_ops_map_alloc(). It should return -ENOTSUPP when trying to
creating a struct_ops BPF_F_LINK map without st_ops->validate and st_ops->update.
