From: Joe Burton <[email protected]>
Add the capability to set a `struct bpf_map` name.
bpf_map__reuse_fd(struct bpf_map *map, int fd) does the following:
1. get the bpf_map_info of the passed-in fd
2. strdup the name from the bpf_map_info
3. assign that name to the map
4. and some other stuff
While `map.name` may initially be arbitrarily long, this operation
truncates it after 15 characters.
We have some infrastructure that uses bpf_map__reuse_fd() to preserve
maps across upgrades. Some of our users have long map names, and are
seeing their maps 'disappear' after an upgrade, due to the name
truncation.
By invoking `bpf_map__set_name()` after `bpf_map__reuse_fd()`, we can
trivially work around the issue.
Signed-off-by: Joe Burton <[email protected]>
---
tools/lib/bpf/libbpf.c | 22 ++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 3 ++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 72548798126b..725baf508e6f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -9089,6 +9089,28 @@ const char *bpf_map__name(const struct bpf_map *map)
return map->name;
}
+int bpf_map__set_name(struct bpf_map *map, const char *name)
+{
+ char *new_name;
+
+ if (!map)
+ return libbpf_err(-EINVAL);
+
+ new_name = strdup(name);
+ if (!new_name)
+ return libbpf_err(-ENOMEM);
+
+ if (map_uses_real_name(map)) {
+ free(map->real_name);
+ map->real_name = new_name;
+ } else {
+ free(map->name);
+ map->name = new_name;
+ }
+
+ return 0;
+}
+
enum bpf_map_type bpf_map__type(const struct bpf_map *map)
{
return map->def.type;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e4d5353f757b..e898c4cb514a 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -731,8 +731,9 @@ LIBBPF_API bool bpf_map__autocreate(const struct bpf_map *map);
*/
LIBBPF_API int bpf_map__fd(const struct bpf_map *map);
LIBBPF_API int bpf_map__reuse_fd(struct bpf_map *map, int fd);
-/* get map name */
+/* get/set map name */
LIBBPF_API const char *bpf_map__name(const struct bpf_map *map);
+LIBBPF_API int bpf_map__set_name(struct bpf_map *map, const char *name);
/* get/set map type */
LIBBPF_API enum bpf_map_type bpf_map__type(const struct bpf_map *map);
LIBBPF_API int bpf_map__set_type(struct bpf_map *map, enum bpf_map_type type);
--
2.37.0.144.g8ac04bfd2-goog
On Wed, Jul 13, 2022 at 2:43 PM Joe Burton <[email protected]> wrote:
>
> From: Joe Burton <[email protected]>
>
> Add the capability to set a `struct bpf_map` name.
>
> bpf_map__reuse_fd(struct bpf_map *map, int fd) does the following:
>
> 1. get the bpf_map_info of the passed-in fd
> 2. strdup the name from the bpf_map_info
> 3. assign that name to the map
> 4. and some other stuff
>
> While `map.name` may initially be arbitrarily long, this operation
> truncates it after 15 characters.
>
> We have some infrastructure that uses bpf_map__reuse_fd() to preserve
> maps across upgrades. Some of our users have long map names, and are
> seeing their maps 'disappear' after an upgrade, due to the name
> truncation.
>
> By invoking `bpf_map__set_name()` after `bpf_map__reuse_fd()`, we can
> trivially work around the issue.
Asked you internally, but not sure I follow. Can you share more on why
the following won't fix it for us:
https://lore.kernel.org/bpf/OSZP286MB1725CEA1C95C5CB8E7CCC53FB8869@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
?
The idea seems to be to get the supplied map name (from the obj)
instead of using pin name? So why is it not enough?
> Signed-off-by: Joe Burton <[email protected]>
> ---
> tools/lib/bpf/libbpf.c | 22 ++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 3 ++-
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 72548798126b..725baf508e6f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9089,6 +9089,28 @@ const char *bpf_map__name(const struct bpf_map *map)
> return map->name;
> }
>
> +int bpf_map__set_name(struct bpf_map *map, const char *name)
> +{
> + char *new_name;
> +
> + if (!map)
> + return libbpf_err(-EINVAL);
> +
> + new_name = strdup(name);
> + if (!new_name)
> + return libbpf_err(-ENOMEM);
> +
> + if (map_uses_real_name(map)) {
> + free(map->real_name);
> + map->real_name = new_name;
> + } else {
> + free(map->name);
> + map->name = new_name;
> + }
> +
> + return 0;
> +}
> +
> enum bpf_map_type bpf_map__type(const struct bpf_map *map)
> {
> return map->def.type;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index e4d5353f757b..e898c4cb514a 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -731,8 +731,9 @@ LIBBPF_API bool bpf_map__autocreate(const struct bpf_map *map);
> */
> LIBBPF_API int bpf_map__fd(const struct bpf_map *map);
> LIBBPF_API int bpf_map__reuse_fd(struct bpf_map *map, int fd);
> -/* get map name */
> +/* get/set map name */
> LIBBPF_API const char *bpf_map__name(const struct bpf_map *map);
> +LIBBPF_API int bpf_map__set_name(struct bpf_map *map, const char *name);
> /* get/set map type */
> LIBBPF_API enum bpf_map_type bpf_map__type(const struct bpf_map *map);
> LIBBPF_API int bpf_map__set_type(struct bpf_map *map, enum bpf_map_type type);
> --
> 2.37.0.144.g8ac04bfd2-goog
>
> Asked you internally, but not sure I follow. Can you share more on why
> the following won't fix it for us:
>
> https://lore.kernel.org/bpf/OSZP286MB1725CEA1C95C5CB8E7CCC53FB8869@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
>
> ?
>
> The idea seems to be to get the supplied map name (from the obj)
> instead of using pin name? So why is it not enough?
You're correct, this approach also resolves the issue. No need for this
new API.
On Wed, Jul 13, 2022 at 3:32 PM Joe Burton <[email protected]> wrote:
>
> > Asked you internally, but not sure I follow. Can you share more on why
> > the following won't fix it for us:
> >
> > https://lore.kernel.org/bpf/OSZP286MB1725CEA1C95C5CB8E7CCC53FB8869@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
> >
> > ?
> >
> > The idea seems to be to get the supplied map name (from the obj)
> > instead of using pin name? So why is it not enough?
>
> You're correct, this approach also resolves the issue. No need for this
> new API.
SG! New helper might still be useful, but I'm not sure how safe that
is, given how much we use the name internally in libbpf
(name/pin_path). So it might be safer to use Anquan's approach for
now.
Andrii, any concerns with [1] ? Should we pull that in?
[1] https://lore.kernel.org/bpf/OSZP286MB1725CEA1C95C5CB8E7CCC53FB8869@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
On Wed, Jul 13, 2022 at 4:02 PM Stanislav Fomichev <[email protected]> wrote:
>
> On Wed, Jul 13, 2022 at 3:32 PM Joe Burton <[email protected]> wrote:
> >
> > > Asked you internally, but not sure I follow. Can you share more on why
> > > the following won't fix it for us:
> > >
> > > https://lore.kernel.org/bpf/OSZP286MB1725CEA1C95C5CB8E7CCC53FB8869@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/
> > >
> > > ?
> > >
> > > The idea seems to be to get the supplied map name (from the obj)
> > > instead of using pin name? So why is it not enough?
> >
> > You're correct, this approach also resolves the issue. No need for this
> > new API.
>
> SG! New helper might still be useful, but I'm not sure how safe that
> is, given how much we use the name internally in libbpf
> (name/pin_path). So it might be safer to use Anquan's approach for
> now.
>
> Andrii, any concerns with [1] ? Should we pull that in?
I already applied [1]. I'm uncomfortable with bpf_map__set_name() in
general, because map name is tied into BTF and other things, so it
needs much more careful thinking how to support sudden map renames.
But given the problem was with bpf_map__reuse_fd(), I think [1] solves
it already.
>
> [1] https://lore.kernel.org/bpf/OSZP286MB1725CEA1C95C5CB8E7CCC53FB8869@OSZP286MB1725.JPNP286.PROD.OUTLOOK.COM/