| Message ID | 20201207200605.650192-1-revest@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v2] bpf: Only call sock_from_file with CONFIG_NET | expand |
| Context | Check | Description |
|---|---|---|
| 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/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: 33 this patch: 33 |
| 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, 11 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 33 this patch: 33 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 12/7/20 12:06 PM, Florent Revest wrote: > This avoids > ld: kernel/trace/bpf_trace.o: in function `bpf_sock_from_file': > bpf_trace.c:(.text+0xe23): undefined reference to `sock_from_file' > When compiling a kernel with BPF and without NET. > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Florent Revest <revest@chromium.org> Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested Thanks. > --- > kernel/trace/bpf_trace.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 0cf0a6331482..29ec2b3b1cc4 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -1272,7 +1272,11 @@ const struct bpf_func_proto bpf_snprintf_btf_proto = { > > BPF_CALL_1(bpf_sock_from_file, struct file *, file) > { > +#ifdef CONFIG_NET > return (unsigned long) sock_from_file(file); > +#else > + return 0; > +#endif > } > > BTF_ID_LIST(bpf_sock_from_file_btf_ids) >
On Mon, Dec 07, 2020 at 09:06:05PM +0100, Florent Revest wrote: > This avoids > ld: kernel/trace/bpf_trace.o: in function `bpf_sock_from_file': > bpf_trace.c:(.text+0xe23): undefined reference to `sock_from_file' > When compiling a kernel with BPF and without NET. > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Florent Revest <revest@chromium.org> > --- > kernel/trace/bpf_trace.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 0cf0a6331482..29ec2b3b1cc4 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -1272,7 +1272,11 @@ const struct bpf_func_proto bpf_snprintf_btf_proto = { > > BPF_CALL_1(bpf_sock_from_file, struct file *, file) > { > +#ifdef CONFIG_NET > return (unsigned long) sock_from_file(file); > +#else > + return 0; > +#endif > } Should bpf_sock_from_file_proto belong to tracing_prog_func_proto() instead of bpf_tracing_func_proto()? bpf_skc_to_*_proto is also in tracing_prog_func_proto() where there is an existing "#ifdef CONFIG_NET". > > BTF_ID_LIST(bpf_sock_from_file_btf_ids) > -- > 2.29.2.576.ga3fc446d84-goog >
On Mon, 2020-12-07 at 13:33 -0800, Martin KaFai Lau wrote: > On Mon, Dec 07, 2020 at 09:06:05PM +0100, Florent Revest wrote: > > This avoids > > ld: kernel/trace/bpf_trace.o: in function `bpf_sock_from_file': > > bpf_trace.c:(.text+0xe23): undefined reference to > > `sock_from_file' > > When compiling a kernel with BPF and without NET. > > > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > > Signed-off-by: Florent Revest <revest@chromium.org> > > --- > > kernel/trace/bpf_trace.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index 0cf0a6331482..29ec2b3b1cc4 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -1272,7 +1272,11 @@ const struct bpf_func_proto > > bpf_snprintf_btf_proto = { > > > > BPF_CALL_1(bpf_sock_from_file, struct file *, file) > > { > > +#ifdef CONFIG_NET > > return (unsigned long) sock_from_file(file); > > +#else > > + return 0; > > +#endif > > } > Should bpf_sock_from_file_proto belong to > tracing_prog_func_proto() instead of bpf_tracing_func_proto()? > bpf_skc_to_*_proto is also in tracing_prog_func_proto() > where there is an existing "#ifdef CONFIG_NET". I'm happy to move bpf_sock_from_file to tracing_prog_func_proto if you'd prefer that. I'm actually unsure what the difference would be, those function names are confusing, but this works for our use-case. :) However, by itself, that wouldn't address the problem reported by Randy since the helper definition would still be compiled and have an undefined reference to sock_from_file. The existing socket helpers (for example skc_to_tcp_sock) can get away without a patch like mine because they are defined in net/core/filter.c which only gets compiled with CONFIG_NET. I will send a v3 where I move the sock_from_file helper definition to net/core/filter.c and also move the usage of the helper to tracing_prog_func_proto under CONFIG_NET and then you can feel free to merge v2 or v3 depending on which approach you prefer (or a followup version if I mess up again... :D)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 0cf0a6331482..29ec2b3b1cc4 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1272,7 +1272,11 @@ const struct bpf_func_proto bpf_snprintf_btf_proto = { BPF_CALL_1(bpf_sock_from_file, struct file *, file) { +#ifdef CONFIG_NET return (unsigned long) sock_from_file(file); +#else + return 0; +#endif } BTF_ID_LIST(bpf_sock_from_file_btf_ids)
This avoids ld: kernel/trace/bpf_trace.o: in function `bpf_sock_from_file': bpf_trace.c:(.text+0xe23): undefined reference to `sock_from_file' When compiling a kernel with BPF and without NET. Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Florent Revest <revest@chromium.org> --- kernel/trace/bpf_trace.c | 4 ++++ 1 file changed, 4 insertions(+)