Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200603081124.1627600-1-matthieu.baerts@tessares.net>
 <CAAej5NZZNg+0EbZsa-SrP0S_sOPMqdzgQ9hS8z6DYpQp9G+yhw@mail.gmail.com>
 <1cb3266c-7c8c-ebe6-0b6e-6d970e0adbd1@tessares.net> <20200603181455.4sajgdyat7rkxxnf@ast-mbp.dhcp.thefacebook.com>
 <3573c0dd-baa8-5313-067a-eec6b04f0f36@tessares.net>
In-Reply-To: <3573c0dd-baa8-5313-067a-eec6b04f0f36@tessares.net>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Wed, 3 Jun 2020 11:43:53 -0700
Message-ID: <CAADnVQ+k7+fQmuNQL=GLLaGUvd5+zZN6GViy-oP7Sfq7aQVG1Q@mail.gmail.com>
Subject: Re: [PATCH bpf] bpf: fix unused-var without NETDEVICES
To:     Matthieu Baerts <matthieu.baerts@tessares.net>
Cc:     Ferenc Fejes <fejes@inf.elte.hu>,
        Network Development <netdev@vger.kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        Andrii Nakryiko <andriin@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@chromium.org>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, bpf <bpf@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jun 3, 2020 at 11:41 AM Matthieu Baerts
<matthieu.baerts@tessares.net> wrote:
>
> Hi Alexei,
>
> On 03/06/2020 20:14, Alexei Starovoitov wrote:
> > On Wed, Jun 03, 2020 at 11:12:01AM +0200, Matthieu Baerts wrote:
> >> Hi Ferenc,
> >>
> >> On 03/06/2020 10:56, Ferenc Fejes wrote:
> >>> Matthieu Baerts <matthieu.baerts@tessares.net> ezt =C3=ADrta (id=C5=
=91pont:
> >>> 2020. j=C3=BAn. 3., Sze, 10:11):
> >>>>
> >>>> A recent commit added new variables only used if CONFIG_NETDEVICES i=
s
> >>>> set.
> >>>
> >>> Thank you for noticing and fixed this!
> >>>
> >>>> A simple fix is to only declare these variables if the same
> >>>> condition is valid.
> >>>>
> >>>> Other solutions could be to move the code related to SO_BINDTODEVICE
> >>>> option from _bpf_setsockopt() function to a dedicated one or only
> >>>> declare these variables in the related "case" section.
> >>>
> >>> Yes thats indeed a cleaner way to approach this. I will prepare a fix=
 for that.
> >>
> >> I should have maybe added that I didn't take this approach because in =
the
> >> rest of the code, I don't see that variables are declared only in a "c=
ase"
> >> section (no "{" ... "}" after "case") and code is generally not moved =
into a
> >> dedicated function in these big switch/cases. But maybe it makes sense=
 here
> >> because of the #ifdef!
> >> At the end, I took the simple approach because it is for -net.
> >>
> >> In other words, I don't know what maintainers would prefer here but I =
am
> >> happy to see any another solutions implemented to remove these compile=
r
> >> warnings :)
> >
> > since CONFIG_NETDEVICES doesn't change anything in .h
> > I think the best is to remove #ifdef CONFIG_NETDEVICES from net/core/fi=
lter.c
> > and rely on sock_bindtoindex() returning ENOPROTOOPT
> > in the extreme case of oddly configured kernels.
>
> Good idea, thank you!
> I can send a patch implementing that.

Please do.
Your 'Notes:' section was absolutely correct in terms of different
trees relationship :)
Thank you.