From: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
Ido Schimmel <idosch@idosch.org>,
John Fastabend <john.fastabend@gmail.com>,
bpf@vger.kernel.org, netdev@vger.kernel.org, ast@kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] bpf: return EOPNOTSUPP when JIT is needed and not possible
Date: Fri, 10 Dec 2021 09:24:04 -0300 [thread overview]
Message-ID: <YbNG5BliqnCyhs4J@mussarela> (raw)
In-Reply-To: <20211209182349.038ac2b8@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Thu, Dec 09, 2021 at 06:23:49PM -0800, Jakub Kicinski wrote:
> On Fri, 10 Dec 2021 00:03:40 +0100 Daniel Borkmann wrote:
> > > Similar issue was discussed in the past. See:
> > > https://lore.kernel.org/netdev/20191204.125135.750458923752225025.davem@davemloft.net/
> >
> > With regards to ENOTSUPP exposure, if the consensus is that we should fix all
> > occurences over to EOPNOTSUPP even if they've been exposed for quite some time
> > (Jakub?),
>
> Did you mean me? :) In case you did - I think we should avoid it
> for new code but changing existing now seems risky. Alexei and Andrii
> would know best but quick search of code bases at work reveals some
> scripts looking for ENOTSUPP.
>
> Thadeu, what motivated the change?
>
> If we're getting those changes fixes based on checkpatch output maybe
> there is a way to mute the checkpatch warnings when it's not run on a
> diff?
>
It was not checkpatch that motivated me.
I was looking into the following commits as we hit a failed test.
be08815c5d3b ("bpf: add also cbpf long jump test cases with heavy expansion")
050fad7c4534 ("bpf: fix truncated jump targets on heavy expansions")
Then, I realized that if given the right number of BPF_LDX | BPF_B | BPF_MSH
instructions, it will pass the bpf_convert_filter stage, but fail at blinding.
And if you have CONFIG_BPF_JIT_ALWAYS_ON, setting the filter will fail with
ENOTSUPP, which should not be sent to userspace.
I noticed other ENOTSUPP, but they seemed to be returned by helpers, and I was
not sure this would be relayed to userspace. So, I went for fixing the observed
case.
I will see if any of the tests I can run is broken by this change and submit it
again with the tests fixed as well.
Cascardo.
> > we could give this patch a try maybe via bpf-next and see if anyone complains.
> >
> > Thadeu, I think you also need to fix up BPF selftests as test_verifier, to mention
> > one example (there are also bunch of others under tools/testing/selftests/), is
> > checking for ENOTSUPP specifically..
prev parent reply other threads:[~2021-12-10 12:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 13:40 [PATCH] bpf: return EOPNOTSUPP when JIT is needed and not possible Thadeu Lima de Souza Cascardo
2021-12-09 19:05 ` John Fastabend
2021-12-09 19:31 ` Ido Schimmel
2021-12-09 23:03 ` Daniel Borkmann
2021-12-10 2:23 ` Jakub Kicinski
2021-12-10 12:24 ` Thadeu Lima de Souza Cascardo [this message]
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=YbNG5BliqnCyhs4J@mussarela \
--to=cascardo@canonical.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=idosch@idosch.org \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/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.