Hi Quentin,
My apologies, I linked to the incorrect line. In particular, I want to continue iterating if the call to bpf_map_get_fd_by_id fails. I cannot disclose at this time our use case due to confidentiality, but I can imagine a scenario for example if a kernel module wants to override the return value for some reason (not our use case, but a legitimate one). Below is my patch.
>From 7f3eb5c045ec0169435c18af448ebe5eeb642cc6 Mon Sep 17 00:00:00 2001
From: Rae Marks ramark@xxxxxxxxxxxxx
Date: Tue, 7 Mar 2023 10:06:34 -0800
Subject: [PATCH] bpftool: Continue iterating if individual map operations fail
If a call to bpf_map_get_fd_by_id or bpf_map_get_info_by_fd fails,
the current behavior is to bail out of the loop, which means no
other maps can be displayed or modified. With this change, the loop
will continue, so an error with one map will not affect the others.
---
src/map.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/map.c b/src/map.c
index aaeb893..17074c1 100644
--- a/src/map.c
+++ b/src/map.c
@@ -705,14 +705,14 @@ static int do_show(int argc, char **argv)
continue;
p_err("can't get map by id (%u): %s",
id, strerror(errno));
- break;
+ continue;
}
err = bpf_map_get_info_by_fd(fd, &info, &len);
if (err) {
p_err("can't get map info: %s", strerror(errno));
close(fd);
- break;
+ continue;
}
if (json_output)
--
2.34.1
Thank you,
Rae
From: Quentin Monnet <quentin@xxxxxxxxxxxxx>
Date: Tuesday, March 7, 2023 at 1:41 AM
To: Rae Marks <Raeanne.Marks@xxxxxxxxxxxxx>, bpf@xxxxxxxxxxxxxxx <bpf@xxxxxxxxxxxxxxx>
Cc: Leonid Liansky <lliansky@xxxxxxxxxxxxx>
Subject: [EXTERNAL] Re: Suggested patch for bpftool
[You don't often get email from quentin@xxxxxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
2023-03-06 21:46 UTC+0000 ~ Rae Marks <Raeanne.Marks@xxxxxxxxxxxxx>
> Hello,
>
> Thank you for your work on bpftool, a great resource.
>
> I have a legitimate reason why bpftool might fail to open an individual map to dump its information. I would like to submit a patch so that it does not bail from the loop iterating over all maps on the first map error. With this change, one map failing to open would not affect showing information about other maps. Specifically, I’d like to change this line (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flibbpf%2Fbpftool%2Fblob%2Fmaster%2Fsrc%2Fmap.c%23L699&data=05%7C01%7CRaeanne.Marks%40microsoft.com%7C851d494f2eb6471a13d408db1ef029a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137789073658744%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=eFnU5gwT%2Fc5Zbk7mYd5cbUnTeMYz2ee6Y%2B8mXjzofJo%3D&reserved=0) to be a continue rather than a break.
>
> How can I submit a patch if you approve of this suggestion? I see that the GitHub mirror of libbpf/bpftool is not the appropriate place, according to the README.
>
> Thank you!
>
> Rae Marks
> Senior Software Engineer | Microsoft
>
Hi Rae,
Thanks for reaching out.
Let me start with how to submit. The GitHub repository is just a mirror
indeed, the sources of bpftool are maintained as part of the Linux
kernel repository. This means that patches should be sent to this
mailing list, please refer to [0] for more details.
Regarding the patch that you propose, I'd welcome a way to keep
iterating if we fail to retrieve the id for the following map, but we
can't just "continue" in the loop if we don't have the id from
bpf_map_get_next_id(): If we don't get a valid id, we can't reuse it on
the following loop iteration to fetch the id of the following map. We
would have to start again with a null id and to loop over all maps again.
Can I ask in what context you saw bpftool stop before printing all the maps?
[0] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.kernel.org%2Fbpf%2Fbpf_devel_QA.html%23submitting-patches&data=05%7C01%7CRaeanne.Marks%40microsoft.com%7C851d494f2eb6471a13d408db1ef029a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137789073658744%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=LVZMI%2Be4och7tG%2BG7JuW6xuWwgoXNHgZjkiT%2B57rGNk%3D&reserved=0
Thanks,
Quentin
