Re: [PATCH v2 bpf-next 5/9] libbpf: use stable map placeholder FDs

[Eduard Zingerman <eddyz87@gmail.com>]

Tbh, it looks like calls to zclose(map->fd) were unnecessary
regardless of this patch, as all maps are closed at the end of
bpf_object_load() in case of an error.

[...]

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index f29cfb344f80..e0085aef17d7 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[...]

> @@ -5275,13 +5289,11 @@ static int bpf_object_create_map(struct bpf_object *obj, struct bpf_map *map, bo
>  		create_attr.btf_value_type_id = 0;
>  		map->btf_key_type_id = 0;
>  		map->btf_value_type_id = 0;
> -		map->fd = bpf_map_create(def->type, map_name,
> -					 def->key_size, def->value_size,
> -					 def->max_entries, &create_attr);
> +		map_fd = bpf_map_create(def->type, map_name,
> +					def->key_size, def->value_size,
> +					def->max_entries, &create_attr);
>  	}
>  
> -	err = map->fd < 0 ? -errno : 0;
> -
>  	if (bpf_map_type_is_map_in_map(def->type) && map->inner_map) {
>  		if (obj->gen_loader)
>  			map->inner_map->fd = -1;
> @@ -5289,7 +5301,19 @@ static int bpf_object_create_map(struct bpf_object *obj, struct bpf_map *map, bo
>  		zfree(&map->inner_map);
>  	}
>  
> -	return err;
> +	if (map_fd < 0)
> +		return -errno;

Nit: this check is now placed after call to bpf_map_destroy(),
     which might call munmap(), which might overwrite "errno",
     set by some of the previous calls to bpf_map_create().

[...]

> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index b5d334754e5d..662a3df1e29f 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -555,6 +555,30 @@ static inline int ensure_good_fd(int fd)
>  	return fd;
>  }
>  
> +static inline int create_placeholder_fd(void)
> +{
> +	int fd;
> +
> +	fd = ensure_good_fd(open("/dev/null", O_WRONLY | O_CLOEXEC));

Stupid question: is it ok to assume that /dev is always mounted?
Googling says that kernel chooses if to mount it automatically
depending on the value of CONFIG_DEVTMPFS_MOUNT option.
Another option might be memfd_create().

> +	if (fd < 0)
> +		return -errno;
> +	return fd;
> +}

[...]