public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Print map ID on successful creation
@ 2025-11-01 19:33 Harshit Mogalapalli
  2025-11-01 19:33 ` [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output Harshit Mogalapalli
  2025-11-01 19:33 ` [PATCH v3 2/2] selftests/bpf: Add test for bpftool map ID printing Harshit Mogalapalli
  0 siblings, 2 replies; 11+ messages in thread
From: Harshit Mogalapalli @ 2025-11-01 19:33 UTC (permalink / raw)
  To: bpf
  Cc: alan.maguire, Harshit Mogalapalli, Quentin Monnet,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, linux-kernel, linux-kselftest

Hi all,

I have tried looking at an issue from the bpftool repository:
https://github.com/libbpf/bpftool/issues/121 and this patch series
tries to add that enhancement.

Summary: Currently when a map creation is successful there is no message
on the terminal, printing IDs on successful creation of maps can help
notify the user and can be used in CI/CD.

The first patch adds the logic for printing and the second patch adds a
simple selftest for the same.

Thank you very much.

V1 --> V2: PATCH 1 updated [Thanks Yonghong for suggesting better way of
error handling with a new label for close(fd); instead of calling
multiple times]

V2 --> V3: Thanks to Quentin.
	PATCH1: drop \n in p_err statement
	PATCH2: Remove messages in cases of successful ID printing. Also
	remove message with a "FAIL:" prefix to make it more consistent.

Regards,
Harshit


Harshit Mogalapalli (2):
  bpftool: Print map ID upon creation and support JSON output
  selftests/bpf: Add test for bpftool map ID printing

 tools/bpf/bpftool/map.c                       | 21 +++++++++---
 .../testing/selftests/bpf/test_bpftool_map.sh | 32 +++++++++++++++++++
 2 files changed, 49 insertions(+), 4 deletions(-)

-- 
2.50.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output
  2025-11-01 19:33 [PATCH v3 0/2] Print map ID on successful creation Harshit Mogalapalli
@ 2025-11-01 19:33 ` Harshit Mogalapalli
  2025-11-04 17:54   ` Alexei Starovoitov
  2025-11-01 19:33 ` [PATCH v3 2/2] selftests/bpf: Add test for bpftool map ID printing Harshit Mogalapalli
  1 sibling, 1 reply; 11+ messages in thread
From: Harshit Mogalapalli @ 2025-11-01 19:33 UTC (permalink / raw)
  To: bpf
  Cc: alan.maguire, Harshit Mogalapalli, Yonghong Song, Quentin Monnet,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
	linux-kernel, linux-kselftest

It is useful to print map ID on successful creation.

JSON case:
$ ./bpftool -j map create /sys/fs/bpf/test_map4 type hash key 4 value 8 entries 128 name map4
{"id":12}

Generic case:
$ ./bpftool  map create /sys/fs/bpf/test_map5 type hash key 4 value 8 entries 128 name map5
Map successfully created with ID: 15

Bpftool Issue: https://github.com/libbpf/bpftool/issues/121
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Reviewed-by: Quentin Monnet <qmo@kernel.org>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v2->v3: remove a line break("\n" ) in p_err statement. [Thanks Quentin]
---
 tools/bpf/bpftool/map.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index c9de44a45778..f32ae5476d76 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -1251,6 +1251,8 @@ static int do_create(int argc, char **argv)
 	LIBBPF_OPTS(bpf_map_create_opts, attr);
 	enum bpf_map_type map_type = BPF_MAP_TYPE_UNSPEC;
 	__u32 key_size = 0, value_size = 0, max_entries = 0;
+	struct bpf_map_info map_info = {};
+	__u32 map_info_len = sizeof(map_info);
 	const char *map_name = NULL;
 	const char *pinfile;
 	int err = -1, fd;
@@ -1353,13 +1355,24 @@ static int do_create(int argc, char **argv)
 	}
 
 	err = do_pin_fd(fd, pinfile);
-	close(fd);
 	if (err)
-		goto exit;
+		goto close_fd;
 
-	if (json_output)
-		jsonw_null(json_wtr);
+	err = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
+	if (err) {
+		p_err("Failed to fetch map info: %s", strerror(errno));
+		goto close_fd;
+	}
 
+	if (json_output) {
+		jsonw_start_object(json_wtr);
+		jsonw_int_field(json_wtr, "id", map_info.id);
+		jsonw_end_object(json_wtr);
+	} else {
+		printf("Map successfully created with ID: %u\n", map_info.id);
+	}
+close_fd:
+	close(fd);
 exit:
 	if (attr.inner_map_fd > 0)
 		close(attr.inner_map_fd);
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/2] selftests/bpf: Add test for bpftool map ID printing
  2025-11-01 19:33 [PATCH v3 0/2] Print map ID on successful creation Harshit Mogalapalli
  2025-11-01 19:33 ` [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output Harshit Mogalapalli
@ 2025-11-01 19:33 ` Harshit Mogalapalli
  2025-11-03 14:31   ` Quentin Monnet
  1 sibling, 1 reply; 11+ messages in thread
From: Harshit Mogalapalli @ 2025-11-01 19:33 UTC (permalink / raw)
  To: bpf
  Cc: alan.maguire, Harshit Mogalapalli, Quentin Monnet,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, linux-kernel, linux-kselftest

Add selftest to check if Map ID is printed on successful creation in
both plain text and json formats.

Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v2-->v3: Removes printing for success cases following the pattern used
in this script. Also remove "FAIL:" prefix for consistency. [ Thanks to
Quentin for the suggestion]
---
 .../testing/selftests/bpf/test_bpftool_map.sh | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_bpftool_map.sh b/tools/testing/selftests/bpf/test_bpftool_map.sh
index 515b1df0501e..d65866497c1b 100755
--- a/tools/testing/selftests/bpf/test_bpftool_map.sh
+++ b/tools/testing/selftests/bpf/test_bpftool_map.sh
@@ -361,6 +361,36 @@ test_map_access_with_btf_list() {
 	fi
 }
 
+# Function to test map ID printing
+# Parameters:
+#   $1: bpftool path
+#   $2: BPF_DIR
+test_map_id_printing() {
+	local bpftool_path="$1"
+	local bpf_dir="$2"
+	local test_map_name="test_map_id"
+	local test_map_path="$bpf_dir/$test_map_name"
+
+	local output
+	output=$("$bpftool_path" map create "$test_map_path" type hash key 4 \
+		value 8 entries 128 name "$test_map_name")
+	if ! echo "$output" | grep -q "Map successfully created with ID:"; then
+		echo "Map ID not printed in plain text output."
+		exit 1
+	fi
+
+	rm -f "$test_map_path"
+
+	output=$("$bpftool_path" map create "$test_map_path" type hash key 4 \
+		value 8 entries 128 name "$test_map_name" --json)
+	if ! echo "$output" | jq -e 'has("id")' >/dev/null 2>&1; then
+		echo "Map ID not printed in JSON output."
+		exit 1
+	fi
+
+	rm -f "$test_map_path"
+}
+
 set -eu
 
 trap cleanup_skip EXIT
@@ -395,4 +425,6 @@ test_map_creation_and_map_of_maps "$BPFTOOL_PATH" "$BPF_DIR"
 
 test_map_access_with_btf_list "$BPFTOOL_PATH"
 
+test_map_id_printing "$BPFTOOL_PATH" "$BPF_DIR"
+
 exit 0
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] selftests/bpf: Add test for bpftool map ID printing
  2025-11-01 19:33 ` [PATCH v3 2/2] selftests/bpf: Add test for bpftool map ID printing Harshit Mogalapalli
@ 2025-11-03 14:31   ` Quentin Monnet
  0 siblings, 0 replies; 11+ messages in thread
