From: Ben Dooks <ben.dooks@codethink.co.uk>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: linux-kernel@lists.codethink.co.uk,
Song Liu <songliubraving@fb.com>,
Jakub Kicinski <jakub.kicinski@netronome.com>,
Jesper Dangaard Brouer <hawk@kernel.org>,
netdev@vger.kernel.org, John Fastabend <john.fastabend@gmail.com>,
Alexei Starovoitov <ast@kernel.org>,
Martin KaFai Lau <kafai@fb.com>, Yonghong Song <yhs@fb.com>,
bpf@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
linux-kernel@vger.kernel.org
Subject: Re: [Linux-kernel] [PATCH] net: bpf: add static in net/core/filter.c
Date: Wed, 16 Oct 2019 14:24:39 +0100 [thread overview]
Message-ID: <16a0fe4a-f63a-7e33-62a2-d3dfbccd8f63@codethink.co.uk> (raw)
In-Reply-To: <e3e81678-6c58-191b-3514-629f5f94def2@codethink.co.uk>
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
next prev parent reply other threads:[~2019-10-16 13:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Ben Dooks [this message]
2019-10-16 17:44 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=16a0fe4a-f63a-7e33-62a2-d3dfbccd8f63@codethink.co.uk \
--to=ben.dooks@codethink.co.uk \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=hawk@kernel.org \
--cc=jakub.kicinski@netronome.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=linux-kernel@lists.codethink.co.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.