Re: [PATCH bpf-next v5] bpftool: Mount bpffs on provided dir instead of parent dir

[Quentin Monnet <qmo@kernel.org>]

On 04/04/2024 20:22, Sahil Siddiq wrote:
> When pinning programs/objects under PATH (eg: during "bpftool prog
> loadall") the bpffs is mounted on the parent dir of PATH in the
> following situations:
> - the given dir exists but it is not bpffs.
> - the given dir doesn't exist and the parent dir is not bpffs.
> 
> Mounting on the parent dir can also have the unintentional side-
> effect of hiding other files located under the parent dir.
> 
> If the given dir exists but is not bpffs, then the bpffs should
> be mounted on the given dir and not its parent dir.
> 
> Similarly, if the given dir doesn't exist and its parent dir is not
> bpffs, then the given dir should be created and the bpffs should be
> mounted on this new dir.
> 
> Link: https://lore.kernel.org/bpf/2da44d24-74ae-a564-1764-afccf395eeec@isovalent.com/T/#t
> Closes: https://github.com/libbpf/bpftool/issues/100
> Fixes: 2a36c26fe3b8 ("bpftool: Support bpffs mountpoint as pin path for prog loadall")
> 
> Changes since v1:
>  - Split "mount_bpffs_for_pin" into two functions.
>    This is done to improve maintainability and readability.
> 
> Changes since v2:
> - mount_bpffs_for_pin: rename to "create_and_mount_bpffs_dir".
> - mount_bpffs_given_file: rename to "mount_bpffs_given_file".
> - create_and_mount_bpffs_dir:
>   - introduce "dir_exists" boolean.
>   - remove new dir if "mnt_fs" fails.
> - improve error handling and error messages.
> 
> Changes since v3:
> - Rectify function name.
> - Improve error messages and formatting.
> - mount_bpffs_for_file:
>   - Check if dir exists before block_mount check.
> 
> Changes since v4:
> - Use strdup instead of strcpy.
> - create_and_mount_bpffs_dir:
>   - Use S_IRWXU instead of 0700.
> - Improve error handling and formatting.
> 
> Signed-off-by: Sahil Siddiq <icegambit91@gmail.com>

Tested-by: Quentin Monnet <qmo@kernel.org>
Reviewed-by: Quentin Monnet <qmo@kernel.org>

Thank you!