From: Quentin Monnet @ 2025-11-03 14:31 UTC (permalink / raw)
  To: Harshit Mogalapalli, bpf
  Cc: alan.maguire, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan, linux-kernel, linux-kselftest

On 01/11/2025 19:33, Harshit Mogalapalli wrote:
> Add selftest to check if Map ID is printed on successful creation in
> both plain text and json formats.
> 
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>

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

Thank you!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output
  2025-11-01 19:33 ` [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output Harshit Mogalapalli
@ 2025-11-04 17:54   ` Alexei Starovoitov
  2025-11-05  9:37     ` Quentin Monnet
  0 siblings, 1 reply; 11+ messages in thread
From: Alexei Starovoitov @ 2025-11-04 17:54 UTC (permalink / raw)
  To: Harshit Mogalapalli
  Cc: bpf, Alan Maguire, Yonghong Song, Quentin Monnet,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
	LKML, open list:KERNEL SELFTEST FRAMEWORK

On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli
<harshit.m.mogalapalli@oracle.com> wrote:
>
> It is useful to print map ID on successful creation.
>
> JSON case:
> $ ./bpftool -j map create /sys/fs/bpf/test_map4 type hash key 4 value 8 entries 128 name map4
> {"id":12}
>
> Generic case:
> $ ./bpftool  map create /sys/fs/bpf/test_map5 type hash key 4 value 8 entries 128 name map5
> Map successfully created with ID: 15
>
> Bpftool Issue: https://github.com/libbpf/bpftool/issues/121
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> Reviewed-by: Quentin Monnet <qmo@kernel.org>
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
> v2->v3: remove a line break("\n" ) in p_err statement. [Thanks Quentin]
> ---
>  tools/bpf/bpftool/map.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index c9de44a45778..f32ae5476d76 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -1251,6 +1251,8 @@ static int do_create(int argc, char **argv)
>         LIBBPF_OPTS(bpf_map_create_opts, attr);
>         enum bpf_map_type map_type = BPF_MAP_TYPE_UNSPEC;
>         __u32 key_size = 0, value_size = 0, max_entries = 0;
> +       struct bpf_map_info map_info = {};
> +       __u32 map_info_len = sizeof(map_info);
>         const char *map_name = NULL;
>         const char *pinfile;
>         int err = -1, fd;
> @@ -1353,13 +1355,24 @@ static int do_create(int argc, char **argv)
>         }
>
>         err = do_pin_fd(fd, pinfile);
> -       close(fd);
>         if (err)
> -               goto exit;
> +               goto close_fd;
>
> -       if (json_output)
> -               jsonw_null(json_wtr);
> +       err = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
> +       if (err) {
> +               p_err("Failed to fetch map info: %s", strerror(errno));
> +               goto close_fd;
> +       }
>
> +       if (json_output) {
> +               jsonw_start_object(json_wtr);
> +               jsonw_int_field(json_wtr, "id", map_info.id);
> +               jsonw_end_object(json_wtr);
> +       } else {
> +               printf("Map successfully created with ID: %u\n", map_info.id);
> +       }

bpftool doesn't print it today and some scripts may depend on that.
Let's drop this 'printf'. Json can do it unconditionally, since
json parsing scripts should filter things they care about.

pw-bot: cr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output
  2025-11-04 17:54   ` Alexei Starovoitov
