[RFC bpf-next 1/2] bpftool: Print map ID upon creation and support JSON output

Harshit Mogalapalli posted 2 patches 3 months, 2 weeks ago
There is a newer version of this series
[RFC bpf-next 1/2] bpftool: Print map ID upon creation and support JSON output
Posted by Harshit Mogalapalli 3 months, 2 weeks ago
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
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
 tools/bpf/bpftool/map.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index c9de44a45778..b6580f25361d 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,27 @@ static int do_create(int argc, char **argv)
 	}
 
 	err = do_pin_fd(fd, pinfile);
-	close(fd);
-	if (err)
+	if (err) {
+		close(fd);
 		goto exit;
+	}
 
-	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\n", strerror(errno));
+		close(fd);
+		goto exit;
+	}
 
+	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);
+	}
 exit:
 	if (attr.inner_map_fd > 0)
 		close(attr.inner_map_fd);
-- 
2.50.1
Re: [RFC bpf-next 1/2] bpftool: Print map ID upon creation and support JSON output
Posted by Yonghong Song 3 months, 1 week ago

On 10/28/25 5:57 AM, Harshit Mogalapalli 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
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
>   tools/bpf/bpftool/map.c | 24 ++++++++++++++++++++----
>   1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index c9de44a45778..b6580f25361d 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,27 @@ static int do_create(int argc, char **argv)
>   	}
>   
>   	err = do_pin_fd(fd, pinfile);
> -	close(fd);
> -	if (err)
> +	if (err) {
> +		close(fd);

I think you can remove close(fd) here,

>   		goto exit;
> +	}
>   
> -	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\n", strerror(errno));
> +		close(fd);

and here

> +		goto exit;
> +	}
>   
> +	close(fd);

and here,

> +
> +	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);
> +	}
>   exit:

and put close(fd) here.

>   	if (attr.inner_map_fd > 0)
>   		close(attr.inner_map_fd);

Re: [RFC bpf-next 1/2] bpftool: Print map ID upon creation and support JSON output
Posted by Harshit Mogalapalli 3 months, 1 week ago
Hi Yonghong,


On 29/10/25 07:44, Yonghong Song wrote:
> 
> 
> On 10/28/25 5:57 AM, Harshit Mogalapalli 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
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> ---
>>   tools/bpf/bpftool/map.c | 24 ++++++++++++++++++++----
>>   1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>> index c9de44a45778..b6580f25361d 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,27 @@ static int do_create(int argc, char **argv)
>>       }
>>       err = do_pin_fd(fd, pinfile);
>> -    close(fd);
>> -    if (err)
>> +    if (err) {
>> +        close(fd);
> 
> I think you can remove close(fd) here,
> 
>>           goto exit;
>> +    }
>> -    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\n", strerror(errno));
>> +        close(fd);
> 
> and here
> 
>> +        goto exit;
>> +    }
>> +    close(fd);
> 
> and here,
> 
>> +
>> +    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);
>> +    }
>>   exit:
> 
> and put close(fd) here.

I think we need one more close_fd: label and then put a close(fd); here. 
As there are other gotos to exit earlier in this function when fd is 
uninitialized, which can the error like:

map.c: In function ‘do_create’:
map.c:1375:9: warning: ‘fd’ may be used uninitialized 
[-Wmaybe-uninitialized]
  1375 |         close(fd);
       |         ^~~~~~~~~
map.c:1258:23: note: ‘fd’ was declared here
  1258 |         int err = -1, fd;
       |                       ^~



So, maybe we could do something like this:

         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\n", 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);

I can prepare a v2 with this change, but wouldn't it be simpler to add a
direct close(fd); on the few error paths instead of introducing an
additional label for close(fd);?

Thoughts/Suggestions ?

Thanks,
Harshit

> 
>>       if (attr.inner_map_fd > 0)
>>           close(attr.inner_map_fd);
> 
> 

Re: [RFC bpf-next 1/2] bpftool: Print map ID upon creation and support JSON output
Posted by Yonghong Song 3 months, 1 week ago

On 10/29/25 9:05 AM, Harshit Mogalapalli wrote:
> Hi Yonghong,
>
>
> On 29/10/25 07:44, Yonghong Song wrote:
>>
>>
>> On 10/28/25 5:57 AM, Harshit Mogalapalli 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
>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>>> ---
>>>   tools/bpf/bpftool/map.c | 24 ++++++++++++++++++++----
>>>   1 file changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>>> index c9de44a45778..b6580f25361d 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,27 @@ static int do_create(int argc, char **argv)
>>>       }
>>>       err = do_pin_fd(fd, pinfile);
>>> -    close(fd);
>>> -    if (err)
>>> +    if (err) {
>>> +        close(fd);
>>
>> I think you can remove close(fd) here,
>>
>>>           goto exit;
>>> +    }
>>> -    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\n", strerror(errno));
>>> +        close(fd);
>>
>> and here
>>
>>> +        goto exit;
>>> +    }
>>> +    close(fd);
>>
>> and here,
>>
>>> +
>>> +    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);
>>> +    }
>>>   exit:
>>
>> and put close(fd) here.
>
> I think we need one more close_fd: label and then put a close(fd); 
> here. As there are other gotos to exit earlier in this function when 
> fd is uninitialized, which can the error like:
>
> map.c: In function ‘do_create’:
> map.c:1375:9: warning: ‘fd’ may be used uninitialized 
> [-Wmaybe-uninitialized]
>  1375 |         close(fd);
>       |         ^~~~~~~~~
> map.c:1258:23: note: ‘fd’ was declared here
>  1258 |         int err = -1, fd;
>       |                       ^~
>
>
>
> So, maybe we could do something like this:
>
>         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\n", 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);
>
> I can prepare a v2 with this change, but wouldn't it be simpler to add a
> direct close(fd); on the few error paths instead of introducing an
> additional label for close(fd);?

The above change LGTM. Thanks!

>
> Thoughts/Suggestions ?
>
> Thanks,
> Harshit
>
>>
>>>       if (attr.inner_map_fd > 0)
>>>           close(attr.inner_map_fd);
>>
>>
>
>

Re: [RFC bpf-next 1/2] bpftool: Print map ID upon creation and support JSON output
Posted by Harshit Mogalapalli 3 months, 1 week ago
Hi Yonghong,

On 29/10/25 07:44, Yonghong Song wrote:
> 
> 
> On 10/28/25 5:57 AM, Harshit Mogalapalli 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
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> ---
>>   tools/bpf/bpftool/map.c | 24 ++++++++++++++++++++----
>>   1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>> index c9de44a45778..b6580f25361d 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,27 @@ static int do_create(int argc, char **argv)
>>       }
>>       err = do_pin_fd(fd, pinfile);
>> -    close(fd);
>> -    if (err)
>> +    if (err) {
>> +        close(fd);
> 
> I think you can remove close(fd) here,
> 
>>           goto exit;
>> +    }
>> -    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\n", strerror(errno));
>> +        close(fd);
> 
> and here
> 
>> +        goto exit;
>> +    }
>> +    close(fd);
> 
> and here,
> 
>> +
>> +    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);
>> +    }
>>   exit:
> 
> and put close(fd) here.
> 

Thanks a lot for the suggestion.

Will do that and send a v2. Thanks for reviewing.


Regards,
Harshit

>>       if (attr.inner_map_fd > 0)
>>           close(attr.inner_map_fd);
> 
>