On Tue, Apr 25, 2023 at 12:43 PM Yonghong Song <yhs@xxxxxxxx> wrote:
>
>
>
> On 4/25/23 11:35 AM, Stanislav Fomichev wrote:
> > On Tue, Apr 25, 2023 at 11:29 AM Yonghong Song <yhs@xxxxxxxx> wrote:
> >>
> >>
> >>
> >> On 4/25/23 11:22 AM, Stanislav Fomichev wrote:
> >>> On Tue, Apr 25, 2023 at 11:10 AM Yonghong Song <yhs@xxxxxxxx> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 4/25/23 10:04 AM, Stanislav Fomichev wrote:
> >>>>> On Mon, Apr 24, 2023 at 6:56 PM Yonghong Song <yhs@xxxxxxxx> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
> >>>>>>> From: Peng Wei <pengweiprc@xxxxxxxxxx>
> >>>>>>>
> >>>>>>> Compiling C++ BPF programs with existing bpf_helper_defs.h is not
> >>>>
> >>>> Just curious, why you want to compile BPF programs with C++?
> >>>> The patch looks good to me. But it would be great to know
> >>>> some reasoning since a lot of stuff, e.g., some CORE related
> >>>> intrinsics, not available for C++.
> >>>
> >>> Can you share more? What's not available? Any pointers to the docs maybe?
> >>
> >> Sorry, it is an attribute, instead of instrinsics.
> >>
> >> The attribute preserve_access_index/btf_type_tag/btf_decl_tag are all C
> >> only.
> >
> > Interesting, thanks! I don't think we use btf_type_tag/btf_decl_tag in
> > the program we want to try c++, but losing preserve_access_index might
> > be unfortunate :-( But we'll see..
> > Btw, any reason these are explicitly opted out from c++? Doesn't seem
> > like there is anything c-specific in them?
>
> Initial use case is C only. If we say to support C++, we will
> need to add attribute processing codes in various other places
> (member functions, templates, other c++ constructs, etc.)
> to convert these attributes to proper debuginfo. There are no use
> cases for this, so we didn't do it in the first place.
I see. In this case, let me respin the patch as is with the clang++
command to reproduce.
We'll experiment with our program internally to see whether these are
the showstoppers.
> > The c++ we are talking about here is mostly "c with classes +
> > templates"; no polymorphism / inheritance.
> >
> >> In llvm-project/clang/include/clang/Basic/Attr.td:
> >>
> >> def BPFPreserveAccessIndex : InheritableAttr,
> >> TargetSpecificAttr<TargetBPF> {
> >> let Spellings = [Clang<"preserve_access_index">];
> >> let Subjects = SubjectList<[Record], ErrorDiag>;
> >> let Documentation = [BPFPreserveAccessIndexDocs];
> >> let LangOpts = [COnly];
> >> }
> >>
> >> def BTFDeclTag : InheritableAttr {
> >> let Spellings = [Clang<"btf_decl_tag">];
> >> let Args = [StringArgument<"BTFDeclTag">];
> >> let Subjects = SubjectList<[Var, Function, Record, Field, TypedefName],
> >> ErrorDiag>;
> >> let Documentation = [BTFDeclTagDocs];
> >> let LangOpts = [COnly];
> >> }
> >>
> >> def BTFTypeTag : TypeAttr {
> >> let Spellings = [Clang<"btf_type_tag">];
> >> let Args = [StringArgument<"BTFTypeTag">];
> >> let Documentation = [BTFTypeTagDocs];
> >> let LangOpts = [COnly];
> >> }
> >>
> >>
> >>
> >>>
> >>> People here want to try to use c++ to see if templating helps with v4
> >>> vs v6 handling.
> >>> We have a bunch of copy-paste around this place and would like to see
> >>> whether c++ could make it a bit more readable.
> >>>
> >>>>>>> possible due to stricter C++ type conversions. C++ complains
> >>>>>>> about (void *) type conversions:
> >>>>>>>
> >>>>>>> bpf_helper_defs.h:57:67: error: invalid conversion from ‘void*’ to ‘void* (*)(void*, const void*)’ [-fpermissive]
> >>>>>>> 57 | static void *(*bpf_map_lookup_elem)(void *map, const void *key) = (void *) 1;
> >>>>>>> | ^~~~~~~~~~
> >>>>>>> | |
> >>>>>>> | void*
> >>>>>>>
> >>>>>>> Extend bpf_doc.py to use proper function type instead of void.
> >>>>>>
> >>>>>> Could you specify what exactly the compilation command triggering the
> >>>>>> above error?
> >>>>>
> >>>>> The following does it for me:
> >>>>> clang++ --include linux/types.h ./tools/lib/bpf/bpf_helper_defs.h
> >>>>
> >>>> Thanks. It would be good if you add the above compilation command
> >>>> in the commit message.
> >>>
> >>> Sure, will add.
> >>>
> >>>>>
> >>>>>
> >>>>>>>
> >>>>>>> Before:
> >>>>>>> static void *(*bpf_map_lookup_elem)(void *map, const void *key) = (void *) 1;
> >>>>>>>
> >>>>>>> After:
> >>>>>>> static void *(*bpf_map_lookup_elem)(void *map, const void *key) = (void *(*)(void *map, const void *key)) 1;
> >>>>>>>
> >>>>>>> Signed-off-by: Peng Wei <pengweiprc@xxxxxxxxxx>
> >>>>>>> Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
> >>>>>>> ---
> >>>>>>> scripts/bpf_doc.py | 7 ++++++-
> >>>>>>> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
> >>>>>>> index eaae2ce78381..fa21137a90e7 100755
> >>>>>>> --- a/scripts/bpf_doc.py
> >>>>>>> +++ b/scripts/bpf_doc.py
> >>>>>>> @@ -827,6 +827,9 @@ COMMANDS
> >>>>>>> print(' *{}{}'.format(' \t' if line else '', line))
> >>>>>>>
> >>>>>>> print(' */')
> >>>>>>> + fptr_type = '%s%s(*)(' % (
> >>>>>>> + self.map_type(proto['ret_type']),
> >>>>>>> + ((' ' + proto['ret_star']) if proto['ret_star'] else ''))
> >>>>>>> print('static %s %s(*%s)(' % (self.map_type(proto['ret_type']),
> >>>>>>> proto['ret_star'], proto['name']), end='')
> >>>>>>> comma = ''
> >>>>>>> @@ -845,8 +848,10 @@ COMMANDS
> >>>>>>> one_arg += '{}'.format(n)
> >>>>>>> comma = ', '
> >>>>>>> print(one_arg, end='')
> >>>>>>> + fptr_type += one_arg
> >>>>>>>
> >>>>>>> - print(') = (void *) %d;' % helper.enum_val)
> >>>>>>> + fptr_type += ')'
> >>>>>>> + print(') = (%s) %d;' % (fptr_type, helper.enum_val))
> >>>>>>> print('')
> >>>>>>>
> >>>>>>> ###############################################################################