@ 2025-11-05  9:37     ` Quentin Monnet
  2025-11-06  1:29       ` Alexei Starovoitov
  0 siblings, 1 reply; 11+ messages in thread
From: Quentin Monnet @ 2025-11-05  9:37 UTC (permalink / raw)
  To: Alexei Starovoitov, Harshit Mogalapalli
  Cc: bpf, Alan Maguire, Yonghong Song, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, John Fastabend, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, LKML,
	open list:KERNEL SELFTEST FRAMEWORK

2025-11-04 09:54 UTC-0800 ~ Alexei Starovoitov
<alexei.starovoitov@gmail.com>
> On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli
> <harshit.m.mogalapalli@oracle.com> wrote:
>>
>> It is useful to print map ID on successful creation.
>>
>> JSON case:
>> $ ./bpftool -j map create /sys/fs/bpf/test_map4 type hash key 4 value 8 entries 128 name map4
>> {"id":12}
>>
>> Generic case:
>> $ ./bpftool  map create /sys/fs/bpf/test_map5 type hash key 4 value 8 entries 128 name map5
>> Map successfully created with ID: 15
>>
>> Bpftool Issue: https://github.com/libbpf/bpftool/issues/121
>> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>> Reviewed-by: Quentin Monnet <qmo@kernel.org>
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> ---
>> v2->v3: remove a line break("\n" ) in p_err statement. [Thanks Quentin]
>> ---
>>  tools/bpf/bpftool/map.c | 21 +++++++++++++++++----
>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>> index c9de44a45778..f32ae5476d76 100644
>> --- a/tools/bpf/bpftool/map.c
>> +++ b/tools/bpf/bpftool/map.c
>> @@ -1251,6 +1251,8 @@ static int do_create(int argc, char **argv)
>>         LIBBPF_OPTS(bpf_map_create_opts, attr);
>>         enum bpf_map_type map_type = BPF_MAP_TYPE_UNSPEC;
>>         __u32 key_size = 0, value_size = 0, max_entries = 0;
>> +       struct bpf_map_info map_info = {};
>> +       __u32 map_info_len = sizeof(map_info);
>>         const char *map_name = NULL;
>>         const char *pinfile;
>>         int err = -1, fd;
>> @@ -1353,13 +1355,24 @@ static int do_create(int argc, char **argv)
>>         }
>>
>>         err = do_pin_fd(fd, pinfile);
>> -       close(fd);
>>         if (err)
>> -               goto exit;
>> +               goto close_fd;
>>
>> -       if (json_output)
>> -               jsonw_null(json_wtr);
>> +       err = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
>> +       if (err) {
>> +               p_err("Failed to fetch map info: %s", strerror(errno));
>> +               goto close_fd;
>> +       }
>>
>> +       if (json_output) {
>> +               jsonw_start_object(json_wtr);
>> +               jsonw_int_field(json_wtr, "id", map_info.id);
>> +               jsonw_end_object(json_wtr);
>> +       } else {
>> +               printf("Map successfully created with ID: %u\n", map_info.id);
>> +       }
> 
> bpftool doesn't print it today and some scripts may depend on that.


Hi Alexei, are you sure we can't add any input at all? I'm concerned
that users won't ever find the IDs for created maps they might want to
use, if they never see it in the plain output.


> Let's drop this 'printf'. Json can do it unconditionally, since
> json parsing scripts should filter things they care about.

I'd say the risk is the same. Scripts should filter things, but in
practise they might just as well be comparing to "null" today, given
that we didn't have any other output for the command so far. Conversely,
what scripts should not do is rely on plain output, we've always
recommended using bpftool's JSON for automation (or the exit code, in
the case of map creation). So I'm not convinced it's justified to
introduce a difference between plain and JSON in the current case.

