bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: bpf: add static in net/core/filter.c
@ 2019-10-16 11:04 Ben Dooks (Codethink)
  2019-10-16 12:26 ` Daniel Borkmann
  0 siblings, 1 reply; 7+ messages in thread
From: Ben Dooks (Codethink) @ 2019-10-16 11:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ben Dooks (Codethink),
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, netdev, bpf,
	linux-kernel

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:8476:31: warning: symbol 'tc_cls_act_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8484:27: warning: symbol 'tc_cls_act_prog_ops' was not declared. Should it be static?
net/core/filter.c:8488:31: warning: symbol 'xdp_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8495:27: warning: symbol 'xdp_prog_ops' was not declared. Should it be static?
net/core/filter.c:8499:31: warning: symbol 'cg_skb_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8505:27: warning: symbol 'cg_skb_prog_ops' was not declared. Should it be static?
net/core/filter.c:8509:31: warning: symbol 'lwt_in_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8515:27: warning: symbol 'lwt_in_prog_ops' was not declared. Should it be static?
net/core/filter.c:8519:31: warning: symbol 'lwt_out_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8525:27: warning: symbol 'lwt_out_prog_ops' was not declared. Should it be static?
net/core/filter.c:8529:31: warning: symbol 'lwt_xmit_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8536:27: warning: symbol 'lwt_xmit_prog_ops' was not declared. Should it be static?
net/core/filter.c:8540:31: warning: symbol 'lwt_seg6local_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8546:27: warning: symbol 'lwt_seg6local_prog_ops' was not declared. Should it be static?
net/core/filter.c:8550:31: warning: symbol 'cg_sock_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8556:27: warning: symbol 'cg_sock_prog_ops' was not declared. Should it be static?
net/core/filter.c:8559:31: warning: symbol 'cg_sock_addr_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8565:27: warning: symbol 'cg_sock_addr_prog_ops' was not declared. Should it be static?
net/core/filter.c:8568:31: warning: symbol 'sock_ops_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8574:27: warning: symbol 'sock_ops_prog_ops' was not declared. Should it be static?
net/core/filter.c:8577:31: warning: symbol 'sk_skb_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8584:27: warning: symbol 'sk_skb_prog_ops' was not declared. Should it be static?
net/core/filter.c:8587:31: warning: symbol 'sk_msg_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8594:27: warning: symbol 'sk_msg_prog_ops' was not declared. Should it be static?
net/core/filter.c:8597:31: warning: symbol 'flow_dissector_verifier_ops' was not declared. Should it be static?
net/core/filter.c:8603:27: warning: symbol 'flow_dissector_prog_ops' was not declared. Should it be static?
net/core/filter.c:8929:31: warning: symbol 'sk_reuseport_verifier_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@codethink.co.uk>
---
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Song Liu <songliubraving@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: netdev@vger.kernel.org
Cc: bpf@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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,
 };
 
-const struct bpf_prog_ops sk_filter_prog_ops = {
+static const struct bpf_prog_ops sk_filter_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
-const struct bpf_verifier_ops tc_cls_act_verifier_ops = {
+static const struct bpf_verifier_ops tc_cls_act_verifier_ops = {
 	.get_func_proto		= tc_cls_act_func_proto,
 	.is_valid_access	= tc_cls_act_is_valid_access,
 	.convert_ctx_access	= tc_cls_act_convert_ctx_access,
@@ -8481,126 +8481,126 @@ const struct bpf_verifier_ops tc_cls_act_verifier_ops = {
 	.gen_ld_abs		= bpf_gen_ld_abs,
 };
 
-const struct bpf_prog_ops tc_cls_act_prog_ops = {
+static const struct bpf_prog_ops tc_cls_act_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
-const struct bpf_verifier_ops xdp_verifier_ops = {
+static const struct bpf_verifier_ops xdp_verifier_ops = {
 	.get_func_proto		= xdp_func_proto,
 	.is_valid_access	= xdp_is_valid_access,
 	.convert_ctx_access	= xdp_convert_ctx_access,
 	.gen_prologue		= bpf_noop_prologue,
 };
 
-const struct bpf_prog_ops xdp_prog_ops = {
+static const struct bpf_prog_ops xdp_prog_ops = {
 	.test_run		= bpf_prog_test_run_xdp,
 };
 
-const struct bpf_verifier_ops cg_skb_verifier_ops = {
+static const struct bpf_verifier_ops cg_skb_verifier_ops = {
 	.get_func_proto		= cg_skb_func_proto,
 	.is_valid_access	= cg_skb_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-const struct bpf_prog_ops cg_skb_prog_ops = {
+static const struct bpf_prog_ops cg_skb_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
-const struct bpf_verifier_ops lwt_in_verifier_ops = {
+static const struct bpf_verifier_ops lwt_in_verifier_ops = {
 	.get_func_proto		= lwt_in_func_proto,
 	.is_valid_access	= lwt_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-const struct bpf_prog_ops lwt_in_prog_ops = {
+static const struct bpf_prog_ops lwt_in_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
-const struct bpf_verifier_ops lwt_out_verifier_ops = {
+static const struct bpf_verifier_ops lwt_out_verifier_ops = {
 	.get_func_proto		= lwt_out_func_proto,
 	.is_valid_access	= lwt_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-const struct bpf_prog_ops lwt_out_prog_ops = {
+static const struct bpf_prog_ops lwt_out_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
-const struct bpf_verifier_ops lwt_xmit_verifier_ops = {
+static const struct bpf_verifier_ops lwt_xmit_verifier_ops = {
 	.get_func_proto		= lwt_xmit_func_proto,
 	.is_valid_access	= lwt_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 	.gen_prologue		= tc_cls_act_prologue,
 };
 
-const struct bpf_prog_ops lwt_xmit_prog_ops = {
+static const struct bpf_prog_ops lwt_xmit_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
-const struct bpf_verifier_ops lwt_seg6local_verifier_ops = {
+static const struct bpf_verifier_ops lwt_seg6local_verifier_ops = {
 	.get_func_proto		= lwt_seg6local_func_proto,
 	.is_valid_access	= lwt_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-const struct bpf_prog_ops lwt_seg6local_prog_ops = {
+static const struct bpf_prog_ops lwt_seg6local_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
-const struct bpf_verifier_ops cg_sock_verifier_ops = {
+static const struct bpf_verifier_ops cg_sock_verifier_ops = {
 	.get_func_proto		= sock_filter_func_proto,
 	.is_valid_access	= sock_filter_is_valid_access,
 	.convert_ctx_access	= bpf_sock_convert_ctx_access,
 };
 
-const struct bpf_prog_ops cg_sock_prog_ops = {
+static const struct bpf_prog_ops cg_sock_prog_ops = {
 };
 
-const struct bpf_verifier_ops cg_sock_addr_verifier_ops = {
+static const struct bpf_verifier_ops cg_sock_addr_verifier_ops = {
 	.get_func_proto		= sock_addr_func_proto,
 	.is_valid_access	= sock_addr_is_valid_access,
 	.convert_ctx_access	= sock_addr_convert_ctx_access,
 };
 
-const struct bpf_prog_ops cg_sock_addr_prog_ops = {
+static const struct bpf_prog_ops cg_sock_addr_prog_ops = {
 };
 
-const struct bpf_verifier_ops sock_ops_verifier_ops = {
+static const struct bpf_verifier_ops sock_ops_verifier_ops = {
 	.get_func_proto		= sock_ops_func_proto,
 	.is_valid_access	= sock_ops_is_valid_access,
 	.convert_ctx_access	= sock_ops_convert_ctx_access,
 };
 
-const struct bpf_prog_ops sock_ops_prog_ops = {
+static const struct bpf_prog_ops sock_ops_prog_ops = {
 };
 
-const struct bpf_verifier_ops sk_skb_verifier_ops = {
+static const struct bpf_verifier_ops sk_skb_verifier_ops = {
 	.get_func_proto		= sk_skb_func_proto,
 	.is_valid_access	= sk_skb_is_valid_access,
 	.convert_ctx_access	= sk_skb_convert_ctx_access,
 	.gen_prologue		= sk_skb_prologue,
 };
 
-const struct bpf_prog_ops sk_skb_prog_ops = {
+static const struct bpf_prog_ops sk_skb_prog_ops = {
 };
 
-const struct bpf_verifier_ops sk_msg_verifier_ops = {
+static const struct bpf_verifier_ops sk_msg_verifier_ops = {
 	.get_func_proto		= sk_msg_func_proto,
 	.is_valid_access	= sk_msg_is_valid_access,
 	.convert_ctx_access	= sk_msg_convert_ctx_access,
 	.gen_prologue		= bpf_noop_prologue,
 };
 
-const struct bpf_prog_ops sk_msg_prog_ops = {
+static const struct bpf_prog_ops sk_msg_prog_ops = {
 };
 
-const struct bpf_verifier_ops flow_dissector_verifier_ops = {
+static const struct bpf_verifier_ops flow_dissector_verifier_ops = {
 	.get_func_proto		= flow_dissector_func_proto,
 	.is_valid_access	= flow_dissector_is_valid_access,
 	.convert_ctx_access	= flow_dissector_convert_ctx_access,
 };
 
-const struct bpf_prog_ops flow_dissector_prog_ops = {
+static const struct bpf_prog_ops flow_dissector_prog_ops = {
 	.test_run		= bpf_prog_test_run_flow_dissector,
 };
 
@@ -8926,12 +8926,12 @@ static u32 sk_reuseport_convert_ctx_access(enum bpf_access_type type,
 	return insn - insn_buf;
 }
 
-const struct bpf_verifier_ops sk_reuseport_verifier_ops = {
+static const struct bpf_verifier_ops sk_reuseport_verifier_ops = {
 	.get_func_proto		= sk_reuseport_func_proto,
 	.is_valid_access	= sk_reuseport_is_valid_access,
 	.convert_ctx_access	= sk_reuseport_convert_ctx_access,
 };
 
-const struct bpf_prog_ops sk_reuseport_prog_ops = {
+static const struct bpf_prog_ops sk_reuseport_prog_ops = {
 };
 #endif /* CONFIG_INET */
-- 
2.23.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: bpf: add static in net/core/filter.c
  2019-10-16 11:04 [PATCH] net: bpf: add static in net/core/filter.c Ben Dooks (Codethink)
@ 2019-10-16 12:26 ` Daniel Borkmann
  2019-10-16 13:02   ` Ben Dooks
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2019-10-16 12:26 UTC (permalink / raw)
  To: Ben Dooks (Codethink)
  Cc: linux-kernel, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, netdev, bpf,
	linux-kernel

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@codethink.co.uk>
> ---
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Song Liu <songliubraving@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Jesper Dangaard Brouer <hawk@kernel.org>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: netdev@vger.kernel.org
> Cc: bpf@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  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 ...

Seen BPF_PROG_TYPE() ?

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: bpf: add static in net/core/filter.c
  2019-10-16 12:26 ` Daniel Borkmann
