On 05/03/2025 14:14, Sebastian Andrzej Siewior wrote:
The statistics gathering code for page_pool statistics has multiple
steps:
- gather statistics from a channel via page_pool_get_stats() to an
on-stack structure.
- copy this data to dedicated rq_stats.
- copy the data from rq_stats global mlx5e_sw_stats structure, and merge
per-queue statistics into one counter.
- Finally copy the data the specific order for the ethtool query.
The downside here is that the individual counter types are expected to
be u64 and if something changes, the code breaks. Also if additional
counter are added to struct page_pool_stats then they are not
automtically picked up by the driver but need to be manually added in
all four spots.
Remove the page_pool_stats fields from rq_stats_desc and use instead
page_pool_ethtool_stats_get_count() for the number of files and
page_pool_ethtool_stats_get_strings() for the strings which are added at
the end.
Remove page_pool_stats members from all structs and add the struct to
mlx5e_sw_stats where the data is gathered directly for all channels.
At the end, use page_pool_ethtool_stats_get() to copy the data to the
output buffer.
Suggested-by: Joe Damato <jdamato@xxxxxxxxxx>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
---
Hi,
Thanks for your patch.
IIUC you remove here the per-ring page_pool stats, and keep only the
summed stats.
I guess the reason for this is that the page_pool strings have no
per-ring variants.
59 static const char pp_stats[][ETH_GSTRING_LEN] = {
60 "rx_pp_alloc_fast",
61 "rx_pp_alloc_slow",
62 "rx_pp_alloc_slow_ho",
63 "rx_pp_alloc_empty",
64 "rx_pp_alloc_refill",
65 "rx_pp_alloc_waive",
66 "rx_pp_recycle_cached",
67 "rx_pp_recycle_cache_full",
68 "rx_pp_recycle_ring",
69 "rx_pp_recycle_ring_full",
70 "rx_pp_recycle_released_ref",
71 };
Is this the only reason?
I like the direction of this patch, but we won't give up the per-ring
counters. Please keep them.
I can think of a new "customized page_pool counters strings" API, where
the strings prefix is provided by the driver, and used to generate the
per-pool strings.
Example: Driver provides "rx5", and gets the strings:
"rx5_pp_alloc_fast",
"rx5_pp_alloc_slow",
"rx5_pp_alloc_slow_ho",
"rx5_pp_alloc_empty",
"rx5_pp_alloc_refill",
"rx5_pp_alloc_waive",
"rx5_pp_recycle_cached",
"rx5_pp_recycle_cache_full",
"rx5_pp_recycle_ring",
"rx5_pp_recycle_ring_full",
"rx5_pp_recycle_released_ref",
Alternatively, page_pool component provides the counters number and the
"stripped" strings, and the driver takes it from there...
"stripped" strings would be:
"pp_alloc_fast",
"pp_alloc_slow",
"pp_alloc_slow_ho",
"pp_alloc_empty",
"pp_alloc_refill",
"pp_alloc_waive",
"pp_recycle_cached",
"pp_recycle_cache_full",
"pp_recycle_ring",
"pp_recycle_ring_full",
"pp_recycle_released_ref",
or maybe even shorter:
"alloc_fast",
"alloc_slow",
"alloc_slow_ho",
"alloc_empty",
"alloc_refill",
"alloc_waive",
"recycle_cached",
"recycle_cache_full",
"recycle_ring",
"recycle_ring_full",
"recycle_released_ref",
Thanks,
Tariq
