* [dpdk-dev] [PATCH 0/2] failsafe: add xstats @ 2019-06-26 22:21 Stephen Hemminger 2019-06-26 22:21 ` [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger ` (4 more replies) 0 siblings, 5 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-06-26 22:21 UTC (permalink / raw) To: gaetan.rivet; +Cc: dev, Stephen Hemminger A useful feature of netvsc PMD is the ability to see how many packets were processed through the VF device. This patch set adds a similar (but more limited) capability to failsafe driver. Since failsafe doesn't have top level xstats, this set uses the generic xstats that exist already as a base then adds the sub-device xstats to that. Stephen Hemminger (2): ethdev: expose basic xstats for driver use failsafe: implement xstats drivers/net/failsafe/failsafe_ops.c | 144 +++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.c | 17 ++- lib/librte_ethdev/rte_ethdev_core.h | 14 +++ lib/librte_ethdev/rte_ethdev_version.map | 3 + 4 files changed, 169 insertions(+), 9 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use 2019-06-26 22:21 [dpdk-dev] [PATCH 0/2] failsafe: add xstats Stephen Hemminger @ 2019-06-26 22:21 ` Stephen Hemminger 2019-06-26 22:21 ` [dpdk-dev] [PATCH 2/2] failsafe: implement xstats Stephen Hemminger ` (3 subsequent siblings) 4 siblings, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-06-26 22:21 UTC (permalink / raw) To: gaetan.rivet; +Cc: dev, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> Avoid duplication by having generic basic xstats available for use by drivers. A later patch uses this for failsafe driver. Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> --- lib/librte_ethdev/rte_ethdev.c | 17 ++++++++--------- lib/librte_ethdev/rte_ethdev_core.h | 14 ++++++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 3 +++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 8ac301608b9c..a83e9727c144 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1996,8 +1996,8 @@ rte_eth_stats_reset(uint16_t port_id) return 0; } -static inline int -get_xstats_basic_count(struct rte_eth_dev *dev) +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev) { uint16_t nb_rxqs, nb_txqs; int count; @@ -2034,7 +2034,7 @@ get_xstats_count(uint16_t port_id) count = 0; - count += get_xstats_basic_count(dev); + count += rte_eth_basic_stats_count(dev); return count; } @@ -2084,7 +2084,7 @@ rte_eth_xstats_get_id_by_name(uint16_t port_id, const char *xstat_name, } /* retrieve basic stats names */ -static int +int rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, struct rte_eth_xstat_name *xstats_names) { @@ -2140,7 +2140,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id, RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); ret = get_xstats_count(port_id); if (ret < 0) return ret; @@ -2268,8 +2268,7 @@ rte_eth_xstats_get_names(uint16_t port_id, return cnt_used_entries; } - -static int +int rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats) { struct rte_eth_dev *dev; @@ -2341,7 +2340,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, expected_entries = (uint16_t)ret; struct rte_eth_xstat xstats[expected_entries]; dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); /* Return max number of stats if no ids given */ if (!ids) { @@ -2355,7 +2354,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, return -EINVAL; if (ids && dev->dev_ops->xstats_get_by_id != NULL && size) { - unsigned int basic_count = get_xstats_basic_count(dev); + unsigned int basic_count = rte_eth_basic_stats_count(dev); uint64_t ids_copy[size]; for (i = 0; i < size; i++) { diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h index 2922d5b7cc95..91ce1880d1c6 100644 --- a/lib/librte_ethdev/rte_ethdev_core.h +++ b/lib/librte_ethdev/rte_ethdev_core.h @@ -517,6 +517,20 @@ struct eth_dev_ops { /**< Test if a port supports specific mempool ops */ }; +/** + * @internal + * Get basic stats for ethdev + */ +int __rte_experimental +rte_eth_basic_stats_count(struct rte_eth_dev *dev); + +int __rte_experimental +rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names); + +int __rte_experimental +rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats); + /** * @internal * Structure used to hold information about the callbacks to be called for a diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map index df9141825c3f..949a79800cbc 100644 --- a/lib/librte_ethdev/rte_ethdev_version.map +++ b/lib/librte_ethdev/rte_ethdev_version.map @@ -239,6 +239,9 @@ DPDK_19.05 { EXPERIMENTAL { global: + rte_eth_basic_stats_count; + rte_eth_basic_stats_get; + rte_eth_basic_stats_get_names; rte_eth_devargs_parse; rte_eth_dev_create; rte_eth_dev_destroy; -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 2/2] failsafe: implement xstats 2019-06-26 22:21 [dpdk-dev] [PATCH 0/2] failsafe: add xstats Stephen Hemminger 2019-06-26 22:21 ` [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger @ 2019-06-26 22:21 ` Stephen Hemminger 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 0/2] failsafe: add xstats Stephen Hemminger ` (2 subsequent siblings) 4 siblings, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-06-26 22:21 UTC (permalink / raw) To: gaetan.rivet; +Cc: dev, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> Add support for extended statistics in failsafe driver. Reports basic statistics for the failsafe driver, and detailed statistics for each sub device. Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> --- drivers/net/failsafe/failsafe_ops.c | 144 ++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 96e05d4dc4b1..9ab32457983d 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -789,6 +789,147 @@ fs_stats_reset(struct rte_eth_dev *dev) fs_unlock(dev, 0); } +static int +fs_xstats_count(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + int r, count; + uint8_t i; + + count = rte_eth_basic_stats_count(dev); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + if (sdev->state < DEV_ACTIVE) + continue; + + r = rte_eth_xstats_get_names(PORT_ID(sdev), NULL, 0); + if (r < 0) { + fs_unlock(dev, 0); + return r; + } + + count += r; + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int limit) +{ + struct sub_device *sdev; + unsigned int count; + uint8_t i; + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; + int j,r; + + if (!xstats_names) + return fs_xstats_count(dev); + + r = rte_eth_basic_stats_get_names(dev, xstats_names); + if (r < 0) + return r; + + count = r; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + struct rte_eth_xstat_name *sub_names = xstats_names + count; + + if (sdev->state < DEV_ACTIVE) + continue; + + if (count >= limit) + break; + + r = rte_eth_xstats_get_names(PORT_ID(sdev), sub_names, + count < limit ? limit - count: 0); + if (r < 0) { + fs_unlock(dev, 0); + return r; + } + + /* add sub_ prefix to names */ + for (j = 0; j < r; j++) { + snprintf(tmp, sizeof(tmp), "sub%u_%s", + i, sub_names[j].name); + memcpy(sub_names[j].name, tmp, + RTE_ETH_XSTATS_NAME_SIZE); + } + + count += r; + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int nstats, j, count, scount; + struct sub_device *sdev; + uint8_t i; + int ret; + + nstats = fs_xstats_count(dev); + if (n < nstats || xstats == NULL) + return nstats; + + ret = rte_eth_basic_stats_get(dev->data->port_id, xstats); + if (ret < 0) + return ret; + + count = ret; + for (j = 0; j < count; j++) + xstats[j].id = j; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + if (sdev->state < DEV_ACTIVE) + continue; + + ret = rte_eth_xstats_get(PORT_ID(sdev), + xstats + count, + n > count ? n - count : 0); + if (ret < 0) { + fs_unlock(dev, 0); + return ret; + } + scount = ret; + + /* add offset to id's from sub-device */ + for (j = 0; j < scount; j++) + xstats[count + j].id += count; + + count += scount; + } + fs_unlock(dev, 0); + + return count; +} + +static void +fs_xstats_reset(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + + rte_eth_stats_reset(dev->data->port_id); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + rte_eth_xstats_reset(PORT_ID(sdev)); + } + fs_unlock(dev, 0); +} + static void fs_dev_merge_desc_lim(struct rte_eth_desc_lim *to, const struct rte_eth_desc_lim *from) @@ -1233,6 +1374,9 @@ const struct eth_dev_ops failsafe_ops = { .link_update = fs_link_update, .stats_get = fs_stats_get, .stats_reset = fs_stats_reset, + .xstats_get = fs_xstats_get, + .xstats_get_names = fs_xstats_get_names, + .xstats_reset = fs_xstats_reset, .dev_infos_get = fs_dev_infos_get, .dev_supported_ptypes_get = fs_dev_supported_ptypes_get, .mtu_set = fs_mtu_set, -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 0/2] failsafe: add xstats 2019-06-26 22:21 [dpdk-dev] [PATCH 0/2] failsafe: add xstats Stephen Hemminger 2019-06-26 22:21 ` [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-06-26 22:21 ` [dpdk-dev] [PATCH 2/2] failsafe: implement xstats Stephen Hemminger @ 2019-06-26 23:33 ` Stephen Hemminger 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats Stephen Hemminger 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Stephen Hemminger 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 0/2] failsafe: " Stephen Hemminger 4 siblings, 2 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-06-26 23:33 UTC (permalink / raw) To: gaetan.rivet; +Cc: dev, Stephen Hemminger A useful feature of netvsc PMD is the ability to see how many packets were processed through the VF device. This patch set adds a similar (but more limited) capability to failsafe driver. Since failsafe doesn't have top level xstats, this set uses the generic xstats that exist already as a base then adds the sub-device xstats to that. v2 - fix checkpatch complaints Stephen Hemminger (2): ethdev: expose basic xstats for driver use failsafe: implement xstats drivers/net/failsafe/failsafe_ops.c | 130 +++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.c | 17 ++- lib/librte_ethdev/rte_ethdev_core.h | 14 +++ lib/librte_ethdev/rte_ethdev_version.map | 3 + 4 files changed, 155 insertions(+), 9 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 0/2] failsafe: add xstats Stephen Hemminger @ 2019-06-26 23:33 ` Stephen Hemminger 2019-07-01 10:54 ` Andrew Rybchenko 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats Stephen Hemminger 1 sibling, 1 reply; 30+ messages in thread From: Stephen Hemminger @ 2019-06-26 23:33 UTC (permalink / raw) To: gaetan.rivet; +Cc: dev, Stephen Hemminger, Stephen Hemminger Avoid duplication by having generic basic xstats available for use by drivers. A later patch uses this for failsafe driver. Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> --- lib/librte_ethdev/rte_ethdev.c | 17 ++++++++--------- lib/librte_ethdev/rte_ethdev_core.h | 14 ++++++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 3 +++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 8ac301608b9c..a83e9727c144 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1996,8 +1996,8 @@ rte_eth_stats_reset(uint16_t port_id) return 0; } -static inline int -get_xstats_basic_count(struct rte_eth_dev *dev) +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev) { uint16_t nb_rxqs, nb_txqs; int count; @@ -2034,7 +2034,7 @@ get_xstats_count(uint16_t port_id) count = 0; - count += get_xstats_basic_count(dev); + count += rte_eth_basic_stats_count(dev); return count; } @@ -2084,7 +2084,7 @@ rte_eth_xstats_get_id_by_name(uint16_t port_id, const char *xstat_name, } /* retrieve basic stats names */ -static int +int rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, struct rte_eth_xstat_name *xstats_names) { @@ -2140,7 +2140,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id, RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); ret = get_xstats_count(port_id); if (ret < 0) return ret; @@ -2268,8 +2268,7 @@ rte_eth_xstats_get_names(uint16_t port_id, return cnt_used_entries; } - -static int +int rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats) { struct rte_eth_dev *dev; @@ -2341,7 +2340,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, expected_entries = (uint16_t)ret; struct rte_eth_xstat xstats[expected_entries]; dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); /* Return max number of stats if no ids given */ if (!ids) { @@ -2355,7 +2354,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, return -EINVAL; if (ids && dev->dev_ops->xstats_get_by_id != NULL && size) { - unsigned int basic_count = get_xstats_basic_count(dev); + unsigned int basic_count = rte_eth_basic_stats_count(dev); uint64_t ids_copy[size]; for (i = 0; i < size; i++) { diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h index 2922d5b7cc95..91ce1880d1c6 100644 --- a/lib/librte_ethdev/rte_ethdev_core.h +++ b/lib/librte_ethdev/rte_ethdev_core.h @@ -517,6 +517,20 @@ struct eth_dev_ops { /**< Test if a port supports specific mempool ops */ }; +/** + * @internal + * Get basic stats for ethdev + */ +int __rte_experimental +rte_eth_basic_stats_count(struct rte_eth_dev *dev); + +int __rte_experimental +rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names); + +int __rte_experimental +rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats); + /** * @internal * Structure used to hold information about the callbacks to be called for a diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map index df9141825c3f..949a79800cbc 100644 --- a/lib/librte_ethdev/rte_ethdev_version.map +++ b/lib/librte_ethdev/rte_ethdev_version.map @@ -239,6 +239,9 @@ DPDK_19.05 { EXPERIMENTAL { global: + rte_eth_basic_stats_count; + rte_eth_basic_stats_get; + rte_eth_basic_stats_get_names; rte_eth_devargs_parse; rte_eth_dev_create; rte_eth_dev_destroy; -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger @ 2019-07-01 10:54 ` Andrew Rybchenko 2019-07-01 14:59 ` Stephen Hemminger 0 siblings, 1 reply; 30+ messages in thread From: Andrew Rybchenko @ 2019-07-01 10:54 UTC (permalink / raw) To: Stephen Hemminger, gaetan.rivet; +Cc: dev, Stephen Hemminger On 27.06.2019 2:33, Stephen Hemminger wrote: > Avoid duplication by having generic basic xstats available > for use by drivers. A later patch uses this for failsafe > driver. > > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> [...] > diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h > index 2922d5b7cc95..91ce1880d1c6 100644 > --- a/lib/librte_ethdev/rte_ethdev_core.h > +++ b/lib/librte_ethdev/rte_ethdev_core.h > @@ -517,6 +517,20 @@ struct eth_dev_ops { > /**< Test if a port supports specific mempool ops */ > }; > > +/** > + * @internal > + * Get basic stats for ethdev > + */ > +int __rte_experimental > +rte_eth_basic_stats_count(struct rte_eth_dev *dev); > + > +int __rte_experimental > +rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, > + struct rte_eth_xstat_name *xstats_names); > + > +int __rte_experimental > +rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats); > + > /** > * @internal > * Structure used to hold information about the callbacks to be called for a It conflicts with __rte_experimenal placing patch which is on the mailing list. Also I've expected to see these functions in rte_ethdev_driver.h to avoid inclusion in rte_ethdev.h. As I understand these functions are for rte_ethdev and drivers only. > diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map > index df9141825c3f..949a79800cbc 100644 > --- a/lib/librte_ethdev/rte_ethdev_version.map > +++ b/lib/librte_ethdev/rte_ethdev_version.map > @@ -239,6 +239,9 @@ DPDK_19.05 { > EXPERIMENTAL { > global: > > + rte_eth_basic_stats_count; > + rte_eth_basic_stats_get; > + rte_eth_basic_stats_get_names; > rte_eth_devargs_parse; > rte_eth_dev_create; > rte_eth_dev_destroy; ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use 2019-07-01 10:54 ` Andrew Rybchenko @ 2019-07-01 14:59 ` Stephen Hemminger 0 siblings, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-07-01 14:59 UTC (permalink / raw) To: Andrew Rybchenko; +Cc: gaetan.rivet, dev, Stephen Hemminger On Mon, 1 Jul 2019 13:54:52 +0300 Andrew Rybchenko <arybchenko@solarflare.com> wrote: > On 27.06.2019 2:33, Stephen Hemminger wrote: > > Avoid duplication by having generic basic xstats available > > for use by drivers. A later patch uses this for failsafe > > driver. > > > > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> > > [...] > > > > diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h > > index 2922d5b7cc95..91ce1880d1c6 100644 > > --- a/lib/librte_ethdev/rte_ethdev_core.h > > +++ b/lib/librte_ethdev/rte_ethdev_core.h > > @@ -517,6 +517,20 @@ struct eth_dev_ops { > > /**< Test if a port supports specific mempool ops */ > > }; > > > > +/** > > + * @internal > > + * Get basic stats for ethdev > > + */ > > +int __rte_experimental > > +rte_eth_basic_stats_count(struct rte_eth_dev *dev); > > + > > +int __rte_experimental > > +rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, > > + struct rte_eth_xstat_name *xstats_names); > > + > > +int __rte_experimental > > +rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats); > > + > > /** > > * @internal > > * Structure used to hold information about the callbacks to be called for a > > It conflicts with __rte_experimenal placing patch which is on the > mailing list. > Also I've expected to see these functions in rte_ethdev_driver.h to > avoid inclusion in rte_ethdev.h. As I understand these functions are for > rte_ethdev and drivers only. Yes. Will change in V3 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 0/2] failsafe: add xstats Stephen Hemminger 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger @ 2019-06-26 23:33 ` Stephen Hemminger 2019-07-01 12:33 ` Gaëtan Rivet 1 sibling, 1 reply; 30+ messages in thread From: Stephen Hemminger @ 2019-06-26 23:33 UTC (permalink / raw) To: gaetan.rivet; +Cc: dev, Stephen Hemminger, Stephen Hemminger Add support for extended statistics in failsafe driver. Reports basic statistics for the failsafe driver, and detailed statistics for each sub device. Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> --- drivers/net/failsafe/failsafe_ops.c | 130 ++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 96e05d4dc4b1..a250c0528965 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -789,6 +789,133 @@ fs_stats_reset(struct rte_eth_dev *dev) fs_unlock(dev, 0); } +static int +fs_xstats_count(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + int count; + + count = rte_eth_basic_stats_count(dev); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + count += rte_eth_xstats_get_names(PORT_ID(sdev), NULL, 0); + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int limit) +{ + struct sub_device *sdev; + unsigned int count; + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; + uint8_t i; + int r; + + if (!xstats_names) + return fs_xstats_count(dev); + + r = rte_eth_basic_stats_get_names(dev, xstats_names); + if (r < 0) + return r; + + count = r; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + struct rte_eth_xstat_name *sub_names = xstats_names + count; + + if (count >= limit) + break; + + r = rte_eth_xstats_get_names(PORT_ID(sdev), + xstats_names + count, + limit - count); + if (r < 0) { + fs_unlock(dev, 0); + return r; + } + + /* add sub_ prefix to names */ + for (i = 0; i < r; i++) { + snprintf(tmp, sizeof(tmp), "sub%u_%s", + i, sub_names[i].name); + memcpy(sub_names[i].name, tmp, + RTE_ETH_XSTATS_NAME_SIZE); + } + + count += r; + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int nstats, j, count, scount; + struct sub_device *sdev; + uint8_t i; + int ret; + + nstats = fs_xstats_count(dev); + if (n < nstats || xstats == NULL) + return nstats; + + ret = rte_eth_basic_stats_get(dev->data->port_id, xstats); + if (ret < 0) + return ret; + + count = ret; + for (j = 0; j < count; j++) + xstats[j].id = j; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + ret = rte_eth_xstats_get(PORT_ID(sdev), + xstats + count, + n > count ? n - count : 0); + if (ret < 0) { + fs_unlock(dev, 0); + return ret; + } + scount = ret; + + /* add offset to id's from sub-device */ + for (j = 0; j < scount; j++) + xstats[count + j].id += count; + + count += scount; + } + fs_unlock(dev, 0); + + return count; +} + +static void +fs_xstats_reset(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + + rte_eth_stats_reset(dev->data->port_id); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + rte_eth_xstats_reset(PORT_ID(sdev)); + } + fs_unlock(dev, 0); +} + static void fs_dev_merge_desc_lim(struct rte_eth_desc_lim *to, const struct rte_eth_desc_lim *from) @@ -1233,6 +1360,9 @@ const struct eth_dev_ops failsafe_ops = { .link_update = fs_link_update, .stats_get = fs_stats_get, .stats_reset = fs_stats_reset, + .xstats_get = fs_xstats_get, + .xstats_get_names = fs_xstats_get_names, + .xstats_reset = fs_xstats_reset, .dev_infos_get = fs_dev_infos_get, .dev_supported_ptypes_get = fs_dev_supported_ptypes_get, .mtu_set = fs_mtu_set, -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats Stephen Hemminger @ 2019-07-01 12:33 ` Gaëtan Rivet 0 siblings, 0 replies; 30+ messages in thread From: Gaëtan Rivet @ 2019-07-01 12:33 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev, Stephen Hemminger Hello Stephen, On Wed, Jun 26, 2019 at 04:33:46PM -0700, Stephen Hemminger wrote: > Add support for extended statistics in failsafe driver. > Reports basic statistics for the failsafe driver, and detailed > statistics for each sub device. > > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> > --- > drivers/net/failsafe/failsafe_ops.c | 130 ++++++++++++++++++++++++++++ > 1 file changed, 130 insertions(+) > > diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c > index 96e05d4dc4b1..a250c0528965 100644 > --- a/drivers/net/failsafe/failsafe_ops.c > +++ b/drivers/net/failsafe/failsafe_ops.c > @@ -789,6 +789,133 @@ fs_stats_reset(struct rte_eth_dev *dev) > fs_unlock(dev, 0); > } > > +static int > +fs_xstats_count(struct rte_eth_dev *dev) > +{ > + struct sub_device *sdev; > + uint8_t i; > + int count; > + > + count = rte_eth_basic_stats_count(dev); > + > + fs_lock(dev, 0); > + FOREACH_SUBDEV(sdev, i, dev) { > + count += rte_eth_xstats_get_names(PORT_ID(sdev), NULL, 0); > + } > + fs_unlock(dev, 0); > + > + return count; > +} > + > +static int > +fs_xstats_get_names(struct rte_eth_dev *dev, > + struct rte_eth_xstat_name *xstats_names, > + unsigned int limit) > +{ > + struct sub_device *sdev; > + unsigned int count; > + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; > + uint8_t i; > + int r; > + > + if (!xstats_names) > + return fs_xstats_count(dev); > + > + r = rte_eth_basic_stats_get_names(dev, xstats_names); > + if (r < 0) > + return r; > + > + count = r; > + > + fs_lock(dev, 0); > + FOREACH_SUBDEV(sdev, i, dev) { > + struct rte_eth_xstat_name *sub_names = xstats_names + count; > + > + if (count >= limit) > + break; > + > + r = rte_eth_xstats_get_names(PORT_ID(sdev), > + xstats_names + count, sub_names here? Or do you want to point our the relationship between + count on the array and - count on the array length? If so that makes sense. > + limit - count); > + if (r < 0) { > + fs_unlock(dev, 0); > + return r; > + } > + > + /* add sub_ prefix to names */ > + for (i = 0; i < r; i++) { > + snprintf(tmp, sizeof(tmp), "sub%u_%s", Prefixing is a good idea, maybe it would be better with an '_' between the sub device name and the stat name? The rest of the implementation seems solid, thanks. You need to update the feature matrix for fail-safe however. Regards, -- Gaëtan Rivet 6WIND ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats 2019-06-26 22:21 [dpdk-dev] [PATCH 0/2] failsafe: add xstats Stephen Hemminger ` (2 preceding siblings ...) 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 0/2] failsafe: add xstats Stephen Hemminger @ 2019-07-03 17:25 ` Stephen Hemminger 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger ` (2 more replies) 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 0/2] failsafe: " Stephen Hemminger 4 siblings, 3 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-07-03 17:25 UTC (permalink / raw) To: gaetan.rivet, arybchenko; +Cc: dev, Stephen Hemminger A useful feature of netvsc PMD is the ability to see how many packets were processed through the VF device. This patch set adds a similar (but more limited) capability to failsafe driver. Since failsafe doesn't have top level xstats, this set uses the generic xstats that exist already as a base then adds the sub-device xstats to that. v2 move generic xstats helpers to rte_ethdev_driver.h change prefix on subdevice stats to sub0_XXXX Stephen Hemminger (2): ethdev: expose basic xstats for driver use failsafe: implement xstats drivers/net/failsafe/failsafe_ops.c | 135 +++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.c | 17 ++- lib/librte_ethdev/rte_ethdev_driver.h | 65 +++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 3 + 4 files changed, 211 insertions(+), 9 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Stephen Hemminger @ 2019-07-03 17:25 ` Stephen Hemminger 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats Stephen Hemminger 2019-07-04 9:56 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Gaëtan Rivet 2 siblings, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-07-03 17:25 UTC (permalink / raw) To: gaetan.rivet, arybchenko; +Cc: dev, Stephen Hemminger Avoid duplication by having generic basic xstats available for use by drivers. A later patch uses this for failsafe driver. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/librte_ethdev/rte_ethdev.c | 17 +++---- lib/librte_ethdev/rte_ethdev_driver.h | 65 ++++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 3 ++ 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 31f02ec2ba3b..82fcac57b1a7 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1996,8 +1996,8 @@ rte_eth_stats_reset(uint16_t port_id) return 0; } -static inline int -get_xstats_basic_count(struct rte_eth_dev *dev) +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev) { uint16_t nb_rxqs, nb_txqs; int count; @@ -2034,7 +2034,7 @@ get_xstats_count(uint16_t port_id) count = 0; - count += get_xstats_basic_count(dev); + count += rte_eth_basic_stats_count(dev); return count; } @@ -2084,7 +2084,7 @@ rte_eth_xstats_get_id_by_name(uint16_t port_id, const char *xstat_name, } /* retrieve basic stats names */ -static int +int rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, struct rte_eth_xstat_name *xstats_names) { @@ -2140,7 +2140,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id, RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); ret = get_xstats_count(port_id); if (ret < 0) return ret; @@ -2268,8 +2268,7 @@ rte_eth_xstats_get_names(uint16_t port_id, return cnt_used_entries; } - -static int +int rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats) { struct rte_eth_dev *dev; @@ -2341,7 +2340,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, expected_entries = (uint16_t)ret; struct rte_eth_xstat xstats[expected_entries]; dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); /* Return max number of stats if no ids given */ if (!ids) { @@ -2355,7 +2354,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, return -EINVAL; if (ids && dev->dev_ops->xstats_get_by_id != NULL && size) { - unsigned int basic_count = get_xstats_basic_count(dev); + unsigned int basic_count = rte_eth_basic_stats_count(dev); uint64_t ids_copy[size]; for (i = 0; i < size; i++) { diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h index 936ff8c98651..489889a72203 100644 --- a/lib/librte_ethdev/rte_ethdev_driver.h +++ b/lib/librte_ethdev/rte_ethdev_driver.h @@ -208,6 +208,71 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev, #endif } +/** + * @internal + * Get basic stats part of xstats for an ethernet device. + * + * @param dev + * Pointer to struct rte_eth_dev. + */ +__rte_experimental +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev); + +/** + * @internal + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Retrieve the names for the basic part of extended statistics. + * + * @param dev + * Pointer to struct rte_eth_dev. + * @param xstats_names + * An rte_eth_xstat_name array of at least *size* elements to + * be filled. If set to NULL, the function returns the required number + * of elements. + * @return + * - A positive value lower or equal to size: success. The return value + * is the number of entries filled in the stats table. + * - A positive value higher than size: error, the given statistics table + * is too small. The return value corresponds to the size that should + * be given to succeed. The entries in the table are not valid and + * shall not be used by the caller. + * - A negative value on error (invalid port id). + */ +__rte_experimental +int +rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names); + +/** + * @internal + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Retrieve the basic part of the extended statistics. + * + * @param dev + * Pointer to struct rte_eth_dev. + * @param xstats + * A pointer to a table of structure of type *rte_eth_xstat* + * to be filled with device statistics ids and values. + * This parameter can be set to NULL if n is 0. + * @param n + * The size of the xstats array (number of elements). + * @return + * - A positive value lower or equal to n: success. The return value + * is the number of entries filled in the stats table. + * - A positive value higher than n: error, the given statistics table + * is too small. The return value corresponds to the size that should + * be given to succeed. The entries in the table are not valid and + * shall not be used by the caller. + * - A negative value on error (invalid port id). + */ +__rte_experimental +int +rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats); + + /** * @warning * @b EXPERIMENTAL: this API may change without prior notice. diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map index df9141825c3f..949a79800cbc 100644 --- a/lib/librte_ethdev/rte_ethdev_version.map +++ b/lib/librte_ethdev/rte_ethdev_version.map @@ -239,6 +239,9 @@ DPDK_19.05 { EXPERIMENTAL { global: + rte_eth_basic_stats_count; + rte_eth_basic_stats_get; + rte_eth_basic_stats_get_names; rte_eth_devargs_parse; rte_eth_dev_create; rte_eth_dev_destroy; -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Stephen Hemminger 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger @ 2019-07-03 17:25 ` Stephen Hemminger 2019-07-04 9:56 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Gaëtan Rivet 2 siblings, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-07-03 17:25 UTC (permalink / raw) To: gaetan.rivet, arybchenko; +Cc: dev, Stephen Hemminger Add support for extended statistics in failsafe driver. Reports basic statistics for the failsafe driver, and detailed statistics for each sub device. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/failsafe/failsafe_ops.c | 135 ++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 96e05d4dc4b1..711fdf302f4f 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -789,6 +789,138 @@ fs_stats_reset(struct rte_eth_dev *dev) fs_unlock(dev, 0); } +/* + * Count how many xstats in total + * Include basic stats for failsafe device + * plus xstats for each sub-device present. + */ +static int +fs_xstats_count(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + int count; + + count = rte_eth_basic_stats_count(dev); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + count += rte_eth_xstats_get_names(PORT_ID(sdev), NULL, 0); + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int limit) +{ + struct sub_device *sdev; + unsigned int count; + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; + uint8_t i; + int r; + + /* Caller only cares about count */ + if (!xstats_names) + return fs_xstats_count(dev); + + r = rte_eth_basic_stats_get_names(dev, xstats_names); + if (r < 0) + return r; + + count = r; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + struct rte_eth_xstat_name *sub_names = xstats_names + count; + + if (count >= limit) + break; + + r = rte_eth_xstats_get_names(PORT_ID(sdev), sub_names, + limit - count); + if (r < 0) { + fs_unlock(dev, 0); + return r; + } + + /* add sub_ prefix to names */ + for (i = 0; i < r; i++) { + snprintf(tmp, sizeof(tmp), "sub%u_%s", + i, sub_names[i].name); + memcpy(sub_names[i].name, tmp, + RTE_ETH_XSTATS_NAME_SIZE); + } + + count += r; + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int nstats, j, count, scount; + struct sub_device *sdev; + uint8_t i; + int ret; + + nstats = fs_xstats_count(dev); + if (n < nstats || xstats == NULL) + return nstats; + + ret = rte_eth_basic_stats_get(dev->data->port_id, xstats); + if (ret < 0) + return ret; + + count = ret; + for (j = 0; j < count; j++) + xstats[j].id = j; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + ret = rte_eth_xstats_get(PORT_ID(sdev), + xstats + count, + n > count ? n - count : 0); + if (ret < 0) { + fs_unlock(dev, 0); + return ret; + } + scount = ret; + + /* add offset to id's from sub-device */ + for (j = 0; j < scount; j++) + xstats[count + j].id += count; + + count += scount; + } + fs_unlock(dev, 0); + + return count; +} + +static void +fs_xstats_reset(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + + rte_eth_stats_reset(dev->data->port_id); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + rte_eth_xstats_reset(PORT_ID(sdev)); + } + fs_unlock(dev, 0); +} + static void fs_dev_merge_desc_lim(struct rte_eth_desc_lim *to, const struct rte_eth_desc_lim *from) @@ -1233,6 +1365,9 @@ const struct eth_dev_ops failsafe_ops = { .link_update = fs_link_update, .stats_get = fs_stats_get, .stats_reset = fs_stats_reset, + .xstats_get = fs_xstats_get, + .xstats_get_names = fs_xstats_get_names, + .xstats_reset = fs_xstats_reset, .dev_infos_get = fs_dev_infos_get, .dev_supported_ptypes_get = fs_dev_supported_ptypes_get, .mtu_set = fs_mtu_set, -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Stephen Hemminger 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats Stephen Hemminger @ 2019-07-04 9:56 ` Gaëtan Rivet 2 siblings, 0 replies; 30+ messages in thread From: Gaëtan Rivet @ 2019-07-04 9:56 UTC (permalink / raw) To: Stephen Hemminger; +Cc: arybchenko, dev On Wed, Jul 03, 2019 at 10:25:10AM -0700, Stephen Hemminger wrote: > A useful feature of netvsc PMD is the ability to see how many packets > were processed through the VF device. This patch set adds a similar > (but more limited) capability to failsafe driver. > > Since failsafe doesn't have top level xstats, this set uses the generic > xstats that exist already as a base then adds the sub-device xstats > to that. > > v2 > move generic xstats helpers to rte_ethdev_driver.h > change prefix on subdevice stats to sub0_XXXX > For the series: Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> > Stephen Hemminger (2): > ethdev: expose basic xstats for driver use > failsafe: implement xstats > > drivers/net/failsafe/failsafe_ops.c | 135 +++++++++++++++++++++++ > lib/librte_ethdev/rte_ethdev.c | 17 ++- > lib/librte_ethdev/rte_ethdev_driver.h | 65 +++++++++++ > lib/librte_ethdev/rte_ethdev_version.map | 3 + > 4 files changed, 211 insertions(+), 9 deletions(-) > > -- > 2.20.1 > -- Gaëtan Rivet 6WIND ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 0/2] failsafe: add xstats 2019-06-26 22:21 [dpdk-dev] [PATCH 0/2] failsafe: add xstats Stephen Hemminger ` (3 preceding siblings ...) 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Stephen Hemminger @ 2019-08-11 16:06 ` Stephen Hemminger 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger ` (3 more replies) 4 siblings, 4 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-08-11 16:06 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger A useful feature of netvsc PMD is the ability to see how many packets were processed through the VF device. This patch set adds a similar (but more limited) capability to failsafe driver. Since failsafe doesn't have top level xstats, this set uses the generic xstats that exist already as a base then adds the sub-device xstats to that. v3 - rebase to 19.08 Stephen Hemminger (2): ethdev: expose basic xstats for driver use net/failsafe: implement xstats drivers/net/failsafe/failsafe_ops.c | 135 +++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.c | 17 ++- lib/librte_ethdev/rte_ethdev_driver.h | 65 +++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 24 ++++ 4 files changed, 232 insertions(+), 9 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 1/2] ethdev: expose basic xstats for driver use 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 0/2] failsafe: " Stephen Hemminger @ 2019-08-11 16:06 ` Stephen Hemminger 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 2/2] net/failsafe: implement xstats Stephen Hemminger ` (2 subsequent siblings) 3 siblings, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-08-11 16:06 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Gaetan Rivet Avoid duplication by having generic basic xstats available for use by drivers. A later patch uses this for failsafe driver. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> --- lib/librte_ethdev/rte_ethdev.c | 17 +++---- lib/librte_ethdev/rte_ethdev_driver.h | 65 ++++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 24 +++++++++ 3 files changed, 97 insertions(+), 9 deletions(-) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 17d183e1f0ec..88e3065d06fd 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1996,8 +1996,8 @@ rte_eth_stats_reset(uint16_t port_id) return 0; } -static inline int -get_xstats_basic_count(struct rte_eth_dev *dev) +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev) { uint16_t nb_rxqs, nb_txqs; int count; @@ -2034,7 +2034,7 @@ get_xstats_count(uint16_t port_id) count = 0; - count += get_xstats_basic_count(dev); + count += rte_eth_basic_stats_count(dev); return count; } @@ -2084,7 +2084,7 @@ rte_eth_xstats_get_id_by_name(uint16_t port_id, const char *xstat_name, } /* retrieve basic stats names */ -static int +int rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, struct rte_eth_xstat_name *xstats_names) { @@ -2140,7 +2140,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id, RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); ret = get_xstats_count(port_id); if (ret < 0) return ret; @@ -2268,8 +2268,7 @@ rte_eth_xstats_get_names(uint16_t port_id, return cnt_used_entries; } - -static int +int rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats) { struct rte_eth_dev *dev; @@ -2341,7 +2340,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, expected_entries = (uint16_t)ret; struct rte_eth_xstat xstats[expected_entries]; dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); /* Return max number of stats if no ids given */ if (!ids) { @@ -2355,7 +2354,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, return -EINVAL; if (ids && dev->dev_ops->xstats_get_by_id != NULL && size) { - unsigned int basic_count = get_xstats_basic_count(dev); + unsigned int basic_count = rte_eth_basic_stats_count(dev); uint64_t ids_copy[size]; for (i = 0; i < size; i++) { diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h index 936ff8c98651..489889a72203 100644 --- a/lib/librte_ethdev/rte_ethdev_driver.h +++ b/lib/librte_ethdev/rte_ethdev_driver.h @@ -208,6 +208,71 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev, #endif } +/** + * @internal + * Get basic stats part of xstats for an ethernet device. + * + * @param dev + * Pointer to struct rte_eth_dev. + */ +__rte_experimental +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev); + +/** + * @internal + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Retrieve the names for the basic part of extended statistics. + * + * @param dev + * Pointer to struct rte_eth_dev. + * @param xstats_names + * An rte_eth_xstat_name array of at least *size* elements to + * be filled. If set to NULL, the function returns the required number + * of elements. + * @return + * - A positive value lower or equal to size: success. The return value + * is the number of entries filled in the stats table. + * - A positive value higher than size: error, the given statistics table + * is too small. The return value corresponds to the size that should + * be given to succeed. The entries in the table are not valid and + * shall not be used by the caller. + * - A negative value on error (invalid port id). + */ +__rte_experimental +int +rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names); + +/** + * @internal + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Retrieve the basic part of the extended statistics. + * + * @param dev + * Pointer to struct rte_eth_dev. + * @param xstats + * A pointer to a table of structure of type *rte_eth_xstat* + * to be filled with device statistics ids and values. + * This parameter can be set to NULL if n is 0. + * @param n + * The size of the xstats array (number of elements). + * @return + * - A positive value lower or equal to n: success. The return value + * is the number of entries filled in the stats table. + * - A positive value higher than n: error, the given statistics table + * is too small. The return value corresponds to the size that should + * be given to succeed. The entries in the table are not valid and + * shall not be used by the caller. + * - A negative value on error (invalid port id). + */ +__rte_experimental +int +rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats); + + /** * @warning * @b EXPERIMENTAL: this API may change without prior notice. diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map index 6df42a47b89d..f2198b0367d1 100644 --- a/lib/librte_ethdev/rte_ethdev_version.map +++ b/lib/librte_ethdev/rte_ethdev_version.map @@ -240,6 +240,25 @@ EXPERIMENTAL { global: # added in 17.11 + rte_eth_devargs_parse; + rte_eth_dev_create; + rte_eth_dev_destroy; + rte_eth_dev_get_module_eeprom; + rte_eth_dev_get_module_info; + rte_eth_dev_is_removed; + rte_eth_dev_owner_delete; + rte_eth_dev_owner_get; + rte_eth_dev_owner_new; + rte_eth_dev_owner_set; + rte_eth_dev_owner_unset; + rte_eth_dev_rx_intr_ctl_q_get_fd; + rte_eth_find_next_of; + rte_eth_find_next_sibling; + rte_eth_read_clock; + rte_eth_switch_domain_alloc; + rte_eth_switch_domain_free; + rte_flow_conv; + rte_flow_expand_rss; rte_mtr_capabilities_get; rte_mtr_create; rte_mtr_destroy; @@ -283,4 +302,9 @@ EXPERIMENTAL { # added in 19.08 rte_eth_read_clock; + + # added in 19.11 + rte_eth_basic_stats_count; + rte_eth_basic_stats_get; + rte_eth_basic_stats_get_names; }; -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 2/2] net/failsafe: implement xstats 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 0/2] failsafe: " Stephen Hemminger 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger @ 2019-08-11 16:06 ` Stephen Hemminger 2019-09-19 12:56 ` [dpdk-dev] [PATCH v4 0/2] failsafe: add xstats Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger 3 siblings, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-08-11 16:06 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Gaetan Rivet Add support for extended statistics in failsafe driver. Reports basic statistics for the failsafe driver, and detailed statistics for each sub device. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> --- drivers/net/failsafe/failsafe_ops.c | 135 ++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 96e05d4dc4b1..711fdf302f4f 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -789,6 +789,138 @@ fs_stats_reset(struct rte_eth_dev *dev) fs_unlock(dev, 0); } +/* + * Count how many xstats in total + * Include basic stats for failsafe device + * plus xstats for each sub-device present. + */ +static int +fs_xstats_count(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + int count; + + count = rte_eth_basic_stats_count(dev); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + count += rte_eth_xstats_get_names(PORT_ID(sdev), NULL, 0); + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int limit) +{ + struct sub_device *sdev; + unsigned int count; + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; + uint8_t i; + int r; + + /* Caller only cares about count */ + if (!xstats_names) + return fs_xstats_count(dev); + + r = rte_eth_basic_stats_get_names(dev, xstats_names); + if (r < 0) + return r; + + count = r; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + struct rte_eth_xstat_name *sub_names = xstats_names + count; + + if (count >= limit) + break; + + r = rte_eth_xstats_get_names(PORT_ID(sdev), sub_names, + limit - count); + if (r < 0) { + fs_unlock(dev, 0); + return r; + } + + /* add sub_ prefix to names */ + for (i = 0; i < r; i++) { + snprintf(tmp, sizeof(tmp), "sub%u_%s", + i, sub_names[i].name); + memcpy(sub_names[i].name, tmp, + RTE_ETH_XSTATS_NAME_SIZE); + } + + count += r; + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int nstats, j, count, scount; + struct sub_device *sdev; + uint8_t i; + int ret; + + nstats = fs_xstats_count(dev); + if (n < nstats || xstats == NULL) + return nstats; + + ret = rte_eth_basic_stats_get(dev->data->port_id, xstats); + if (ret < 0) + return ret; + + count = ret; + for (j = 0; j < count; j++) + xstats[j].id = j; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + ret = rte_eth_xstats_get(PORT_ID(sdev), + xstats + count, + n > count ? n - count : 0); + if (ret < 0) { + fs_unlock(dev, 0); + return ret; + } + scount = ret; + + /* add offset to id's from sub-device */ + for (j = 0; j < scount; j++) + xstats[count + j].id += count; + + count += scount; + } + fs_unlock(dev, 0); + + return count; +} + +static void +fs_xstats_reset(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + + rte_eth_stats_reset(dev->data->port_id); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + rte_eth_xstats_reset(PORT_ID(sdev)); + } + fs_unlock(dev, 0); +} + static void fs_dev_merge_desc_lim(struct rte_eth_desc_lim *to, const struct rte_eth_desc_lim *from) @@ -1233,6 +1365,9 @@ const struct eth_dev_ops failsafe_ops = { .link_update = fs_link_update, .stats_get = fs_stats_get, .stats_reset = fs_stats_reset, + .xstats_get = fs_xstats_get, + .xstats_get_names = fs_xstats_get_names, + .xstats_reset = fs_xstats_reset, .dev_infos_get = fs_dev_infos_get, .dev_supported_ptypes_get = fs_dev_supported_ptypes_get, .mtu_set = fs_mtu_set, -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v4 0/2] failsafe: add xstats 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 0/2] failsafe: " Stephen Hemminger 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 2/2] net/failsafe: implement xstats Stephen Hemminger @ 2019-09-19 12:56 ` Stephen Hemminger 2019-09-19 12:56 ` [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-09-19 12:56 ` [dpdk-dev] [PATCH 2/2] net/failsafe: implement xstats Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger 3 siblings, 2 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-09-19 12:56 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger A useful feature of netvsc PMD is the ability to see how many packets were processed through the VF device. This patch set adds a similar (but more limited) capability to failsafe driver. Since failsafe doesn't have top level xstats, this set uses the generic xstats that exist already as a base then adds the sub-device xstats to that. v4 - rebase to 19.11-rc v3 - rebase to 19.08 Stephen Hemminger (2): ethdev: expose basic xstats for driver use net/failsafe: implement xstats drivers/net/failsafe/failsafe_ops.c | 135 +++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.c | 17 ++- lib/librte_ethdev/rte_ethdev_driver.h | 65 +++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 24 ++++ 4 files changed, 232 insertions(+), 9 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use 2019-09-19 12:56 ` [dpdk-dev] [PATCH v4 0/2] failsafe: add xstats Stephen Hemminger @ 2019-09-19 12:56 ` Stephen Hemminger 2019-09-19 12:56 ` [dpdk-dev] [PATCH 2/2] net/failsafe: implement xstats Stephen Hemminger 1 sibling, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-09-19 12:56 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Avoid duplication by having generic basic xstats available for use by drivers. A later patch uses this for failsafe driver. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> --- lib/librte_ethdev/rte_ethdev.c | 17 +++---- lib/librte_ethdev/rte_ethdev_driver.h | 65 ++++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 24 +++++++++ 3 files changed, 97 insertions(+), 9 deletions(-) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 17d183e1f0ec..88e3065d06fd 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1996,8 +1996,8 @@ rte_eth_stats_reset(uint16_t port_id) return 0; } -static inline int -get_xstats_basic_count(struct rte_eth_dev *dev) +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev) { uint16_t nb_rxqs, nb_txqs; int count; @@ -2034,7 +2034,7 @@ get_xstats_count(uint16_t port_id) count = 0; - count += get_xstats_basic_count(dev); + count += rte_eth_basic_stats_count(dev); return count; } @@ -2084,7 +2084,7 @@ rte_eth_xstats_get_id_by_name(uint16_t port_id, const char *xstat_name, } /* retrieve basic stats names */ -static int +int rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, struct rte_eth_xstat_name *xstats_names) { @@ -2140,7 +2140,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id, RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); ret = get_xstats_count(port_id); if (ret < 0) return ret; @@ -2268,8 +2268,7 @@ rte_eth_xstats_get_names(uint16_t port_id, return cnt_used_entries; } - -static int +int rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats) { struct rte_eth_dev *dev; @@ -2341,7 +2340,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, expected_entries = (uint16_t)ret; struct rte_eth_xstat xstats[expected_entries]; dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); /* Return max number of stats if no ids given */ if (!ids) { @@ -2355,7 +2354,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, return -EINVAL; if (ids && dev->dev_ops->xstats_get_by_id != NULL && size) { - unsigned int basic_count = get_xstats_basic_count(dev); + unsigned int basic_count = rte_eth_basic_stats_count(dev); uint64_t ids_copy[size]; for (i = 0; i < size; i++) { diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h index 936ff8c98651..489889a72203 100644 --- a/lib/librte_ethdev/rte_ethdev_driver.h +++ b/lib/librte_ethdev/rte_ethdev_driver.h @@ -208,6 +208,71 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev, #endif } +/** + * @internal + * Get basic stats part of xstats for an ethernet device. + * + * @param dev + * Pointer to struct rte_eth_dev. + */ +__rte_experimental +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev); + +/** + * @internal + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Retrieve the names for the basic part of extended statistics. + * + * @param dev + * Pointer to struct rte_eth_dev. + * @param xstats_names + * An rte_eth_xstat_name array of at least *size* elements to + * be filled. If set to NULL, the function returns the required number + * of elements. + * @return + * - A positive value lower or equal to size: success. The return value + * is the number of entries filled in the stats table. + * - A positive value higher than size: error, the given statistics table + * is too small. The return value corresponds to the size that should + * be given to succeed. The entries in the table are not valid and + * shall not be used by the caller. + * - A negative value on error (invalid port id). + */ +__rte_experimental +int +rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names); + +/** + * @internal + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Retrieve the basic part of the extended statistics. + * + * @param dev + * Pointer to struct rte_eth_dev. + * @param xstats + * A pointer to a table of structure of type *rte_eth_xstat* + * to be filled with device statistics ids and values. + * This parameter can be set to NULL if n is 0. + * @param n + * The size of the xstats array (number of elements). + * @return + * - A positive value lower or equal to n: success. The return value + * is the number of entries filled in the stats table. + * - A positive value higher than n: error, the given statistics table + * is too small. The return value corresponds to the size that should + * be given to succeed. The entries in the table are not valid and + * shall not be used by the caller. + * - A negative value on error (invalid port id). + */ +__rte_experimental +int +rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats); + + /** * @warning * @b EXPERIMENTAL: this API may change without prior notice. diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map index 6df42a47b89d..f2198b0367d1 100644 --- a/lib/librte_ethdev/rte_ethdev_version.map +++ b/lib/librte_ethdev/rte_ethdev_version.map @@ -240,6 +240,25 @@ EXPERIMENTAL { global: # added in 17.11 + rte_eth_devargs_parse; + rte_eth_dev_create; + rte_eth_dev_destroy; + rte_eth_dev_get_module_eeprom; + rte_eth_dev_get_module_info; + rte_eth_dev_is_removed; + rte_eth_dev_owner_delete; + rte_eth_dev_owner_get; + rte_eth_dev_owner_new; + rte_eth_dev_owner_set; + rte_eth_dev_owner_unset; + rte_eth_dev_rx_intr_ctl_q_get_fd; + rte_eth_find_next_of; + rte_eth_find_next_sibling; + rte_eth_read_clock; + rte_eth_switch_domain_alloc; + rte_eth_switch_domain_free; + rte_flow_conv; + rte_flow_expand_rss; rte_mtr_capabilities_get; rte_mtr_create; rte_mtr_destroy; @@ -283,4 +302,9 @@ EXPERIMENTAL { # added in 19.08 rte_eth_read_clock; + + # added in 19.11 + rte_eth_basic_stats_count; + rte_eth_basic_stats_get; + rte_eth_basic_stats_get_names; }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 2/2] net/failsafe: implement xstats 2019-09-19 12:56 ` [dpdk-dev] [PATCH v4 0/2] failsafe: add xstats Stephen Hemminger 2019-09-19 12:56 ` [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger @ 2019-09-19 12:56 ` Stephen Hemminger 1 sibling, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-09-19 12:56 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Add support for extended statistics in failsafe driver. Reports basic statistics for the failsafe driver, and detailed statistics for each sub device. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> --- drivers/net/failsafe/failsafe_ops.c | 135 ++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 96e05d4dc4b1..711fdf302f4f 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -789,6 +789,138 @@ fs_stats_reset(struct rte_eth_dev *dev) fs_unlock(dev, 0); } +/* + * Count how many xstats in total + * Include basic stats for failsafe device + * plus xstats for each sub-device present. + */ +static int +fs_xstats_count(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + int count; + + count = rte_eth_basic_stats_count(dev); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + count += rte_eth_xstats_get_names(PORT_ID(sdev), NULL, 0); + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int limit) +{ + struct sub_device *sdev; + unsigned int count; + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; + uint8_t i; + int r; + + /* Caller only cares about count */ + if (!xstats_names) + return fs_xstats_count(dev); + + r = rte_eth_basic_stats_get_names(dev, xstats_names); + if (r < 0) + return r; + + count = r; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + struct rte_eth_xstat_name *sub_names = xstats_names + count; + + if (count >= limit) + break; + + r = rte_eth_xstats_get_names(PORT_ID(sdev), sub_names, + limit - count); + if (r < 0) { + fs_unlock(dev, 0); + return r; + } + + /* add sub_ prefix to names */ + for (i = 0; i < r; i++) { + snprintf(tmp, sizeof(tmp), "sub%u_%s", + i, sub_names[i].name); + memcpy(sub_names[i].name, tmp, + RTE_ETH_XSTATS_NAME_SIZE); + } + + count += r; + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int nstats, j, count, scount; + struct sub_device *sdev; + uint8_t i; + int ret; + + nstats = fs_xstats_count(dev); + if (n < nstats || xstats == NULL) + return nstats; + + ret = rte_eth_basic_stats_get(dev->data->port_id, xstats); + if (ret < 0) + return ret; + + count = ret; + for (j = 0; j < count; j++) + xstats[j].id = j; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + ret = rte_eth_xstats_get(PORT_ID(sdev), + xstats + count, + n > count ? n - count : 0); + if (ret < 0) { + fs_unlock(dev, 0); + return ret; + } + scount = ret; + + /* add offset to id's from sub-device */ + for (j = 0; j < scount; j++) + xstats[count + j].id += count; + + count += scount; + } + fs_unlock(dev, 0); + + return count; +} + +static void +fs_xstats_reset(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + + rte_eth_stats_reset(dev->data->port_id); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + rte_eth_xstats_reset(PORT_ID(sdev)); + } + fs_unlock(dev, 0); +} + static void fs_dev_merge_desc_lim(struct rte_eth_desc_lim *to, const struct rte_eth_desc_lim *from) @@ -1233,6 +1365,9 @@ const struct eth_dev_ops failsafe_ops = { .link_update = fs_link_update, .stats_get = fs_stats_get, .stats_reset = fs_stats_reset, + .xstats_get = fs_xstats_get, + .xstats_get_names = fs_xstats_get_names, + .xstats_reset = fs_xstats_reset, .dev_infos_get = fs_dev_infos_get, .dev_supported_ptypes_get = fs_dev_supported_ptypes_get, .mtu_set = fs_mtu_set, -- 2.17.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 0/2] failsafe: " Stephen Hemminger ` (2 preceding siblings ...) 2019-09-19 12:56 ` [dpdk-dev] [PATCH v4 0/2] failsafe: add xstats Stephen Hemminger @ 2019-09-19 13:17 ` Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger ` (2 more replies) 3 siblings, 3 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-09-19 13:17 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger A useful feature of netvsc PMD is the ability to see how many packets were processed through the VF device. This patch set adds a similar (but more limited) capability to failsafe driver. Since failsafe doesn't have top level xstats, this set uses the generic xstats that exist already as a base then adds the sub-device xstats to that. v5 - fix ethdev map file v4 - rebase to 19.11 v3 - rebase to 19.08 Stephen Hemminger (2): ethdev: expose basic xstats for driver use net/failsafe: implement xstats drivers/net/failsafe/failsafe_ops.c | 135 +++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.c | 17 ++- lib/librte_ethdev/rte_ethdev_driver.h | 65 +++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 5 + 4 files changed, 213 insertions(+), 9 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger @ 2019-09-19 13:17 ` Stephen Hemminger 2019-09-26 12:46 ` Andrew Rybchenko 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 2/2] net/failsafe: implement xstats Stephen Hemminger 2019-10-14 17:04 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger 2 siblings, 1 reply; 30+ messages in thread From: Stephen Hemminger @ 2019-09-19 13:17 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Avoid duplication by having generic basic xstats available for use by drivers. A later patch uses this for failsafe driver. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> --- lib/librte_ethdev/rte_ethdev.c | 17 +++---- lib/librte_ethdev/rte_ethdev_driver.h | 65 ++++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev_version.map | 5 ++ 3 files changed, 78 insertions(+), 9 deletions(-) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 17d183e1f0ec..88e3065d06fd 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1996,8 +1996,8 @@ rte_eth_stats_reset(uint16_t port_id) return 0; } -static inline int -get_xstats_basic_count(struct rte_eth_dev *dev) +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev) { uint16_t nb_rxqs, nb_txqs; int count; @@ -2034,7 +2034,7 @@ get_xstats_count(uint16_t port_id) count = 0; - count += get_xstats_basic_count(dev); + count += rte_eth_basic_stats_count(dev); return count; } @@ -2084,7 +2084,7 @@ rte_eth_xstats_get_id_by_name(uint16_t port_id, const char *xstat_name, } /* retrieve basic stats names */ -static int +int rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, struct rte_eth_xstat_name *xstats_names) { @@ -2140,7 +2140,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id, RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); ret = get_xstats_count(port_id); if (ret < 0) return ret; @@ -2268,8 +2268,7 @@ rte_eth_xstats_get_names(uint16_t port_id, return cnt_used_entries; } - -static int +int rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats) { struct rte_eth_dev *dev; @@ -2341,7 +2340,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, expected_entries = (uint16_t)ret; struct rte_eth_xstat xstats[expected_entries]; dev = &rte_eth_devices[port_id]; - basic_count = get_xstats_basic_count(dev); + basic_count = rte_eth_basic_stats_count(dev); /* Return max number of stats if no ids given */ if (!ids) { @@ -2355,7 +2354,7 @@ rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, return -EINVAL; if (ids && dev->dev_ops->xstats_get_by_id != NULL && size) { - unsigned int basic_count = get_xstats_basic_count(dev); + unsigned int basic_count = rte_eth_basic_stats_count(dev); uint64_t ids_copy[size]; for (i = 0; i < size; i++) { diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h index 936ff8c98651..489889a72203 100644 --- a/lib/librte_ethdev/rte_ethdev_driver.h +++ b/lib/librte_ethdev/rte_ethdev_driver.h @@ -208,6 +208,71 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev, #endif } +/** + * @internal + * Get basic stats part of xstats for an ethernet device. + * + * @param dev + * Pointer to struct rte_eth_dev. + */ +__rte_experimental +int +rte_eth_basic_stats_count(struct rte_eth_dev *dev); + +/** + * @internal + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Retrieve the names for the basic part of extended statistics. + * + * @param dev + * Pointer to struct rte_eth_dev. + * @param xstats_names + * An rte_eth_xstat_name array of at least *size* elements to + * be filled. If set to NULL, the function returns the required number + * of elements. + * @return + * - A positive value lower or equal to size: success. The return value + * is the number of entries filled in the stats table. + * - A positive value higher than size: error, the given statistics table + * is too small. The return value corresponds to the size that should + * be given to succeed. The entries in the table are not valid and + * shall not be used by the caller. + * - A negative value on error (invalid port id). + */ +__rte_experimental +int +rte_eth_basic_stats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names); + +/** + * @internal + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Retrieve the basic part of the extended statistics. + * + * @param dev + * Pointer to struct rte_eth_dev. + * @param xstats + * A pointer to a table of structure of type *rte_eth_xstat* + * to be filled with device statistics ids and values. + * This parameter can be set to NULL if n is 0. + * @param n + * The size of the xstats array (number of elements). + * @return + * - A positive value lower or equal to n: success. The return value + * is the number of entries filled in the stats table. + * - A positive value higher than n: error, the given statistics table + * is too small. The return value corresponds to the size that should + * be given to succeed. The entries in the table are not valid and + * shall not be used by the caller. + * - A negative value on error (invalid port id). + */ +__rte_experimental +int +rte_eth_basic_stats_get(uint16_t port_id, struct rte_eth_xstat *xstats); + + /** * @warning * @b EXPERIMENTAL: this API may change without prior notice. diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map index 6df42a47b89d..df7e83a90603 100644 --- a/lib/librte_ethdev/rte_ethdev_version.map +++ b/lib/librte_ethdev/rte_ethdev_version.map @@ -283,4 +283,9 @@ EXPERIMENTAL { # added in 19.08 rte_eth_read_clock; + + # added in 19.11 + rte_eth_basic_stats_count; + rte_eth_basic_stats_get; + rte_eth_basic_stats_get_names; }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger @ 2019-09-26 12:46 ` Andrew Rybchenko 2019-09-26 16:09 ` Stephen Hemminger 2019-10-31 16:40 ` Stephen Hemminger 0 siblings, 2 replies; 30+ messages in thread From: Andrew Rybchenko @ 2019-09-26 12:46 UTC (permalink / raw) To: Stephen Hemminger, dev On 9/19/19 4:17 PM, Stephen Hemminger wrote: > Avoid duplication by having generic basic xstats available > for use by drivers. A later patch uses this for failsafe > driver. > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com> > diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h > index 936ff8c98651..489889a72203 100644 > --- a/lib/librte_ethdev/rte_ethdev_driver.h > +++ b/lib/librte_ethdev/rte_ethdev_driver.h > @@ -208,6 +208,71 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev, > #endif > } > > +/** > + * @internal > + * Get basic stats part of xstats for an ethernet device. > + * > + * @param dev > + * Pointer to struct rte_eth_dev. > + */ > +__rte_experimental Does it make sense to mark internal API as experimental? I thought that @internal is not a part of API/ABI. [snip] ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use 2019-09-26 12:46 ` Andrew Rybchenko @ 2019-09-26 16:09 ` Stephen Hemminger 2019-10-31 16:40 ` Stephen Hemminger 1 sibling, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-09-26 16:09 UTC (permalink / raw) To: Andrew Rybchenko; +Cc: dev On Thu, 26 Sep 2019 15:46:52 +0300 Andrew Rybchenko <arybchenko@solarflare.com> wrote: > On 9/19/19 4:17 PM, Stephen Hemminger wrote: > > Avoid duplication by having generic basic xstats available > > for use by drivers. A later patch uses this for failsafe > > driver. > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > > Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com> > > > diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h > > index 936ff8c98651..489889a72203 100644 > > --- a/lib/librte_ethdev/rte_ethdev_driver.h > > +++ b/lib/librte_ethdev/rte_ethdev_driver.h > > @@ -208,6 +208,71 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev, > > #endif > > } > > > > +/** > > + * @internal > > + * Get basic stats part of xstats for an ethernet device. > > + * > > + * @param dev > > + * Pointer to struct rte_eth_dev. > > + */ > > +__rte_experimental > > Does it make sense to mark internal API as experimental? > I thought that @internal is not a part of API/ABI. agree, but checkpatch doesn't understand @internal tag. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use 2019-09-26 12:46 ` Andrew Rybchenko 2019-09-26 16:09 ` Stephen Hemminger @ 2019-10-31 16:40 ` Stephen Hemminger 1 sibling, 0 replies; 30+ messages in thread From: Stephen Hemminger @ 2019-10-31 16:40 UTC (permalink / raw) To: Andrew Rybchenko; +Cc: dev On Thu, 26 Sep 2019 15:46:52 +0300 Andrew Rybchenko <arybchenko@solarflare.com> wrote: > On 9/19/19 4:17 PM, Stephen Hemminger wrote: > > Avoid duplication by having generic basic xstats available > > for use by drivers. A later patch uses this for failsafe > > driver. > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > > Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com> > > > diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h > > index 936ff8c98651..489889a72203 100644 > > --- a/lib/librte_ethdev/rte_ethdev_driver.h > > +++ b/lib/librte_ethdev/rte_ethdev_driver.h > > @@ -208,6 +208,71 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev, > > #endif > > } > > > > +/** > > + * @internal > > + * Get basic stats part of xstats for an ethernet device. > > + * > > + * @param dev > > + * Pointer to struct rte_eth_dev. > > + */ > > +__rte_experimental > > Does it make sense to mark internal API as experimental? > I thought that @internal is not a part of API/ABI. Checkpatch picks on it ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v5 2/2] net/failsafe: implement xstats 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger @ 2019-09-19 13:17 ` Stephen Hemminger 2019-10-17 14:54 ` Ferruh Yigit 2019-10-14 17:04 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger 2 siblings, 1 reply; 30+ messages in thread From: Stephen Hemminger @ 2019-09-19 13:17 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Add support for extended statistics in failsafe driver. Reports basic statistics for the failsafe driver, and detailed statistics for each sub device. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> --- drivers/net/failsafe/failsafe_ops.c | 135 ++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 96e05d4dc4b1..711fdf302f4f 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -789,6 +789,138 @@ fs_stats_reset(struct rte_eth_dev *dev) fs_unlock(dev, 0); } +/* + * Count how many xstats in total + * Include basic stats for failsafe device + * plus xstats for each sub-device present. + */ +static int +fs_xstats_count(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + int count; + + count = rte_eth_basic_stats_count(dev); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + count += rte_eth_xstats_get_names(PORT_ID(sdev), NULL, 0); + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int limit) +{ + struct sub_device *sdev; + unsigned int count; + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; + uint8_t i; + int r; + + /* Caller only cares about count */ + if (!xstats_names) + return fs_xstats_count(dev); + + r = rte_eth_basic_stats_get_names(dev, xstats_names); + if (r < 0) + return r; + + count = r; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + struct rte_eth_xstat_name *sub_names = xstats_names + count; + + if (count >= limit) + break; + + r = rte_eth_xstats_get_names(PORT_ID(sdev), sub_names, + limit - count); + if (r < 0) { + fs_unlock(dev, 0); + return r; + } + + /* add sub_ prefix to names */ + for (i = 0; i < r; i++) { + snprintf(tmp, sizeof(tmp), "sub%u_%s", + i, sub_names[i].name); + memcpy(sub_names[i].name, tmp, + RTE_ETH_XSTATS_NAME_SIZE); + } + + count += r; + } + fs_unlock(dev, 0); + + return count; +} + +static int +fs_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int nstats, j, count, scount; + struct sub_device *sdev; + uint8_t i; + int ret; + + nstats = fs_xstats_count(dev); + if (n < nstats || xstats == NULL) + return nstats; + + ret = rte_eth_basic_stats_get(dev->data->port_id, xstats); + if (ret < 0) + return ret; + + count = ret; + for (j = 0; j < count; j++) + xstats[j].id = j; + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + ret = rte_eth_xstats_get(PORT_ID(sdev), + xstats + count, + n > count ? n - count : 0); + if (ret < 0) { + fs_unlock(dev, 0); + return ret; + } + scount = ret; + + /* add offset to id's from sub-device */ + for (j = 0; j < scount; j++) + xstats[count + j].id += count; + + count += scount; + } + fs_unlock(dev, 0); + + return count; +} + +static void +fs_xstats_reset(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + + rte_eth_stats_reset(dev->data->port_id); + + fs_lock(dev, 0); + FOREACH_SUBDEV(sdev, i, dev) { + rte_eth_xstats_reset(PORT_ID(sdev)); + } + fs_unlock(dev, 0); +} + static void fs_dev_merge_desc_lim(struct rte_eth_desc_lim *to, const struct rte_eth_desc_lim *from) @@ -1233,6 +1365,9 @@ const struct eth_dev_ops failsafe_ops = { .link_update = fs_link_update, .stats_get = fs_stats_get, .stats_reset = fs_stats_reset, + .xstats_get = fs_xstats_get, + .xstats_get_names = fs_xstats_get_names, + .xstats_reset = fs_xstats_reset, .dev_infos_get = fs_dev_infos_get, .dev_supported_ptypes_get = fs_dev_supported_ptypes_get, .mtu_set = fs_mtu_set, -- 2.17.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v5 2/2] net/failsafe: implement xstats 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 2/2] net/failsafe: implement xstats Stephen Hemminger @ 2019-10-17 14:54 ` Ferruh Yigit 0 siblings, 0 replies; 30+ messages in thread From: Ferruh Yigit @ 2019-10-17 14:54 UTC (permalink / raw) To: Stephen Hemminger, dev; +Cc: Andrew Rybchenko, Gaetan Rivet On 9/19/2019 2:17 PM, Stephen Hemminger wrote: > Add support for extended statistics in failsafe driver. > Reports basic statistics for the failsafe driver, and detailed > statistics for each sub device. +1, but doesn't need to pull basic stats to the xstats in the PMD level, that part is already done by xstats API [1], so 'fs_xstats_get.*()' functions return only sub-device basic stats is good enough which won't require the new 'rte_eth_basic_stats_.*()' APIs. Current implementation cause basic stats part duplicated for failsafe device. [1] rte_eth_xstats_get_names cnt_used_entries = rte_eth_basic_stats_get_names(dev, xstats_names) cnt_driver_entries = (*dev->dev_ops->xstats_get_names)(...) cnt_used_entries += cnt_driver_entries return cnt_used_entries; btw, while testing this I have observed an always reproducible seg fault by failsafe on testpmd quit [2], but didn't checked more, @Gaetan can you please check this when possible? [2] Shutting down port 1... Closing ports... Done Segmentation fault (core dumped) With command: testpmd -w 0:0.0 --vdev net_null0 --vdev "net_failsafe0,dev(86:00.1),dev(86:00.3)" -- -i > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> <...> > +static int > +fs_xstats_get_names(struct rte_eth_dev *dev, > + struct rte_eth_xstat_name *xstats_names, > + unsigned int limit) > +{ > + struct sub_device *sdev; > + unsigned int count; > + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; > + uint8_t i; > + int r; > + > + /* Caller only cares about count */ > + if (!xstats_names) > + return fs_xstats_count(dev); > + > + r = rte_eth_basic_stats_get_names(dev, xstats_names); > + if (r < 0) > + return r; > + > + count = r; > + > + fs_lock(dev, 0); > + FOREACH_SUBDEV(sdev, i, dev) { > + struct rte_eth_xstat_name *sub_names = xstats_names + count; > + > + if (count >= limit) > + break; > + > + r = rte_eth_xstats_get_names(PORT_ID(sdev), sub_names, > + limit - count); > + if (r < 0) { > + fs_unlock(dev, 0); > + return r; > + } > + > + /* add sub_ prefix to names */ > + for (i = 0; i < r; i++) { 'i' is used in outer loop as well thanks to 'C' to let us do these kind of things without any kind of warning. Also it shows maintainer acked without testing :( > + snprintf(tmp, sizeof(tmp), "sub%u_%s", > + i, sub_names[i].name); > + memcpy(sub_names[i].name, tmp, > + RTE_ETH_XSTATS_NAME_SIZE); > + } > + > + count += r; > + } > + fs_unlock(dev, 0); > + > + return count; > +} <...> ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 2/2] net/failsafe: implement xstats Stephen Hemminger @ 2019-10-14 17:04 ` Stephen Hemminger 2019-10-15 9:08 ` Ferruh Yigit 2 siblings, 1 reply; 30+ messages in thread From: Stephen Hemminger @ 2019-10-14 17:04 UTC (permalink / raw) To: dev On Thu, 19 Sep 2019 15:17:27 +0200 Stephen Hemminger <stephen@networkplumber.org> wrote: > A useful feature of netvsc PMD is the ability to see how many > packets > were processed through the VF device. This patch set adds a similar > (but more limited) capability to failsafe driver. > > Since failsafe doesn't have top level xstats, this set uses the generic > xstats that exist already as a base then adds the sub-device xstats > to that. > > v5 - fix ethdev map file > v4 - rebase to 19.11 > v3 - rebase to 19.08 > > Stephen Hemminger (2): > ethdev: expose basic xstats for driver use > net/failsafe: implement xstats > > drivers/net/failsafe/failsafe_ops.c | 135 +++++++++++++++++++++++ > lib/librte_ethdev/rte_ethdev.c | 17 ++- > lib/librte_ethdev/rte_ethdev_driver.h | 65 +++++++++++ > lib/librte_ethdev/rte_ethdev_version.map | 5 + > 4 files changed, 213 insertions(+), 9 deletions(-) > Why are these patches still not merged? ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats 2019-10-14 17:04 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger @ 2019-10-15 9:08 ` Ferruh Yigit 0 siblings, 0 replies; 30+ messages in thread From: Ferruh Yigit @ 2019-10-15 9:08 UTC (permalink / raw) To: Stephen Hemminger, dev On 10/14/2019 6:04 PM, Stephen Hemminger wrote: > On Thu, 19 Sep 2019 15:17:27 +0200 > Stephen Hemminger <stephen@networkplumber.org> wrote: > >> A useful feature of netvsc PMD is the ability to see how many >> packets >> were processed through the VF device. This patch set adds a similar >> (but more limited) capability to failsafe driver. >> >> Since failsafe doesn't have top level xstats, this set uses the generic >> xstats that exist already as a base then adds the sub-device xstats >> to that. >> >> v5 - fix ethdev map file >> v4 - rebase to 19.11 >> v3 - rebase to 19.08 >> >> Stephen Hemminger (2): >> ethdev: expose basic xstats for driver use >> net/failsafe: implement xstats >> >> drivers/net/failsafe/failsafe_ops.c | 135 +++++++++++++++++++++++ >> lib/librte_ethdev/rte_ethdev.c | 17 ++- >> lib/librte_ethdev/rte_ethdev_driver.h | 65 +++++++++++ >> lib/librte_ethdev/rte_ethdev_version.map | 5 + >> 4 files changed, 213 insertions(+), 9 deletions(-) >> > > Why are these patches still not merged? > Because patches are not reviewed yet, specially ethdev one. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 0/2] xstats related patches @ 2019-11-01 20:12 Stephen Hemminger 2019-11-01 20:12 ` [dpdk-dev] [PATCH 2/2] failsafe: implement xstats Stephen Hemminger 0 siblings, 1 reply; 30+ messages in thread From: Stephen Hemminger @ 2019-11-01 20:12 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger This is an updated simplified version of previous failsafe xstats patch and a bug fix for testpmd. The failsafe xstats patch is almostly a complete redo. Stephen Hemminger (2): app/testpmd: block xstats for hidden ports failsafe: implement xstats app/test-pmd/config.c | 8 ++ drivers/net/failsafe/failsafe_ops.c | 152 ++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+) -- 2.20.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 2/2] failsafe: implement xstats 2019-11-01 20:12 [dpdk-dev] [PATCH 0/2] xstats related patches Stephen Hemminger @ 2019-11-01 20:12 ` Stephen Hemminger 2019-11-08 18:13 ` Ferruh Yigit 0 siblings, 1 reply; 30+ messages in thread From: Stephen Hemminger @ 2019-11-01 20:12 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Add support for extended statistics in failsafe driver. Reports detailed statistics for each sub device. Example: testpmd> show port xstats 1 rx_good_packets: 0 tx_good_packets: 0 rx_good_bytes: 0 tx_good_bytes: 0 rx_missed_errors: 0 rx_errors: 0 tx_errors: 0 rx_mbuf_allocation_errors: 0 rx_q0packets: 0 rx_q0bytes: 0 rx_q0errors: 0 tx_q0packets: 0 tx_q0bytes: 0 rx_sub0_good_packets: 0 tx_sub0_good_packets: 0 ... rx_sub1_good_packets: 0 tx_sub1_good_packets: 0 rx_sub1_good_bytes: 0 Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/failsafe/failsafe_ops.c | 152 ++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 0afae6c71feb..a87e49b97d33 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -14,6 +14,7 @@ #include <rte_flow.h> #include <rte_cycles.h> #include <rte_ethdev.h> +#include <rte_string_fns.h> #include "failsafe_private.h" @@ -881,6 +882,154 @@ fs_stats_reset(struct rte_eth_dev *dev) return 0; } +static int +__fs_xstats_count(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + int count = 0; + uint8_t i; + int ret; + + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { + ret = rte_eth_xstats_get_names(PORT_ID(sdev), NULL, 0); + if (ret < 0) + return ret; + count += ret; + } + + return count; +} + +static int +__fs_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int limit) +{ + struct sub_device *sdev; + unsigned int count = 0; + uint8_t i; + + /* Caller only cares about count */ + if (!xstats_names) + return __fs_xstats_count(dev); + + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { + struct rte_eth_xstat_name *sub_names = xstats_names + count; + int j, r; + + if (count >= limit) + break; + + r = rte_eth_xstats_get_names(PORT_ID(sdev), + sub_names, limit - count); + if (r < 0) + return r; + + /* add subN_ prefix to names */ + for (j = 0; j < r; j++) { + char *xname = sub_names[j].name; + char tmp[RTE_ETH_XSTATS_NAME_SIZE]; + + if ((xname[0] == 't' || xname[0] == 'r') && + xname[1] == 'x' && xname[2] == '_') + snprintf(tmp, sizeof(tmp), "%.3ssub%u_%s", + xname, i, xname + 3); + else + snprintf(tmp, sizeof(tmp), "sub%u_%s", + i, xname); + + strlcpy(xname, tmp, RTE_ETH_XSTATS_NAME_SIZE); + } + count += r; + } + return count; +} + +static int +fs_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int limit) +{ + int ret; + + fs_lock(dev, 0); + ret = __fs_xstats_get_names(dev, xstats_names, limit); + fs_unlock(dev, 0); + return ret; +} + +static int +__fs_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int count = 0; + struct sub_device *sdev; + uint8_t i; + int j, ret; + + ret = __fs_xstats_count(dev); + /* + * if error + * or caller did not give enough space + * or just querying + */ + if (ret < 0 || ret > (int)n || xstats == NULL) + return ret; + + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { + ret = rte_eth_xstats_get(PORT_ID(sdev), xstats, n); + if (ret < 0) + return ret; + + if (ret > (int)n) + return n + count; + + /* add offset to id's from sub-device */ + for (j = 0; j < ret; j++) + xstats[j].id += count; + + xstats += ret; + n -= ret; + count += ret; + } + + return count; +} + +static int +fs_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, + unsigned int n) +{ + int ret; + + fs_lock(dev, 0); + ret = __fs_xstats_get(dev, xstats, n); + fs_unlock(dev, 0); + + return ret; +} + + +static int +fs_xstats_reset(struct rte_eth_dev *dev) +{ + struct sub_device *sdev; + uint8_t i; + int r = 0; + + fs_lock(dev, 0); + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { + r = rte_eth_xstats_reset(PORT_ID(sdev)); + if (r < 0) + break; + } + fs_unlock(dev, 0); + + return r; +} + static void fs_dev_merge_desc_lim(struct rte_eth_desc_lim *to, const struct rte_eth_desc_lim *from) @@ -1331,6 +1480,9 @@ const struct eth_dev_ops failsafe_ops = { .link_update = fs_link_update, .stats_get = fs_stats_get, .stats_reset = fs_stats_reset, + .xstats_get = fs_xstats_get, + .xstats_get_names = fs_xstats_get_names, + .xstats_reset = fs_xstats_reset, .dev_infos_get = fs_dev_infos_get, .dev_supported_ptypes_get = fs_dev_supported_ptypes_get, .mtu_set = fs_mtu_set, -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] failsafe: implement xstats 2019-11-01 20:12 ` [dpdk-dev] [PATCH 2/2] failsafe: implement xstats Stephen Hemminger @ 2019-11-08 18:13 ` Ferruh Yigit 0 siblings, 0 replies; 30+ messages in thread From: Ferruh Yigit @ 2019-11-08 18:13 UTC (permalink / raw) To: Stephen Hemminger, dev, Gaetan Rivet On 11/1/2019 8:12 PM, Stephen Hemminger wrote: > Add support for extended statistics in failsafe driver. > Reports detailed statistics for each sub device. > > Example: > > testpmd> show port xstats 1 > rx_good_packets: 0 > tx_good_packets: 0 > rx_good_bytes: 0 > tx_good_bytes: 0 > rx_missed_errors: 0 > rx_errors: 0 > tx_errors: 0 > rx_mbuf_allocation_errors: 0 > rx_q0packets: 0 > rx_q0bytes: 0 > rx_q0errors: 0 > tx_q0packets: 0 > tx_q0bytes: 0 > rx_sub0_good_packets: 0 > tx_sub0_good_packets: 0 > ... > rx_sub1_good_packets: 0 > tx_sub1_good_packets: 0 > rx_sub1_good_bytes: 0 > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> Hi Stephen, Geatan, What do you think about adding Stephen as additional failsafe maintainer? ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2019-11-08 18:13 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-26 22:21 [dpdk-dev] [PATCH 0/2] failsafe: add xstats Stephen Hemminger 2019-06-26 22:21 ` [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-06-26 22:21 ` [dpdk-dev] [PATCH 2/2] failsafe: implement xstats Stephen Hemminger 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 0/2] failsafe: add xstats Stephen Hemminger 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-07-01 10:54 ` Andrew Rybchenko 2019-07-01 14:59 ` Stephen Hemminger 2019-06-26 23:33 ` [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats Stephen Hemminger 2019-07-01 12:33 ` Gaëtan Rivet 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Stephen Hemminger 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-07-03 17:25 ` [dpdk-dev] [PATCH v2 2/2] failsafe: implement xstats Stephen Hemminger 2019-07-04 9:56 ` [dpdk-dev] [PATCH v2 0/2] *failsafe: add xstats Gaëtan Rivet 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 0/2] failsafe: " Stephen Hemminger 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-08-11 16:06 ` [dpdk-dev] [PATCH v3 2/2] net/failsafe: implement xstats Stephen Hemminger 2019-09-19 12:56 ` [dpdk-dev] [PATCH v4 0/2] failsafe: add xstats Stephen Hemminger 2019-09-19 12:56 ` [dpdk-dev] [PATCH 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-09-19 12:56 ` [dpdk-dev] [PATCH 2/2] net/failsafe: implement xstats Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 1/2] ethdev: expose basic xstats for driver use Stephen Hemminger 2019-09-26 12:46 ` Andrew Rybchenko 2019-09-26 16:09 ` Stephen Hemminger 2019-10-31 16:40 ` Stephen Hemminger 2019-09-19 13:17 ` [dpdk-dev] [PATCH v5 2/2] net/failsafe: implement xstats Stephen Hemminger 2019-10-17 14:54 ` Ferruh Yigit 2019-10-14 17:04 ` [dpdk-dev] [PATCH v5 0/2] failsafe: add xstats Stephen Hemminger 2019-10-15 9:08 ` Ferruh Yigit 2019-11-01 20:12 [dpdk-dev] [PATCH 0/2] xstats related patches Stephen Hemminger 2019-11-01 20:12 ` [dpdk-dev] [PATCH 2/2] failsafe: implement xstats Stephen Hemminger 2019-11-08 18:13 ` Ferruh Yigit
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).