[PATCH bpf-next 0/2] BPF FS mount options parsing follow ups

[PATCH bpf-next 0/2] BPF FS mount options parsing follow ups

[Andrii Nakryiko]

Original BPF token patch set ([0]) added delegate_xxx mount options which
supported only special "any" value and hexadecimal bitmask. This patch set
attempts to make specifying and inspecting these mount options more
human-friendly by supporting string constants matching corresponding bpf_cmd,
bpf_map_type, bpf_prog_type, and bpf_attach_type enumerators.

This implementation relies on BTF information to find all supported symbolic
names. If kernel wasn't built with BTF, BPF FS will still support "any" and
hex-based mask.

  [0] https://patchwork.kernel.org/project/netdevbpf/list/?series=805707&state=*

Andrii Nakryiko (2):
  bpf: support symbolic BPF FS delegation mount options
  selftests/bpf: utilize string values for delegate_xxx mount options

 kernel/bpf/inode.c                            | 231 +++++++++++++++---
 .../testing/selftests/bpf/prog_tests/token.c  |  52 ++--
 2 files changed, 225 insertions(+), 58 deletions(-)

-- 
2.34.1

[PATCH bpf-next 1/2] bpf: support symbolic BPF FS delegation mount options

[Andrii Nakryiko]

Besides already supported special "any" value and hex bit mask, support
string-based parsing of delegation masks based on exact enumerator
names. Utilize BTF information of `enum bpf_cmd`, `enum bpf_map_type`,
`enum bpf_prog_type`, and `enum bpf_attach_type` types to find supported
symbolic names (ignoring __MAX_xxx guard values). So "BPF_PROG_LOAD" and
"BPF_MAP_CREATE" are valid values to specify for delegate_cmds options,
"BPF_MAP_TYPE_ARRAY" is among supported for map types, etc.

Besides supporting string values, we also support multiple values
specified at the same time, using colon (':') separator.

There are corresponding changes on bpf_show_options side to use known
values to print them in human-readable format, falling back to hex mask
printing, if there are any unrecognized bits. This shouldn't be
necessary when enum BTF information is present, but in general we should
always be able to fall back to this even if kernel was built without BTF.

Example below shows various ways to specify delegate_cmds options
through mount command and how mount options are printed back:

  $ sudo mkdir -p /sys/fs/bpf/token
  $ sudo mount -t bpf bpffs /sys/fs/bpf/token \
               -o delegate_cmds=BPF_PROG_LOAD \
               -o delegate_cmds=BPF_MAP_CREATE \
               -o delegate_cmds=BPF_TOKEN_CREATE:BPF_BTF_LOAD:BPF_LINK_CREATE
  $ mount | grep token
  bpffs on /sys/fs/bpf/token type bpf (rw,relatime,delegate_cmds=BPF_MAP_CREATE:BPF_PROG_LOAD:BPF_BTF_LOAD:BPF_LINK_CREATE:BPF_TOKEN_CREATE)

Same approach works across delegate_maps, delegate_progs, and
delegate_attachs masks as well.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/inode.c | 231 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 193 insertions(+), 38 deletions(-)

diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 5359a0929c35..e90d4be5c759 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -595,6 +595,127 @@ struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type typ
 }
 EXPORT_SYMBOL(bpf_prog_get_type_path);
 
