On Wed, Nov 5, 2025 at 6:05 PM Quentin Monnet <[email protected]> wrote: > > 2025-11-05 17:29 UTC-0800 ~ Alexei Starovoitov > <[email protected]> > > On Wed, Nov 5, 2025 at 1:38 AM Quentin Monnet <[email protected]> wrote: > >> > >> 2025-11-04 09:54 UTC-0800 ~ Alexei Starovoitov > >> <[email protected]> > >>> On Sat, Nov 1, 2025 at 12:34 PM Harshit Mogalapalli > >>> <[email protected]> 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 <[email protected]> > >>>> Reviewed-by: Quentin Monnet <[email protected]> > >>>> Signed-off-by: Harshit Mogalapalli <[email protected]> > >>>> --- > >>>> 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.