@ 2019-10-16 13:02   ` Ben Dooks
  2019-10-16 13:10     ` Daniel Borkmann
  0 siblings, 1 reply; 7+ messages in thread
From: Ben Dooks @ 2019-10-16 13:02 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: linux-kernel, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, netdev, bpf,
	linux-kernel

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@codethink.co.uk>
>> ---
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Martin KaFai Lau <kafai@fb.com>
>> Cc: Song Liu <songliubraving@fb.com>
>> Cc: Yonghong Song <yhs@fb.com>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
>> Cc: Jesper Dangaard Brouer <hawk@kernel.org>
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Cc: netdev@vger.kernel.org
>> Cc: bpf@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>   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.

> Seen BPF_PROG_TYPE() ?




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

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: bpf: add static in net/core/filter.c
  2019-10-16 13:02   ` Ben Dooks
@ 2019-10-16 13:10     ` Daniel Borkmann
  2019-10-16 13:11       ` Ben Dooks
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2019-10-16 13:10 UTC (permalink / raw)
  To: Ben Dooks
  Cc: linux-kernel, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, netdev, bpf,
	linux-kernel

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@codethink.co.uk>
> > > ---
> > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > Cc: Martin KaFai Lau <kafai@fb.com>
> > > Cc: Song Liu <songliubraving@fb.com>
> > > Cc: Yonghong Song <yhs@fb.com>
> > > Cc: "David S. Miller" <davem@davemloft.net>
> > > Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > Cc: Jesper Dangaard Brouer <hawk@kernel.org>
> > > Cc: John Fastabend <john.fastabend@gmail.com>
> > > Cc: netdev@vger.kernel.org
> > > Cc: bpf@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > ---
> > >   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:

[...]
  CC      net/core/dev_ioctl.o
  CC      net/core/tso.o
net/core/filter.c:8467:38: error: static declaration of ‘sk_filter_verifier_ops’ follows non-static declaration
 8467 | static const struct bpf_verifier_ops sk_filter_verifier_ops = {
      |                                      ^~~~~~~~~~~~~~~~~~~~~~
In file included from ./include/linux/bpf-cgroup.h:5,
                 from ./include/linux/cgroup-defs.h:22,
                 from ./include/linux/cgroup.h:28,
                 from ./include/net/netprio_cgroup.h:11,
                 from ./include/linux/netdevice.h:42,
                 from ./include/net/sock.h:46,
                 from ./include/linux/sock_diag.h:8,
                 from net/core/filter.c:25:
./include/linux/bpf_types.h:5:44: note: previous declaration of ‘sk_filter_verifier_ops’ was here
    5 | BPF_PROG_TYPE(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter)
      |                                            ^~~~~~~~~
./include/linux/bpf.h:625:39: note: in definition of macro ‘BPF_PROG_TYPE’
  625 |  extern const struct bpf_verifier_ops _name ## _verifier_ops;
      |                                       ^~~~~
[...] ( and more of the same errors ... )

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: bpf: add static in net/core/filter.c
  2019-10-16 13:10     ` Daniel Borkmann
