[dpdk-dev] [PATCH 0/3] service: add attribute get and reset

[dpdk-dev] [PATCH 0/3] service: add attribute get and reset

[Harry van Haaren]

This patchset adds two new functions to the service cores API, allowing
the service library to expose generic attributes about the services,
and to reset them. These attributes can be the cycle counts that cores
have spent in the service-function. The 3rd patch in the series adds a
new attribute, enabling the application to retrieve the number of calls
to any service.

Harry van Haaren (3):
  service: add attribute get function
  service: add reset all attributes for service
  service: add attribute for number of invokations

 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 +
 lib/librte_eal/common/include/rte_service.h     | 32 +++++++++
 lib/librte_eal/common/rte_service.c             | 45 +++++++++++--
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 +
 test/test/test_service_cores.c                  | 88 +++++++++++++++++++++++++
 5 files changed, 163 insertions(+), 6 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/3] service: add attribute get function

[Harry van Haaren]

This commit adds a new function to the service API to allow
the application to retrieve items about each individual service
in the system. A unit test checks the return values of a variety
of invalid and valid calls.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/include/rte_service.h     | 17 +++++++
 lib/librte_eal/common/rte_service.c             | 18 ++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 test/test/test_service_cores.c                  | 61 +++++++++++++++++++++++++
 5 files changed, 98 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 47a09ea..8c9c5f6 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -212,6 +212,7 @@ EXPERIMENTAL {
 	rte_eal_devargs_remove;
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
+	rte_service_attr_get;
 	rte_service_component_register;
 	rte_service_component_unregister;
 	rte_service_component_runstate_set;
diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index 21da739..9267a0e 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -360,6 +360,23 @@ int32_t rte_service_lcore_count_services(uint32_t lcore);
  */
 int32_t rte_service_dump(FILE *f, uint32_t id);
 
+/**
+ * Returns the number of cycles that this service has consumed
+ */
+#define RTE_SERVICE_ATTR_CYCLES 0
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Get an attribute from a service.
+ *
+ * @retval 0 Success, the attribute value has been written to *attr_value*.
+ *         -EINVAL Invalid id, attr_id or attr_value was NULL.
+ */
+int32_t rte_service_attr_get(uint32_t id, uint32_t attr_id,
+			     uint32_t *attr_value);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index e598e16..4935804 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -644,6 +644,24 @@ rte_service_lcore_stop(uint32_t lcore)
 	return 0;
 }
 
+int32_t
+rte_service_attr_get(uint32_t id, uint32_t attr_id, uint32_t *attr_value)
+{
+	struct rte_service_spec_impl *s;
+	SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
+
+	if (!attr_value)
+		return -EINVAL;
+
+	switch (attr_id) {
+	case RTE_SERVICE_ATTR_CYCLES:
+		*attr_value = s->cycles_spent;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
 static void
 rte_service_dump_one(FILE *f, struct rte_service_spec_impl *s,
 		     uint64_t all_cycles, uint32_t reset)
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 8c08b8d..c2fad10 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -217,6 +217,7 @@ EXPERIMENTAL {
 	rte_eal_devargs_remove;
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
+	rte_service_attr_get;
 	rte_service_component_register;
 	rte_service_component_unregister;
 	rte_service_component_runstate_set;
diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c
index ee8313e..7d2e42d 100644
--- a/test/test/test_service_cores.c
+++ b/test/test/test_service_cores.c
@@ -266,6 +266,66 @@ service_name(void)
 	return unregister_all();
 }
 
+/* verify service attr get */
+static int
+service_attr_get(void)
+{
+	struct rte_service_spec service;
+	memset(&service, 0, sizeof(struct rte_service_spec));
+	service.callback = dummy_cb;
+	snprintf(service.name, sizeof(service.name), DUMMY_SERVICE_NAME);
+	service.capabilities |= RTE_SERVICE_CAP_MT_SAFE;
+	uint32_t id;
+	TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, &id),
+			"Register of  service failed");
+	rte_service_component_runstate_set(id, 1);
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(id, 1),
+			"Error: Service start returned non-zero");
+	rte_service_set_stats_enable(id, 1);
+
+	uint32_t attr_id = UINT32_MAX;
+	uint32_t attr_value = 0xdead;
+	/* check error return values */
+	TEST_ASSERT_EQUAL(-EINVAL, rte_service_attr_get(id, attr_id,
+							&attr_value),
+			"Invalid attr_id didn't return -EINVAL");
+
+	attr_id = RTE_SERVICE_ATTR_CYCLES;
+	TEST_ASSERT_EQUAL(-EINVAL, rte_service_attr_get(UINT32_MAX, attr_id,
+							&attr_value),
+			"Invalid service id didn't return -EINVAL");
+
+	TEST_ASSERT_EQUAL(-EINVAL, rte_service_attr_get(id, attr_id, NULL),
+			"Invalid attr_value pointer id didn't return -EINVAL");
+
+	/* check correct (zero) return value and correct value (zero) */
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_id, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(0, attr_value,
+			"attr_get() call didn't set correct cycles (zero)");
+
+	/* Call service to increment cycle count */
+	TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id),
+			"Service core add did not return zero");
+	TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(id, slcore_id, 1),
+			"Enabling valid service and core failed");
+	TEST_ASSERT_EQUAL(0, rte_service_lcore_start(slcore_id),
+			"Starting service core failed");
+
+	/* wait for the service lcore to run */
+	rte_delay_ms(200);
+
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_id, &attr_value),
+			"Valid attr_get() call didn't return success");
+	int cycles_gt_zero = attr_value > 0;
+	TEST_ASSERT_EQUAL(1, cycles_gt_zero,
+			"attr_get() failed to get cycles (expected > zero)");
+
+	rte_service_lcore_stop(slcore_id);
+
+	return unregister_all();
+}
+
 /* verify service dump */
 static int
 service_dump(void)
