linux-kernel - Re: [PATCH net-next 5/9] sunvnet: add memory barrier before check for tx enable

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]

Date:   Fri, 03 Feb 2017 16:52:32 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     shannon.nelson@...cle.com
Cc:     eric.dumazet@...il.com, netdev@...r.kernel.org,
        sparclinux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 5/9] sunvnet: add memory barrier before check
 for tx enable

From: Shannon Nelson <shannon.nelson@...cle.com>
Date: Fri, 3 Feb 2017 13:20:43 -0800

> On 2/3/2017 9:56 AM, Eric Dumazet wrote:
>> On Fri, 2017-02-03 at 09:42 -0800, Shannon Nelson wrote:
>>> In order to allow the underlying LDC and outstanding memory operations
>>> to potentially catch up with the driver's Tx requests, add a memory
>>> barrier before checking again for available tx descriptors.
>>>
>>> Signed-off-by: Shannon Nelson <shannon.nelson@...cle.com>
>>> ---
>>>  drivers/net/ethernet/sun/sunvnet_common.c |    1 +
>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c
>>> b/drivers/net/ethernet/sun/sunvnet_common.c
>>> index 5d0d386..98e758e 100644
>>> --- a/drivers/net/ethernet/sun/sunvnet_common.c
>>> +++ b/drivers/net/ethernet/sun/sunvnet_common.c
>>> @@ -1467,6 +1467,7 @@ ldc_start_done:
>>>  	dr->prod = (dr->prod + 1) & (VNET_TX_RING_SIZE - 1);
>>>  	if (unlikely(vnet_tx_dring_avail(dr) < 1)) {
>>>  		netif_tx_stop_queue(txq);
>>> +		dma_wmb();
>>
>> This does not look right.
>>
>> I believe you need smp_rmb() here.
> 
> Well, it probably should be dma_rmb(), since regardless of the number
> of cores we think we have, we're communicating with a peer ldom that
> has its own core(s).  Either way, on sparc they all seem to boil down
> to the same bit of asm, but using the "rmb" part makes more logical
> sense. I'll respin with dma_rmb().

DMA barriers are for ordering between CPUs and devices.

SMP barriers are for ordering between CPUs, which is your situation
here.

It is completely inappropriate to use DMA barriers in a virutalization
device driver.