[dpdk-dev] [PATCH] raw/ioat: enable xstats reset for ioat device

[dpdk-dev] [PATCH] raw/ioat: enable xstats reset for ioat device

[Ciara Power]

The rawdev xstats_reset function is now enabled.  It is called when the
ioat autotest completes, to reset all xstat values after they have been
modified during testing.

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 drivers/raw/ioat/ioat_rawdev.c      | 38 +++++++++++++++++++++++++++++
 drivers/raw/ioat/ioat_rawdev_test.c |  6 +++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c
index 7270ad7aa..af8414b34 100644
--- a/drivers/raw/ioat/ioat_rawdev.c
+++ b/drivers/raw/ioat/ioat_rawdev.c
@@ -161,6 +161,43 @@ ioat_xstats_get_names(const struct rte_rawdev *dev,
 	return RTE_DIM(xstat_names);
 }
 
+static int
+ioat_xstats_reset(struct rte_rawdev *dev, const uint32_t *ids, uint32_t nb_ids)
+{
+	struct rte_ioat_rawdev *ioat = dev->dev_private;
+	unsigned int i;
+
+	if (!ids) {
+		ioat->enqueue_failed = 0;
+		ioat->enqueued = 0;
+		ioat->started = 0;
+		ioat->completed = 0;
+		return 0;
+	}
+
+	for (i = 0; i < nb_ids; i++) {
+		switch (ids[i]) {
+		case 0:
+			ioat->enqueue_failed = 0;
+			break;
+		case 1:
+			ioat->enqueued = 0;
+			break;
+		case 2:
+			ioat->started = 0;
+			break;
+		case 3:
+			ioat->completed = 0;
+			break;
+		default:
+			IOAT_PMD_WARN("Invalid xstat id - cannot reset value");
+			break;
+		}
+	}
+
+	return 0;
+}
+
 extern int ioat_rawdev_test(uint16_t dev_id);
 
 static int
@@ -173,6 +210,7 @@ ioat_rawdev_create(const char *name, struct rte_pci_device *dev)
 			.dev_info_get = ioat_dev_info_get,
 			.xstats_get = ioat_xstats_get,
 			.xstats_get_names = ioat_xstats_get_names,
+			.xstats_reset = ioat_xstats_reset,
 			.dev_selftest = ioat_rawdev_test,
 	};
 
diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c
index f6c7dbb80..61d4bd066 100644
--- a/drivers/raw/ioat/ioat_rawdev_test.c
+++ b/drivers/raw/ioat/ioat_rawdev_test.c
@@ -220,6 +220,11 @@ ioat_rawdev_test(uint16_t dev_id)
 	}
 	printf("\n");
 
+	if (rte_rawdev_xstats_reset(dev_id, NULL, 0) != 0) {
+		printf("Error resetting xstat values\n");
+		goto err;
+	}
+
 	rte_mempool_free(pool);
 	free(snames);
 	free(stats);
@@ -227,6 +232,7 @@ ioat_rawdev_test(uint16_t dev_id)
 	return 0;
 
 err:
+	rte_rawdev_xstats_reset(dev_id, NULL, 0);
 	rte_mempool_free(pool);
 	free(snames);
 	free(stats);
-- 
2.17.1

Re: [dpdk-dev] [PATCH] raw/ioat: enable xstats reset for ioat device

[Bruce Richardson]

On Wed, Oct 16, 2019 at 02:16:26PM +0100, Ciara Power wrote:
> The rawdev xstats_reset function is now enabled.  It is called when the
> ioat autotest completes, to reset all xstat values after they have been
> modified during testing.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH] raw/ioat: enable xstats reset for ioat device

[David Marchand]

On Wed, Oct 16, 2019 at 3:36 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Oct 16, 2019 at 02:16:26PM +0100, Ciara Power wrote:
> > The rawdev xstats_reset function is now enabled.  It is called when the
> > ioat autotest completes, to reset all xstat values after they have been
> > modified during testing.
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

This patch conflicts with the other series on this driver.
Is it really necessary to reset those statistics once the device has
been stopped?


-- 
David Marchand

Re: [dpdk-dev] [PATCH] raw/ioat: enable xstats reset for ioat device

[Power, Ciara]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Sunday 27 October 2019 13:27
> To: Power, Ciara <ciara.power@intel.com>
> Cc: dev <dev@dpdk.org>; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] raw/ioat: enable xstats reset for ioat device
> 
> On Wed, Oct 16, 2019 at 3:36 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Wed, Oct 16, 2019 at 02:16:26PM +0100, Ciara Power wrote:
> > > The rawdev xstats_reset function is now enabled.  It is called when
> > > the ioat autotest completes, to reset all xstat values after they
> > > have been modified during testing.
> > >
> > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> >
> > Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> This patch conflicts with the other series on this driver.
> Is it really necessary to reset those statistics once the device has been
> stopped?
> 
> 
> --
> David Marchand