+struct bpffs_btf_enums {
+	const struct btf *btf;
+	const struct btf_type *cmd_t;
+	const struct btf_type *map_t;
+	const struct btf_type *prog_t;
+	const struct btf_type *attach_t;
+};
+
+static int find_bpffs_btf_enums(struct bpffs_btf_enums *info)
+{
+	const struct btf *btf;
+	const struct btf_type *t;
+	const char *name;
+	int i, n;
+
+	memset(info, 0, sizeof(*info));
+
+	btf = bpf_get_btf_vmlinux();
+	if (IS_ERR(btf))
+		return PTR_ERR(btf);
+	if (!btf)
+		return -ENOENT;
+
+	info->btf = btf;
+
+	for (i = 1, n = btf_nr_types(btf); i < n; i++) {
+		t = btf_type_by_id(btf, i);
+		if (!btf_type_is_enum(t))
+			continue;
+
+		name = btf_name_by_offset(btf, t->name_off);
+		if (!name)
+			continue;
+
+		if (strcmp(name, "bpf_cmd") == 0)
+			info->cmd_t = t;
+		else if (strcmp(name, "bpf_map_type") == 0)
+			info->map_t = t;
+		else if (strcmp(name, "bpf_prog_type") == 0)
+			info->prog_t = t;
+		else if (strcmp(name, "bpf_attach_type") == 0)
+			info->attach_t = t;
+		else
+			continue;
+
+		if (info->cmd_t && info->map_t && info->prog_t && info->attach_t)
+			return 0;
+	}
+
+	return -ESRCH;
+}
+
+static bool find_btf_enum_const(const struct btf *btf, const struct btf_type *enum_t,
+			        const char *str, int *value)
+{
+	const struct btf_enum *e;
+	const char *name;
+	int i, n;
+
+	*value = 0;
+
+	if (!btf || !enum_t)
+		return false;
+
+	for (i = 0, n = btf_vlen(enum_t); i < n; i++) {
+		e = &btf_enum(enum_t)[i];
+
+		name = btf_name_by_offset(btf, e->name_off);
+		if (!name || (name[0] == '_' && name[1] == '_'))
+			continue;
+
+		if (strcmp(name, str) == 0) {
+			*value = e->val;
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static void seq_print_delegate_opts(struct seq_file *m,
+				    const char *opt_name,
+				    const struct btf *btf,
+				    const struct btf_type *enum_t,
+				    u64 delegate_msk, u64 any_msk)
+{
+	const struct btf_enum *e;
+	bool first = true;
+	const char *name;
+	u64 msk;
+	int i, n;
+
+	delegate_msk &= any_msk; /* clear unknown bits */
+
+	if (delegate_msk == 0)
+		return;
+
+	seq_printf(m, ",%s", opt_name);
+	if (delegate_msk == any_msk) {
+		seq_printf(m, "=any");
+		return;
+	}
+
+	if (btf && enum_t) {
+		for (i = 0, n = btf_vlen(enum_t); i < n; i++) {
+			e = &btf_enum(enum_t)[i];
+			name = btf_name_by_offset(btf, e->name_off);
+			if (!name || (name[0] == '_' && name[1] == '_'))
+				continue;
+			msk = 1ULL << e->val;
+			if (delegate_msk & msk) {
+				seq_printf(m, "%c%s", first ? '=' : ':', name);
+				delegate_msk &= ~msk;
+				first = false;
+			}
+		}
+	}
+	if (delegate_msk)
+		seq_printf(m, "%c0x%llx", first ? '=' : ':', delegate_msk);
+}
+
 /*
  * Display the mount options in /proc/mounts.
  */
@@ -607,29 +728,30 @@ static int bpf_show_options(struct seq_file *m, struct dentry *root)
 	if (mode != S_IRWXUGO)
 		seq_printf(m, ",mode=%o", mode);
 
-	mask = (1ULL << __MAX_BPF_CMD) - 1;
-	if ((opts->delegate_cmds & mask) == mask)
-		seq_printf(m, ",delegate_cmds=any");
-	else if (opts->delegate_cmds)
-		seq_printf(m, ",delegate_cmds=0x%llx", opts->delegate_cmds);
-
-	mask = (1ULL << __MAX_BPF_MAP_TYPE) - 1;
-	if ((opts->delegate_maps & mask) == mask)
-		seq_printf(m, ",delegate_maps=any");
-	else if (opts->delegate_maps)
-		seq_printf(m, ",delegate_maps=0x%llx", opts->delegate_maps);
-
-	mask = (1ULL << __MAX_BPF_PROG_TYPE) - 1;
-	if ((opts->delegate_progs & mask) == mask)
-		seq_printf(m, ",delegate_progs=any");
-	else if (opts->delegate_progs)
-		seq_printf(m, ",delegate_progs=0x%llx", opts->delegate_progs);
-
-	mask = (1ULL << __MAX_BPF_ATTACH_TYPE) - 1;
-	if ((opts->delegate_attachs & mask) == mask)
-		seq_printf(m, ",delegate_attachs=any");
-	else if (opts->delegate_attachs)
-		seq_printf(m, ",delegate_attachs=0x%llx", opts->delegate_attachs);
+	if (opts->delegate_cmds || opts->delegate_maps ||
+	    opts->delegate_progs || opts->delegate_attachs) {
+		struct bpffs_btf_enums info;
+
+		/* ignore errors, fallback to hex */
+		(void)find_bpffs_btf_enums(&info);
+
+		mask = (1ULL << __MAX_BPF_CMD) - 1;
+		seq_print_delegate_opts(m, "delegate_cmds", info.btf, info.cmd_t,
+					opts->delegate_cmds, mask);
+
+		mask = (1ULL << __MAX_BPF_MAP_TYPE) - 1;
+		seq_print_delegate_opts(m, "delegate_maps", info.btf, info.map_t,
+					opts->delegate_maps, mask);
+
+		mask = (1ULL << __MAX_BPF_PROG_TYPE) - 1;
+		seq_print_delegate_opts(m, "delegate_progs", info.btf, info.prog_t,
+					opts->delegate_progs, mask);
+
+		mask = (1ULL << __MAX_BPF_ATTACH_TYPE) - 1;
+		seq_print_delegate_opts(m, "delegate_attachs", info.btf, info.attach_t,
+					opts->delegate_attachs, mask);
+	}
+
 	return 0;
 }
 
@@ -673,7 +795,6 @@ static int bpf_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	struct bpf_mount_opts *opts = fc->s_fs_info;
 	struct fs_parse_result result;
 	int opt, err;
-	u64 msk;
 
 	opt = fs_parse(fc, bpf_fs_parameters, param, &result);
 	if (opt < 0) {
@@ -700,24 +821,58 @@ static int bpf_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	case OPT_DELEGATE_CMDS:
 	case OPT_DELEGATE_MAPS:
 	case OPT_DELEGATE_PROGS:
-	case OPT_DELEGATE_ATTACHS:
-		if (strcmp(param->string, "any") == 0) {
-			msk = ~0ULL;
-		} else {
-			err = kstrtou64(param->string, 0, &msk);
-			if (err)
-				return err;
+	case OPT_DELEGATE_ATTACHS: {
+		struct bpffs_btf_enums info;
+		const struct btf_type *enum_t;
+		u64 *delegate_msk, msk = 0;
+		char *p;
+		int val;
+
+		/* ignore errors, fallback to hex */
+		(void)find_bpffs_btf_enums(&info);
+
+		switch (opt) {
+		case OPT_DELEGATE_CMDS:
+			delegate_msk = &opts->delegate_cmds;
+			enum_t = info.cmd_t;
+			break;
+		case OPT_DELEGATE_MAPS:
+			delegate_msk = &opts->delegate_maps;
+			enum_t = info.map_t;
+			break;
+		case OPT_DELEGATE_PROGS:
+			delegate_msk = &opts->delegate_progs;
+			enum_t = info.prog_t;
+			break;
+		case OPT_DELEGATE_ATTACHS:
+			delegate_msk = &opts->delegate_attachs;
+			enum_t = info.attach_t;
+			break;
+		default:
+			return -EINVAL;
 		}
+
+		while ((p = strsep(&param->string, ":"))) {
+			if (strcmp(p, "any") == 0) {
+				msk |= ~0ULL;
+			} else if (find_btf_enum_const(info.btf, enum_t, p, &val)) {
+				msk |= 1ULL << val;
+			} else {
+				err = kstrtou64(p, 0, &msk);
+				if (err)
+					return err;
+			}
+		}
+
 		/* Setting delegation mount options requires privileges */
 		if (msk && !capable(CAP_SYS_ADMIN))
 			return -EPERM;
-		switch (opt) {
-		case OPT_DELEGATE_CMDS: opts->delegate_cmds |= msk; break;
-		case OPT_DELEGATE_MAPS: opts->delegate_maps |= msk; break;
-		case OPT_DELEGATE_PROGS: opts->delegate_progs |= msk; break;
-		case OPT_DELEGATE_ATTACHS: opts->delegate_attachs |= msk; break;
-		default: return -EINVAL;
-		}
+
+		*delegate_msk |= msk;
+		break;
+	}
+	default:
+		/* ignore unknown mount options */
 		break;
 	}
 
-- 
2.34.1

RE: [PATCH bpf-next 1/2] bpf: support symbolic BPF FS delegation mount options

[John Fastabend]

Andrii Nakryiko wrote:
> Besides already supported special "any" value and hex bit mask, support
> string-based parsing of delegation masks based on exact enumerator
> names. Utilize BTF information of `enum bpf_cmd`, `enum bpf_map_type`,
> `enum bpf_prog_type`, and `enum bpf_attach_type` types to find supported
> symbolic names (ignoring __MAX_xxx guard values). So "BPF_PROG_LOAD" and
> "BPF_MAP_CREATE" are valid values to specify for delegate_cmds options,
> "BPF_MAP_TYPE_ARRAY" is among supported for map types, etc.
> 
> Besides supporting string values, we also support multiple values
> specified at the same time, using colon (':') separator.
> 
> There are corresponding changes on bpf_show_options side to use known
> values to print them in human-readable format, falling back to hex mask
> printing, if there are any unrecognized bits. This shouldn't be
> necessary when enum BTF information is present, but in general we should
> always be able to fall back to this even if kernel was built without BTF.
> 
> Example below shows various ways to specify delegate_cmds options
> through mount command and how mount options are printed back:
> 
>   $ sudo mkdir -p /sys/fs/bpf/token
>   $ sudo mount -t bpf bpffs /sys/fs/bpf/token \
>                -o delegate_cmds=BPF_PROG_LOAD \
>                -o delegate_cmds=BPF_MAP_CREATE \
>                -o delegate_cmds=BPF_TOKEN_CREATE:BPF_BTF_LOAD:BPF_LINK_CREATE
>   $ mount | grep token
>   bpffs on /sys/fs/bpf/token type bpf (rw,relatime,delegate_cmds=BPF_MAP_CREATE:BPF_PROG_LOAD:BPF_BTF_LOAD:BPF_LINK_CREATE:BPF_TOKEN_CREATE)
> 
> Same approach works across delegate_maps, delegate_progs, and
> delegate_attachs masks as well.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

Acked-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH bpf-next 1/2] bpf: support symbolic BPF FS delegation mount options

[Daniel Borkmann]

On 12/13/23 11:23 PM, Andrii Nakryiko wrote:
> Besides already supported special "any" value and hex bit mask, support
> string-based parsing of delegation masks based on exact enumerator
> names. Utilize BTF information of `enum bpf_cmd`, `enum bpf_map_type`,
> `enum bpf_prog_type`, and `enum bpf_attach_type` types to find supported
> symbolic names (ignoring __MAX_xxx guard values). So "BPF_PROG_LOAD" and
> "BPF_MAP_CREATE" are valid values to specify for delegate_cmds options,
> "BPF_MAP_TYPE_ARRAY" is among supported for map types, etc.
> 
> Besides supporting string values, we also support multiple values
> specified at the same time, using colon (':') separator.
> 
> There are corresponding changes on bpf_show_options side to use known
> values to print them in human-readable format, falling back to hex mask
> printing, if there are any unrecognized bits. This shouldn't be
> necessary when enum BTF information is present, but in general we should
> always be able to fall back to this even if kernel was built without BTF.
> 
> Example below shows various ways to specify delegate_cmds options
> through mount command and how mount options are printed back:
> 
>    $ sudo mkdir -p /sys/fs/bpf/token
>    $ sudo mount -t bpf bpffs /sys/fs/bpf/token \
>                 -o delegate_cmds=BPF_PROG_LOAD \
>                 -o delegate_cmds=BPF_MAP_CREATE \
>                 -o delegate_cmds=BPF_TOKEN_CREATE:BPF_BTF_LOAD:BPF_LINK_CREATE
>    $ mount | grep token
>    bpffs on /sys/fs/bpf/token type bpf (rw,relatime,delegate_cmds=BPF_MAP_CREATE:BPF_PROG_LOAD:BPF_BTF_LOAD:BPF_LINK_CREATE:BPF_TOKEN_CREATE)
> 
> Same approach works across delegate_maps, delegate_progs, and
> delegate_attachs masks as well.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

LGTM, this needs a small rebase though given:

commit 750e785796bb72423b97cac21ecd0fa3b3b65610
Author: Jie Jiang <jiejiang@chromium.org>
Date:   Tue Dec 12 09:39:23 2023 +0000

     bpf: Support uid and gid when mounting bpffs

Thanks,
Daniel

Re: [PATCH bpf-next 1/2] bpf: support symbolic BPF FS delegation mount options

[Alexei Starovoitov]

On Wed, Dec 13, 2023 at 02:23:26PM -0800, Andrii Nakryiko wrote:
> Besides already supported special "any" value and hex bit mask, support
> string-based parsing of delegation masks based on exact enumerator
> names. Utilize BTF information of `enum bpf_cmd`, `enum bpf_map_type`,
> `enum bpf_prog_type`, and `enum bpf_attach_type` types to find supported
> symbolic names (ignoring __MAX_xxx guard values). So "BPF_PROG_LOAD" and
> "BPF_MAP_CREATE" are valid values to specify for delegate_cmds options,
> "BPF_MAP_TYPE_ARRAY" is among supported for map types, etc.
> 
> Besides supporting string values, we also support multiple values
> specified at the same time, using colon (':') separator.
> 
> There are corresponding changes on bpf_show_options side to use known
> values to print them in human-readable format, falling back to hex mask
> printing, if there are any unrecognized bits. This shouldn't be
> necessary when enum BTF information is present, but in general we should
> always be able to fall back to this even if kernel was built without BTF.
> 
> Example below shows various ways to specify delegate_cmds options
> through mount command and how mount options are printed back:
> 
>   $ sudo mkdir -p /sys/fs/bpf/token
>   $ sudo mount -t bpf bpffs /sys/fs/bpf/token \
>                -o delegate_cmds=BPF_PROG_LOAD \
>                -o delegate_cmds=BPF_MAP_CREATE \
>                -o delegate_cmds=BPF_TOKEN_CREATE:BPF_BTF_LOAD:BPF_LINK_CREATE
>   $ mount | grep token
>   bpffs on /sys/fs/bpf/token type bpf (rw,relatime,delegate_cmds=BPF_MAP_CREATE:BPF_PROG_LOAD:BPF_BTF_LOAD:BPF_LINK_CREATE:BPF_TOKEN_CREATE)

imo this is too verbose.
For cmds it doesn't look as bad. "BPF_" prefix is repetitive, but not overly so.
But for maps and progs it will be bad.
It will look like:
delegate_progs=BPF_PROG_TYPE_SOCKET_FILTER:BPF_PROG_TYPE_SOCKET_XDP:BPF_PROG_TYPE_SCHED_CLS
which is not readable.
delegate_progs=SOCKET_FILTER:XDP:SCHED_CLS
is much better.

And I would go further (like libbpf does) and lower case them for output
while allow both upper and lower for input:
delegate_progs=socket_filter:xdp:sched_cls.

Because of stripping the prefix for maps and progs I would strip the prefix for cmds too
for consistency.

pw-bot: cr

Re: [PATCH bpf-next 1/2] bpf: support symbolic BPF FS delegation mount options

[Andrii Nakryiko]

On Thu, Dec 14, 2023 at 7:58 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Dec 13, 2023 at 02:23:26PM -0800, Andrii Nakryiko wrote:
> > Besides already supported special "any" value and hex bit mask, support
> > string-based parsing of delegation masks based on exact enumerator
> > names. Utilize BTF information of `enum bpf_cmd`, `enum bpf_map_type`,
> > `enum bpf_prog_type`, and `enum bpf_attach_type` types to find supported
> > symbolic names (ignoring __MAX_xxx guard values). So "BPF_PROG_LOAD" and
> > "BPF_MAP_CREATE" are valid values to specify for delegate_cmds options,
> > "BPF_MAP_TYPE_ARRAY" is among supported for map types, etc.
> >
> > Besides supporting string values, we also support multiple values
> > specified at the same time, using colon (':') separator.
> >
> > There are corresponding changes on bpf_show_options side to use known
> > values to print them in human-readable format, falling back to hex mask
> > printing, if there are any unrecognized bits. This shouldn't be
> > necessary when enum BTF information is present, but in general we should
> > always be able to fall back to this even if kernel was built without BTF.
> >
> > Example below shows various ways to specify delegate_cmds options
> > through mount command and how mount options are printed back:
> >
> >   $ sudo mkdir -p /sys/fs/bpf/token
> >   $ sudo mount -t bpf bpffs /sys/fs/bpf/token \
> >                -o delegate_cmds=BPF_PROG_LOAD \
> >                -o delegate_cmds=BPF_MAP_CREATE \
> >                -o delegate_cmds=BPF_TOKEN_CREATE:BPF_BTF_LOAD:BPF_LINK_CREATE
> >   $ mount | grep token
> >   bpffs on /sys/fs/bpf/token type bpf (rw,relatime,delegate_cmds=BPF_MAP_CREATE:BPF_PROG_LOAD:BPF_BTF_LOAD:BPF_LINK_CREATE:BPF_TOKEN_CREATE)
>
> imo this is too verbose.
> For cmds it doesn't look as bad. "BPF_" prefix is repetitive, but not overly so.
> But for maps and progs it will be bad.
> It will look like:
> delegate_progs=BPF_PROG_TYPE_SOCKET_FILTER:BPF_PROG_TYPE_SOCKET_XDP:BPF_PROG_TYPE_SCHED_CLS
> which is not readable.
> delegate_progs=SOCKET_FILTER:XDP:SCHED_CLS
> is much better.
>
> And I would go further (like libbpf does) and lower case them for output
> while allow both upper and lower for input:
> delegate_progs=socket_filter:xdp:sched_cls.

Ok, I was wondering if someone would complain about this :) Makes
sense, I'll do it more flexibly and succinctly.

>
> Because of stripping the prefix for maps and progs I would strip the prefix for cmds too
> for consistency.
>
> pw-bot: cr
>

[PATCH bpf-next 2/2] selftests/bpf: utilize string values for delegate_xxx mount options

[Andrii Nakryiko]

Use both hex-based and string-based way to specify delegate mount
options for BPF FS.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../testing/selftests/bpf/prog_tests/token.c  | 52 ++++++++++++-------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/token.c b/tools/testing/selftests/bpf/prog_tests/token.c
index dc03790c6272..90ca7ae9649a 100644
--- a/tools/testing/selftests/bpf/prog_tests/token.c
+++ b/tools/testing/selftests/bpf/prog_tests/token.c
@@ -55,14 +55,22 @@ static int restore_priv_caps(__u64 old_caps)
 	return cap_enable_effective(old_caps, NULL);
 }
 
