netdev - Re: [PATCH v1 net-next 01/15] iov_iter: Skip copy in memcpy_to

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: <eb974de5-01c7-a4e1-b08d-d2e64bfed4a8@gmail.com>
Date:   Tue, 8 Dec 2020 16:30:58 +0200
From:   Boris Pismenny <borispismenny@...il.com>
To:     David Ahern <dsahern@...il.com>,
        Boris Pismenny <borisp@...lanox.com>, kuba@...nel.org,
        davem@...emloft.net, saeedm@...dia.com, hch@....de,
        sagi@...mberg.me, axboe@...com, kbusch@...nel.org,
        viro@...iv.linux.org.uk, edumazet@...gle.com
Cc:     boris.pismenny@...il.com, linux-nvme@...ts.infradead.org,
        netdev@...r.kernel.org, benishay@...dia.com, ogerlitz@...dia.com,
        yorayz@...dia.com, Ben Ben-Ishay <benishay@...lanox.com>,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Yoray Zack <yorayz@...lanox.com>, borisp@...dia.com
Subject: Re: [PATCH v1 net-next 01/15] iov_iter: Skip copy in memcpy_to_page
 if src==dst

On 08/12/2020 2:39, David Ahern wrote:
> On 12/7/20 2:06 PM, Boris Pismenny wrote:
>> When using direct data placement the NIC writes some of the payload
>> directly to the destination buffer, and constructs the SKB such that it
>> points to this data. As a result, the skb_copy datagram_iter call will
>> attempt to copy data when it is not necessary.
>>
>> This patch adds a check to avoid this copy, and a static_key to enabled
>> it when TCP direct data placement is possible.
>>
> Why not mark the skb as ZEROCOPY -- an Rx version of the existing
> SKBTX_DEV_ZEROCOPY and skb_shared_info->tx_flags? Use that as a generic
> way of indicating to the stack what is happening.
>
>

[Re-sending as the previous one didn't hit the mailing list]

Interesting idea. But, unlike SKBTX_DEV_ZEROCOPY this SKB can be inspected/modified by the stack without the need to copy things out. Additionally, the SKB may contain both data that is already placed in its final destination buffer (PDU data) and data that isn't (PDU header); it doesn't matter. Therefore, labeling the entire SKB as zerocopy doesn't convey the desired information. Moreover, skipping copies in the stack to receive zerocopy SKBs will require more invasive changes.

Our goal in this approach was to provide the smallest change that enables the desired functionality while preserving the performance of existing flows that do not care for it. An alternative approach, that doesn't affect existing flows at all, which we considered was to make a special version of memcpy_to_page to be used by DDP providers (nvme-tcp). This alternative will require creating corresponding special versions for users of this function such skb_copy_datagram_iter. Thit is more invasive, thus in this patchset we decided to avoid it.