* [PATCH bpf-next] Support load time binding of bpf-helpers.
@ 2022-03-07 23:48 Alan Jowett
2022-03-08 23:49 ` Andrii Nakryiko
0 siblings, 1 reply; 4+ messages in thread
From: Alan Jowett @ 2022-03-07 23:48 UTC (permalink / raw)
To: bpf
BPF helper function names are common across different platforms, but the
id assigned to helper functions may vary. This change provides an option
to generate eBPF ELF files with relocations for helper functions which
permits resolving helper function names to ids at load time instead of
at compile time.
Add a level of indirection to bpf_doc.py (and generated bpf_helper_defs.h)
to permit compile time generation of bpf-helper function declarations to
facilitating late binding of bpf-helpers to helper id for cases where
different platforms use different helper ids, but the same helper
functions.
Example use case would be:
"#define BPF_HELPER(return_type, name, args, id) \
extern return_type name args"
To generate:
extern void * bpf_map_lookup_elem (void *map, const void *key);
Instead of:
static void *(*bpf_map_lookup_elem) (void *map, const void *key) \
= (void*) 1;
This would result in the bpf-helpers having external linkage and permit
late binding of BPF programs to helper function ids.
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
---
scripts/bpf_helpers_doc.py | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
index 867ada23281c..442b5e87687e 100755
--- a/scripts/bpf_helpers_doc.py
+++ b/scripts/bpf_helpers_doc.py
@@ -519,6 +519,10 @@ class PrinterHelpers(Printer):
for fwd in self.type_fwds:
print('%s;' % fwd)
print('')
+ print('#ifndef BPF_HELPER')
+ print('#define BPF_HELPER(return_type, name, args, id) static return_type(*name) args = (void*) id')
+ print('#endif')
+ print('')
def print_footer(self):
footer = ''
@@ -558,7 +562,7 @@ class PrinterHelpers(Printer):
print(' *{}{}'.format(' \t' if line else '', line))
print(' */')
- print('static %s %s(*%s)(' % (self.map_type(proto['ret_type']),
+ print('BPF_HELPER(%s%s, %s, (' % (self.map_type(proto['ret_type']),
proto['ret_star'], proto['name']), end='')
comma = ''
for i, a in enumerate(proto['args']):
@@ -577,7 +581,7 @@ class PrinterHelpers(Printer):
comma = ', '
print(one_arg, end='')
- print(') = (void *) %d;' % len(self.seen_helpers))
+ print('), %d);' % len(self.seen_helpers))
print('')
###############################################################################
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] Support load time binding of bpf-helpers.
2022-03-07 23:48 [PATCH bpf-next] Support load time binding of bpf-helpers Alan Jowett
@ 2022-03-08 23:49 ` Andrii Nakryiko
2022-03-11 18:37 ` [EXTERNAL] " Alan Jowett
0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2022-03-08 23:49 UTC (permalink / raw)
To: Alan Jowett; +Cc: bpf
On Tue, Mar 8, 2022 at 9:20 AM Alan Jowett <Alan.Jowett@microsoft.com> wrote:
>
> BPF helper function names are common across different platforms, but the
> id assigned to helper functions may vary. This change provides an option
> to generate eBPF ELF files with relocations for helper functions which
> permits resolving helper function names to ids at load time instead of
> at compile time.
>
> Add a level of indirection to bpf_doc.py (and generated bpf_helper_defs.h)
> to permit compile time generation of bpf-helper function declarations to
> facilitating late binding of bpf-helpers to helper id for cases where
> different platforms use different helper ids, but the same helper
> functions.
>
> Example use case would be:
> "#define BPF_HELPER(return_type, name, args, id) \
> extern return_type name args"
>
> To generate:
> extern void * bpf_map_lookup_elem (void *map, const void *key);
>
> Instead of:
> static void *(*bpf_map_lookup_elem) (void *map, const void *key) \
> = (void*) 1;
>
> This would result in the bpf-helpers having external linkage and permit
> late binding of BPF programs to helper function ids.
>
Ugh...
BPF_HELPER(void*, bpf_map_lookup_elem, (void *map, const void *key), 1);
Looks pretty awful :(
But I also have the question about why different platforms should have
different IDs for the same subset of BPF helpers? Wouldn't it be
better to preserve IDs across platforms to simplify cross-platform BPF
object files?
We can probably also define some range for platform-specific BPF
helpers starting from some high ID?
> Signed-off-by: Alan Jowett <alanjo@microsoft.com>
> ---
> scripts/bpf_helpers_doc.py | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
> index 867ada23281c..442b5e87687e 100755
> --- a/scripts/bpf_helpers_doc.py
> +++ b/scripts/bpf_helpers_doc.py
> @@ -519,6 +519,10 @@ class PrinterHelpers(Printer):
> for fwd in self.type_fwds:
> print('%s;' % fwd)
> print('')
> + print('#ifndef BPF_HELPER')
> + print('#define BPF_HELPER(return_type, name, args, id) static return_type(*name) args = (void*) id')
> + print('#endif')
> + print('')
>
> def print_footer(self):
> footer = ''
> @@ -558,7 +562,7 @@ class PrinterHelpers(Printer):
> print(' *{}{}'.format(' \t' if line else '', line))
>
> print(' */')
> - print('static %s %s(*%s)(' % (self.map_type(proto['ret_type']),
> + print('BPF_HELPER(%s%s, %s, (' % (self.map_type(proto['ret_type']),
> proto['ret_star'], proto['name']), end='')
> comma = ''
> for i, a in enumerate(proto['args']):
> @@ -577,7 +581,7 @@ class PrinterHelpers(Printer):
> comma = ', '
> print(one_arg, end='')
>
> - print(') = (void *) %d;' % len(self.seen_helpers))
> + print('), %d);' % len(self.seen_helpers))
> print('')
>
> ###############################################################################
> --
> 2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [EXTERNAL] Re: [PATCH bpf-next] Support load time binding of bpf-helpers.
2022-03-08 23:49 ` Andrii Nakryiko
@ 2022-03-11 18:37 ` Alan Jowett
2022-03-21 22:23 ` Andrii Nakryiko
0 siblings, 1 reply; 4+ messages in thread
From: Alan Jowett @ 2022-03-11 18:37 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf
Thanks for the feedback, I appreciate it. Any suggestions on how to make the output more visually appealing?
If there was a IANA equivalent for assigning helper function id's then it might be possible to support having the same IDs on multiple platforms, but as it exists today the various platforms are developed separately, with no simple way to co-ordinate. The benefit of using symbols over numeric id's is that it reduces the possibility of identifier collision, where different platforms might define helper ID # to mean different things, but it's less likely that different platforms will define bpf_foo_bar to have different semantics.
Conceptually, this is like the relocations done using BTF data for CORE, where the offsets into structures can vary depending on the kernel version. In this scenario the helper IDs can vary with the relocation being done based on symbols stored in the ELF file.
Regards,
Alan Jowett
-----Original Message-----
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Sent: Tuesday, March 8, 2022 4:49 PM
To: Alan Jowett <Alan.Jowett@microsoft.com>
Cc: bpf@vger.kernel.org
Subject: [EXTERNAL] Re: [PATCH bpf-next] Support load time binding of bpf-helpers.
On Tue, Mar 8, 2022 at 9:20 AM Alan Jowett <Alan.Jowett@microsoft.com> wrote:
>
> BPF helper function names are common across different platforms, but
> the id assigned to helper functions may vary. This change provides an
> option to generate eBPF ELF files with relocations for helper
> functions which permits resolving helper function names to ids at load
> time instead of at compile time.
>
> Add a level of indirection to bpf_doc.py (and generated
> bpf_helper_defs.h) to permit compile time generation of bpf-helper
> function declarations to facilitating late binding of bpf-helpers to
> helper id for cases where different platforms use different helper
> ids, but the same helper functions.
>
> Example use case would be:
> "#define BPF_HELPER(return_type, name, args, id) \
> extern return_type name args"
>
> To generate:
> extern void * bpf_map_lookup_elem (void *map, const void *key);
>
> Instead of:
> static void *(*bpf_map_lookup_elem) (void *map, const void *key) \
> = (void*) 1;
>
> This would result in the bpf-helpers having external linkage and
> permit late binding of BPF programs to helper function ids.
>
Ugh...
BPF_HELPER(void*, bpf_map_lookup_elem, (void *map, const void *key), 1);
Looks pretty awful :(
But I also have the question about why different platforms should have different IDs for the same subset of BPF helpers? Wouldn't it be better to preserve IDs across platforms to simplify cross-platform BPF object files?
We can probably also define some range for platform-specific BPF helpers starting from some high ID?
> Signed-off-by: Alan Jowett <alanjo@microsoft.com>
> ---
> scripts/bpf_helpers_doc.py | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
> index 867ada23281c..442b5e87687e 100755
> --- a/scripts/bpf_helpers_doc.py
> +++ b/scripts/bpf_helpers_doc.py
> @@ -519,6 +519,10 @@ class PrinterHelpers(Printer):
> for fwd in self.type_fwds:
> print('%s;' % fwd)
> print('')
> + print('#ifndef BPF_HELPER')
> + print('#define BPF_HELPER(return_type, name, args, id) static return_type(*name) args = (void*) id')
> + print('#endif')
> + print('')
>
> def print_footer(self):
> footer = ''
> @@ -558,7 +562,7 @@ class PrinterHelpers(Printer):
> print(' *{}{}'.format(' \t' if line else '', line))
>
> print(' */')
> - print('static %s %s(*%s)(' % (self.map_type(proto['ret_type']),
> + print('BPF_HELPER(%s%s, %s, (' %
> + (self.map_type(proto['ret_type']),
> proto['ret_star'], proto['name']), end='')
> comma = ''
> for i, a in enumerate(proto['args']):
> @@ -577,7 +581,7 @@ class PrinterHelpers(Printer):
> comma = ', '
> print(one_arg, end='')
>
> - print(') = (void *) %d;' % len(self.seen_helpers))
> + print('), %d);' % len(self.seen_helpers))
> print('')
>
>
> ######################################################################
> #########
> --
> 2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [EXTERNAL] Re: [PATCH bpf-next] Support load time binding of bpf-helpers.
2022-03-11 18:37 ` [EXTERNAL] " Alan Jowett
@ 2022-03-21 22:23 ` Andrii Nakryiko
0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2022-03-21 22:23 UTC (permalink / raw)
To: Alan Jowett; +Cc: bpf
On Fri, Mar 11, 2022 at 10:37 AM Alan Jowett <Alan.Jowett@microsoft.com> wrote:
>
> Thanks for the feedback, I appreciate it. Any suggestions on how to make the output more visually appealing?
>
> If there was a IANA equivalent for assigning helper function id's then it might be possible to support having the same IDs on multiple platforms, but as it exists today the various platforms are developed separately, with no simple way to co-ordinate. The benefit of using symbols over numeric id's is that it reduces the possibility of identifier collision, where different platforms might define helper ID # to mean different things, but it's less likely that different platforms will define bpf_foo_bar to have different semantics.
>
> Conceptually, this is like the relocations done using BTF data for CORE, where the offsets into structures can vary depending on the kernel version. In this scenario the helper IDs can vary with the relocation being done based on symbols stored in the ELF file.
Please don't top post, reply inline instead.
We have a BPF Foundation and BPF Steering Committee for coordinating
things like this across different platforms. Adding extra relocations
just for BPF helper IDs seems to go in the wrong direction from
minimizing the amount of runtime BPF instruction adjustments (to
simplify things like BPF signing, etc).
>
> Regards,
> Alan Jowett
>
> -----Original Message-----
> From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Sent: Tuesday, March 8, 2022 4:49 PM
> To: Alan Jowett <Alan.Jowett@microsoft.com>
> Cc: bpf@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH bpf-next] Support load time binding of bpf-helpers.
>
> On Tue, Mar 8, 2022 at 9:20 AM Alan Jowett <Alan.Jowett@microsoft.com> wrote:
> >
> > BPF helper function names are common across different platforms, but
> > the id assigned to helper functions may vary. This change provides an
> > option to generate eBPF ELF files with relocations for helper
> > functions which permits resolving helper function names to ids at load
> > time instead of at compile time.
> >
> > Add a level of indirection to bpf_doc.py (and generated
> > bpf_helper_defs.h) to permit compile time generation of bpf-helper
> > function declarations to facilitating late binding of bpf-helpers to
> > helper id for cases where different platforms use different helper
> > ids, but the same helper functions.
> >
> > Example use case would be:
> > "#define BPF_HELPER(return_type, name, args, id) \
> > extern return_type name args"
> >
> > To generate:
> > extern void * bpf_map_lookup_elem (void *map, const void *key);
> >
> > Instead of:
> > static void *(*bpf_map_lookup_elem) (void *map, const void *key) \
> > = (void*) 1;
> >
> > This would result in the bpf-helpers having external linkage and
> > permit late binding of BPF programs to helper function ids.
> >
>
> Ugh...
>
> BPF_HELPER(void*, bpf_map_lookup_elem, (void *map, const void *key), 1);
>
> Looks pretty awful :(
>
> But I also have the question about why different platforms should have different IDs for the same subset of BPF helpers? Wouldn't it be better to preserve IDs across platforms to simplify cross-platform BPF object files?
>
> We can probably also define some range for platform-specific BPF helpers starting from some high ID?
>
> > Signed-off-by: Alan Jowett <alanjo@microsoft.com>
> > ---
> > scripts/bpf_helpers_doc.py | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
> > index 867ada23281c..442b5e87687e 100755
> > --- a/scripts/bpf_helpers_doc.py
> > +++ b/scripts/bpf_helpers_doc.py
> > @@ -519,6 +519,10 @@ class PrinterHelpers(Printer):
> > for fwd in self.type_fwds:
> > print('%s;' % fwd)
> > print('')
> > + print('#ifndef BPF_HELPER')
> > + print('#define BPF_HELPER(return_type, name, args, id) static return_type(*name) args = (void*) id')
> > + print('#endif')
> > + print('')
> >
> > def print_footer(self):
> > footer = ''
> > @@ -558,7 +562,7 @@ class PrinterHelpers(Printer):
> > print(' *{}{}'.format(' \t' if line else '', line))
> >
> > print(' */')
> > - print('static %s %s(*%s)(' % (self.map_type(proto['ret_type']),
> > + print('BPF_HELPER(%s%s, %s, (' %
> > + (self.map_type(proto['ret_type']),
> > proto['ret_star'], proto['name']), end='')
> > comma = ''
> > for i, a in enumerate(proto['args']):
> > @@ -577,7 +581,7 @@ class PrinterHelpers(Printer):
> > comma = ', '
> > print(one_arg, end='')
> >
> > - print(') = (void *) %d;' % len(self.seen_helpers))
> > + print('), %d);' % len(self.seen_helpers))
> > print('')
> >
> >
> > ######################################################################
> > #########
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-21 22:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 23:48 [PATCH bpf-next] Support load time binding of bpf-helpers Alan Jowett
2022-03-08 23:49 ` Andrii Nakryiko
2022-03-11 18:37 ` [EXTERNAL] " Alan Jowett
2022-03-21 22:23 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).