linux-kernel - RE: [PATCH v2] net :mana : Add per-cpu stats for MANA device

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]

Date: Fri, 8 Mar 2024 19:31:17 +0000
From: Haiyang Zhang <haiyangz@...rosoft.com>
To: Shradha Gupta <shradhagupta@...ux.microsoft.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>, Ajay Sharma <sharmaajay@...rosoft.com>, Leon
 Romanovsky <leon@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Sebastian
 Andrzej Siewior <bigeasy@...utronix.de>, KY Srinivasan <kys@...rosoft.com>,
	Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>, Long Li
	<longli@...rosoft.com>, Shradha Gupta <shradhagupta@...rosoft.com>
Subject: RE: [PATCH v2] net :mana : Add per-cpu stats for MANA device



> -----Original Message-----
> From: Shradha Gupta <shradhagupta@...ux.microsoft.com>
> Sent: Friday, March 8, 2024 5:45 AM
> To: linux-kernel@...r.kernel.org; linux-hyperv@...r.kernel.org; linux-
> rdma@...r.kernel.org; netdev@...r.kernel.org
> Cc: Shradha Gupta <shradhagupta@...ux.microsoft.com>; Eric Dumazet
> <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>; Paolo Abeni
> <pabeni@...hat.com>; Ajay Sharma <sharmaajay@...rosoft.com>; Leon
> Romanovsky <leon@...nel.org>; Thomas Gleixner <tglx@...utronix.de>;
> Sebastian Andrzej Siewior <bigeasy@...utronix.de>; KY Srinivasan
> <kys@...rosoft.com>; Haiyang Zhang <haiyangz@...rosoft.com>; Wei Liu
> <wei.liu@...nel.org>; Dexuan Cui <decui@...rosoft.com>; Long Li
> <longli@...rosoft.com>; Shradha Gupta <shradhagupta@...rosoft.com>
> Subject: [PATCH v2] net :mana : Add per-cpu stats for MANA device
> 
> Extend 'ethtool -S' output for mana devices to include per-CPU packet
> stats
> 
> Signed-off-by: Shradha Gupta <shradhagupta@...ux.microsoft.com>
> ---
>  Changes in v2
>  * Corrected the patch description to remove redundant built and
>    Tested info
>  * Used num_possible_cpus() as suggested
>  * Added the missing allocation and deallocation sections for
>    per-CPU counters.
> ---
>  drivers/net/ethernet/microsoft/mana/mana_en.c | 30 ++++++++++++++
>  .../ethernet/microsoft/mana/mana_ethtool.c    | 41 ++++++++++++++++++-
>  include/net/mana/mana.h                       | 12 ++++++
>  3 files changed, 81 insertions(+), 2 deletions(-)


> @@ -2660,6 +2682,7 @@ int mana_detach(struct net_device *ndev, bool
> from_close)
> 
>  	apc->port_st_save = apc->port_is_up;
>  	apc->port_is_up = false;
> +	free_percpu(apc->pcpu_stats);

Mana_detach() is called in multiple places like changing MTU, 
#channels, etc. After that you will use freed ptr.

This should be in mana_remove() before calling free_netdev(): 

                rtnl_unlock();

+		free_percpu(apc->pcpu_stats);
                free_netdev(ndev);

You should test with multiple changing mtu, #channels, and 
reloading driver to ensure the stats are working correctly, and no 
panics/errors in the dmesg.


> diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
> index 76147feb0d10..9a2414ee7f02 100644
> --- a/include/net/mana/mana.h
> +++ b/include/net/mana/mana.h
> @@ -51,6 +51,8 @@ enum TRI_STATE {
>  /* Update this count whenever the respective structures are changed */
>  #define MANA_STATS_RX_COUNT 5
>  #define MANA_STATS_TX_COUNT 11
> +#define MANA_STATS_RX_PCPU 2
> +#define MANA_STATS_TX_PCPU 3
Move these defines just above struct mana_pcpu_stats,
so people will remember to update them together.

Thanks,
-Haiyang