netdev - Re: [PATCH net-next 1/5] nfp: make use of the DMA_ATTR_SKIP_CPU

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAJpBn1zyDv_U-hXD2cmoUCH5+_L8ibo1s2-eXV08VEmBcsfifg@mail.gmail.com>
Date:   Fri, 21 Apr 2017 10:10:59 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Alexander Duyck <alexander.duyck@...il.com>
Cc:     Netdev <netdev@...r.kernel.org>, oss-drivers@...ronome.com,
        Jakub Kicinski <kubakici@...pl>
Subject: Re: [PATCH net-next 1/5] nfp: make use of the DMA_ATTR_SKIP_CPU_SYNC attr

On Fri, Apr 21, 2017 at 8:07 AM, Alexander Duyck
<alexander.duyck@...il.com> wrote:
> On Fri, Apr 21, 2017 at 7:20 AM, Jakub Kicinski
> <jakub.kicinski@...ronome.com> wrote:
>> DMA unmap may destroy changes CPU made to the buffer.  To make XDP
>> run correctly on non-x86 platforms we should use the
>> DMA_ATTR_SKIP_CPU_SYNC attribute.
>>
>> Thanks to using the attribute we can now push the sync operation to
>> the common code path from XDP handler.
>>
>> A little bit of variable name reshuffling is required to bring the
>> code back to readable state.
>>
>> Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>
> So I see where you added the sync_single_for_cpu, but what about the
> sync_single_for_device? It needs to be called for the buffer before
> you assign it for Rx. On x86 it won't really matter but for proper
> utilization of the DMA API you need to sync the buffer even for Rx
> just to make certain that the cache lines are evicted prior to the
> device attempting to write to the buffer.

Ah, indeed, thanks for catching this.  We need to invalidate the
caches in case they are dirty and could get written back between the
device DMA and sync_for_cpu, correct?