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: <20200715214312.2266839-1-haoluo@google.com> <20200715214312.2266839-3-haoluo@google.com>
 <CAEf4BzYxWk9OmN0QhDrvE943YsYd2Opdkbt7NQTO9-YM6c4aGw@mail.gmail.com>
 <CA+khW7i9wq0+2P_M46pEv-onGXL_=sW7xE=10CYeP_yjPh-Rpw@mail.gmail.com> <CAEf4BzY=6PH4YS8sX1SRFOj+6oQnfAk-f0P8+0XWMGMS+RJ0pw@mail.gmail.com>
In-Reply-To: <CAEf4BzY=6PH4YS8sX1SRFOj+6oQnfAk-f0P8+0XWMGMS+RJ0pw@mail.gmail.com>
From:   Hao Luo <haoluo@google.com>
Date:   Tue, 21 Jul 2020 09:45:26 -0700
Message-ID: <CA+khW7j93gtB-e8x1-Fd9sTrOaHHD7suzVaBh9gPjnOJux+-AA@mail.gmail.com>
Subject: Re: [RFC PATCH bpf-next 2/2] selftests/bpf: Test __ksym externs with BTF
To:     Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc:     Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Andrii Nakryiko <andriin@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        KP Singh <kpsingh@chromium.org>,
        Stanislav Fomichev <sdf@google.com>,
        Quentin Monnet <quentin@isovalent.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Ack. Will have that in v2.

Hao

On Mon, Jul 20, 2020 at 7:37 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jul 20, 2020 at 1:28 PM Hao Luo <haoluo@google.com> wrote:
> >
> > >
> > > This should ideally look like a real global variable extern:
> > >
> > > extern const struct rq runqueues __ksym;
> > >
> > >
> > > But that's the case for non-per-cpu variables. You didn't seem to
> > > address per-CPU variables in this patch set. How did you intend to
> > > handle that? We should look at a possible BPF helper to access such
> > > variables as well and how the verifier will prevent direct memory
> > > accesses for such variables.
> > >
> > > We should have some BPF helper that accepts per-CPU PTR_TO_BTF_ID, and
> > > returns PTR_TO_BTF_ID, but adjusted to desired CPU. And verifier
> > > ideally would allow direct memory access on that resulting
> > > PTR_TO_BTF_ID, but not on per-CPU one. Not sure yet how this should
> > > look like, but the verifier probably needs to know that variable
> > > itself is per-cpu, no?
> > >
> >
> > Yes, that's what I was unclear about, so I don't have that part in
> > this patchset. But your explanation helped me organize my thoughts. :)
> >
> > Actually, the verifier can tell whether a var is percpu from the
> > DATASEC, since we have encoded "percpu" DATASEC in btf. I think the
> > following should work:
> >
> > We may introduce a new PTR_TO_BTF_VAR_ID. In ld_imm, libbpf replaces
> > ksyms with btf_id. The btf id points to a KIND_VAR. If the pointed VAR
> > is found in the "percpu" DATASEC, dst_reg is set to PTR_TO_BTF_VAR_ID;
> > otherwise, it will be a PTR_TO_BTF_ID. For PTR_TO_BTF_VAR_ID,
> > reg->btf_id is the id of the VAR. For PTR_TO_BTF_ID, reg->btf_id is
> > the id of the actual kernel type. The verifier would reject direct
> > memory access on PTR_TO_BTF_VAR_ID, but the new BPF helper can convert
> > a PTR_TO_BTF_VAR_ID to PTR_TO_BTF_ID.
>
> Sounds good to me as a plan, except that PTR_TO_BTF_VAR_ID is a
> misleading name. It's always a variable. The per-CPU part is crucial,
> though, so maybe something like PTR_TO_PERCPU_BTF_ID?
>
> >
> > Hao