Quentin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output
  2025-11-05  9:37     ` Quentin Monnet
@ 2025-11-06  1:29       ` Alexei Starovoitov
  2025-11-06  2:05         ` Quentin Monnet
  0 siblings, 1 reply; 11+ messages in thread
From: Alexei Starovoitov @ 2025-11-06  1:29 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Harshit Mogalapalli, bpf, Alan Maguire, Yonghong Song,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
	LKML, open list:KERNEL SELFTEST FRAMEWORK

On Wed, Nov 5, 2025 at 1:38 AM Quentin Monnet <qmo@kernel.org> wrote:
>
> 2025-11-04 09:54 UTC-0800 ~ Alexei Starovoitov
> <alexei.starovoitov@gmail.com>
> > On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli
> > <harshit.m.mogalapalli@oracle.com> wrote:
> >>
> >> It is useful to print map ID on successful creation.
> >>
> >> JSON case:
> >> $ ./bpftool -j map create /sys/fs/bpf/test_map4 type hash key 4 value 8 entries 128 name map4
> >> {"id":12}
> >>
> >> Generic case:
> >> $ ./bpftool  map create /sys/fs/bpf/test_map5 type hash key 4 value 8 entries 128 name map5
> >> Map successfully created with ID: 15
> >>
> >> Bpftool Issue: https://github.com/libbpf/bpftool/issues/121
> >> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> >> Reviewed-by: Quentin Monnet <qmo@kernel.org>
> >> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> >> ---
> >> v2->v3: remove a line break("\n" ) in p_err statement. [Thanks Quentin]
> >> ---
> >>  tools/bpf/bpftool/map.c | 21 +++++++++++++++++----
> >>  1 file changed, 17 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> >> index c9de44a45778..f32ae5476d76 100644
> >> --- a/tools/bpf/bpftool/map.c
> >> +++ b/tools/bpf/bpftool/map.c
> >> @@ -1251,6 +1251,8 @@ static int do_create(int argc, char **argv)
> >>         LIBBPF_OPTS(bpf_map_create_opts, attr);
> >>         enum bpf_map_type map_type = BPF_MAP_TYPE_UNSPEC;
> >>         __u32 key_size = 0, value_size = 0, max_entries = 0;
> >> +       struct bpf_map_info map_info = {};
> >> +       __u32 map_info_len = sizeof(map_info);
> >>         const char *map_name = NULL;
> >>         const char *pinfile;
> >>         int err = -1, fd;
> >> @@ -1353,13 +1355,24 @@ static int do_create(int argc, char **argv)
> >>         }
> >>
> >>         err = do_pin_fd(fd, pinfile);
> >> -       close(fd);
> >>         if (err)
> >> -               goto exit;
> >> +               goto close_fd;
> >>
> >> -       if (json_output)
> >> -               jsonw_null(json_wtr);
> >> +       err = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
> >> +       if (err) {
> >> +               p_err("Failed to fetch map info: %s", strerror(errno));
> >> +               goto close_fd;
> >> +       }
> >>
> >> +       if (json_output) {
> >> +               jsonw_start_object(json_wtr);
> >> +               jsonw_int_field(json_wtr, "id", map_info.id);
> >> +               jsonw_end_object(json_wtr);
> >> +       } else {
> >> +               printf("Map successfully created with ID: %u\n", map_info.id);
> >> +       }
> >
> > bpftool doesn't print it today and some scripts may depend on that.
>
>
> Hi Alexei, are you sure we can't add any input at all? I'm concerned
> that users won't ever find the IDs for created maps they might want to
> use, if they never see it in the plain output.
>
>
> > Let's drop this 'printf'. Json can do it unconditionally, since
> > json parsing scripts should filter things they care about.
>
> I'd say the risk is the same. Scripts should filter things, but in
> practise they might just as well be comparing to "null" today, given
> that we didn't have any other output for the command so far. Conversely,
> what scripts should not do is rely on plain output, we've always
> recommended using bpftool's JSON for automation (or the exit code, in
> the case of map creation). So I'm not convinced it's justified to
> introduce a difference between plain and JSON in the current case.

tbh the "map create" feature suppose to create and pin and if both
are successful then the map will be there and bpftool will
exit with success.
Now you're arguing that there could be a race with another
bpftool/something that pins a different map in the same location
and success of bpftool doesn't mean that exact that map is there.
Other tool could have unpinned/deleted map, pinned another one, etc.
Sure, such races are possible, but returning map id still
looks pointless. It doesn't solve any race.
So the whole 'lets print id' doesn't quite make sense to me.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output
  2025-11-06  1:29       ` Alexei Starovoitov
@ 2025-11-06  2:05         ` Quentin Monnet
  2025-11-06  2:14           ` Alexei Starovoitov
  0 siblings, 1 reply; 11+ messages in thread
From: Quentin Monnet @ 2025-11-06  2:05 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Harshit Mogalapalli, bpf, Alan Maguire, Yonghong Song,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
	LKML, open list:KERNEL SELFTEST FRAMEWORK

2025-11-05 17:29 UTC-0800 ~ Alexei Starovoitov
<alexei.starovoitov@gmail.com>
> On Wed, Nov 5, 2025 at 1:38 AM Quentin Monnet <qmo@kernel.org> wrote:
>>
>> 2025-11-04 09:54 UTC-0800 ~ Alexei Starovoitov
>> <alexei.starovoitov@gmail.com>
>>> On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli
>>> <harshit.m.mogalapalli@oracle.com> wrote:
>>>>
>>>> It is useful to print map ID on successful creation.
>>>>
>>>> JSON case:
>>>> $ ./bpftool -j map create /sys/fs/bpf/test_map4 type hash key 4 value 8 entries 128 name map4
>>>> {"id":12}
>>>>
>>>> Generic case:
>>>> $ ./bpftool  map create /sys/fs/bpf/test_map5 type hash key 4 value 8 entries 128 name map5
>>>> Map successfully created with ID: 15
>>>>
>>>> Bpftool Issue: https://github.com/libbpf/bpftool/issues/121
>>>> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>>>> Reviewed-by: Quentin Monnet <qmo@kernel.org>
>>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>>>> ---
>>>> v2->v3: remove a line break("\n" ) in p_err statement. [Thanks Quentin]
>>>> ---
>>>>  tools/bpf/bpftool/map.c | 21 +++++++++++++++++----
>>>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>>>> index c9de44a45778..f32ae5476d76 100644
>>>> --- a/tools/bpf/bpftool/map.c
>>>> +++ b/tools/bpf/bpftool/map.c
>>>> @@ -1251,6 +1251,8 @@ static int do_create(int argc, char **argv)
>>>>         LIBBPF_OPTS(bpf_map_create_opts, attr);
>>>>         enum bpf_map_type map_type = BPF_MAP_TYPE_UNSPEC;
>>>>         __u32 key_size = 0, value_size = 0, max_entries = 0;
>>>> +       struct bpf_map_info map_info = {};
>>>> +       __u32 map_info_len = sizeof(map_info);
>>>>         const char *map_name = NULL;
>>>>         const char *pinfile;
>>>>         int err = -1, fd;
>>>> @@ -1353,13 +1355,24 @@ static int do_create(int argc, char **argv)
>>>>         }
>>>>
>>>>         err = do_pin_fd(fd, pinfile);
>>>> -       close(fd);
>>>>         if (err)
>>>> -               goto exit;
>>>> +               goto close_fd;
>>>>
>>>> -       if (json_output)
>>>> -               jsonw_null(json_wtr);
>>>> +       err = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
>>>> +       if (err) {
>>>> +               p_err("Failed to fetch map info: %s", strerror(errno));
>>>> +               goto close_fd;
>>>> +       }
>>>>
>>>> +       if (json_output) {
>>>> +               jsonw_start_object(json_wtr);
>>>> +               jsonw_int_field(json_wtr, "id", map_info.id);
>>>> +               jsonw_end_object(json_wtr);
>>>> +       } else {
>>>> +               printf("Map successfully created with ID: %u\n", map_info.id);
>>>> +       }
>>>
>>> bpftool doesn't print it today and some scripts may depend on that.
>>
>>
>> Hi Alexei, are you sure we can't add any input at all? I'm concerned
>> that users won't ever find the IDs for created maps they might want to
>> use, if they never see it in the plain output.
>>
>>
>>> Let's drop this 'printf'. Json can do it unconditionally, since
>>> json parsing scripts should filter things they care about.
>>
>> I'd say the risk is the same. Scripts should filter things, but in
>> practise they might just as well be comparing to "null" today, given
>> that we didn't have any other output for the command so far. Conversely,
>> what scripts should not do is rely on plain output, we've always
>> recommended using bpftool's JSON for automation (or the exit code, in
>> the case of map creation). So I'm not convinced it's justified to
>> introduce a difference between plain and JSON in the current case.
> 
> tbh the "map create" feature suppose to create and pin and if both
> are successful then the map will be there and bpftool will
> exit with success.
> Now you're arguing that there could be a race with another
> bpftool/something that pins a different map in the same location
> and success of bpftool doesn't mean that exact that map is there.
> Other tool could have unpinned/deleted map, pinned another one, etc.
> Sure, such races are possible, but returning map id still
> looks pointless. It doesn't solve any race.
> So the whole 'lets print id' doesn't quite make sense to me.