-static int set_delegate_mask(int fs_fd, const char *key, __u64 mask)
+static int set_delegate_mask(int fs_fd, const char *key, __u64 mask, const char *mask_str)
 {
 	char buf[32];
 	int err;
 
-	snprintf(buf, sizeof(buf), "0x%llx", (unsigned long long)mask);
+	if (!mask_str) {
+		if (mask == ~0ULL) {
+			mask_str = "any";
+		} else {
+			snprintf(buf, sizeof(buf), "0x%llx", (unsigned long long)mask);
+			mask_str = buf;
+		}
+	}
+
 	err = sys_fsconfig(fs_fd, FSCONFIG_SET_STRING, key,
-			   mask == ~0ULL ? "any" : buf, 0);
+			   mask_str, 0);
 	if (err < 0)
 		err = -errno;
 	return err;
@@ -75,6 +83,10 @@ struct bpffs_opts {
 	__u64 maps;
 	__u64 progs;
 	__u64 attachs;
+	const char *cmds_str;
+	const char *maps_str;
+	const char *progs_str;
+	const char *attachs_str;
 };
 
 static int create_bpffs_fd(void)
@@ -93,16 +105,16 @@ static int materialize_bpffs_fd(int fs_fd, struct bpffs_opts *opts)
 	int mnt_fd, err;
 
 	/* set up token delegation mount options */
