Re: [RFC bpf-next v2 1/3] libbpf: Create a shadow copy for each struct_ops map if necessary.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/15/24 15:55, Andrii Nakryiko wrote:
On Tue, Feb 13, 2024 at 6:08 PM <thinker.li@xxxxxxxxx> wrote:

From: Kui-Feng Lee <thinker.li@xxxxxxxxx>

If the user has passed a shadow info for a struct_ops map along with struct
bpf_object_open_opts, a shadow copy will be created for the map and
returned from bpf_map__initial_value().

The user can read and write shadow variables through the shadow copy, which
is placed in the struct pointed by skel->struct_ops.FOO, where FOO is the
map name.

The value of a shadow variable will be used to update the value of the map
when loading the map to the kernel.

Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
---
  tools/lib/bpf/libbpf.c          | 195 ++++++++++++++++++++++++++++++--
  tools/lib/bpf/libbpf.h          |  34 +++++-
  tools/lib/bpf/libbpf.map        |   1 +
  tools/lib/bpf/libbpf_internal.h |   1 +
  4 files changed, 220 insertions(+), 11 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 01f407591a92..ce9c4cdb2dc5 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -487,6 +487,14 @@ struct bpf_struct_ops {
          * from "data".
          */
         void *kern_vdata;
+       /* Description of the layout that a shadow copy should look like.
+        */
+       const struct bpf_struct_ops_map_info *shadow_info;
+       /* A shadow copy of the struct_ops data created according to the
+        * layout described by shadow_info.
+        */
+       void *shadow_data;
+       __u32 shadow_data_size;

what I mentioned on cover letter, just a few lines above, before
kern_vdata we have just `void *data` which initially contains whatever
was set in ELF. Just expose that through bpf_map__initial_value() and
teach bpftool to generate section with variables for that memory and
that should be all we need, no?

I am not sure if read your question correctly.
Padding & alignments can vary in different platforms. BPF and
user space programs are supposed to be in different platforms.
So, I can not expect that the same struct has the same layout in
BPF/x86/and ARM, right?


         __u32 type_id;
  };

@@ -1027,7 +1035,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
         struct module_btf *mod_btf;
         void *data, *kern_data;
         const char *tname;
-       int err;
+       int err, j;

         st_ops = map->st_ops;
         type = st_ops->type;

[...]

  void *bpf_map__initial_value(struct bpf_map *map, size_t *psize)
  {
+       if (bpf_map__is_struct_ops(map)) {
+               if (psize)
+                       *psize = map->st_ops->shadow_data_size;
+               return map->st_ops->shadow_data;
+       }
+
         if (!map->mmaped)
                 return NULL;
         *psize = map->def.value_size;
@@ -13462,3 +13632,8 @@ void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s)
         free(s->progs);
         free(s);
  }
+
+__u32 bpf_map__struct_ops_type(const struct bpf_map *map)
+{
+       return map->st_ops->type_id;
+}

we can expose this st_ops->type_id as map->def.value_type_id so that
existing bpf_map__btf_value_type_id() API can be used, no need to add
more struct_ops-specific APIs

OK!


diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5723cbbfcc41..b435cafefe7a 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -109,6 +109,27 @@ LIBBPF_API libbpf_print_fn_t libbpf_set_print(libbpf_print_fn_t fn);
  /* Hide internal to user */
  struct bpf_object;

+/* Description of a member in the struct_ops type for a map. */
+struct bpf_struct_ops_member_info {
+       const char *name;
+       __u32 offset;
+       __u32 size;
+};
+
+/* Description of the layout of a shadow copy for a struct_ops map. */
+struct bpf_struct_ops_map_info {
+       /* The name of the struct_ops map */
+       const char *name;
+       const struct bpf_struct_ops_member_info *members;
+       __u32 cnt;
+       __u32 data_size;
+};
+
+struct bpf_struct_ops_shadow_info {
+       const struct bpf_struct_ops_map_info *maps;
+       __u32 cnt;
+};
+
  struct bpf_object_open_opts {
         /* size of this struct, for forward/backward compatibility */
         size_t sz;
@@ -197,9 +218,18 @@ struct bpf_object_open_opts {
          */
         const char *bpf_token_path;

+       /* A list of shadow info for every struct_ops map.  A shadow info
+        * provides the information used by libbpf to map the offsets of
+        * struct members of a struct_ops type from BTF to the offsets of
+        * the corresponding members in the shadow copy in the user
+        * space. It ensures that the shadow copy provided by the libbpf
+        * can be accessed by the user space program correctly.
+        */
+       const struct bpf_struct_ops_shadow_info *struct_ops_shadow;
+

I still don't follow. bpftool will generate memory-layout compatible
structure for user-space, they can just work directly with that
memory. We shouldn't need all this extra info structs.

Libbpf can just check that fields that are supposed to be BPF prog
references are correct `struct bpf_program *` pointers.

Check the explanation above.


         size_t :0;
  };
-#define bpf_object_open_opts__last_field bpf_token_path
+#define bpf_object_open_opts__last_field struct_ops_shadow

  /**
   * @brief **bpf_object__open()** creates a bpf_object by opening
@@ -839,6 +869,8 @@ struct bpf_map;
  LIBBPF_API struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map);
  LIBBPF_API int bpf_link__update_map(struct bpf_link *link, const struct bpf_map *map);

+LIBBPF_API __u32 bpf_map__struct_ops_type(const struct bpf_map *map);
+
  struct bpf_iter_attach_opts {
         size_t sz; /* size of this struct for forward/backward compatibility */
         union bpf_iter_link_info *link_info;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 86804fd90dd1..e0efc85114df 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -413,4 +413,5 @@ LIBBPF_1.4.0 {
                 bpf_token_create;
                 btf__new_split;
                 btf_ext__raw_data;
+               bpf_map__struct_ops_type;
  } LIBBPF_1.3.0;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index ad936ac5e639..aec6d57fe5d1 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -234,6 +234,7 @@ struct btf_type;
  struct btf_type *btf_type_by_id(const struct btf *btf, __u32 type_id);
  const char *btf_kind_str(const struct btf_type *t);
  const struct btf_type *skip_mods_and_typedefs(const struct btf *btf, __u32 id, __u32 *res_id);
+const struct btf_type *resolve_func_ptr(const struct btf *btf, __u32 id, __u32 *res_id);

  static inline enum btf_func_linkage btf_func_linkage(const struct btf_type *t)
  {
--
2.34.1





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux