On 2/28/24 17:03, Andrii Nakryiko wrote:
On Wed, Feb 28, 2024 at 4:44 PM Kui-Feng Lee <sinquersw@xxxxxxxxx> wrote:
On 2/28/24 16:09, Andrii Nakryiko wrote:
On Wed, Feb 28, 2024 at 2:28 PM Kui-Feng Lee <sinquersw@xxxxxxxxx> wrote:
On 2/28/24 13:21, Kui-Feng Lee wrote:
Will fix most of issues.
On 2/28/24 10:25, Andrii Nakryiko wrote:
On Mon, Feb 26, 2024 at 5:04 PM Kui-Feng Lee <thinker.li@xxxxxxxxx>
wrote:
+ * type. Accessing them through the generated names may unintentionally
+ * corrupt data.
+ */
+static int gen_st_ops_shadow_type(struct btf *btf, const char *ident,
+ const struct bpf_map *map)
+{
+ int err;
+
+ printf("\t\tstruct {\n");
would it be useful to still name this type? E.g., if it is `struct
bpf_struct_ops_tcp_congestion_ops in vmlinux BTF` we can name this one
as <skeleton-name>__bpf_struct_ops_tcp_congestion_ops. We have a
similar pattern for bss/data/rodata sections, having names is useful.
If a user defines several struct_ops maps with the same name and type in
different files, it can cause name conflicts. Unless we also prefix the
name with the name of the skeleton. I am not sure if it is a good idea
to generate such long names. If a user want to refer to the type, he
still can use typeof(). WDYT?
I misread your words. So, you were saying to prefix the skeleton name,
not map names. It is doable.
I did say to prefix with skeleton name, but *that* actually can lead
to a conflict if you have two struct_ops maps that use the same BTF
type. On the other hand, map names are unique, they are forced to be
global symbols, so there is no way for them to conflict (it would be
link-time error).
I avoided conflicts by checking if the definition of a type is already
generated.
For example, if there are two maps with the same type, they would looks
like.
struct XXXSekelton {
...
struct {
struct struct_ops_type {
....
} *map1;
struct struct_ops_type *map2;
It's kind of non-uniform. I think we are overengineering this, let's
just do <skel>__<map>__<type> and see how it goes? No checks, no
nothing, pure string generation.
Got it
} struct_ops;
...
};
WDYT?
How about we append both skeleton name, map name, and map's underlying
BTF type? So:
<skel>__<map>__bpf_struct_ops_tcp_congestion_ops
?
Is there any problem with having a long name?
No a big problem! Just not convenient to use.
+
+ err = walk_st_ops_shadow_vars(btf, ident, map);
+ if (err)
+ return err;
+
+ printf("\t\t} *%s;\n", ident);
+
+ return 0;
+}
+
