* [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
@ 2023-04-25 0:01 Stanislav Fomichev
2023-04-25 1:56 ` Yonghong Song
0 siblings, 1 reply; 9+ messages in thread
From: Stanislav Fomichev @ 2023-04-25 0:01 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, Peng Wei
From: Peng Wei <pengweiprc@google.com>
Compiling C++ BPF programs with existing bpf_helper_defs.h is not
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.
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@google.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
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('')
###############################################################################
--
2.40.0.634.g4ca3ef3211-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
2023-04-25 0:01 [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly Stanislav Fomichev
@ 2023-04-25 1:56 ` Yonghong Song
2023-04-25 17:04 ` Stanislav Fomichev
0 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2023-04-25 1:56 UTC (permalink / raw)
To: Stanislav Fomichev, bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, Peng Wei
On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
> From: Peng Wei <pengweiprc@google.com>
>
> Compiling C++ BPF programs with existing bpf_helper_defs.h is not
> 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?
>
> 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@google.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> 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('')
>
> ###############################################################################
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
2023-04-25 1:56 ` Yonghong Song
@ 2023-04-25 17:04 ` Stanislav Fomichev
2023-04-25 18:10 ` Yonghong Song
0 siblings, 1 reply; 9+ messages in thread
From: Stanislav Fomichev @ 2023-04-25 17:04 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, Peng Wei
On Mon, Apr 24, 2023 at 6:56 PM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
> > From: Peng Wei <pengweiprc@google.com>
> >
> > Compiling C++ BPF programs with existing bpf_helper_defs.h is not
> > 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
> >
> > 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@google.com>
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > 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('')
> >
> > ###############################################################################
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
2023-04-25 17:04 ` Stanislav Fomichev
@ 2023-04-25 18:10 ` Yonghong Song
2023-04-25 18:22 ` Stanislav Fomichev
0 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2023-04-25 18:10 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, Peng Wei
On 4/25/23 10:04 AM, Stanislav Fomichev wrote:
> On Mon, Apr 24, 2023 at 6:56 PM Yonghong Song <yhs@meta.com> wrote:
>>
>>
>>
>> On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
>>> From: Peng Wei <pengweiprc@google.com>
>>>
>>> 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++.
>>> 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.
>
>
>>>
>>> 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@google.com>
>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>>> ---
>>> 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('')
>>>
>>> ###############################################################################
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
2023-04-25 18:10 ` Yonghong Song
@ 2023-04-25 18:22 ` Stanislav Fomichev
2023-04-25 18:29 ` Yonghong Song
0 siblings, 1 reply; 9+ messages in thread
From: Stanislav Fomichev @ 2023-04-25 18:22 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, Peng Wei
On Tue, Apr 25, 2023 at 11:10 AM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> On 4/25/23 10:04 AM, Stanislav Fomichev wrote:
> > On Mon, Apr 24, 2023 at 6:56 PM Yonghong Song <yhs@meta.com> wrote:
> >>
> >>
> >>
> >> On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
> >>> From: Peng Wei <pengweiprc@google.com>
> >>>
> >>> 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?
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@google.com>
> >>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> >>> ---
> >>> 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('')
> >>>
> >>> ###############################################################################
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
2023-04-25 18:22 ` Stanislav Fomichev
@ 2023-04-25 18:29 ` Yonghong Song
2023-04-25 18:35 ` Stanislav Fomichev
0 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2023-04-25 18:29 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, Peng Wei
On 4/25/23 11:22 AM, Stanislav Fomichev wrote:
> On Tue, Apr 25, 2023 at 11:10 AM Yonghong Song <yhs@meta.com> wrote:
>>
>>
>>
>> On 4/25/23 10:04 AM, Stanislav Fomichev wrote:
>>> On Mon, Apr 24, 2023 at 6:56 PM Yonghong Song <yhs@meta.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
>>>>> From: Peng Wei <pengweiprc@google.com>
>>>>>
>>>>> 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.
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@google.com>
>>>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>>>>> ---
>>>>> 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('')
>>>>>
>>>>> ###############################################################################
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
2023-04-25 18:29 ` Yonghong Song
@ 2023-04-25 18:35 ` Stanislav Fomichev
2023-04-25 19:43 ` Yonghong Song
0 siblings, 1 reply; 9+ messages in thread
From: Stanislav Fomichev @ 2023-04-25 18:35 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, Peng Wei
On Tue, Apr 25, 2023 at 11:29 AM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> On 4/25/23 11:22 AM, Stanislav Fomichev wrote:
> > On Tue, Apr 25, 2023 at 11:10 AM Yonghong Song <yhs@meta.com> wrote:
> >>
> >>
> >>
> >> On 4/25/23 10:04 AM, Stanislav Fomichev wrote:
> >>> On Mon, Apr 24, 2023 at 6:56 PM Yonghong Song <yhs@meta.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
> >>>>> From: Peng Wei <pengweiprc@google.com>
> >>>>>
> >>>>> 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?
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@google.com>
> >>>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> >>>>> ---
> >>>>> 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('')
> >>>>>
> >>>>> ###############################################################################
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
2023-04-25 18:35 ` Stanislav Fomichev
@ 2023-04-25 19:43 ` Yonghong Song
2023-04-26 15:52 ` Stanislav Fomichev
0 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2023-04-25 19:43 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, Peng Wei
On 4/25/23 11:35 AM, Stanislav Fomichev wrote:
> On Tue, Apr 25, 2023 at 11:29 AM Yonghong Song <yhs@meta.com> wrote:
>>
>>
>>
>> On 4/25/23 11:22 AM, Stanislav Fomichev wrote:
>>> On Tue, Apr 25, 2023 at 11:10 AM Yonghong Song <yhs@meta.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/25/23 10:04 AM, Stanislav Fomichev wrote:
>>>>> On Mon, Apr 24, 2023 at 6:56 PM Yonghong Song <yhs@meta.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
>>>>>>> From: Peng Wei <pengweiprc@google.com>
>>>>>>>
>>>>>>> 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.
> 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@google.com>
>>>>>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>>>>>>> ---
>>>>>>> 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('')
>>>>>>>
>>>>>>> ###############################################################################
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly
2023-04-25 19:43 ` Yonghong Song
@ 2023-04-26 15:52 ` Stanislav Fomichev
0 siblings, 0 replies; 9+ messages in thread
From: Stanislav Fomichev @ 2023-04-26 15:52 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, Peng Wei
On Tue, Apr 25, 2023 at 12:43 PM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> On 4/25/23 11:35 AM, Stanislav Fomichev wrote:
> > On Tue, Apr 25, 2023 at 11:29 AM Yonghong Song <yhs@meta.com> wrote:
> >>
> >>
> >>
> >> On 4/25/23 11:22 AM, Stanislav Fomichev wrote:
> >>> On Tue, Apr 25, 2023 at 11:10 AM Yonghong Song <yhs@meta.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 4/25/23 10:04 AM, Stanislav Fomichev wrote:
> >>>>> On Mon, Apr 24, 2023 at 6:56 PM Yonghong Song <yhs@meta.com> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 4/24/23 5:01 PM, Stanislav Fomichev wrote:
> >>>>>>> From: Peng Wei <pengweiprc@google.com>
> >>>>>>>
> >>>>>>> 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@google.com>
> >>>>>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> >>>>>>> ---
> >>>>>>> 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('')
> >>>>>>>
> >>>>>>> ###############################################################################
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-04-26 15:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-25 0:01 [PATCH bpf-next] bpf: Make bpf_helper_defs.h c++ friendly Stanislav Fomichev
2023-04-25 1:56 ` Yonghong Song
2023-04-25 17:04 ` Stanislav Fomichev
2023-04-25 18:10 ` Yonghong Song
2023-04-25 18:22 ` Stanislav Fomichev
2023-04-25 18:29 ` Yonghong Song
2023-04-25 18:35 ` Stanislav Fomichev
2023-04-25 19:43 ` Yonghong Song
2023-04-26 15:52 ` Stanislav Fomichev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.