Re: [PATCH for-6.1 1/2] io_uring/net: fail zc send for unsupported protocols

[Jens Axboe <axboe@kernel.dk>]

On 10/20/22 2:13 AM, Stefan Metzmacher wrote:
> Hi Pavel,
> 
>> If a protocol doesn't support zerocopy it will silently fall back to
>> copying. This type of behaviour has always been a source of troubles
>> so it's better to fail such requests instead. For now explicitly
>> whitelist supported protocols in io_uring, which should be turned later
>> into a socket flag.
>>
>> Cc: <stable@vger.kernel.org> # 6.0
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   io_uring/net.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/io_uring/net.c b/io_uring/net.c
>> index 8c7226b5bf41..28127f1de1f0 100644
>> --- a/io_uring/net.c
>> +++ b/io_uring/net.c
>> @@ -120,6 +120,13 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
>>       }
>>   }
>>   +static inline bool io_sock_support_zc(struct socket *sock)
>> +{
>> +    return likely(sock->sk && sk_fullsock(sock->sk) &&
>> +             (sock->sk->sk_protocol == IPPROTO_TCP ||
>> +              sock->sk->sk_protocol == IPPROTO_UDP));
>> +}
> 
> Can we please make this more generic (at least for 6.1, which is likely be an lts release)
> 
> It means my out of tree smbdirect driver would not be able to provide SENDMSG_ZC.
> 
> Currently sk_setsockopt has this logic:
> 
>         case SO_ZEROCOPY:
>                 if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6) {
>                         if (!(sk_is_tcp(sk) ||
>                               (sk->sk_type == SOCK_DGRAM &&
>                                sk->sk_protocol == IPPROTO_UDP)))
>                                 ret = -EOPNOTSUPP;
>                 } else if (sk->sk_family != PF_RDS) {
>                         ret = -EOPNOTSUPP;
>                 }
>                 if (!ret) {
>                         if (val < 0 || val > 1)
>                                 ret = -EINVAL;
>                         else
>                                 sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
>                 }
>                 break;
> 
> Maybe the socket creation code could set
> unsigned char skc_so_zerocopy_supported:1;
> and/or
> unsigned char skc_zerocopy_msg_ubuf_supported:1;
> 
> In order to avoid the manual complex tests.

I agree that would be cleaner, even for 6.1. Let's drop these two
for now.

-- 
Jens Axboe