On 03/04/2021 12:56, Greg KH wrote:
> On Wed, Mar 31, 2021 at 09:37:28PM +0300, Zidenberg, Tsahi wrote:
>> On 30/03/2021 20:21, Sasha Levin wrote:
>>
>>> What stops us from taking that API back to 5.4? I took a look at the
>>> dependencies and they don't look too scary.
>>>
>>> Can we try that route instead? We really don't want to diverge from
>>> upstream that much.
>>>
>> probe_read_{user,kernel}* functions themselves seem simple enough.
>> Importing them in a forward-compliant way to 5.4 would require either
>> adding an unused entry in bpf.h's __BPF_FUNC_MAPPER or also pulling
>> skb_output bpf helper functions into 5.4. To me, it seems like too
>> much of a UAPI change to go into a stable release.
> Why is anything changing in the user api here? The user api should not
> be changing in incompatible ways, otherwise you would not be able to
> upgrade to new kernels without breaking things.
>
>> Another option would be to import more code to make it somewhat closer
>> to upstream implementation without changing UAPI. As in v5.8, I could
>> internally map these helpers to probe_read_compat* functions, which
>> will in turn call probe_read_{user,kernel}*_common functions.
>> Func names might seem weird out of context, but it will be closer.
>> Implementation will still defer, e.g. to avoid warnings on platforms
>> without ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>
>> Does that sound like a reasonable solution?
> Again that would make things different from Linus's tree, which is what
> we want to avoid if at all possible.
>
> What commits in 5.8 are you talking about here, if the changes are there
> that work here in 5.4, that's fine.
In 5.5 (mostly 6ae08ae3dea2) BPF UAPI was changed, bpf_probe_read was split
into compat (original), user and kernel variants. Compat here just calls the
kernel variant, which means it's still broken.
In 5.8 (8d92db5c04d10), compat was fixed to call user/kernel variants
according to address in machines where it makes sense, and disabled on other
machines. I am trying to take the fix for machines where it's possible, and
leave other machines untouched.
As I understand it, there are 3 options:
1) Implement the same fix as v5.8, while staying with v5.4 code/API.
That's what my original patch did.
2) Import the new 5.5 API + 5.8 fix. It's not trivial to get API-compatibility
right. Specifically - need to solve skb_output (a7658e1a4164c), another
entry in the BPF enum, introduced before the new read variants.
3) Take some internal code from v5.8 without changing the user-facing API.
It will still not be cherry-picks and differ from Linus's tree. Result
would be somewhat closer to v5.8 code, at the price of having a larger
change.
I still like option 1, but I'd be happy to implement any other option you
prefer. I could also submit an identical patchset with a better commit
message.
Thank you!
Tsahi.
