netdev - Re: [RFC net-next] net: ena: Use u64_stats_t with u64_stats

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: <76564BD3-B685-45EC-B53F-06534512EBC7@amazon.com>
Date: Wed, 28 Jan 2026 13:21:27 +0000
From: "Arinzon, David" <darinzon@...zon.com>
To: "mmyangfl@...il.com" <mmyangfl@...il.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
CC: "mmyangfl@...il.com" <mmyangfl@...il.com>, "Allen, Neil"
	<shayagr@...zon.com>, "Kiyanovski, Arthur" <akiyano@...zon.com>, "Bshara,
 Saeed" <saeedb@...zon.com>, "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "ast@...nel.org" <ast@...nel.org>,
	"daniel@...earbox.net" <daniel@...earbox.net>, "hawk@...nel.org"
	<hawk@...nel.org>, "john.fastabend@...il.com" <john.fastabend@...il.com>,
	"sdf@...ichev.me" <sdf@...ichev.me>, "Bernstein, Amit" <amitbern@...zon.com>,
	"horms@...nel.org" <horms@...nel.org>, "bhelgaas@...gle.com"
	<bhelgaas@...gle.com>, "leitao@...ian.org" <leitao@...ian.org>, "Enju, Kohei"
	<enjuk@...zon.co.jp>, "ahmed.zaki@...el.com" <ahmed.zaki@...el.com>,
	"mingo@...nel.org" <mingo@...nel.org>, "tglx@...nel.org" <tglx@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: Re: [RFC net-next] net: ena: Use u64_stats_t with u64_stats_sync
 properly

> Hi David,
> 
> 
> Thank you for submitting the patch.
> 
> 
> > On 64bit arches, struct u64_stats_sync is empty and provides no help
> > against load/store tearing. Convert to u64_stats_t to ensure atomic
> > operations.
> > 
> 
> 
> We would like to run some quick performance tests internally before signing off.
> 

We have conducted initial internal performance testing and observed no measurable impact from
the proposed changes to the driver statistics.

> > Signed-off-by: David Yang <mmyangfl@...il.com <mailto:mmyangfl@...il.com>>
> > ---
> > RFC Comment:
> > 
> > The write side of u64_stats should ensure mutual exclusion, however I couldn't
> > find the synchronization mechanism in use (for example, ena_up / ena_io_poll /
> > ena_start_xmit). Should this be considered an issue?
> 
> 
> We reviewed other drivers and it looks like none add extra synchronization on the write side.
> Adding extra synchronization would degrade datapath performance to fix extremely rare missed counts.
> 
> 
> You've submitted similar patches for other drivers but only commented on write-side
> synchronization for ENA. Is there something specific to ENA that differs from other drivers?

We're still waiting for your response to our previous question: You've submitted similar patches
for other drivers but only commented on write-side synchronization for ENA.
Is there something specific to ENA that differs from other drivers?

> > @@ -440,42 +440,42 @@ struct ena_admin_eni_stats {
> >  	/* The number of packets shaped due to inbound aggregate BW
> >  	 * allowance being exceeded
> >  	 */
> > -	u64 bw_in_allowance_exceeded;
> > +	u64_stats_t bw_in_allowance_exceeded;
> >  
> >  	/* The number of packets shaped due to outbound aggregate BW
> >  	 * allowance being exceeded
> >  	 */
> > -	u64 bw_out_allowance_exceeded;
> > +	u64_stats_t bw_out_allowance_exceeded;
> >  
> >  	/* The number of packets shaped due to PPS allowance being exceeded */
> > -	u64 pps_allowance_exceeded;
> > +	u64_stats_t pps_allowance_exceeded;
> >  
> >  	/* The number of packets shaped due to connection tracking
> >  	 * allowance being exceeded and leading to failure in establishment
> >  	 * of new connections
> >  	 */
> > -	u64 conntrack_allowance_exceeded;
> > +	u64_stats_t conntrack_allowance_exceeded;
> >  
> >  	/* The number of packets shaped due to linklocal packet rate
> >  	 * allowance being exceeded
> >  	 */
> > -	u64 linklocal_allowance_exceeded;
> > +	u64_stats_t linklocal_allowance_exceeded;
> >  };

Regarding the modifications to ena_admin_defs.h: these structures define the hardware-software
interface between the host driver and the ENA device.
The fields are specified as native u64 types to ensure consistent interpretation between the driver
and device across different architectures.
We're concerned that converting these interface definitions to u64_stats_t could potentially
affect the structure layout or introduce subtle compatibility issues with the device.
Since these definitions represent a contract with the hardware, we'd prefer to keep them
unchanged while accepting the driver-internal statistics changes.

Would you be willing to split the patch to exclude the ena_admin_defs.h changes and any
code that touches these hardware interface fields?