Re: [PATCH net-next 1/2] net: ti: icssg: Derive stats array lengths from ARRAY_SIZE

[Jacob Keller <jacob.e.keller@intel.com>]

On 5/13/2026 9:56 PM, MD Danish Anwar wrote:
> Hi Jacob,
> 
> On 14/05/26 1:30 am, Jacob Keller wrote:
>> The way we solved this in the Intel drivers is to use a single array
>> which contains both the stat name as well as the offset from the
>> structure where the stat resides.
>>
>> The stat string code just iterates over the stat list for the strings,
>> while the stat value code iterates the array and computes the stat
>> address from the offset and size and base structure pointer. Each object
>> that has stats has its own stat array structure.
>>
>> This is probably overkill, but the advantage is that the strings and
>> their values are stored together and adding a new stat is as simple as
>> adding a new entry to that list.
>>
>> I.e.
>>
>> struct ice_stats {
>>         char stat_string[ETH_GSTRING_LEN];
>>         int sizeof_stat;
>>         int stat_offset;
>> };
>>
>> #define ICE_STAT(_type, _name, _stat) { \
>>         .stat_string = _name, \
>>         .sizeof_stat = sizeof_field(_type, _stat), \
>>         .stat_offset = offsetof(_type, _stat) \
>> }
>>
>> #define ICE_VSI_STAT(_name, _stat) \
>>                 ICE_STAT(struct ice_vsi, _name, _stat)
>> #define ICE_PF_STAT(_name, _stat) \
>>                 ICE_STAT(struct ice_pf, _name, _stat)
>>
>>
>> Then the stats for the individial arrays are defined like this:
>>
>> static const struct ice_stats ice_gstrings_vsi_stats[] = {
>>         ICE_VSI_STAT(ICE_RX_UNICAST, eth_stats.rx_unicast),
>>         ICE_VSI_STAT(ICE_TX_UNICAST, eth_stats.tx_unicast),
>>         ICE_VSI_STAT(ICE_RX_MULTICAST, eth_stats.rx_multicast),
>>         ICE_VSI_STAT(ICE_TX_MULTICAST, eth_stats.tx_multicast),
>>         ICE_VSI_STAT(ICE_RX_BROADCAST, eth_stats.rx_broadcast),
>>         ICE_VSI_STAT(ICE_TX_BROADCAST, eth_stats.tx_broadcast),
>> 	...
>> };
>>
>> (Note, ICE_RX_UNICAST is a macro that defines the string value.. I don't
>> recall who changed this to macros or why vs just having the strings be
>> directly in the definition...)
>>
> 
> Thanks for sharing the ice driver pattern — that's a clean design.
> 
>> This is probably a lot bigger refactor to make work, and may not be
>> exactly suitable for your driver. I've considered "upgrading" these data
> 
> Yes, I need to see if refactoring is applicable to ICSSG or not. I will
> look into this and send a separate patch / series in future if
> applicable. For this series I will stick with what David Carlier suggested.
> 
Makes sense. No need to bloat this by more work immediately. I just
wanted to point it out.

I'd like to uplift this infrastructure into core helpers so that other
drivers can more easily benefit without duplicating the scaffolding, but
its always been a low priority task especially since the private driver
stats are somewhat of a legacy/deprecated interface. (Maintainers don't
generally like driver developers making up their own statistics...)