Hi David,

It is a necessary change because stopping the device does not reset the stats, which will result in them accumulating if the test is run multiple times.
Resetting the stats after stopping the device solves this problem.

I can rebase onto master to resolve the conflict, and send a new version for this patch if it helps.

Thanks,
- Ciara  

Re: [dpdk-dev] [PATCH] raw/ioat: enable xstats reset for ioat device

[David Marchand]

On Tue, Oct 29, 2019 at 12:52 PM Power, Ciara <ciara.power@intel.com> wrote:
> > This patch conflicts with the other series on this driver.
> > Is it really necessary to reset those statistics once the device has been
> > stopped?
> >> It is a necessary change because stopping the device does not reset the stats, which will result in them accumulating if the test is run multiple times.
> Resetting the stats after stopping the device solves this problem.

Ok, if this is intended.

> I can rebase onto master to resolve the conflict, and send a new version for this patch if it helps.

Yes please.


-- 
David Marchand

[dpdk-dev] [PATCH v2] raw/ioat: enable xstats reset for ioat device

[Ciara Power]

The rawdev xstats_reset function is now enabled.  It is called when the
ioat autotest completes, to reset all xstat values after they have been
modified during testing.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

---
v2: Rebased onto master
---
 drivers/raw/ioat/ioat_rawdev.c      | 38 +++++++++++++++++++++++++++++
 drivers/raw/ioat/ioat_rawdev_test.c |  6 +++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c
index 7270ad7aa..af8414b34 100644
--- a/drivers/raw/ioat/ioat_rawdev.c
+++ b/drivers/raw/ioat/ioat_rawdev.c
@@ -161,6 +161,43 @@ ioat_xstats_get_names(const struct rte_rawdev *dev,
 	return RTE_DIM(xstat_names);
 }
 
+static int
+ioat_xstats_reset(struct rte_rawdev *dev, const uint32_t *ids, uint32_t nb_ids)
+{
+	struct rte_ioat_rawdev *ioat = dev->dev_private;
+	unsigned int i;
+
+	if (!ids) {
+		ioat->enqueue_failed = 0;
+		ioat->enqueued = 0;
+		ioat->started = 0;
+		ioat->completed = 0;
+		return 0;
+	}
+
+	for (i = 0; i < nb_ids; i++) {
+		switch (ids[i]) {
+		case 0:
+			ioat->enqueue_failed = 0;
+			break;
+		case 1:
+			ioat->enqueued = 0;
+			break;
+		case 2:
+			ioat->started = 0;
+			break;
+		case 3:
+			ioat->completed = 0;
+			break;
+		default:
+			IOAT_PMD_WARN("Invalid xstat id - cannot reset value");
+			break;
+		}
+	}
+
+	return 0;
+}
+
 extern int ioat_rawdev_test(uint16_t dev_id);
 
 static int
@@ -173,6 +210,7 @@ ioat_rawdev_create(const char *name, struct rte_pci_device *dev)
 			.dev_info_get = ioat_dev_info_get,
 			.xstats_get = ioat_xstats_get,
 			.xstats_get_names = ioat_xstats_get_names,
+			.xstats_reset = ioat_xstats_reset,
 			.dev_selftest = ioat_rawdev_test,
 	};
 
diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c
index e99f97ec4..c37351af2 100644
--- a/drivers/raw/ioat/ioat_rawdev_test.c
+++ b/drivers/raw/ioat/ioat_rawdev_test.c
@@ -223,6 +223,11 @@ ioat_rawdev_test(uint16_t dev_id)
 	printf("\n");
 
 	rte_rawdev_stop(dev_id);
+	if (rte_rawdev_xstats_reset(dev_id, NULL, 0) != 0) {
+		printf("Error resetting xstat values\n");
+		goto err;
+	}
+
 	rte_mempool_free(pool);
 	free(snames);
 	free(stats);
@@ -231,6 +236,7 @@ ioat_rawdev_test(uint16_t dev_id)
 
 err:
 	rte_rawdev_stop(dev_id);
+	rte_rawdev_xstats_reset(dev_id, NULL, 0);
 	rte_mempool_free(pool);
 	free(snames);
 	free(stats);
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2] raw/ioat: enable xstats reset for ioat device

[David Marchand]

On Tue, Oct 29, 2019 at 2:45 PM Ciara Power <ciara.power@intel.com> wrote:
>
> The rawdev xstats_reset function is now enabled.  It is called when the
> ioat autotest completes, to reset all xstat values after they have been
> modified during testing.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks.


-- 
David Marchand