-	err = set_delegate_mask(fs_fd, "delegate_cmds", opts->cmds);
+	err = set_delegate_mask(fs_fd, "delegate_cmds", opts->cmds, opts->cmds_str);
 	if (!ASSERT_OK(err, "fs_cfg_cmds"))
 		return err;
-	err = set_delegate_mask(fs_fd, "delegate_maps", opts->maps);
+	err = set_delegate_mask(fs_fd, "delegate_maps", opts->maps, opts->maps_str);
 	if (!ASSERT_OK(err, "fs_cfg_maps"))
 		return err;
-	err = set_delegate_mask(fs_fd, "delegate_progs", opts->progs);
+	err = set_delegate_mask(fs_fd, "delegate_progs", opts->progs, opts->progs_str);
 	if (!ASSERT_OK(err, "fs_cfg_progs"))
 		return err;
-	err = set_delegate_mask(fs_fd, "delegate_attachs", opts->attachs);
+	err = set_delegate_mask(fs_fd, "delegate_attachs", opts->attachs, opts->attachs_str);
 	if (!ASSERT_OK(err, "fs_cfg_attachs"))
 		return err;
 
@@ -284,13 +296,13 @@ static void child(int sock_fd, struct bpffs_opts *opts, child_callback_fn callba
 	}
 
 	/* ensure unprivileged child cannot set delegation options */
-	err = set_delegate_mask(fs_fd, "delegate_cmds", 0x1);
+	err = set_delegate_mask(fs_fd, "delegate_cmds", 0x1, NULL);
 	ASSERT_EQ(err, -EPERM, "delegate_cmd_eperm");
-	err = set_delegate_mask(fs_fd, "delegate_maps", 0x1);
+	err = set_delegate_mask(fs_fd, "delegate_maps", 0x1, NULL);
 	ASSERT_EQ(err, -EPERM, "delegate_maps_eperm");
-	err = set_delegate_mask(fs_fd, "delegate_progs", 0x1);
+	err = set_delegate_mask(fs_fd, "delegate_progs", 0x1, NULL);
 	ASSERT_EQ(err, -EPERM, "delegate_progs_eperm");
-	err = set_delegate_mask(fs_fd, "delegate_attachs", 0x1);
+	err = set_delegate_mask(fs_fd, "delegate_attachs", 0x1, NULL);
 	ASSERT_EQ(err, -EPERM, "delegate_attachs_eperm");
 
 	/* pass BPF FS context object to parent */
@@ -314,22 +326,22 @@ static void child(int sock_fd, struct bpffs_opts *opts, child_callback_fn callba
 	}
 
 	/* ensure unprivileged child cannot reconfigure to set delegation options */
