Message-ID: <ccbb1ed5-fc35-4084-9c13-1e4b1ede96f4@oracle.com>
Date: Thu, 6 Nov 2025 18:42:40 +0530
From: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>
To: Quentin Monnet <qmo@...nel.org>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: bpf <bpf@...r.kernel.org>, Alan Maguire <alan.maguire@...cle.com>,
        Yonghong Song <yonghong.song@...ux.dev>,
        Alexei Starovoitov
 <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau
 <martin.lau@...ux.dev>,
        Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        Shuah Khan <shuah@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v3 1/2] bpftool: Print map ID upon creation and support
 JSON output

Hi Alexei and Quentin,

On 06/11/25 07:55, Quentin Monnet wrote:
> 2025-11-05 18:14 UTC-0800 ~ Alexei Starovoitov
> <alexei.starovoitov@...il.com>
>> On Wed, Nov 5, 2025 at 6:05 PM Quentin Monnet <qmo@...nel.org> wrote:
>>>
>>> 2025-11-05 17:29 UTC-0800 ~ Alexei Starovoitov
>>> <alexei.starovoitov@...il.com>
>>>> On Wed, Nov 5, 2025 at 1:38 AM Quentin Monnet <qmo@...nel.org> wrote:
>>>>>
>>>>> 2025-11-04 09:54 UTC-0800 ~ Alexei Starovoitov
>>>>> <alexei.starovoitov@...il.com>
>>>>>> On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli
>>>>>> <harshit.m.mogalapalli@...cle.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@...ux.dev>
>>>>>>> Reviewed-by: Quentin Monnet <qmo@...nel.org>
>>>>>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.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

Powered by blists - more mailing lists