On 2/16/23 11:17 AM, Kui-Feng Lee wrote:
+static int bpf_struct_ops_map_link_update(struct bpf_link *link, struct
bpf_map *new_map)
+{
+ struct bpf_struct_ops_value *kvalue;
+ struct bpf_struct_ops_map *st_map, *old_st_map;
+ struct bpf_map *old_map;
+ int err;
+
+ if (new_map->map_type != BPF_MAP_TYPE_STRUCT_OPS || !(new_map->map_flags
& BPF_F_LINK))
+ return -EINVAL;
+
+ old_map = link->map;
+
+ /* It does nothing if the new map is the same as the old one.
+ * A struct_ops that backs a bpf_link can not be updated or
+ * its kvalue would be updated and causes inconsistencies.
+ */
+ if (old_map == new_map)
+ return 0;
+
+ /* The new and old struct_ops must be the same type. */
+ st_map = (struct bpf_struct_ops_map *)new_map;
+ old_st_map = (struct bpf_struct_ops_map *)old_map;
+ if (st_map->st_ops != old_st_map->st_ops)
+ return -EINVAL;
+
+ /* Assure the struct_ops is updated (has value) and not
+ * backing any other link.
+ */
+ kvalue = &st_map->kvalue;
+ if (kvalue->state != BPF_STRUCT_OPS_STATE_INUSE ||
+ refcount_read(&kvalue->refcnt) != 0)
+ return -EINVAL;
+
+ bpf_map_inc(new_map);
+ refcount_set(&kvalue->refcnt, 1);
+
+ set_memory_rox((long)st_map->image, 1);
+ err = st_map->st_ops->update(kvalue->data, old_st_map->kvalue.data);
+ if (err) {
+ refcount_set(&kvalue->refcnt, 0);
+
+ set_memory_nx((long)st_map->image, 1);
+ set_memory_rw((long)st_map->image, 1);
+ bpf_map_put(new_map);
+ return err;
+ }
+
+ link->map = new_map;
Similar here, does this link_update operation needs a lock?
The update function of tcp_ca checks if the name is unique with the protection
of a lock. bpf_struct_ops_map_update_elem() also check and update state of the
kvalue to prevent changing kvalue. Only one of thread will success to register
or update at any moment.
hmm... meaning the lock inside the "->update()" function? There are many
variables outside of update() that this function
(bpf_struct_ops_map_link_update) is setting and testing without a lock. eg. the
succeeded thread will set refcnt to 1 while the failed thread will set it back
to 0...
+
+static int link_update_struct_ops(struct bpf_link *link, union bpf_attr *attr)
+{
+ struct bpf_map *new_map;
+ int ret = 0;
+
+ new_map = bpf_map_get(attr->link_update.new_map_fd);
+ if (IS_ERR(new_map))
+ return -EINVAL;
+
+ if (new_map->map_type != BPF_MAP_TYPE_STRUCT_OPS) {
+ ret = -EINVAL;
+ goto out_put_map;
+ }
How about BPF_F_REPLACE?
Do you mean the new_map should be labeled with BPF_F_REPLACE to replace the old
one?
was asking if BPF_F_REPLACE is supported.
