[v5,2/2] net/failsafe: implement xstats
Checks
Commit Message
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(+)
Comments
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;
> +}
<...>
@@ -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,