netdev - Re: [PATCH v2 1/2] net: nps_enet: Sync access to packet sent flag

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] [day] [month] [year] [list]

Message-ID: <57322951.3040109@gmx.de>
Date:	Tue, 10 May 2016 20:32:49 +0200
From:	Lino Sanfilippo <LinoSanfilippo@....de>
To:	Elad Kanfi <eladkan@...lanox.com>,
	Lino Sanfilippo <lsanfil@...vell.com>,
	David Miller <davem@...emloft.net>
Cc:	Noam Camus <noamca@...lanox.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"abrodkin@...opsys.com" <abrodkin@...opsys.com>,
	Tal Zilcer <talz@...lanox.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] net: nps_enet: Sync access to packet sent flag

Hi Elad,

On 08.05.2016 15:44, Elad Kanfi wrote:

>   
> After reviewing the code and your suggestion, it seems that we can do without the flag tx_packet_sent and therefor the first issue becomes irrelevant.
> The indication that a packet was sent is (tx_skb != NULL) , and the sequence will be:
> 
> CPU A:
> 1. tx_skb = skb
> 2. wmb() /* make sure tx_skb reaches the RAM before the HW is informed and the IRQ is fired */
> 3. nps_enet_reg_set(priv, NPS_ENET_REG_TX_CTL, tx_ctrl.value); /* send frame */
> 
> CPU B:
> 1. read tx_skb 
> 2. if( tx_skb != NULL ) handle tx_skb
> 3. tx_skb = NULL 
> 
> 

Ok, without the tx_packet_sent flag the code becomes simpler. But it
does not mean that we can toss the smp_rmb in the irq handler
completely. We still have to use a read barrier there to ensure that we
see the most recent value of tx_skb. E.g like this:

if (priv->tx_skb != NULL ) {
	smp_rmb()
	/ * handle tx_skb */
}

With both barriers in place the code should work as expected.

Regards,
Lino