netdev - Re: [EXTERNAL] Re: [PATCH net v2 2/4] octeon_ep: remove firmware stats fetch in ndo_get

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: <Z2GowEuPU0nILAoW@lzaremba-mobl.ger.corp.intel.com>
Date: Tue, 17 Dec 2024 17:37:20 +0100
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Shinas Rasheed <srasheed@...vell.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Haseeb Gani
	<hgani@...vell.com>, Sathesh B Edara <sedara@...vell.com>, Vimlesh Kumar
	<vimleshk@...vell.com>, "thaller@...hat.com" <thaller@...hat.com>,
	"wizhao@...hat.com" <wizhao@...hat.com>, "kheib@...hat.com"
	<kheib@...hat.com>, "konguyen@...hat.com" <konguyen@...hat.com>,
	"horms@...nel.org" <horms@...nel.org>, "einstein.xue@...axg.com"
	<einstein.xue@...axg.com>, Veerasenareddy Burru <vburru@...vell.com>, "Andrew
 Lunn" <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Satananda Burla <sburla@...vell.com>, Abhijit Ayarekar
	<aayarekar@...vell.com>
Subject: Re: [EXTERNAL] Re: [PATCH net v2 2/4] octeon_ep: remove firmware
 stats fetch in ndo_get_stats64

On Mon, Dec 16, 2024 at 06:31:05PM +0000, Shinas Rasheed wrote:
> Hi Larysa,
> 
> 
> > On Sun, Dec 15, 2024 at 11:58:40PM -0800, Shinas Rasheed wrote:
> > > The per queue stats are available already and are retrieved
> > > from register reads during ndo_get_stats64. The firmware stats
> > > fetch call that happens in ndo_get_stats64() is currently not
> > > required
> > >
> > > Fixes: 6a610a46bad1 ("octeon_ep: add support for ndo ops")
> > > Signed-off-by: Shinas Rasheed <srasheed@...vell.com>
> > > @@ -1019,10 +1013,6 @@ static void octep_get_stats64(struct net_device
> > *netdev,
> > >  	stats->tx_bytes = tx_bytes;
> > >  	stats->rx_packets = rx_packets;
> > >  	stats->rx_bytes = rx_bytes;
> > > -	stats->multicast = oct->iface_rx_stats.mcast_pkts;
> > > -	stats->rx_errors = oct->iface_rx_stats.err_pkts;
> > > -	stats->collisions = oct->iface_tx_stats.xscol;
> > > -	stats->tx_fifo_errors = oct->iface_tx_stats.undflw;
> > 
> > I do not see, how it is a fix to remove some fields from stats. If this is a
> > cleanup, it should not go to the stable tree.
> > 
> > >  }
> > >
> 
> The fix part of this patch is to remove the call to firmware to retrieve stats, which could block and cause rcu read lock warnings.
> The fields that are retrieved by this stats call can be neglected for use cases concerning the DPU, and the necessary stats are already
> read from per queue hardware stats registers. Hence, why the code is removed.
>

Please, provide the warinings in question in the commit message, this is an 
important context for anyone that would need to look through code history.