* [PATCH v2 net-next 0/2] net: mvneta: add support for page_pool_get_stats @ 2022-04-08 8:23 Lorenzo Bianconi 2022-04-08 8:23 ` [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats Lorenzo Bianconi 2022-04-08 8:24 ` [PATCH v2 net-next 2/2] net: mvneta: add support for page_pool_get_stats Lorenzo Bianconi 0 siblings, 2 replies; 8+ messages in thread From: Lorenzo Bianconi @ 2022-04-08 8:23 UTC (permalink / raw) To: netdev Cc: lorenzo.bianconi, davem, kuba, pabeni, thomas.petazzoni, ilias.apalodimas, jbrouer, andrew, jdamato 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(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats 2022-04-08 8:23 [PATCH v2 net-next 0/2] net: mvneta: add support for page_pool_get_stats Lorenzo Bianconi @ 2022-04-08 8:23 ` Lorenzo Bianconi 2022-04-08 13:33 ` Andrew Lunn 2022-04-08 13:45 ` Andrew Lunn 2022-04-08 8:24 ` [PATCH v2 net-next 2/2] net: mvneta: add support for page_pool_get_stats Lorenzo Bianconi 1 sibling, 2 replies; 8+ messages in thread From: Lorenzo Bianconi @ 2022-04-08 8:23 UTC (permalink / raw) To: netdev Cc: lorenzo.bianconi, davem, kuba, pabeni, thomas.petazzoni, ilias.apalodimas, jbrouer, andrew, jdamato Introduce page_pool APIs to report stats through ethtool and reduce duplicated code in each driver. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- include/net/page_pool.h | 24 ++++++++++++ net/core/page_pool.c | 81 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/include/net/page_pool.h b/include/net/page_pool.h index ea5fb70e5101..ada528df1843 100644 --- a/include/net/page_pool.h +++ b/include/net/page_pool.h @@ -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 diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 4af55d28ffa3..bffc3037d7f6 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -18,6 +18,7 @@ #include <linux/page-flags.h> #include <linux/mm.h> /* for __put_page() */ #include <linux/poison.h> +#include <linux/ethtool.h> #include <trace/events/page_pool.h> @@ -50,7 +51,13 @@ bool page_pool_get_stats(struct page_pool *pool, if (!stats) return false; - memcpy(&stats->alloc_stats, &pool->alloc_stats, sizeof(pool->alloc_stats)); + /* The caller is responsible to initialize stats. */ + stats->alloc_stats.fast += pool->alloc_stats.fast; + stats->alloc_stats.slow += pool->alloc_stats.slow; + stats->alloc_stats.slow_high_order += pool->alloc_stats.slow_high_order; + stats->alloc_stats.empty += pool->alloc_stats.empty; + stats->alloc_stats.refill += pool->alloc_stats.refill; + stats->alloc_stats.waive += pool->alloc_stats.waive; for_each_possible_cpu(cpu) { const struct page_pool_recycle_stats *pcpu = @@ -66,6 +73,78 @@ bool page_pool_get_stats(struct page_pool *pool, return true; } EXPORT_SYMBOL(page_pool_get_stats); + +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", + }; + int i; + + for (i = 0; i < PP_ETHTOOL_STATS_MAX; i++) { + memcpy(data, stats[i], ETH_GSTRING_LEN); + data += ETH_GSTRING_LEN; + } + + return data; +} +EXPORT_SYMBOL(page_pool_ethtool_stats_get_strings); + +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) { + 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: + *data++ = stats->alloc_stats.empty; + break; + case PP_ETHTOOL_ALLOC_REFILL: + *data++ = stats->alloc_stats.refill; + break; + case PP_ETHTOOL_ALLOC_WAIVE: + *data++ = stats->alloc_stats.waive; + break; + case PP_ETHTOOL_RECYCLE_CACHED: + *data++ = stats->recycle_stats.cached; + break; + case PP_ETHTOOL_RECYCLE_CACHE_FULL: + *data++ = stats->recycle_stats.cache_full; + break; + case PP_ETHTOOL_RECYCLE_RING: + *data++ = stats->recycle_stats.ring; + break; + case PP_ETHTOOL_RECYCLE_RING_FULL: + *data++ = stats->recycle_stats.ring_full; + break; + case PP_ETHTOOL_RECYCLE_RELEASED_REF: + *data++ = stats->recycle_stats.released_refcnt; + break; + } + } + + return data; +} +EXPORT_SYMBOL(page_pool_ethtool_stats_get); #else #define alloc_stat_inc(pool, __stat) #define recycle_stat_inc(pool, __stat) -- 2.35.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats 2022-04-08 8:23 ` [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats Lorenzo Bianconi @ 2022-04-08 13:33 ` Andrew Lunn 2022-04-08 13:45 ` Andrew Lunn 1 sibling, 0 replies; 8+ messages in thread From: Andrew Lunn @ 2022-04-08 13:33 UTC (permalink / raw) To: Lorenzo Bianconi Cc: netdev, lorenzo.bianconi, davem, kuba, pabeni, thomas.petazzoni, ilias.apalodimas, jbrouer, jdamato > +/* 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) { > + 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats 2022-04-08 8:23 ` [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats Lorenzo Bianconi 2022-04-08 13:33 ` Andrew Lunn @ 2022-04-08 13:45 ` Andrew Lunn 2022-04-08 17:14 ` Lorenzo Bianconi 1 sibling, 1 reply; 8+ messages in thread From: Andrew Lunn @ 2022-04-08 13:45 UTC (permalink / raw) To: Lorenzo Bianconi Cc: netdev, lorenzo.bianconi, davem, kuba, pabeni, thomas.petazzoni, ilias.apalodimas, jbrouer, jdamato > +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", > + }; > + 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats 2022-04-08 13:45 ` Andrew Lunn @ 2022-04-08 17:14 ` Lorenzo Bianconi 0 siblings, 0 replies; 8+ messages in thread From: Lorenzo Bianconi @ 2022-04-08 17:14 UTC (permalink / raw) To: Andrew Lunn Cc: Lorenzo Bianconi, netdev, davem, kuba, pabeni, thomas.petazzoni, ilias.apalodimas, jbrouer, jdamato [-- Attachment #1: Type: text/plain, Size: 867 bytes --] On Apr 08, Andrew Lunn wrote: > > +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", > > + }; > > + 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. ack, I will fix it. Regards, Lorenzo > > Andrew > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 net-next 2/2] net: mvneta: add support for page_pool_get_stats 2022-04-08 8:23 [PATCH v2 net-next 0/2] net: mvneta: add support for page_pool_get_stats Lorenzo Bianconi 2022-04-08 8:23 ` [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats Lorenzo Bianconi @ 2022-04-08 8:24 ` Lorenzo Bianconi 2022-04-08 13:42 ` Andrew Lunn 1 sibling, 1 reply; 8+ messages in thread From: Lorenzo Bianconi @ 2022-04-08 8:24 UTC (permalink / raw) To: netdev Cc: lorenzo.bianconi, davem, kuba, pabeni, thomas.petazzoni, ilias.apalodimas, jbrouer, andrew, jdamato Introduce support for the page_pool_get_stats API to mvneta driver Report page_pool stats through ethtool. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- drivers/net/ethernet/marvell/Kconfig | 1 + drivers/net/ethernet/marvell/mvneta.c | 38 +++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig index fe0989c0fc25..1240cb2dc07f 100644 --- a/drivers/net/ethernet/marvell/Kconfig +++ b/drivers/net/ethernet/marvell/Kconfig @@ -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 diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index 934f6dd90992..7cc20d25a7a3 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -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"); -- 2.35.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 net-next 2/2] net: mvneta: add support for page_pool_get_stats 2022-04-08 8:24 ` [PATCH v2 net-next 2/2] net: mvneta: add support for page_pool_get_stats Lorenzo Bianconi @ 2022-04-08 13:42 ` Andrew Lunn 2022-04-08 17:14 ` Lorenzo Bianconi 0 siblings, 1 reply; 8+ messages in thread From: Andrew Lunn @ 2022-04-08 13:42 UTC (permalink / raw) To: Lorenzo Bianconi Cc: netdev, lorenzo.bianconi, davem, kuba, pabeni, thomas.petazzoni, ilias.apalodimas, jbrouer, jdamato > @@ -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: > + > + page_pool_ethtool_stats_get_strings(data + ETH_GSTRING_LEN * ARRAY_SIZE(mvneta_staticstics))); > } > } > > @@ -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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 net-next 2/2] net: mvneta: add support for page_pool_get_stats 2022-04-08 13:42 ` Andrew Lunn @ 2022-04-08 17:14 ` Lorenzo Bianconi 0 siblings, 0 replies; 8+ messages in thread From: Lorenzo Bianconi @ 2022-04-08 17:14 UTC (permalink / raw) To: Andrew Lunn Cc: Lorenzo Bianconi, netdev, davem, kuba, pabeni, thomas.petazzoni, ilias.apalodimas, jbrouer, jdamato [-- Attachment #1: Type: text/plain, Size: 1325 bytes --] > > @@ -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: ack, it was just to avoid a long line, I will fix it. > > > + > > + page_pool_ethtool_stats_get_strings(data + > ETH_GSTRING_LEN * ARRAY_SIZE(mvneta_staticstics))); > > } > > } > > > > @@ -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. ops, I need to drop this, thanks. Regards, Lorenzo > > Andrew > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-04-08 17:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-04-08 8:23 [PATCH v2 net-next 0/2] net: mvneta: add support for page_pool_get_stats Lorenzo Bianconi 2022-04-08 8:23 ` [PATCH v2 net-next 1/2] net: page_pool: introduce ethtool stats Lorenzo Bianconi 2022-04-08 13:33 ` Andrew Lunn 2022-04-08 13:45 ` Andrew Lunn 2022-04-08 17:14 ` Lorenzo Bianconi 2022-04-08 8:24 ` [PATCH v2 net-next 2/2] net: mvneta: add support for page_pool_get_stats Lorenzo Bianconi 2022-04-08 13:42 ` Andrew Lunn 2022-04-08 17:14 ` Lorenzo Bianconi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).