On 9/16/22 2:31 PM, Martin KaFai Lau wrote:
On 9/16/22 1:35 PM, Kumar Kartikeya Dwivedi wrote:
On Fri, 16 Sept 2022 at 22:20, Martin KaFai Lau <martin.lau@xxxxxxxxx>
wrote:
On 9/11/22 11:19 AM, Daniel Xu wrote:
We're seeing the following new warnings on netdev/build_32bit and
netdev/build_allmodconfig_warn CI jobs:
../net/core/filter.c:8608:1: warning: symbol
'nf_conn_btf_access_lock' was not declared. Should it be static?
../net/core/filter.c:8611:5: warning: symbol 'nfct_bsa' was not
declared. Should it be static?
Fix by ensuring extern declaration is present while compiling filter.o.
Signed-off-by: Daniel Xu <dxu@xxxxxxxxx>
---
include/linux/filter.h | 6 ++++++
include/net/netfilter/nf_conntrack_bpf.h | 7 +------
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 527ae1d64e27..96de256b2c8d 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -567,6 +567,12 @@ struct sk_filter {
DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
+extern struct mutex nf_conn_btf_access_lock;
+extern int (*nfct_bsa)(struct bpf_verifier_log *log, const struct
btf *btf,
+ const struct btf_type *t, int off, int size,
+ enum bpf_access_type atype, u32 *next_btf_id,
+ enum bpf_type_flag *flag);
Can it avoid leaking the nfct specific details like
'nf_conn_btf_access_lock' and the null checking on 'nfct_bsa' to
filter.c? In particular, this code snippet in filter.c:
mutex_lock(&nf_conn_btf_access_lock);
if (nfct_bsa)
ret = nfct_bsa(log, btf, ....);
mutex_unlock(&nf_conn_btf_access_lock);
Can the lock and null check be done as one function (eg.
nfct_btf_struct_access()) in nf_conntrack_bpf.c and use it in filter.c
instead?
Don't think so, no. Because we want nf_conntrack to work as a module
as well.
Ah, got it.
I don't see nf_conntrack_btf_struct_access() in nf_conntrack_bpf.h is
used anywhere. Can be removed?
I was the one who suggested nf_conn specific names for now. There is
no other user of such module supplied
btf_struct_access callbacks yet, when one appears, we should instead
make registration of such callbacks properly generic (i.e. also
enforce it is only for module BTF ID etc.).
But that would be a lot of code without any users right now.
The lock is the only one needed to be in btf.c and
nfct_btf_struct_access() can be an inline in nf_conntrack_bpf.h instead?
nm. brain leaks. nfct_bsa pointer is still needed :( I was just
thinking if it can avoid this nfct specific bits here.
