Re: [PATCH v1] net: Add distinct sk_psock field

[Chuck Lever III <chuck.lever@oracle.com>]

> On Jul 13, 2022, at 9:51 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> On Wed, 13 Jul 2022 10:26:21 -0400 Chuck Lever wrote:
>> The sk_psock facility populates the sk_user_data field with the
>> address of an extra bit of metadata. User space sockets never
>> populate the sk_user_data field, so this has worked out fine.
>> 
>> However, kernel socket consumers such as the RPC client and server
>> do populate the sk_user_data field. The sk_psock() function cannot
>> tell that the content of sk_user_data does not point to psock
>> metadata, so it will happily return a pointer to something else,
>> cast to a struct sk_psock.
>> 
>> Thus kernel socket consumers and psock currently cannot co-exist.
>> 
>> We could educate sk_psock() to return NULL if sk_user_data does
>> not point to a struct sk_psock. However, a more general solution
>> that enables full co-existence psock and other uses of sk_user_data
>> might be more interesting.
>> 
>> Move the struct sk_psock address to its own pointer field so that
>> the contents of the sk_user_data field is preserved.
>> 
>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> 
> Thanks for posting separately. We already have the (somewhat
> nondescript) SK_USER_DATA_BPF, can we use another bit for psock?
> Or add a u8 user_data type and have TCP ULP reject if the type is
> anything but psock. I'm not sure why psock is special to deserve 
> its own pointer.

Hi Jakub, for an informed answer, you will need to ask the folks
who maintain psock. My guess is that kernel consumers might need
to populate both BPF/psock and sk_user_data concurrently for
separate purposes. If concurrent usage is never necessary, then
you can probably get away with a small enumerator that describes
the content of sk_user_data. But after some code auditing it didn't
look to me like that would be adequate.


--
Chuck Lever