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).
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?
>
> >
> >>
> >>> +
> >>> + err = walk_st_ops_shadow_vars(btf, ident, map);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + printf("\t\t} *%s;\n", ident);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