@ 2019-10-16 13:11       ` Ben Dooks
  2019-10-16 13:24         ` [Linux-kernel] " Ben Dooks
  2019-10-16 17:44         ` David Miller
  0 siblings, 2 replies; 7+ messages in thread
From: Ben Dooks @ 2019-10-16 13:11 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: linux-kernel, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, netdev, bpf,
	linux-kernel

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@codethink.co.uk>
>>>> ---
>>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>>> Cc: Martin KaFai Lau <kafai@fb.com>
>>>> Cc: Song Liu <songliubraving@fb.com>
>>>> Cc: Yonghong Song <yhs@fb.com>
>>>> Cc: "David S. Miller" <davem@davemloft.net>
>>>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
>>>> Cc: Jesper Dangaard Brouer <hawk@kernel.org>
>>>> Cc: John Fastabend <john.fastabend@gmail.com>
>>>> Cc: netdev@vger.kernel.org
>>>> Cc: bpf@vger.kernel.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> ---
>>>>    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.

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

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Linux-kernel] [PATCH] net: bpf: add static in net/core/filter.c
  2019-10-16 13:11       ` Ben Dooks
@ 2019-10-16 13:24         ` Ben Dooks
  2019-10-16 17:44         ` David Miller
  1 sibling, 0 replies; 7+ messages in thread