OK "solving races" is not accurate, but returning the ID gives a unique
handle to work with the map, if a user runs a follow-up invocation to
update entries using the ID they can be sure they're working with the
same map - whatever happened with the bpffs. Or they can have the update
fail if you really want that particular map but, for example, it's been
recreated in the meantime. At the moment there's no way to uniquely
identify the map we've created with bpftool, and that seems weird to me.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output
  2025-11-06  2:05         ` Quentin Monnet
@ 2025-11-06  2:14           ` Alexei Starovoitov
  2025-11-06  2:25             ` Quentin Monnet
  0 siblings, 1 reply; 11+ messages in thread
From: Alexei Starovoitov @ 2025-11-06  2:14 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Harshit Mogalapalli, bpf, Alan Maguire, Yonghong Song,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
	LKML, open list:KERNEL SELFTEST FRAMEWORK

On Wed, Nov 5, 2025 at 6:05 PM Quentin Monnet <qmo@kernel.org> wrote:
>
> 2025-11-05 17:29 UTC-0800 ~ Alexei Starovoitov
> <alexei.starovoitov@gmail.com>
> > On Wed, Nov 5, 2025 at 1:38 AM Quentin Monnet <qmo@kernel.org> wrote:
> >>
> >> 2025-11-04 09:54 UTC-0800 ~ Alexei Starovoitov
> >> <alexei.starovoitov@gmail.com>
> >>> On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli
> >>> <harshit.m.mogalapalli@oracle.com> wrote:
> >>>>
> >>>> It is useful to print map ID on successful creation.
> >>>>
> >>>> JSON case:
> >>>> $ ./bpftool -j map create /sys/fs/bpf/test_map4 type hash key 4 value 8 entries 128 name map4
> >>>> {"id":12}
> >>>>
> >>>> Generic case:
> >>>> $ ./bpftool  map create /sys/fs/bpf/test_map5 type hash key 4 value 8 entries 128 name map5
> >>>> Map successfully created with ID: 15
> >>>>
> >>>> Bpftool Issue: https://github.com/libbpf/bpftool/issues/121
> >>>> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> >>>> Reviewed-by: Quentin Monnet <qmo@kernel.org>
> >>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> >>>> ---
> >>>> v2->v3: remove a line break("\n" ) in p_err statement. [Thanks Quentin]
> >>>> ---
> >>>>  tools/bpf/bpftool/map.c | 21 +++++++++++++++++----
> >>>>  1 file changed, 17 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> >>>> index c9de44a45778..f32ae5476d76 100644
> >>>> --- a/tools/bpf/bpftool/map.c
> >>>> +++ b/tools/bpf/bpftool/map.c
> >>>> @@ -1251,6 +1251,8 @@ static int do_create(int argc, char **argv)
> >>>>         LIBBPF_OPTS(bpf_map_create_opts, attr);
> >>>>         enum bpf_map_type map_type = BPF_MAP_TYPE_UNSPEC;
> >>>>         __u32 key_size = 0, value_size = 0, max_entries = 0;
> >>>> +       struct bpf_map_info map_info = {};
> >>>> +       __u32 map_info_len = sizeof(map_info);
> >>>>         const char *map_name = NULL;
> >>>>         const char *pinfile;
> >>>>         int err = -1, fd;
> >>>> @@ -1353,13 +1355,24 @@ static int do_create(int argc, char **argv)
> >>>>         }
> >>>>
> >>>>         err = do_pin_fd(fd, pinfile);
> >>>> -       close(fd);
> >>>>         if (err)
> >>>> -               goto exit;
> >>>> +               goto close_fd;
> >>>>
> >>>> -       if (json_output)
> >>>> -               jsonw_null(json_wtr);
> >>>> +       err = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
> >>>> +       if (err) {
> >>>> +               p_err("Failed to fetch map info: %s", strerror(errno));
> >>>> +               goto close_fd;
> >>>> +       }
> >>>>
> >>>> +       if (json_output) {
> >>>> +               jsonw_start_object(json_wtr);
> >>>> +               jsonw_int_field(json_wtr, "id", map_info.id);
> >>>> +               jsonw_end_object(json_wtr);
> >>>> +       } else {
> >>>> +               printf("Map successfully created with ID: %u\n", map_info.id);
> >>>> +       }
> >>>
> >>> bpftool doesn't print it today and some scripts may depend on that.
> >>
> >>
> >> Hi Alexei, are you sure we can't add any input at all? I'm concerned
> >> that users won't ever find the IDs for created maps they might want to
> >> use, if they never see it in the plain output.
> >>
> >>
> >>> Let's drop this 'printf'. Json can do it unconditionally, since
> >>> json parsing scripts should filter things they care about.
> >>
> >> I'd say the risk is the same. Scripts should filter things, but in
> >> practise they might just as well be comparing to "null" today, given
> >> that we didn't have any other output for the command so far. Conversely,
> >> what scripts should not do is rely on plain output, we've always
> >> recommended using bpftool's JSON for automation (or the exit code, in
> >> the case of map creation). So I'm not convinced it's justified to
> >> introduce a difference between plain and JSON in the current case.
> >
> > tbh the "map create" feature suppose to create and pin and if both
> > are successful then the map will be there and bpftool will
> > exit with success.
> > Now you're arguing that there could be a race with another
> > bpftool/something that pins a different map in the same location
> > and success of bpftool doesn't mean that exact that map is there.
> > Other tool could have unpinned/deleted map, pinned another one, etc.
> > Sure, such races are possible, but returning map id still
> > looks pointless. It doesn't solve any race.
> > So the whole 'lets print id' doesn't quite make sense to me.
>
> OK "solving races" is not accurate, but returning the ID gives a unique
> handle to work with the map, if a user runs a follow-up invocation to
> update entries using the ID they can be sure they're working with the
> same map - whatever happened with the bpffs. Or they can have the update
> fail if you really want that particular map but, for example, it's been
> recreated in the meantime. At the moment there's no way to uniquely
> identify the map we've created with bpftool, and that seems weird to me.

ID is not unique. If somebody rm -rf bpffs. That ID will not point anywhere.
Also it's 31-bit space and folks in the past demonstrated an attack
to recycle the same ID.
So the users cannot be sure what ID is this.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output
  2025-11-06  2:14           ` Alexei Starovoitov
