Re: [PATCH] bpf: return EOPNOTSUPP when JIT is needed and not possible

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:

quoted

quoted

Similar issue was discussed in the past. See:
https://lore.kernel.org/netdev/20191204.125135.750458923752225025.davem@davemloft.net/ (local)  

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.

quoted

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..