@@ -606,6 +666,7 @@ static struct unit_test_suite service_tests  = {
 		TEST_CASE_ST(dummy_register, NULL, service_name),
 		TEST_CASE_ST(dummy_register, NULL, service_get_by_name),
 		TEST_CASE_ST(dummy_register, NULL, service_dump),
+		TEST_CASE_ST(dummy_register, NULL, service_attr_get),
 		TEST_CASE_ST(dummy_register, NULL, service_probe_capability),
 		TEST_CASE_ST(dummy_register, NULL, service_start_stop),
 		TEST_CASE_ST(dummy_register, NULL, service_lcore_add_del),
-- 
2.7.4

[dpdk-dev] [PATCH 2/3] service: add reset all attributes for service

[Harry van Haaren]

This commit introduces a new API, allowing the application to
reset attributes of a service like the cycle count. Given this
functionality is now exposed to the user, remove the resetting
of stats during a dump() call.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/include/rte_service.h     | 10 ++++++++++
 lib/librte_eal/common/rte_service.c             | 24 ++++++++++++++++++------
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 test/test/test_service_cores.c                  | 11 +++++++++++
 5 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 8c9c5f6..e654283 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -213,6 +213,7 @@ EXPERIMENTAL {
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
 	rte_service_attr_get;
+	rte_service_attr_reset_all;
 	rte_service_component_register;
 	rte_service_component_unregister;
 	rte_service_component_runstate_set;
diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index 9267a0e..bf7b50b 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -377,6 +377,16 @@ int32_t rte_service_dump(FILE *f, uint32_t id);
 int32_t rte_service_attr_get(uint32_t id, uint32_t attr_id,
 			     uint32_t *attr_value);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Reset all attributes of a service.
+ * @retval 0 Successfully reset attributes
+ *         -EINVAL Invalid service id provided
+ */
+int32_t rte_service_attr_reset_all(uint32_t id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 4935804..3f274f4 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -674,15 +674,27 @@ rte_service_dump_one(FILE *f, struct rte_service_spec_impl *s,
 	if (s->calls != 0)
 		calls = s->calls;
 
+	if (reset) {
+		s->cycles_spent = 0;
+		s->calls = 0;
+		return;
+	}
+
 	fprintf(f, "  %s: stats %d\tcalls %"PRIu64"\tcycles %"
 			PRIu64"\tavg: %"PRIu64"\n",
 			s->spec.name, service_stats_enabled(s), s->calls,
 			s->cycles_spent, s->cycles_spent / calls);
+}
 
-	if (reset) {
-		s->cycles_spent = 0;
-		s->calls = 0;
-	}
+int32_t
+rte_service_attr_reset_all(uint32_t id)
+{
+	struct rte_service_spec_impl *s;
+	SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
+
+	int reset = 1;
+	rte_service_dump_one(NULL, s, 0, reset);
+	return 0;
 }
 
 static void
@@ -730,7 +742,7 @@ int32_t rte_service_dump(FILE *f, uint32_t id)
 	for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) {
 		if (!service_valid(i))
 			continue;
-		uint32_t reset = 1;
+		uint32_t reset = 0;
 		rte_service_dump_one(f, &rte_services[i], total_cycles, reset);
 	}
 
@@ -739,7 +751,7 @@ int32_t rte_service_dump(FILE *f, uint32_t id)
 		if (lcore_config[i].core_role != ROLE_SERVICE)
 			continue;
 
-		uint32_t reset = 1;
+		uint32_t reset = 0;
 		service_dump_calls_per_lcore(f, i, reset);
 	}
 
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index c2fad10..e5a0f04 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -218,6 +218,7 @@ EXPERIMENTAL {
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
 	rte_service_attr_get;
+	rte_service_attr_reset_all;
 	rte_service_component_register;
 	rte_service_component_unregister;
 	rte_service_component_runstate_set;
diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c
index 7d2e42d..dd3bf75 100644
--- a/test/test/test_service_cores.c
+++ b/test/test/test_service_cores.c
@@ -270,6 +270,9 @@ service_name(void)
 static int
 service_attr_get(void)
 {
+	/* ensure all services unregistered so cycle counts are zero */
+	unregister_all();
+
 	struct rte_service_spec service;
 	memset(&service, 0, sizeof(struct rte_service_spec));
 	service.callback = dummy_cb;
@@ -323,6 +326,14 @@ service_attr_get(void)
 
 	rte_service_lcore_stop(slcore_id);
 
+	TEST_ASSERT_EQUAL(0, rte_service_attr_reset_all(id),
+			"Valid attr_reset_all() return success");
+
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_id, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(0, attr_value,
+			"attr_get() call didn't set correct cycles (zero)");
+
 	return unregister_all();
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH 3/3] service: add attribute for number of invokations

[Harry van Haaren]

This commit adds a new attribute to the service cores attributes
API, which allows the application to retrieve the number of times
that a service-core called the service to perform its action.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/librte_eal/common/include/rte_service.h |  5 +++++
 lib/librte_eal/common/rte_service.c         |  3 +++
 test/test/test_service_cores.c              | 16 ++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index bf7b50b..4684985 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -366,6 +366,11 @@ int32_t rte_service_dump(FILE *f, uint32_t id);
 #define RTE_SERVICE_ATTR_CYCLES 0
 
 /**
+ * Returns the count of invokations of this service function
+ */
+#define RTE_SERVICE_ATTR_CALL_COUNT 1
+
+/**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice
  *
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 3f274f4..fe0b8cf 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -657,6 +657,9 @@ rte_service_attr_get(uint32_t id, uint32_t attr_id, uint32_t *attr_value)
 	case RTE_SERVICE_ATTR_CYCLES:
 		*attr_value = s->cycles_spent;
 		return 0;
+	case RTE_SERVICE_ATTR_CALL_COUNT:
+		*attr_value = s->calls;
+		return 0;
 	default:
 		return -EINVAL;
 	}
diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c
index dd3bf75..cabf5de 100644
--- a/test/test/test_service_cores.c
+++ b/test/test/test_service_cores.c
@@ -306,6 +306,12 @@ service_attr_get(void)
 			"Valid attr_get() call didn't return success");
 	TEST_ASSERT_EQUAL(0, attr_value,
 			"attr_get() call didn't set correct cycles (zero)");
+	/* check correct call count */
+	const int attr_calls = RTE_SERVICE_ATTR_CALL_COUNT;
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_calls, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(0, attr_value,
+			"attr_get() call didn't get call count (zero)");
 
 	/* Call service to increment cycle count */
 	TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id),
@@ -326,6 +332,11 @@ service_attr_get(void)
 
 	rte_service_lcore_stop(slcore_id);
 
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_calls, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(1, (attr_value > 0),
+			"attr_get() call didn't get call count (zero)");
+
 	TEST_ASSERT_EQUAL(0, rte_service_attr_reset_all(id),
 			"Valid attr_reset_all() return success");
 
@@ -333,6 +344,11 @@ service_attr_get(void)
 			"Valid attr_get() call didn't return success");
 	TEST_ASSERT_EQUAL(0, attr_value,
 			"attr_get() call didn't set correct cycles (zero)");
