netdev - Re: [PATCH net-next v6 2/2] net: ti: icssg-prueth: Add support for PA 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: <42f29ba8-e341-474e-9e2a-59f55850803a@kernel.org>
Date: Thu, 22 Aug 2024 14:29:34 +0300
From: Roger Quadros <rogerq@...nel.org>
To: MD Danish Anwar <danishanwar@...com>, Suman Anna <s-anna@...com>,
 Sai Krishna <saikrishnag@...vell.com>, Jan Kiszka <jan.kiszka@...mens.com>,
 Dan Carpenter <dan.carpenter@...aro.org>, Diogo Ivo <diogo.ivo@...mens.com>,
 Kory Maincent <kory.maincent@...tlin.com>,
 Heiner Kallweit <hkallweit1@...il.com>, Andrew Lunn <andrew@...n.ch>,
 Simon Horman <horms@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
 "David S. Miller" <davem@...emloft.net>, Conor Dooley <conor+dt@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>,
 Santosh Shilimkar <ssantosh@...nel.org>, Nishanth Menon <nm@...com>
Cc: netdev@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 srk@...com, Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH net-next v6 2/2] net: ti: icssg-prueth: Add support for PA
 Stats



On 22/08/2024 08:28, MD Danish Anwar wrote:
> 
> 
> On 21/08/24 6:05 pm, Roger Quadros wrote:
>>
>>
>> On 20/08/2024 12:16, MD Danish Anwar wrote:
>>> Add support for dumping PA stats registers via ethtool.
>>> Firmware maintained stats are stored at PA Stats registers.
>>> Also modify emac_get_strings() API to use ethtool_puts().
>>>
>>> This commit also renames the array icssg_all_stats to icssg_mii_g_rt_stats
>>> and creates a new array named icssg_all_pa_stats for PA Stats.
>>>
>>> Signed-off-by: MD Danish Anwar <danishanwar@...com>
>>> ---
> 
> [ ... ]
> 
>>> +
>>>  #define ICSSG_STATS(field, stats_type)			\
>>>  {							\
>>>  	#field,						\
>>> @@ -84,13 +98,24 @@ struct miig_stats_regs {
>>>  	stats_type					\
>>>  }
>>>  
>>> +#define ICSSG_PA_STATS(field)			\
>>> +{						\
>>> +	#field,					\
>>> +	offsetof(struct pa_stats_regs, field),	\
>>> +}
>>> +
>>>  struct icssg_stats {
>>
>> icssg_mii_stats?
>>
> 
> Sure Roger. I will name it icssg_miig_stats to be consistent with
> 'struct miig_stats_regs'
> 
>>>  	char name[ETH_GSTRING_LEN];
>>>  	u32 offset;
>>>  	bool standard_stats;
>>>  };
>>>  
>>> -static const struct icssg_stats icssg_all_stats[] = {
>>> +struct icssg_pa_stats {
>>> +	char name[ETH_GSTRING_LEN];
>>> +	u32 offset;
>>> +};
>>> +
>>> +static const struct icssg_stats icssg_mii_g_rt_stats[] = {
>>
>> icssg_all_mii_stats? to be consistend with the newly added
>> icssg_pa_stats and icssg_all_pa_stats.
>>
>> Could you please group all mii_stats data strucutres and arrays together
>> followed by pa_stats data structures and arrays?
>>
> 
> Sure Roger, I will group all mii stats related data structures and
> pa_stats related data structures together.
> 
> The sequence and naming will be something like this,
> 
> struct miig_stats_regs
> #define ICSSG_MIIG_STATS(field, stats_type)
> struct icssg_miig_stats
> static const struct icssg_miig_stats icssg_all_miig_stats[]
> 
> struct pa_stats_regs
> #define ICSSG_PA_STATS(field)
> struct icssg_pa_stats
> static const struct icssg_pa_stats icssg_all_pa_stats[]
> 
> Let me know if this looks ok to you.

This is good. Thanks!

> 
>>>  	/* Rx */
>>>  	ICSSG_STATS(rx_packets, true),
>>>  	ICSSG_STATS(rx_broadcast_frames, false),
>>> @@ -155,4 +180,11 @@ static const struct icssg_stats icssg_all_stats[] = {
>>>  	ICSSG_STATS(tx_bytes, true),t
>>>  };
>>>  
>>> +static const struct icssg_pa_stats icssg_all_pa_stats[] = > +	ICSSG_PA_STATS(fw_rx_cnt),
>>> +	ICSSG_PA_STATS(fw_tx_cnt),
>>> +	ICSSG_PA_STATS(fw_tx_pre_overflow),
>>> +	ICSSG_PA_STATS(fw_tx_exp_overflow),
>>> +};
>>> +
>>>  #endif /* __NET_TI_ICSSG_STATS_H */
>>
> 

-- 
cheers,
-roger