Re: [Linux-kernel] [PATCH] net: bpf: add static in net/core/filter.c

From: Ben Dooks
Date: Wed Oct 16 2019 - 09:24:47 EST


On 16/10/2019 14:11, Ben Dooks wrote:
On 16/10/2019 14:10, Daniel Borkmann wrote:
On Wed, Oct 16, 2019 at 02:02:31PM +0100, Ben Dooks wrote:
On 16/10/2019 13:26, Daniel Borkmann wrote:
On Wed, Oct 16, 2019 at 12:04:46PM +0100, Ben Dooks (Codethink) wrote:
There are a number of structs in net/core/filter.c
that are not exported or declared outside of the
file. Fix the following warnings by making these
all static:

net/core/filter.c:8465:31: warning: symbol 'sk_filter_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8472:27: warning: symbol 'sk_filter_prog_ops' was not declared. Should it be static?
[...]
net/core/filter.c:8935:27: warning: symbol 'sk_reuseport_prog_ops' was not declared. Should it be static?

Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
---
Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Cc: Martin KaFai Lau <kafai@xxxxxx>
Cc: Song Liu <songliubraving@xxxxxx>
Cc: Yonghong Song <yhs@xxxxxx>
Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
Cc: Jakub Kicinski <jakub.kicinski@xxxxxxxxxxxxx>
Cc: Jesper Dangaard Brouer <hawk@xxxxxxxxxx>
Cc: John Fastabend <john.fastabend@xxxxxxxxx>
Cc: netdev@xxxxxxxxxxxxxxx
Cc: bpf@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
---
ÂÂ net/core/filter.c | 60 +++++++++++++++++++++++------------------------
ÂÂ 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index ed6563622ce3..f7338fee41f8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -8462,18 +8462,18 @@ static u32 sk_msg_convert_ctx_access(enum bpf_access_type type,
ÂÂÂÂÂÂ return insn - insn_buf;
ÂÂ }
-const struct bpf_verifier_ops sk_filter_verifier_ops = {
+static const struct bpf_verifier_ops sk_filter_verifier_ops = {
ÂÂÂÂÂÂ .get_func_protoÂÂÂÂÂÂÂ = sk_filter_func_proto,
ÂÂÂÂÂÂ .is_valid_accessÂÂÂ = sk_filter_is_valid_access,
ÂÂÂÂÂÂ .convert_ctx_accessÂÂÂ = bpf_convert_ctx_access,
ÂÂÂÂÂÂ .gen_ld_absÂÂÂÂÂÂÂ = bpf_gen_ld_abs,
ÂÂ };

Big obvious NAK. I'm puzzled that you try to fix a compile warning, but without
even bothering to compile the result after your patch ...

builds fine. maybe some effort to stop this happening again should be made.

It doesn't build, because they are used/needed outside:

Hmm, your config it does, I get /none/ of these warnings.

I guess a lot of this is being built whether or not is then used.

Without CONFIG_BPF_SYSCALL, a part of net/core/filter.c is being
built but then not declared or used. Should this be split up or
the areas not being built be removed?

--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html