+	/* ensure call count > zero */
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_calls, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(0, (attr_value > 0),
+			"attr_get() call didn't get call count (zero)");
 
 	return unregister_all();
 }
-- 
2.7.4

Re: [dpdk-dev] [PATCH 0/3] service: add attribute get and reset

[Thomas Monjalon]

04/10/2017 12:54, Harry van Haaren:
> This patchset adds two new functions to the service cores API, allowing
> the service library to expose generic attributes about the services,
> and to reset them. These attributes can be the cycle counts that cores
> have spent in the service-function. The 3rd patch in the series adds a
> new attribute, enabling the application to retrieve the number of calls
> to any service.
> 
> Harry van Haaren (3):
>   service: add attribute get function
>   service: add reset all attributes for service
>   service: add attribute for number of invokations

Please could you rebase this series?

There is also a typo in title and code comment:
	invokation -> invocation

Re: [dpdk-dev] [PATCH 0/3] service: add attribute get and reset

[Van Haaren, Harry]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> <snip>
> >
> > Harry van Haaren (3):
> >   service: add attribute get function
> >   service: add reset all attributes for service
> >   service: add attribute for number of invokations
> 
> Please could you rebase this series?

Yes no problem, on the way.


> There is also a typo in title and code comment:
> 	invokation -> invocation

