netdev - Re: [PATCH net-next v4 11/27] tcp: support externally provided ubufs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e28b6ac9-c191-c36c-3769-4a43d49a86c2@kernel.org>
Date:   Wed, 13 Jul 2022 16:38:24 -0700
From:   David Ahern <dsahern@...nel.org>
To:     Pavel Begunkov <asml.silence@...il.com>, io-uring@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Willem de Bruijn <willemb@...gle.com>,
        Jens Axboe <axboe@...nel.dk>, kernel-team@...com
Subject: Re: [PATCH net-next v4 11/27] tcp: support externally provided ubufs

On 7/8/22 7:03 AM, Pavel Begunkov wrote:
>>> @@ -1356,9 +1362,11 @@ int tcp_sendmsg_locked(struct sock *sk, struct
>>> msghdr *msg, size_t size)
>>>                 copy = min_t(int, copy, pfrag->size - pfrag->offset);
>>>   -            if (tcp_downgrade_zcopy_pure(sk, skb))
>>> -                goto wait_for_space;
>>> -
>>> +            if (unlikely(skb_zcopy_pure(skb) ||
>>> skb_zcopy_managed(skb))) {
>>> +                if (tcp_downgrade_zcopy_pure(sk, skb))
>>> +                    goto wait_for_space;
>>> +                skb_zcopy_downgrade_managed(skb);
>>> +            }
>>>               copy = tcp_wmem_schedule(sk, copy);
>>>               if (!copy)
>>>                   goto wait_for_space;
>>
>> You dropped the msg->msg_ubuf checks on jump labels. Removing the one
>> you had at 'out_nopush' I agree with based on my tests (i.e, it is not
>> needed).
> 
> It was an optimisation, which I dropped for simplicity. Will be sending it
> and couple more afterwards.
> 
> 
>> The one at 'out_err' seems like it is needed - but it has been a few
>> weeks since I debugged that case. I believe the error path I was hitting
>> was sk_stream_wait_memory with MSG_DONTWAIT flag set meaning timeout is
>> 0 and it jumps there with EPIPE.
> 
> Currently, it's consistent with MSG_ZEROCOPY ubuf_info, we grab a ubuf_info
> reference at the beginning (msg_zerocopy_realloc() for MSG_ZEROCOPY and
> net_zcopy_get() for msg_ubuf), and then release it at the end
> with net_zcopy_put() or net_zcopy_put_abort().
> 

my fault; I somehow dropped a line in the port to the 5.13 kernel.