| Message ID | 20210915050943.679062-7-memxor@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Support kernel module function calls from eBPF | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 2 maintainers not CCed: john.fastabend@gmail.com kpsingh@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 3346 this patch: 3346 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 10 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 3433 this patch: 3433 |
| netdev/header_inline | success | Link |
| bpf/vmtest-bpf-next | fail | VM_Test |
On Wed, Sep 15, 2021 at 10:39:39AM +0530, Kumar Kartikeya Dwivedi wrote: > Increase the maximum stack size accessible to BPF program to 768 bytes. > This is done so that gen_loader can use 94 additional fds for kfunc BTFs > that it passes in to fd_array from the remaining space available for the > loader_stack struct to expand. > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > --- > include/linux/filter.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/filter.h b/include/linux/filter.h > index 4a93c12543ee..b214189ece62 100644 > --- a/include/linux/filter.h > +++ b/include/linux/filter.h > @@ -82,8 +82,8 @@ struct ctl_table_header; > */ > #define BPF_SYM_ELF_TYPE 't' > > -/* BPF program can access up to 512 bytes of stack space. */ > -#define MAX_BPF_STACK 512 > +/* BPF program can access up to 768 bytes of stack space. */ > +#define MAX_BPF_STACK 768 Yikes. I guess you meant as RFC, right? You didn't really propose to increase prog stack size just for that, right? In the later patch: +/* MAX_BPF_STACK is 768 bytes, so (64 + 32 + 94 (MAX_KFUNC_DESCS) + 2) * 4 */ #define MAX_USED_MAPS 64 #define MAX_USED_PROGS 32 @@ -31,6 +33,8 @@ struct loader_stack { __u32 btf_fd; __u32 map_fd[MAX_USED_MAPS]; __u32 prog_fd[MAX_USED_PROGS]; + /* Update insn->off store when reordering kfunc_btf_fd */ + __u32 kfunc_btf_fd[MAX_KFUNC_DESCS]; __u32 inner_map_fd; }; There are few other ways to do that. For example: A: rename map_fd[] into fds[] and store both map and btf FDs in there. B: move map and btf FDs into data instead of stack.
On Wed, Sep 15, 2021 at 10:03:53PM IST, Alexei Starovoitov wrote: > On Wed, Sep 15, 2021 at 10:39:39AM +0530, Kumar Kartikeya Dwivedi wrote: > > Increase the maximum stack size accessible to BPF program to 768 bytes. > > This is done so that gen_loader can use 94 additional fds for kfunc BTFs > > that it passes in to fd_array from the remaining space available for the > > loader_stack struct to expand. > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > --- > > include/linux/filter.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/filter.h b/include/linux/filter.h > > index 4a93c12543ee..b214189ece62 100644 > > --- a/include/linux/filter.h > > +++ b/include/linux/filter.h > > @@ -82,8 +82,8 @@ struct ctl_table_header; > > */ > > #define BPF_SYM_ELF_TYPE 't' > > > > -/* BPF program can access up to 512 bytes of stack space. */ > > -#define MAX_BPF_STACK 512 > > +/* BPF program can access up to 768 bytes of stack space. */ > > +#define MAX_BPF_STACK 768 > > Yikes. > I guess you meant as RFC, right? You didn't really propose > to increase prog stack size just for that, right? > Yes, and right, it's ugly :/. > In the later patch: > +/* MAX_BPF_STACK is 768 bytes, so (64 + 32 + 94 (MAX_KFUNC_DESCS) + 2) * 4 */ > #define MAX_USED_MAPS 64 > #define MAX_USED_PROGS 32 > > @@ -31,6 +33,8 @@ struct loader_stack { > __u32 btf_fd; > __u32 map_fd[MAX_USED_MAPS]; > __u32 prog_fd[MAX_USED_PROGS]; > + /* Update insn->off store when reordering kfunc_btf_fd */ > + __u32 kfunc_btf_fd[MAX_KFUNC_DESCS]; > __u32 inner_map_fd; > }; > > There are few other ways to do that. > For example: > A: rename map_fd[] into fds[] and store both map and btf FDs in there. > B: move map and btf FDs into data instead of stack. Both are great suggestions, I thought about A but not B, but it will be better (even though it requires more changes, we can do full 256 BTF fds using B). Thanks! -- Kartikeya
On Wed, Sep 15, 2021 at 10:57 AM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > On Wed, Sep 15, 2021 at 10:03:53PM IST, Alexei Starovoitov wrote: > > On Wed, Sep 15, 2021 at 10:39:39AM +0530, Kumar Kartikeya Dwivedi wrote: > > > Increase the maximum stack size accessible to BPF program to 768 bytes. > > > This is done so that gen_loader can use 94 additional fds for kfunc BTFs > > > that it passes in to fd_array from the remaining space available for the > > > loader_stack struct to expand. > > > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > > --- > > > include/linux/filter.h | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/linux/filter.h b/include/linux/filter.h > > > index 4a93c12543ee..b214189ece62 100644 > > > --- a/include/linux/filter.h > > > +++ b/include/linux/filter.h > > > @@ -82,8 +82,8 @@ struct ctl_table_header; > > > */ > > > #define BPF_SYM_ELF_TYPE 't' > > > > > > -/* BPF program can access up to 512 bytes of stack space. */ > > > -#define MAX_BPF_STACK 512 > > > +/* BPF program can access up to 768 bytes of stack space. */ > > > +#define MAX_BPF_STACK 768 > > > > Yikes. > > I guess you meant as RFC, right? You didn't really propose > > to increase prog stack size just for that, right? > > > > Yes, and right, it's ugly :/. > > > In the later patch: > > +/* MAX_BPF_STACK is 768 bytes, so (64 + 32 + 94 (MAX_KFUNC_DESCS) + 2) * 4 */ > > #define MAX_USED_MAPS 64 > > #define MAX_USED_PROGS 32 > > > > @@ -31,6 +33,8 @@ struct loader_stack { > > __u32 btf_fd; > > __u32 map_fd[MAX_USED_MAPS]; > > __u32 prog_fd[MAX_USED_PROGS]; > > + /* Update insn->off store when reordering kfunc_btf_fd */ > > + __u32 kfunc_btf_fd[MAX_KFUNC_DESCS]; > > __u32 inner_map_fd; > > }; > > > > There are few other ways to do that. > > For example: > > A: rename map_fd[] into fds[] and store both map and btf FDs in there. > > B: move map and btf FDs into data instead of stack. > > Both are great suggestions, I thought about A but not B, but it will be better > (even though it requires more changes, we can do full 256 BTF fds using B). > Thanks! btw fd_array doesn't have to have valid FDs in all slots when passed into the prog_load command. If bpf prog doesn't index into them they can have garbage. Just mentioning in case that simplifies the implementation. I suspect packing btf_fds right after map_fds with no gaps will not be hard to do, but in case it's somehow too painful there could be a gap.
diff --git a/include/linux/filter.h b/include/linux/filter.h index 4a93c12543ee..b214189ece62 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -82,8 +82,8 @@ struct ctl_table_header; */ #define BPF_SYM_ELF_TYPE 't' -/* BPF program can access up to 512 bytes of stack space. */ -#define MAX_BPF_STACK 512 +/* BPF program can access up to 768 bytes of stack space. */ +#define MAX_BPF_STACK 768 /* Helper macros for filter block array initializers. */
Increase the maximum stack size accessible to BPF program to 768 bytes. This is done so that gen_loader can use 94 additional fds for kfunc BTFs that it passes in to fd_array from the remaining space available for the loader_stack struct to expand. Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> --- include/linux/filter.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)