[dpdk-dev] [PATCH v2 1/3] service: add attribute get function

[Harry van Haaren]

This commit adds a new function to the service API to allow
the application to retrieve items about each individual service
in the system. A unit test checks the return values of a variety
of invalid and valid calls.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v2:
- Rebase on master, fix version.map conflicts
---
 lib/librte_eal/common/include/rte_service.h | 17 ++++++++
 lib/librte_eal/common/rte_service.c         | 18 +++++++++
 lib/librte_eal/rte_eal_version.map          |  1 +
 test/test/test_service_cores.c              | 61 +++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index 95def4c..4898e76 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -394,6 +394,23 @@ int32_t rte_service_lcore_count_services(uint32_t lcore);
  */
 int32_t rte_service_dump(FILE *f, uint32_t id);
 
+/**
+ * Returns the number of cycles that this service has consumed
+ */
+#define RTE_SERVICE_ATTR_CYCLES 0
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Get an attribute from a service.
+ *
+ * @retval 0 Success, the attribute value has been written to *attr_value*.
+ *         -EINVAL Invalid id, attr_id or attr_value was NULL.
+ */
+int32_t rte_service_attr_get(uint32_t id, uint32_t attr_id,
+			     uint32_t *attr_value);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 1065392..63db475 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -682,6 +682,24 @@ rte_service_lcore_stop(uint32_t lcore)
 	return 0;
 }
 
+int32_t
+rte_service_attr_get(uint32_t id, uint32_t attr_id, uint32_t *attr_value)
+{
+	struct rte_service_spec_impl *s;
+	SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
+
+	if (!attr_value)
+		return -EINVAL;
+
+	switch (attr_id) {
+	case RTE_SERVICE_ATTR_CYCLES:
+		*attr_value = s->cycles_spent;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
 static void
 rte_service_dump_one(FILE *f, struct rte_service_spec_impl *s,
 		     uint64_t all_cycles, uint32_t reset)
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index b3dc5a7..744de04 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -216,6 +216,7 @@ EXPERIMENTAL {
 	rte_eal_devargs_remove;
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
+	rte_service_attr_get;
 	rte_service_component_register;
 	rte_service_component_unregister;
 	rte_service_component_runstate_set;
diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c
index 2972a80..a0768c9 100644
--- a/test/test/test_service_cores.c
+++ b/test/test/test_service_cores.c
@@ -238,6 +238,66 @@ service_name(void)
 	return unregister_all();
 }
 
+/* verify service attr get */
+static int
+service_attr_get(void)
+{
+	struct rte_service_spec service;
+	memset(&service, 0, sizeof(struct rte_service_spec));
+	service.callback = dummy_cb;
+	snprintf(service.name, sizeof(service.name), DUMMY_SERVICE_NAME);
+	service.capabilities |= RTE_SERVICE_CAP_MT_SAFE;
+	uint32_t id;
+	TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, &id),
+			"Register of  service failed");
+	rte_service_component_runstate_set(id, 1);
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(id, 1),
+			"Error: Service start returned non-zero");
+	rte_service_set_stats_enable(id, 1);
+
+	uint32_t attr_id = UINT32_MAX;
+	uint32_t attr_value = 0xdead;
+	/* check error return values */
+	TEST_ASSERT_EQUAL(-EINVAL, rte_service_attr_get(id, attr_id,
+							&attr_value),
+			"Invalid attr_id didn't return -EINVAL");
+
+	attr_id = RTE_SERVICE_ATTR_CYCLES;
+	TEST_ASSERT_EQUAL(-EINVAL, rte_service_attr_get(UINT32_MAX, attr_id,
+							&attr_value),
+			"Invalid service id didn't return -EINVAL");
+
+	TEST_ASSERT_EQUAL(-EINVAL, rte_service_attr_get(id, attr_id, NULL),
+			"Invalid attr_value pointer id didn't return -EINVAL");
+
+	/* check correct (zero) return value and correct value (zero) */
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_id, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(0, attr_value,
+			"attr_get() call didn't set correct cycles (zero)");
+
+	/* Call service to increment cycle count */
+	TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id),
+			"Service core add did not return zero");
+	TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(id, slcore_id, 1),
+			"Enabling valid service and core failed");
+	TEST_ASSERT_EQUAL(0, rte_service_lcore_start(slcore_id),
+			"Starting service core failed");
+
+	/* wait for the service lcore to run */
+	rte_delay_ms(200);
+
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_id, &attr_value),
+			"Valid attr_get() call didn't return success");
+	int cycles_gt_zero = attr_value > 0;
+	TEST_ASSERT_EQUAL(1, cycles_gt_zero,
+			"attr_get() failed to get cycles (expected > zero)");
+
+	rte_service_lcore_stop(slcore_id);
+
+	return unregister_all();
+}
+
 /* verify service dump */
 static int
 service_dump(void)