@ 2025-11-06  2:25             ` Quentin Monnet
  2025-11-06 13:12               ` Harshit Mogalapalli
  0 siblings, 1 reply; 11+ messages in thread
From: Quentin Monnet @ 2025-11-06  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Harshit Mogalapalli, bpf, Alan Maguire, Yonghong Song,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
	LKML, open list:KERNEL SELFTEST FRAMEWORK

2025-11-05 18:14 UTC-0800 ~ Alexei Starovoitov
<alexei.starovoitov@gmail.com>
> On Wed, Nov 5, 2025 at 6:05 PM Quentin Monnet <qmo@kernel.org> wrote:
>>
>> 2025-11-05 17:29 UTC-0800 ~ Alexei Starovoitov
>> <alexei.starovoitov@gmail.com>
>>> On Wed, Nov 5, 2025 at 1:38 AM Quentin Monnet <qmo@kernel.org> wrote:
>>>>
>>>> 2025-11-04 09:54 UTC-0800 ~ Alexei Starovoitov
>>>> <alexei.starovoitov@gmail.com>
>>>>> On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli
>>>>> <harshit.m.mogalapalli@oracle.com> wrote:
>>>>>>
>>>>>> It is useful to print map ID on successful creation.
>>>>>>
>>>>>> JSON case:
>>>>>> $ ./bpftool -j map create /sys/fs/bpf/test_map4 type hash key 4 value 8 entries 128 name map4
>>>>>> {"id":12}
>>>>>>
>>>>>> Generic case:
>>>>>> $ ./bpftool  map create /sys/fs/bpf/test_map5 type hash key 4 value 8 entries 128 name map5
>>>>>> Map successfully created with ID: 15
>>>>>>
>>>>>> Bpftool Issue: https://github.com/libbpf/bpftool/issues/121
>>>>>> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>>>>>> Reviewed-by: Quentin Monnet <qmo@kernel.org>
>>>>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>>>>>> ---
>>>>>> v2->v3: remove a line break("\n" ) in p_err statement. [Thanks Quentin]
>>>>>> ---
>>>>>>  tools/bpf/bpftool/map.c | 21 +++++++++++++++++----
>>>>>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>>>>>> index c9de44a45778..f32ae5476d76 100644
>>>>>> --- a/tools/bpf/bpftool/map.c
>>>>>> +++ b/tools/bpf/bpftool/map.c
>>>>>> @@ -1251,6 +1251,8 @@ static int do_create(int argc, char **argv)
>>>>>>         LIBBPF_OPTS(bpf_map_create_opts, attr);
>>>>>>         enum bpf_map_type map_type = BPF_MAP_TYPE_UNSPEC;
>>>>>>         __u32 key_size = 0, value_size = 0, max_entries = 0;
>>>>>> +       struct bpf_map_info map_info = {};
>>>>>> +       __u32 map_info_len = sizeof(map_info);
>>>>>>         const char *map_name = NULL;
>>>>>>         const char *pinfile;
>>>>>>         int err = -1, fd;
>>>>>> @@ -1353,13 +1355,24 @@ static int do_create(int argc, char **argv)
>>>>>>         }
>>>>>>
>>>>>>         err = do_pin_fd(fd, pinfile);
>>>>>> -       close(fd);
>>>>>>         if (err)
>>>>>> -               goto exit;
>>>>>> +               goto close_fd;
>>>>>>
>>>>>> -       if (json_output)
>>>>>> -               jsonw_null(json_wtr);
>>>>>> +       err = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
>>>>>> +       if (err) {
>>>>>> +               p_err("Failed to fetch map info: %s", strerror(errno));
>>>>>> +               goto close_fd;
>>>>>> +       }
>>>>>>
>>>>>> +       if (json_output) {
>>>>>> +               jsonw_start_object(json_wtr);
>>>>>> +               jsonw_int_field(json_wtr, "id", map_info.id);
>>>>>> +               jsonw_end_object(json_wtr);
>>>>>> +       } else {
>>>>>> +               printf("Map successfully created with ID: %u\n", map_info.id);
>>>>>> +       }
>>>>>
>>>>> bpftool doesn't print it today and some scripts may depend on that.
>>>>
>>>>
>>>> Hi Alexei, are you sure we can't add any input at all? I'm concerned
>>>> that users won't ever find the IDs for created maps they might want to
>>>> use, if they never see it in the plain output.
>>>>
>>>>
>>>>> Let's drop this 'printf'. Json can do it unconditionally, since
>>>>> json parsing scripts should filter things they care about.
>>>>
>>>> I'd say the risk is the same. Scripts should filter things, but in
>>>> practise they might just as well be comparing to "null" today, given
>>>> that we didn't have any other output for the command so far. Conversely,
>>>> what scripts should not do is rely on plain output, we've always
>>>> recommended using bpftool's JSON for automation (or the exit code, in
>>>> the case of map creation). So I'm not convinced it's justified to
>>>> introduce a difference between plain and JSON in the current case.
>>>
>>> tbh the "map create" feature suppose to create and pin and if both
>>> are successful then the map will be there and bpftool will
>>> exit with success.
>>> Now you're arguing that there could be a race with another
>>> bpftool/something that pins a different map in the same location
>>> and success of bpftool doesn't mean that exact that map is there.
>>> Other tool could have unpinned/deleted map, pinned another one, etc.
>>> Sure, such races are possible, but returning map id still
>>> looks pointless. It doesn't solve any race.
>>> So the whole 'lets print id' doesn't quite make sense to me.
>>
>> OK "solving races" is not accurate, but returning the ID gives a unique
>> handle to work with the map, if a user runs a follow-up invocation to
>> update entries using the ID they can be sure they're working with the
>> same map - whatever happened with the bpffs. Or they can have the update
>> fail if you really want that particular map but, for example, it's been
>> recreated in the meantime. At the moment there's no way to uniquely
>> identify the map we've created with bpftool, and that seems weird to me.
> 
> ID is not unique. If somebody rm -rf bpffs. That ID will not point anywhere.
> Also it's 31-bit space and folks in the past demonstrated an attack
> to recycle the same ID.
> So the users cannot be sure what ID is this.
> 

