Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190426154848.23490-1-alban@kinvolk.io> <20190426140323.4edf1127@cakuba.netronome.com>
 <CADZs7q7O9TL5wXFBq_SgAdNCkzO=3tyLnF1_chthX3jao=PKqA@mail.gmail.com> <20190427113940.223fd4d1@cakuba.netronome.com>
In-Reply-To: <20190427113940.223fd4d1@cakuba.netronome.com>
From:   Alban Crequy <alban@kinvolk.io>
Date:   Mon, 29 Apr 2019 17:42:27 +0200
Message-ID: <CADZs7q6OORsEh=agQ5gzwr8uzj9emXU5x-317tF3POsO_Z2nKw@mail.gmail.com>
Subject: Re: [PATCH bpf-next v3 1/4] bpf: sock ops: add netns ino and dev in
 bpf context
To:     Jakub Kicinski <jakub.kicinski@netronome.com>
Cc:     Alban Crequy <alban.crequy@gmail.com>,
        John Fastabend <john.fastabend@gmail.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        bpf <bpf@vger.kernel.org>, netdev <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        =?UTF-8?Q?Iago_L=C3=B3pez_Galeiras?= <iago@kinvolk.io>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Apr 27, 2019 at 8:39 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Sat, 27 Apr 2019 12:48:25 +0200, Alban Crequy wrote:
> > On Fri, Apr 26, 2019 at 11:03 PM Jakub Kicinski
> > <jakub.kicinski@netronome.com> wrote:
> > >
> > > On Fri, 26 Apr 2019 17:48:45 +0200, Alban Crequy wrote:
> > > > In the unlikely case where network namespaces are not compiled in
> > > > (CONFIG_NET_NS=n), the verifier will not allow access to ->netns_*.
> > >
> > > Naive question - why return an error?  init_net should always be there,
> > > no?
> >
> > True for netns_dev. However, without CONFIG_NET_NS, we cannot access netns_ino:
> >
> > (struct sock_common).possible_net_t.(struct net *):
> >
> > typedef struct {
> > #ifdef CONFIG_NET_NS
> >         struct net *net;
> > #endif
> > } possible_net_t;
> >
> > And I don't think it would make much sense to allow access to
> > netns_dev but not netns_ino.
>
> Right, if CONFIG_NET_NS=n we could just take the pointer to init_net
> directly, and not worry about the field.  IMHO it'd be preferable to
> changing the UAPI based on kernel config, but I don't feel super
> strongly.

I see the point about not changing the UAPI. So I will update the patch to:
- return netns_dev unconditionally, regardless of CONFIG_NET_NS
- return netns_ino with either the correct value or zero depending on
CONFIG_NET_NS.