From: Ben Dooks @ 2019-10-16 13:24 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: linux-kernel, Song Liu, Jakub Kicinski, Jesper Dangaard Brouer,
	netdev, John Fastabend, Alexei Starovoitov, Martin KaFai Lau,
	Yonghong Song, bpf, David S. Miller, linux-kernel

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@codethink.co.uk>
>>>>> ---
>>>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>>>> Cc: Martin KaFai Lau <kafai@fb.com>
>>>>> Cc: Song Liu <songliubraving@fb.com>
>>>>> Cc: Yonghong Song <yhs@fb.com>
>>>>> Cc: "David S. Miller" <davem@davemloft.net>
>>>>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
>>>>> Cc: Jesper Dangaard Brouer <hawk@kernel.org>
>>>>> Cc: John Fastabend <john.fastabend@gmail.com>
>>>>> Cc: netdev@vger.kernel.org
>>>>> Cc: bpf@vger.kernel.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> ---
>>>>>    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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: bpf: add static in net/core/filter.c
  2019-10-16 13:11       ` Ben Dooks
  2019-10-16 13:24         ` [Linux-kernel] " Ben Dooks
@ 2019-10-16 17:44         ` David Miller
  1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2019-10-16 17:44 UTC (permalink / raw)
  To: ben.dooks
  Cc: daniel, linux-kernel, ast, kafai, songliubraving, yhs,
	jakub.kicinski, hawk, john.fastabend, netdev, bpf, linux-kernel

From: Ben Dooks <ben.dooks@codethink.co.uk>
Date: Wed, 16 Oct 2019 14:11:52 +0100

> 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.

When you are making changes like this, unless you have done a full grep
over the tree and are %100 sure it is unrefrenced you should do at
a minimum an allmodconfig build.

Otherwise by definition you are not testing the build of this change.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-10-16 17:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-16 11:04 [PATCH] net: bpf: add static in net/core/filter.c Ben Dooks (Codethink)
2019-10-16 12:26 ` Daniel Borkmann
2019-10-16 13:02   ` Ben Dooks
2019-10-16 13:10     ` Daniel Borkmann
2019-10-16 13:11       ` Ben Dooks
2019-10-16 13:24         ` [Linux-kernel] " Ben Dooks
2019-10-16 17:44         ` David Miller

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).