Ah. I did assume it was unique :/. My bad, then in that case it doesn't
make too much sense, indeed.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output
  2025-11-06  2:25             ` Quentin Monnet
@ 2025-11-06 13:12               ` Harshit Mogalapalli
  0 siblings, 0 replies; 11+ messages in thread
From: Harshit Mogalapalli @ 2025-11-06 13:12 UTC (permalink / raw)
  To: Quentin Monnet, Alexei Starovoitov
  Cc: bpf, Alan Maguire, Yonghong Song, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, John Fastabend, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, LKML,
	open list:KERNEL SELFTEST FRAMEWORK

Hi Alexei and Quentin,

On 06/11/25 07:55, Quentin Monnet wrote:
> 2025-11-05 18:14 UTC-0800 ~ Alexei Starovoitov
> <alexei.starovoitov@gmail.com>
>> On Wed, Nov 5, 2025 at 6:05 PM Quentin Monnet <qmo@kernel.org> wrote:
>>>
>>> 2025-11-05 17:29 UTC-0800 ~ Alexei Starovoitov
>>> <alexei.starovoitov@gmail.com>
>>>> On Wed, Nov 5, 2025 at 1:38 AM Quentin Monnet <qmo@kernel.org> wrote:
>>>>>
>>>>> 2025-11-04 09:54 UTC-0800 ~ Alexei Starovoitov
>>>>> <alexei.starovoitov@gmail.com>
>>>>>> On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli
>>>>>> <harshit.m.mogalapalli@oracle.com> wrote:
>>>>>>>
>>>>>>> It is useful to print map ID on successful creation.
>>>>>>>
>>>>>>> JSON case:
>>>>>>> $ ./bpftool -j map create /sys/fs/bpf/test_map4 type hash key 4 value 8 entries 128 name map4
>>>>>>> {"id":12}
>>>>>>>
>>>>>>> Generic case:
>>>>>>> $ ./bpftool  map create /sys/fs/bpf/test_map5 type hash key 4 value 8 entries 128 name map5
>>>>>>> Map successfully created with ID: 15
>>>>>>>
>>>>>>> Bpftool Issue: https://github.com/libbpf/bpftool/issues/121
>>>>>>> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>>>>>>> Reviewed-by: Quentin Monnet <qmo@kernel.org>
>>>>>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>>>>>>> ---
>>>>>>> v2->v3: remove a line break("\n" ) in p_err statement. [Thanks Quentin]
>>>>>>> ---
>>>>>>>   tools/bpf/bpftool/map.c | 21 +++++++++++++++++----
>>>>>>>   1 file changed, 17 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>>>>>>> index c9de44a45778..f32ae5476d76 100644
>>>>>>> --- a/tools/bpf/bpftool/map.c
>>>>>>> +++ b/tools/bpf/bpftool/map.c
>>>>>>> @@ -1251,6 +1251,8 @@ static int do_create(int argc, char **argv)
>>>>>>>          LIBBPF_OPTS(bpf_map_create_opts, attr);
>>>>>>>          enum bpf_map_type map_type = BPF_MAP_TYPE_UNSPEC;
>>>>>>>          __u32 key_size = 0, value_size = 0, max_entries = 0;
>>>>>>> +       struct bpf_map_info map_info = {};
>>>>>>> +       __u32 map_info_len = sizeof(map_info);
>>>>>>>          const char *map_name = NULL;
>>>>>>>          const char *pinfile;
>>>>>>>          int err = -1, fd;
>>>>>>> @@ -1353,13 +1355,24 @@ static int do_create(int argc, char **argv)
>>>>>>>          }
>>>>>>>
>>>>>>>          err = do_pin_fd(fd, pinfile);
>>>>>>> -       close(fd);
>>>>>>>          if (err)
>>>>>>> -               goto exit;
>>>>>>> +               goto close_fd;
>>>>>>>
>>>>>>> -       if (json_output)
>>>>>>> -               jsonw_null(json_wtr);
>>>>>>> +       err = bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
>>>>>>> +       if (err) {
>>>>>>> +               p_err("Failed to fetch map info: %s", strerror(errno));
>>>>>>> +               goto close_fd;
>>>>>>> +       }
>>>>>>>
>>>>>>> +       if (json_output) {
>>>>>>> +               jsonw_start_object(json_wtr);
>>>>>>> +               jsonw_int_field(json_wtr, "id", map_info.id);
>>>>>>> +               jsonw_end_object(json_wtr);
>>>>>>> +       } else {
>>>>>>> +               printf("Map successfully created with ID: %u\n", map_info.id);
>>>>>>> +       }
>>>>>>
>>>>>> bpftool doesn't print it today and some scripts may depend on that.
>>>>>
>>>>>
>>>>> Hi Alexei, are you sure we can't add any input at all? I'm concerned
>>>>> that users won't ever find the IDs for created maps they might want to
>>>>> use, if they never see it in the plain output.
>>>>>
>>>>>
>>>>>> Let's drop this 'printf'. Json can do it unconditionally, since
>>>>>> json parsing scripts should filter things they care about.
>>>>>
>>>>> I'd say the risk is the same. Scripts should filter things, but in
>>>>> practise they might just as well be comparing to "null" today, given
>>>>> that we didn't have any other output for the command so far. Conversely,
>>>>> what scripts should not do is rely on plain output, we've always
>>>>> recommended using bpftool's JSON for automation (or the exit code, in
>>>>> the case of map creation). So I'm not convinced it's justified to
>>>>> introduce a difference between plain and JSON in the current case.
>>>>
>>>> tbh the "map create" feature suppose to create and pin and if both
>>>> are successful then the map will be there and bpftool will
>>>> exit with success.
>>>> Now you're arguing that there could be a race with another
>>>> bpftool/something that pins a different map in the same location
>>>> and success of bpftool doesn't mean that exact that map is there.
>>>> Other tool could have unpinned/deleted map, pinned another one, etc.
>>>> Sure, such races are possible, but returning map id still
>>>> looks pointless. It doesn't solve any race.
>>>> So the whole 'lets print id' doesn't quite make sense to me.
>>>
>>> OK "solving races" is not accurate, but returning the ID gives a unique
>>> handle to work with the map, if a user runs a follow-up invocation to
>>> update entries using the ID they can be sure they're working with the
>>> same map - whatever happened with the bpffs. Or they can have the update
>>> fail if you really want that particular map but, for example, it's been
>>> recreated in the meantime. At the moment there's no way to uniquely
>>> identify the map we've created with bpftool, and that seems weird to me.
>>
>> ID is not unique. If somebody rm -rf bpffs. That ID will not point anywhere.
>> Also it's 31-bit space and folks in the past demonstrated an attack
>> to recycle the same ID.
>> So the users cannot be sure what ID is this.
>>
> 
> Ah. I did assume it was unique :/. My bad, then in that case it doesn't
> make too much sense, indeed.

Thanks a lot for your inputs on this. I have learnt a lot by following 
this discussion.

Thanks,
Harshit


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-11-06 13:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-01 19:33 [PATCH v3 0/2] Print map ID on successful creation Harshit Mogalapalli
2025-11-01 19:33 ` [PATCH v3 1/2] bpftool: Print map ID upon creation and support JSON output Harshit Mogalapalli
2025-11-04 17:54   ` Alexei Starovoitov
2025-11-05  9:37     ` Quentin Monnet
2025-11-06  1:29       ` Alexei Starovoitov
2025-11-06  2:05         ` Quentin Monnet
2025-11-06  2:14           ` Alexei Starovoitov
2025-11-06  2:25             ` Quentin Monnet
2025-11-06 13:12               ` Harshit Mogalapalli
2025-11-01 19:33 ` [PATCH v3 2/2] selftests/bpf: Add test for bpftool map ID printing Harshit Mogalapalli
2025-11-03 14:31   ` Quentin Monnet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox