On Tue, Mar 12, 2024 at 5:37 PM Kui-Feng Lee <sinquersw@xxxxxxxxx> wrote:
>
>
>
> On 3/12/24 17:27, Andrii Nakryiko wrote:
> > On Tue, Mar 12, 2024 at 5:08 PM Kui-Feng Lee <sinquersw@xxxxxxxxx> wrote:
> >>
> >>
> >>
> >> On 3/12/24 15:47, Andrii Nakryiko wrote:
> >>> On Mon, Mar 11, 2024 at 6:38 PM Kui-Feng Lee <thinker.li@xxxxxxxxx> wrote:
> >>>>
> >>>> According to a report, skeletons fail to assign shadow pointers when being
> >>>> compiled with C++ programs. Unlike C doing implicit casting for void
> >>>> pointers, C++ requires an explicit casting.
> >>>>
> >>>> To support C++, we do explicit casting for each shadow pointer.
> >>>>
> >>>> Cc: yhs@xxxxxxxx
> >>>> Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
> >>>> ---
> >>>> tools/bpf/bpftool/gen.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> >>>> index 4fa4ade1ce74..dedafea0c127 100644
> >>>> --- a/tools/bpf/bpftool/gen.c
> >>>> +++ b/tools/bpf/bpftool/gen.c
> >>>> @@ -1131,7 +1131,7 @@ static void gen_st_ops_shadow_init(struct btf *btf, struct bpf_object *obj)
> >>>> continue;
> >>>> codegen("\
> >>>> \n\
> >>>> - obj->struct_ops.%1$s = bpf_map__initial_value(obj->maps.%1$s, NULL);\n\
> >>>> + obj->struct_ops.%1$s = (typeof(obj->struct_ops.%1$s))bpf_map__initial_value(obj->maps.%1$s, NULL);\n\
> >>>
> >>> Given we have a named struct type for this and we use explicit type
> >>> names in other parts of generated skeleton code, let's maybe use
> >>> "struct %s__%s__%s" explicitly here (passing in obj_name, ident,
> >>> type_name)?
> >>
> >> I have considered about this solution. But, C++ works differently. It
> >> has nested namespaces. That means it should be referred as
> >> "XXX_skeleton::OOO_st_ops_map" in C++. Then, we need #if #else #endif
> >> directives to provide two separated casting.
> >>
> >
> > we cast to (struct <skeleton> *) by name of the skeleton, so it should
> > be fine, I don't see why we'd need to do something C++ specific here
>
> The skeleton looks like
>
> struct struct_ops_module {
> ......
> struct {
> struct
> struct_ops_module__testmod_zeroed__bpf_testmod_ops___zeroed {
> ....
> } testmod_zeroed;
> } struct_ops;
> };
>
> struct struct_ops_module__testmod_zeroed__bpf_testmod_ops___zeroed is
> inside of struct struct_ops_module. In C++, it should be referred as
> "struct_ops_module::struct_ops_module__testmod_zeroed__bpf_testmod_ops___zeroed".
ah, makes sense, thanks for elaborating
>
> The other option is moving definitions of these types to the top scope.
no, it's fine the way you did it in this patch, I'll land it once
bpf-next tree is open for new patches, thanks
>
> >
> >>>
> >>> No strong preferences, but feels like a consistent approach here would be nice.
> >>>
> >>>> \n\
> >>>> ", ident);
> >>>> }
> >>>> --
> >>>> 2.34.1
> >>>>
