net: mvneta: add support for page_pool_get_stats
Subsystems (4)
v2 cover net: mvneta: add support for page_pool_get_stats
Introduce page_pool stats ethtool APIs in order to avoid driver duplicated code.
Changes since v1:
- move stats accounting to page_pool code
- move stats string management to page_pool code
Lorenzo Bianconi (2):
net: page_pool: introduce ethtool stats
net: mvneta: add support for page_pool_get_stats
drivers/net/ethernet/marvell/Kconfig | 1 + drivers/net/ethernet/marvell/mvneta.c | 38 +++++++++++-- include/net/page_pool.h | 24 ++++++++ net/core/page_pool.c | 81 ++++++++++++++++++++++++++- 4 files changed, 138 insertions(+), 6 deletions(-)
Signature · 1 line
2.35.1
PATCH v2 1/2 net: page_pool: introduce ethtool stats
Introduce page_pool APIs to report stats through ethtool and reduce duplicated code in each driver.
--- include/net/page_pool.h | 24 ++++++++++++ net/core/page_pool.c | 81 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 1 deletion(-)
include/net/page_pool.h +24 -0
@@ -117,6 +117,30 @@ struct page_pool_stats {struct page_pool_recycle_stats recycle_stats; }; +/* List of page_pool stats exported through ethtool. */ +enum { +PP_ETHTOOL_ALLOC_FAST , +PP_ETHTOOL_ALLOC_SLOW , +PP_ETHTOOL_ALLOC_SLOW_HIGH_ORDER , +PP_ETHTOOL_ALLOC_EMPTY , +PP_ETHTOOL_ALLOC_REFILL , +PP_ETHTOOL_ALLOC_WAIVE , +PP_ETHTOOL_RECYCLE_CACHED , +PP_ETHTOOL_RECYCLE_CACHE_FULL , +PP_ETHTOOL_RECYCLE_RING , +PP_ETHTOOL_RECYCLE_RING_FULL , +PP_ETHTOOL_RECYCLE_RELEASED_REF , +PP_ETHTOOL_STATS_MAX , +}; + +static inline int page_pool_ethtool_stats_get_count (void ) +{ +return PP_ETHTOOL_STATS_MAX ; +} + +u8 *page_pool_ethtool_stats_get_strings (u8 *data); +u64 *page_pool_ethtool_stats_get (u64 *data,struct page_pool_stats *stats); +/* * Drivers that wish to harvest page pool stats and report them to users * (perhaps via ethtool, debugfs, or another mechanism) can allocate a
net/core/page_pool.c +80 -1
99 lines not on this page · raw message
Signature · 1 line
2.35.1
Re: [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats
Quoted 21 lines
include/net/page_pool.h +20
-0+/* List of page_pool stats exported through ethtool. */ +enum { +PP_ETHTOOL_ALLOC_FAST , +PP_ETHTOOL_ALLOC_SLOW , +PP_ETHTOOL_ALLOC_SLOW_HIGH_ORDER , +PP_ETHTOOL_ALLOC_EMPTY , +PP_ETHTOOL_ALLOC_REFILL , +PP_ETHTOOL_ALLOC_WAIVE , +PP_ETHTOOL_RECYCLE_CACHED , +PP_ETHTOOL_RECYCLE_CACHE_FULL , +PP_ETHTOOL_RECYCLE_RING , +PP_ETHTOOL_RECYCLE_RING_FULL , +PP_ETHTOOL_RECYCLE_RELEASED_REF , +PP_ETHTOOL_STATS_MAX , +u64 *page_pool_ethtool_stats_get(u64 *data, struct page_pool_stats *stats) +{ +int i; + +for (i =0 ; i <PP_ETHTOOL_STATS_MAX ; i++) { +switch (i) {
include/net/page_pool.h struct page_pool_stats {
Quoted 10 lines
include/net/page_pool.h +10
-0+case PP_ETHTOOL_ALLOC_FAST : + *data++ = stats->alloc_stats.fast; +break ; +case PP_ETHTOOL_ALLOC_SLOW : + *data++ = stats->alloc_stats.slow; +break ; +case PP_ETHTOOL_ALLOC_SLOW_HIGH_ORDER : + *data++ = stats->alloc_stats.slow_high_order; +break ; +case PP_ETHTOOL_ALLOC_EMPTY :
What is the purpose of this enum? The order should be fixed, so just do:
*data++ = stats->alloc_stats.fast; *data++ = stats->alloc_stats.slow; *data++ = stats->alloc_stats.slow_high_order;
and don't use the enum.
Andrew
Re: [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats
In reply to Lorenzo BianconiQuoted 8 lines
net/core/page_pool.c +8
-0+u8 *page_pool_ethtool_stats_get_strings (u8 *data) +{ +static const char stats[PP_ETHTOOL_STATS_MAX ][ETH_GSTRING_LEN ] = { +"rx_pp_alloc_fast" , +"rx_pp_alloc_slow" , +"rx_pp_alloc_slow_ho" , +"rx_pp_alloc_empty" , +"rx_pp_alloc_refill" ,
net/core/page_pool.c bool page_pool_get_stats(struct page_pool *pool, · line 94
Quoted 10 lines
net/core/page_pool.c +10
-0+"rx_pp_alloc_waive" , +"rx_pp_recycle_cached" , +"rx_pp_recycle_cache_full" , +"rx_pp_recycle_ring" , +"rx_pp_recycle_ring_full" , + "rx_pp_recycle_released_ref", + }; +int i; + +for (i =0 ; i <PP_ETHTOOL_STATS_MAX ; i++) {
I suggest you move this stats array out of the function, and then you can use ARRAY_SIZE(stats) instead of PP_ETHTOOL_STATS_MAX. That is a pretty common patters for ethtool statistics.
Andrew
Re: [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats
Quoted 14 lines · Andrew Lunn
Quoted 14 lines
net/core/page_pool.c +14
-0+u8 *page_pool_ethtool_stats_get_strings (u8 *data) +{ +static const char stats[PP_ETHTOOL_STATS_MAX ][ETH_GSTRING_LEN ] = { +"rx_pp_alloc_fast" , +"rx_pp_alloc_slow" , +"rx_pp_alloc_slow_ho" , +"rx_pp_alloc_empty" , +"rx_pp_alloc_refill" , +"rx_pp_alloc_waive" , +"rx_pp_recycle_cached" , +"rx_pp_recycle_cache_full" , +"rx_pp_recycle_ring" , +"rx_pp_recycle_ring_full" , +"rx_pp_recycle_released_ref" ,
Quoted 8 lines
net/core/page_pool.c bool page_pool_get_stats(struct page_pool *pool, · line 94
Quoted 4 lines
net/core/page_pool.c +4
-0+ }; +int i; + +for (i =0 ; i <PP_ETHTOOL_STATS_MAX ; i++) {I suggest you move this stats array out of the function, and then you can use
ARRAY_SIZE(stats)instead ofPP_ETHTOOL_STATS_MAX. That is a pretty common patters for ethtool statistics.
ack, I will fix it.
Regards,
Lorenzo
Quoted 3 lines
Andrew
PATCH v2 2/2 net: mvneta: add support for page_pool_get_stats
In reply to Lorenzo BianconiIntroduce support for the page_pool_get_stats API to mvneta driver Report page_pool stats through ethtool.
--- drivers/net/ethernet/marvell/Kconfig | 1 + drivers/net/ethernet/marvell/mvneta.c | 38 +++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 5 deletions(-)
drivers/net/ethernet/marvell/Kconfig +1 -0
@@ -62,6 +62,7 @@ config MVNETA select MVMDIO select PHYLINK select PAGE_POOL + select PAGE_POOL_STATS help This driver supports the network interface units in the Marvell ARMADA XP, ARMADA 370, ARMADA 38x and
drivers/net/ethernet/marvell/mvneta.c +33 -5
@@ -540,7 +540,7 @@ struct mvneta_port {bool eee_active;bool tx_lpi_enabled; -u64 ethtool_stats[ARRAY_SIZE (mvneta_statistics)]; +u64 *ethtool_stats;u32 indir[MVNETA_RSS_LU_TABLE_SIZE ];
@@ -4732,9 +4732,13 @@ static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset,if (sset ==ETH_SS_STATS ) {int i; -for (i =0 ; i <ARRAY_SIZE (mvneta_statistics); i++) -memcpy (data + i *ETH_GSTRING_LEN , - mvneta_statistics[i].name,ETH_GSTRING_LEN ); +for (i =0 ; i <ARRAY_SIZE (mvneta_statistics); i++) { +memcpy (data, mvneta_statistics[i].name, +ETH_GSTRING_LEN ); + data +=ETH_GSTRING_LEN ; + } + +page_pool_ethtool_stats_get_strings (data); } }
@@ -4847,6 +4851,17 @@ static void mvneta_ethtool_update_stats(struct mvneta_port *pp) } } +static void mvneta_ethtool_pp_stats (struct mvneta_port *pp,u64 *data) +{ +struct page_pool_stats stats = {}; +int i; + +for (i =0 ; i < rxq_number; i++) +page_pool_get_stats (pp->rxqs[i].page_pool, &stats); + +page_pool_ethtool_stats_get (data, &stats); +} +static void mvneta_ethtool_get_stats (struct net_device *dev,struct ethtool_stats *stats,u64 *data) {
@@ -4857,12 +4872,16 @@ static void mvneta_ethtool_get_stats(struct net_device *dev,for (i =0 ; i <ARRAY_SIZE (mvneta_statistics); i++) *data++ = pp->ethtool_stats[i]; + +mvneta_ethtool_pp_stats (pp, data); }static int mvneta_ethtool_get_sset_count (struct net_device *dev,int sset) {if (sset ==ETH_SS_STATS ) -return ARRAY_SIZE (mvneta_statistics); +return ARRAY_SIZE (mvneta_statistics) + +page_pool_ethtool_stats_get_count (); +return -EOPNOTSUPP ; }
@@ -5372,6 +5391,7 @@ static int mvneta_probe(struct platform_device *pdev)phy_interface_t phy_mode;const char *mac_from;int tx_csum_limit; +int stats_len;int err;int cpu;
@@ -5392,6 +5412,14 @@ static int mvneta_probe(struct platform_device *pdev) pp->rxq_def = rxq_def; pp->indir[0 ] = rxq_def; + stats_len =ARRAY_SIZE (mvneta_statistics) + +page_pool_ethtool_stats_get_count (); + pp->ethtool_stats =devm_kzalloc (&pdev->dev, +sizeof (*pp->ethtool_stats) * stats_len, +GFP_KERNEL ); +if (!pp->ethtool_stats) +return -ENOMEM ; + err =of_get_phy_mode (dn, &phy_mode);if (err) {dev_err (&pdev->dev,"incorrect phy-mode\n" );
Signature · 1 line
2.35.1
drivers/net/ethernet/marvell/mvneta.c static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset, · line 4739
Quoted 12 lines
drivers/net/ethernet/marvell/mvneta.c +5
-3@@ -4732,9 +4732,13 @@ static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset,if (sset ==ETH_SS_STATS ) {int i; -for (i =0 ; i <ARRAY_SIZE (mvneta_statistics); i++) -memcpy (data + i *ETH_GSTRING_LEN , - mvneta_statistics[i].name,ETH_GSTRING_LEN ); +for (i =0 ; i <ARRAY_SIZE (mvneta_statistics); i++) { +memcpy (data, mvneta_statistics[i].name, +ETH_GSTRING_LEN ); + data +=ETH_GSTRING_LEN ; + }
You don't need to touch this loop, you can just do:
Review static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset, · line 4741
Quoted 2 lines
(quoted diff) +2
-0+ + page_pool_ethtool_stats_get_strings(data +
ETH_GSTRING_LEN * ARRAY_SIZE(mvneta_staticstics)));
drivers/net/ethernet/marvell/mvneta.c static int mvneta_probe(struct platform_device *pdev) · line 5419
Quoted 12 lines
drivers/net/ethernet/marvell/mvneta.c +5
-0} }@@ -5392,6 +5412,14 @@ static int mvneta_probe(struct platform_device *pdev) pp->rxq_def = rxq_def; pp->indir[0 ] = rxq_def; + stats_len =ARRAY_SIZE (mvneta_statistics) + +page_pool_ethtool_stats_get_count (); + pp->ethtool_stats =devm_kzalloc (&pdev->dev, +sizeof (*pp->ethtool_stats) * stats_len, +GFP_KERNEL );
Why do you do this? The page_pool stats are never stored in pp->ethtool_stats.
Andrew
Quoted 4 lines
Quoted 4 lines
drivers/net/ethernet/marvell/mvneta.c +0
-0@@ -4732,9 +4732,13 @@ static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset,if (sset ==ETH_SS_STATS ) {int i;
Quoted 10 lines
drivers/net/ethernet/marvell/mvneta.c static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset, · line 4739
Quoted 8 lines
drivers/net/ethernet/marvell/mvneta.c +5
-3-for (i =0 ; i <ARRAY_SIZE (mvneta_statistics); i++) -memcpy (data + i *ETH_GSTRING_LEN , - mvneta_statistics[i].name,ETH_GSTRING_LEN ); +for (i =0 ; i <ARRAY_SIZE (mvneta_statistics); i++) { +memcpy (data, mvneta_statistics[i].name, +ETH_GSTRING_LEN ); + data +=ETH_GSTRING_LEN ; + }You don't need to touch this loop, you can just do:
ack, it was just to avoid a long line, I will fix it.
Quoted 11 lines
Review static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset, · line 4741
Quoted 3 lines
(quoted diff) +2
-0+ + page_pool_ethtool_stats_get_strings(data +ETH_GSTRING_LEN * ARRAY_SIZE(mvneta_staticstics)));Quoted 7 lines
drivers/net/ethernet/marvell/mvneta.c +0
-0} }@@ -5392,6 +5412,14 @@ static int mvneta_probe(struct platform_device *pdev) pp->rxq_def = rxq_def; pp->indir[0 ] = rxq_def;
Quoted 8 lines
drivers/net/ethernet/marvell/mvneta.c static int mvneta_probe(struct platform_device *pdev) · line 5419
Quoted 5 lines
drivers/net/ethernet/marvell/mvneta.c +5
-0+ stats_len =ARRAY_SIZE (mvneta_statistics) + +page_pool_ethtool_stats_get_count (); + pp->ethtool_stats =devm_kzalloc (&pdev->dev, +sizeof (*pp->ethtool_stats) * stats_len, +GFP_KERNEL );Why do you do this? The
page_poolstats are never stored inpp->ethtool_stats.
ops, I need to drop this, thanks.
Regards,
Lorenzo
Quoted 3 lines
Andrew