@@ -693,6 +753,7 @@ static struct unit_test_suite service_tests  = {
 		TEST_CASE_ST(dummy_register, NULL, service_name),
 		TEST_CASE_ST(dummy_register, NULL, service_get_by_name),
 		TEST_CASE_ST(dummy_register, NULL, service_dump),
+		TEST_CASE_ST(dummy_register, NULL, service_attr_get),
 		TEST_CASE_ST(dummy_register, NULL, service_probe_capability),
 		TEST_CASE_ST(dummy_register, NULL, service_start_stop),
 		TEST_CASE_ST(dummy_register, NULL, service_lcore_add_del),
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/3] service: add reset all attributes for service

[Harry van Haaren]

This commit introduces a new API, allowing the application to
reset attributes of a service like the cycle count. Given this
functionality is now exposed to the user, remove the resetting
of stats during a dump() call.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v2:
- Improve doxygen with @param value
---
 lib/librte_eal/common/include/rte_service.h | 12 ++++++++++++
 lib/librte_eal/common/rte_service.c         | 24 ++++++++++++++++++------
 lib/librte_eal/rte_eal_version.map          |  1 +
 test/test/test_service_cores.c              | 11 +++++++++++
 4 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index 4898e76..460dbbf 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -411,6 +411,18 @@ int32_t rte_service_dump(FILE *f, uint32_t id);
 int32_t rte_service_attr_get(uint32_t id, uint32_t attr_id,
 			     uint32_t *attr_value);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Reset all attribute values of a service.
+ *
+ * @param id The service to reset all statistics of
+ * @retval 0 Successfully reset attributes
+ *         -EINVAL Invalid service id provided
+ */
+int32_t rte_service_attr_reset_all(uint32_t id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 63db475..1b54b7e 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -712,15 +712,27 @@ rte_service_dump_one(FILE *f, struct rte_service_spec_impl *s,
 	if (s->calls != 0)
 		calls = s->calls;
 
+	if (reset) {
+		s->cycles_spent = 0;
+		s->calls = 0;
+		return;
+	}
+
 	fprintf(f, "  %s: stats %d\tcalls %"PRIu64"\tcycles %"
 			PRIu64"\tavg: %"PRIu64"\n",
 			s->spec.name, service_stats_enabled(s), s->calls,
 			s->cycles_spent, s->cycles_spent / calls);
+}
 
-	if (reset) {
-		s->cycles_spent = 0;
-		s->calls = 0;
-	}
+int32_t
+rte_service_attr_reset_all(uint32_t id)
+{
+	struct rte_service_spec_impl *s;
+	SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
+
+	int reset = 1;
+	rte_service_dump_one(NULL, s, 0, reset);
+	return 0;
 }
 
 static void
@@ -768,7 +780,7 @@ int32_t rte_service_dump(FILE *f, uint32_t id)
 	for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) {
 		if (!service_valid(i))
 			continue;
-		uint32_t reset = 1;
+		uint32_t reset = 0;
 		rte_service_dump_one(f, &rte_services[i], total_cycles, reset);
 	}
 
@@ -777,7 +789,7 @@ int32_t rte_service_dump(FILE *f, uint32_t id)
 		if (lcore_config[i].core_role != ROLE_SERVICE)
 			continue;
 
