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

Message-ID: <HE1PR0501MB2026E62285B397A5C7CC9705CD790@HE1PR0501MB2026.eurprd05.prod.outlook.com>
Date:	Mon, 2 May 2016 10:21:19 +0000
From:	Elad Kanfi <eladkan@...lanox.com>
To:	David Miller <davem@...emloft.net>,
	Lino Sanfilippo <lsanfil@...vell.com>
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



-----Original Message-----
From: David Miller [mailto:davem@...emloft.net] 
Sent: Friday, April 29, 2016 12:11 AM
To: Elad Kanfi
Cc: Noam Camus; linux-kernel@...r.kernel.org; abrodkin@...opsys.com; Tal Zilcer; netdev@...r.kernel.org
Subject: Re: [PATCH v2 1/2] net: nps_enet: Sync access to packet sent flag

From: Elad Kanfi <eladkan@...lanox.com>
Date: Wed, 27 Apr 2016 16:18:29 +0300

> From: Elad Kanfi <eladkan@...lanox.com>
> 
> Below is a description of a possible problematic sequence. CPU-A is 
> sending a frame and CPU-B handles the interrupt that indicates the 
> frame was sent. CPU-B reads an invalid value of tx_packet_sent.
> 
> 	CPU-A				CPU-B
> 	-----				-----
> 	nps_enet_send_frame
> 	.
> 	.
> 	tx_packet_sent = true
> 	order HW to start tx
> 	.
> 	.
> 	HW complete tx
> 			    ------> 	get tx complete interrupt
> 					.
> 					.
> 					if(tx_packet_sent == true)
> 
> 	end memory transaction
> 	(tx_packet_sent actually
> 	 written)
> 
> Problem solution:
> 
> Add a memory barrier after setting tx_packet_sent, in order to make 
> sure that it is written before the packet is sent.
> 
> Signed-off-by: Elad Kanfi <eladkan@...lanox.com>
> 
> Acked-by: Noam Camus <noamca@...lanox.com>
>
> Please address the feedback about memory barrier pairing.
>
> Also, for both patches, do not put empty lines between the various tags at the end of the commit message.  They should all be together in one continuous group.

Since this driver handles one frame at a time, I couldn't find a case that requires the paired barrier.

The order of reads is not critical once it is assured that the value is valid.

If there is something I'm missing or for the sake of good order, I can add it. 

Please let me know your opinion on this.

I will remove the empty lines in the commit messages at the next patches version.

Elad.