On Mon, Apr 29, 2024 at 2:36 PM Kui-Feng Lee <thinker.li@xxxxxxxxx> wrote:
>
> To facilitate the upcoming unregistring struct_ops objects from the systems
> consuming objects, a pointer of the attached link is added to allow for
> accessing the attached link of a bpf_struct_ops_map directly from the map
> itself.
>
> Previously, a st_map could be attached to multiple links. This patch now
> enforces only one link attached at most.
I'd like to avoid this restriction, in principle. We don't enforce
that BPF program should be attached through a single BPF link, so I
don't think we should allow that for maps. Worst case you can keep a
list of attached links.
>
> Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
> ---
> kernel/bpf/bpf_struct_ops.c | 47 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 86c7884abaf8..072e3416c987 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -20,6 +20,8 @@ struct bpf_struct_ops_value {
>
> #define MAX_TRAMP_IMAGE_PAGES 8
>
> +struct bpf_struct_ops_link;
> +
> struct bpf_struct_ops_map {
> struct bpf_map map;
> struct rcu_head rcu;
> @@ -39,6 +41,8 @@ struct bpf_struct_ops_map {
> void *image_pages[MAX_TRAMP_IMAGE_PAGES];
> /* The owner moduler's btf. */
> struct btf *btf;
> + /* The link is attached by this map. */
> + struct bpf_struct_ops_link __rcu *attached;
> /* uvalue->data stores the kernel struct
> * (e.g. tcp_congestion_ops) that is more useful
> * to userspace than the kvalue. For example,
> @@ -1048,6 +1052,22 @@ static bool bpf_struct_ops_valid_to_reg(struct bpf_map *map)
> smp_load_acquire(&st_map->kvalue.common.state) == BPF_STRUCT_OPS_STATE_READY;
> }
>
> +/* Set the attached link of a map.
> + *
> + * Return the current value of the st_map->attached.
> + */
> +static inline struct bpf_struct_ops_link *map_attached(struct bpf_struct_ops_map *st_map,
> + struct bpf_struct_ops_link *st_link)
> +{
> + return unrcu_pointer(cmpxchg(&st_map->attached, NULL, st_link));
> +}
> +
> +/* Reset the attached link of a map */
> +static inline void map_attached_null(struct bpf_struct_ops_map *st_map)
> +{
> + rcu_assign_pointer(st_map->attached, NULL);
> +}
> +
> static void bpf_struct_ops_map_link_dealloc(struct bpf_link *link)
> {
> struct bpf_struct_ops_link *st_link;
> @@ -1061,6 +1081,7 @@ static void bpf_struct_ops_map_link_dealloc(struct bpf_link *link)
> * bpf_struct_ops_link_create() fails to register.
> */
> st_map->st_ops_desc->st_ops->unreg(&st_map->kvalue.data);
> + map_attached_null(st_map);
> bpf_map_put(&st_map->map);
> }
> kfree(st_link);
> @@ -1125,9 +1146,21 @@ static int bpf_struct_ops_map_link_update(struct bpf_link *link, struct bpf_map
> goto err_out;
> }
>
> + if (likely(st_map != old_st_map) && map_attached(st_map, st_link)) {
> + /* The map is already in use */
> + err = -EBUSY;
> + goto err_out;
> + }
> +
> err = st_map->st_ops_desc->st_ops->update(st_map->kvalue.data, old_st_map->kvalue.data);
> - if (err)
> + if (err) {
> + if (st_map != old_st_map)
> + map_attached_null(st_map);
> goto err_out;
> + }
> +
> + if (likely(st_map != old_st_map))
> + map_attached_null(old_st_map);
>
> bpf_map_inc(new_map);
> rcu_assign_pointer(st_link->map, new_map);
> @@ -1172,20 +1205,28 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
> }
> bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS, &bpf_struct_ops_map_lops, NULL);
>
> + if (map_attached(st_map, link)) {
> + err = -EBUSY;
> + goto err_out;
> + }
> +
> err = bpf_link_prime(&link->link, &link_primer);
> if (err)
> - goto err_out;
> + goto err_out_attached;
>
> err = st_map->st_ops_desc->st_ops->reg(st_map->kvalue.data);
> if (err) {
> bpf_link_cleanup(&link_primer);
> + /* The link has been free by bpf_link_cleanup() */
> link = NULL;
> - goto err_out;
> + goto err_out_attached;
> }
> RCU_INIT_POINTER(link->map, map);
>
> return bpf_link_settle(&link_primer);
>
> +err_out_attached:
> + map_attached_null(st_map);
> err_out:
> bpf_map_put(map);
> kfree(link);
> --
> 2.34.1
>