-	err = set_delegate_mask(fs_fd, "delegate_cmds", ~0ULL);
+	err = set_delegate_mask(fs_fd, "delegate_cmds", 0, "any");
 	if (!ASSERT_EQ(err, -EPERM, "delegate_cmd_eperm_reconfig")) {
 		err = -EINVAL;
 		goto cleanup;
 	}
-	err = set_delegate_mask(fs_fd, "delegate_maps", ~0ULL);
+	err = set_delegate_mask(fs_fd, "delegate_maps", 0, "any");
 	if (!ASSERT_EQ(err, -EPERM, "delegate_maps_eperm_reconfig")) {
 		err = -EINVAL;
 		goto cleanup;
 	}
-	err = set_delegate_mask(fs_fd, "delegate_progs", ~0ULL);
+	err = set_delegate_mask(fs_fd, "delegate_progs", 0, "any");
 	if (!ASSERT_EQ(err, -EPERM, "delegate_progs_eperm_reconfig")) {
 		err = -EINVAL;
 		goto cleanup;
 	}
-	err = set_delegate_mask(fs_fd, "delegate_attachs", ~0ULL);
+	err = set_delegate_mask(fs_fd, "delegate_attachs", 0, "any");
 	if (!ASSERT_EQ(err, -EPERM, "delegate_attachs_eperm_reconfig")) {
 		err = -EINVAL;
 		goto cleanup;
@@ -647,8 +659,8 @@ void test_token(void)
 {
 	if (test__start_subtest("map_token")) {
 		struct bpffs_opts opts = {
-			.cmds = 1ULL << BPF_MAP_CREATE,
-			.maps = 1ULL << BPF_MAP_TYPE_STACK,
+			.cmds_str = "BPF_MAP_CREATE",
+			.maps_str = "BPF_MAP_TYPE_STACK",
 		};
 
 		subtest_userns(&opts, userns_map_create);
@@ -662,9 +674,9 @@ void test_token(void)
 	}
 	if (test__start_subtest("prog_token")) {
 		struct bpffs_opts opts = {
-			.cmds = 1ULL << BPF_PROG_LOAD,
-			.progs = 1ULL << BPF_PROG_TYPE_XDP,
-			.attachs = 1ULL << BPF_XDP,
+			.cmds_str = "BPF_PROG_LOAD",
+			.progs_str = "BPF_PROG_TYPE_XDP",
+			.attachs_str = "BPF_XDP",
 		};
 
 		subtest_userns(&opts, userns_prog_load);
-- 
2.34.1

RE: [PATCH bpf-next 2/2] selftests/bpf: utilize string values for delegate_xxx mount options

[John Fastabend]

Andrii Nakryiko wrote:
> Use both hex-based and string-based way to specify delegate mount
> options for BPF FS.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

Acked-by: John Fastabend <john.fastabend@gmail.com>

RE: [PATCH bpf-next 0/2] BPF FS mount options parsing follow ups

[John Fastabend]

Andrii Nakryiko wrote:
> Original BPF token patch set ([0]) added delegate_xxx mount options which
> supported only special "any" value and hexadecimal bitmask. This patch set
> attempts to make specifying and inspecting these mount options more
> human-friendly by supporting string constants matching corresponding bpf_cmd,
> bpf_map_type, bpf_prog_type, and bpf_attach_type enumerators.
> 
> This implementation relies on BTF information to find all supported symbolic
> names. If kernel wasn't built with BTF, BPF FS will still support "any" and
> hex-based mask.
> 
>   [0] https://patchwork.kernel.org/project/netdevbpf/list/?series=805707&state=*
> 
> Andrii Nakryiko (2):
>   bpf: support symbolic BPF FS delegation mount options
>   selftests/bpf: utilize string values for delegate_xxx mount options
> 
>  kernel/bpf/inode.c                            | 231 +++++++++++++++---
>  .../testing/selftests/bpf/prog_tests/token.c  |  52 ++--
>  2 files changed, 225 insertions(+), 58 deletions(-)
> 
> -- 
> 2.34.1
> 
> 

Acked-by: John Fastabend <john.fastabend@gmail.com>