-		uint32_t reset = 1;
+		uint32_t reset = 0;
 		service_dump_calls_per_lcore(f, i, reset);
 	}
 
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 744de04..3fa1e13 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -217,6 +217,7 @@ EXPERIMENTAL {
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
 	rte_service_attr_get;
+	rte_service_attr_reset_all;
 	rte_service_component_register;
 	rte_service_component_unregister;
 	rte_service_component_runstate_set;
diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c
index a0768c9..e510643 100644
--- a/test/test/test_service_cores.c
+++ b/test/test/test_service_cores.c
@@ -242,6 +242,9 @@ service_name(void)
 static int
 service_attr_get(void)
 {
+	/* ensure all services unregistered so cycle counts are zero */
+	unregister_all();
+
 	struct rte_service_spec service;
 	memset(&service, 0, sizeof(struct rte_service_spec));
 	service.callback = dummy_cb;
@@ -295,6 +298,14 @@ service_attr_get(void)
 
 	rte_service_lcore_stop(slcore_id);
 
+	TEST_ASSERT_EQUAL(0, rte_service_attr_reset_all(id),
+			"Valid attr_reset_all() return success");
+
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_id, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(0, attr_value,
+			"attr_get() call didn't set correct cycles (zero)");
+
 	return unregister_all();
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 3/3] service: add attribute for number of invocations

[Harry van Haaren]

This commit adds a new attribute to the service cores attributes
API, which allows the application to retrieve the number of times
that a service-core called the service to perform its action.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v2:
- Fix typo in "invocations"
---
 lib/librte_eal/common/include/rte_service.h |  5 +++++
 lib/librte_eal/common/rte_service.c         |  3 +++
 test/test/test_service_cores.c              | 16 ++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index 460dbbf..85e964b 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -400,6 +400,11 @@ int32_t rte_service_dump(FILE *f, uint32_t id);
 #define RTE_SERVICE_ATTR_CYCLES 0
 
 /**
+ * Returns the count of invocations of this service function
+ */
+#define RTE_SERVICE_ATTR_CALL_COUNT 1
+
+/**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice
  *
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 1b54b7e..5f97d85 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -695,6 +695,9 @@ rte_service_attr_get(uint32_t id, uint32_t attr_id, uint32_t *attr_value)
 	case RTE_SERVICE_ATTR_CYCLES:
 		*attr_value = s->cycles_spent;
 		return 0;
+	case RTE_SERVICE_ATTR_CALL_COUNT:
+		*attr_value = s->calls;
+		return 0;
 	default:
 		return -EINVAL;
 	}
diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c
index e510643..86b4073 100644
--- a/test/test/test_service_cores.c
+++ b/test/test/test_service_cores.c
@@ -278,6 +278,12 @@ service_attr_get(void)
 			"Valid attr_get() call didn't return success");
 	TEST_ASSERT_EQUAL(0, attr_value,
 			"attr_get() call didn't set correct cycles (zero)");
+	/* check correct call count */
+	const int attr_calls = RTE_SERVICE_ATTR_CALL_COUNT;
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_calls, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(0, attr_value,
+			"attr_get() call didn't get call count (zero)");
 
 	/* Call service to increment cycle count */
 	TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id),
@@ -298,6 +304,11 @@ service_attr_get(void)
 
 	rte_service_lcore_stop(slcore_id);
 
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_calls, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(1, (attr_value > 0),
+			"attr_get() call didn't get call count (zero)");
+
 	TEST_ASSERT_EQUAL(0, rte_service_attr_reset_all(id),
 			"Valid attr_reset_all() return success");
 
@@ -305,6 +316,11 @@ service_attr_get(void)
 			"Valid attr_get() call didn't return success");
 	TEST_ASSERT_EQUAL(0, attr_value,
 			"attr_get() call didn't set correct cycles (zero)");
+	/* ensure call count > zero */
+	TEST_ASSERT_EQUAL(0, rte_service_attr_get(id, attr_calls, &attr_value),
+			"Valid attr_get() call didn't return success");
+	TEST_ASSERT_EQUAL(0, (attr_value > 0),
+			"attr_get() call didn't get call count (zero)");
 
 	return unregister_all();
 }
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 1/3] service: add attribute get function

[Thomas Monjalon]

12/01/2018 11:27, Harry van Haaren:
> This commit adds a new function to the service API to allow
> the application to retrieve items about each individual service
> in the system. A unit test checks the return values of a variety
> of invalid and valid calls.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

Series applied, thanks