From: Will Deacon <will@kernel.org>
To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: bpf@vger.kernel.org, ardb@kernel.org, naresh.kamboju@linaro.org,
Jean-Philippe Brucker <jean-philippe@linaro.org>,
Yauheni Kaliuta <yauheni.kaliuta@redhat.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Alexei Starovoitov <ast@kernel.org>,
Zi Shen Lim <zlim.lnx@gmail.com>,
Catalin Marinas <catalin.marinas@arm.com>,
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>,
Jesper Dangaard Brouer <hawk@kernel.org>,
netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: bpf: Fix branch offset in JIT
Date: Mon, 14 Sep 2020 15:01:15 +0100 [thread overview]
Message-ID: <20200914140114.GG24441@willie-the-truck> (raw)
In-Reply-To: <20200914132350.GA126552@apalos.home>
Hi Ilias,
On Mon, Sep 14, 2020 at 04:23:50PM +0300, Ilias Apalodimas wrote:
> On Mon, Sep 14, 2020 at 03:35:04PM +0300, Ilias Apalodimas wrote:
> > On Mon, Sep 14, 2020 at 01:20:43PM +0100, Will Deacon wrote:
> > > On Mon, Sep 14, 2020 at 11:36:21AM +0300, Ilias Apalodimas wrote:
> > > > Running the eBPF test_verifier leads to random errors looking like this:
[...]
> > > > The reason seems to be the offset[] creation and usage ctx->offset[]
> > >
> > > "seems to be"? Are you unsure?
> >
> > Reading the history and other ports of the JIT implementation, I couldn't
> > tell if the decision on skipping the 1st entry was deliberate or not on
> > Aarch64. Reading through the mailist list didn't help either [1].
> > Skipping the 1st entry seems indeed to cause the problem.
> > I did run the patch though the BPF tests and showed no regressions + fixing
> > the error.
>
> I'll correct myself here.
> Looking into 7c2e988f400e ("bpf: fix x64 JIT code generation for jmp to 1st insn")
> explains things a bit better.
> Jumping back to the 1st insn wasn't allowed until eBPF bounded loops were
> introduced. That's what the 1st instruction was not saved in the original code.
>
> > >
> > > No Fixes: tag?
> >
> > I'll re-spin and apply one
> >
> Any suggestion on any Fixes I should apply? The original code was 'correct' and
> broke only when bounded loops and their self-tests were introduced.
Ouch, that's pretty bad as it means nobody is regression testing BPF on
arm64 with mainline. Damn.
The Fixes: tag should identify the commit beyond which we don't need to
backport the fix, so it sounds like introduction of bounded loops, according
to your analysis.
Will
next prev parent reply other threads:[~2020-09-14 14:04 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-14 8:36 [PATCH] arm64: bpf: Fix branch offset in JIT Ilias Apalodimas
2020-09-14 12:20 ` Will Deacon
2020-09-14 12:35 ` Ilias Apalodimas
2020-09-14 13:23 ` Ilias Apalodimas
2020-09-14 14:01 ` Will Deacon [this message]
2020-09-14 16:02 ` Ilias Apalodimas
2020-09-14 16:12 ` Jesper Dangaard Brouer
2020-09-14 17:02 ` Ilias Apalodimas
2020-09-14 17:47 ` Xi Wang
2020-09-14 17:55 ` Ilias Apalodimas
2020-09-14 18:08 ` Xi Wang
2020-09-14 18:21 ` Luke Nelson
2020-09-14 18:36 ` Ilias Apalodimas
2020-09-14 18:27 ` Ilias Apalodimas
2020-09-14 18:52 ` Xi Wang
2020-09-14 19:24 ` Ilias Apalodimas
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=20200914140114.GG24441@willie-the-truck \
--to=will@kernel.org \
--cc=andriin@fb.com \
--cc=ardb@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=catalin.marinas@arm.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=hawk@kernel.org \
--cc=ilias.apalodimas@linaro.org \
--cc=jean-philippe@linaro.org \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=naresh.kamboju@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=yauheni.kaliuta@redhat.com \
--cc=yhs@fb.com \
--cc=zlim.lnx@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox