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

[Haiyang Zhang]

> -----Original Message-----
> From: Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx>
> Sent: Friday, March 8, 2024 5:45 AM
> To: linux-kernel@xxxxxxxxxxxxxxx; linux-hyperv@xxxxxxxxxxxxxxx; linux-
> rdma@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx
> Cc: Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Ajay Sharma <sharmaajay@xxxxxxxxxxxxx>; Leon
> Romanovsky <leon@xxxxxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>;
> Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>; KY Srinivasan
> <kys@xxxxxxxxxxxxx>; Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>; Wei Liu
> <wei.liu@xxxxxxxxxx>; Dexuan Cui <decui@xxxxxxxxxxxxx>; Long Li
> <longli@xxxxxxxxxxxxx>; Shradha Gupta <shradhagupta@xxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxx>
> ---
>  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