[dpdk-dev,v7,08/17] test: change params to distributor autotest

Message ID 1487647073-129064-9-git-send-email-david.hunt@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Hunt, David Feb. 21, 2017, 3:17 a.m. UTC
  In the next few patches, we'll want to test old and new API,
so here we're allowing different parameters to be passed to
the tests, instead of just a distributor struct.

Signed-off-by: David Hunt <david.hunt@intel.com>
---
 app/test/test_distributor.c | 64 +++++++++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 22 deletions(-)
  

Comments

Bruce Richardson Feb. 24, 2017, 2:14 p.m. UTC | #1
On Tue, Feb 21, 2017 at 03:17:44AM +0000, David Hunt wrote:
> In the next few patches, we'll want to test old and new API,
> so here we're allowing different parameters to be passed to
> the tests, instead of just a distributor struct.
> 
> Signed-off-by: David Hunt <david.hunt@intel.com>
> ---
>  app/test/test_distributor.c | 64 +++++++++++++++++++++++++++++----------------
>  1 file changed, 42 insertions(+), 22 deletions(-)
> 
> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
> index 6a4e20b..fdfa793 100644
> --- a/app/test/test_distributor.c
> +++ b/app/test/test_distributor.c
> @@ -45,6 +45,13 @@
>  #define BURST 32
>  #define BIG_BATCH 1024
>  
> +struct worker_params {
> +	char name[64];
> +	struct rte_distributor_v20 *dist;
> +};
> +
> +struct worker_params worker_params;
> +
>  /* statics - all zero-initialized by default */
>  static volatile int quit;      /**< general quit variable for all threads */
>  static volatile int zero_quit; /**< var for when we just want thr0 to quit*/
> @@ -81,7 +88,8 @@ static int
>  handle_work(void *arg)
>  {
>  	struct rte_mbuf *pkt = NULL;
> -	struct rte_distributor_v20 *d = arg;
> +	struct worker_params *wp = arg;
> +	struct rte_distributor_v20 *d = wp->dist;

The cover letter indicated that using new vs old API was just a matter
of passing a different parameter to create. I therefore would not expect
to see references to v20 APIs or structures in any code outside the lib
itself. Am I missing something?

/Bruce
  
Hunt, David March 1, 2017, 10:06 a.m. UTC | #2
On 24/2/2017 2:14 PM, Bruce Richardson wrote:
> On Tue, Feb 21, 2017 at 03:17:44AM +0000, David Hunt wrote:
>> In the next few patches, we'll want to test old and new API,
>> so here we're allowing different parameters to be passed to
>> the tests, instead of just a distributor struct.
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
>> ---
>>   app/test/test_distributor.c | 64 +++++++++++++++++++++++++++++----------------
>>   1 file changed, 42 insertions(+), 22 deletions(-)
>>
>> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
>> index 6a4e20b..fdfa793 100644
>> --- a/app/test/test_distributor.c
>> +++ b/app/test/test_distributor.c
>> @@ -45,6 +45,13 @@
>>   #define BURST 32
>>   #define BIG_BATCH 1024
>>   
>> +struct worker_params {
>> +	char name[64];
>> +	struct rte_distributor_v20 *dist;
>> +};
>> +
>> +struct worker_params worker_params;
>> +
>>   /* statics - all zero-initialized by default */
>>   static volatile int quit;      /**< general quit variable for all threads */
>>   static volatile int zero_quit; /**< var for when we just want thr0 to quit*/
>> @@ -81,7 +88,8 @@ static int
>>   handle_work(void *arg)
>>   {
>>   	struct rte_mbuf *pkt = NULL;
>> -	struct rte_distributor_v20 *d = arg;
>> +	struct worker_params *wp = arg;
>> +	struct rte_distributor_v20 *d = wp->dist;
> The cover letter indicated that using new vs old API was just a matter
> of passing a different parameter to create. I therefore would not expect
> to see references to v20 APIs or structures in any code outside the lib
> itself. Am I missing something?
>
> /Bruce
>

Patchset has now been reworked so the api switches over and apps 
migrated to new SPI in one step, so _v20 never exposed.

Dave.
  

Patch

diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
index 6a4e20b..fdfa793 100644
--- a/app/test/test_distributor.c
+++ b/app/test/test_distributor.c
@@ -45,6 +45,13 @@ 
 #define BURST 32
 #define BIG_BATCH 1024
 
+struct worker_params {
+	char name[64];
+	struct rte_distributor_v20 *dist;
+};
+
+struct worker_params worker_params;
+
 /* statics - all zero-initialized by default */
 static volatile int quit;      /**< general quit variable for all threads */
 static volatile int zero_quit; /**< var for when we just want thr0 to quit*/
@@ -81,7 +88,8 @@  static int
 handle_work(void *arg)
 {
 	struct rte_mbuf *pkt = NULL;
-	struct rte_distributor_v20 *d = arg;
+	struct worker_params *wp = arg;
+	struct rte_distributor_v20 *d = wp->dist;
 	unsigned count = 0;
 	unsigned id = __sync_fetch_and_add(&worker_idx, 1);
 
@@ -107,8 +115,9 @@  handle_work(void *arg)
  *   not necessarily in the same order (as different flows).
  */
 static int
-sanity_test(struct rte_distributor_v20 *d, struct rte_mempool *p)
+sanity_test(struct worker_params *wp, struct rte_mempool *p)
 {
+	struct rte_distributor_v20 *d = wp->dist;
 	struct rte_mbuf *bufs[BURST];
 	unsigned i;
 
@@ -249,7 +258,8 @@  static int
 handle_work_with_free_mbufs(void *arg)
 {
 	struct rte_mbuf *pkt = NULL;
-	struct rte_distributor_v20 *d = arg;
+	struct worker_params *wp = arg;
+	struct rte_distributor_v20 *d = wp->dist;
 	unsigned count = 0;
 	unsigned id = __sync_fetch_and_add(&worker_idx, 1);
 
@@ -270,9 +280,9 @@  handle_work_with_free_mbufs(void *arg)
  * library.
  */
 static int
-sanity_test_with_mbuf_alloc(struct rte_distributor_v20 *d,
-		struct rte_mempool *p)
+sanity_test_with_mbuf_alloc(struct worker_params *wp, struct rte_mempool *p)
 {
+	struct rte_distributor_v20 *d = wp->dist;
 	unsigned i;
 	struct rte_mbuf *bufs[BURST];
 
@@ -306,7 +316,8 @@  static int
 handle_work_for_shutdown_test(void *arg)
 {
 	struct rte_mbuf *pkt = NULL;
-	struct rte_distributor_v20 *d = arg;
+	struct worker_params *wp = arg;
+	struct rte_distributor_v20 *d = wp->dist;
 	unsigned count = 0;
 	const unsigned id = __sync_fetch_and_add(&worker_idx, 1);
 
@@ -345,9 +356,10 @@  handle_work_for_shutdown_test(void *arg)
  * library.
  */
 static int
-sanity_test_with_worker_shutdown(struct rte_distributor_v20 *d,
+sanity_test_with_worker_shutdown(struct worker_params *wp,
 		struct rte_mempool *p)
 {
+	struct rte_distributor_v20 *d = wp->dist;
 	struct rte_mbuf *bufs[BURST];
 	unsigned i;
 
@@ -402,9 +414,10 @@  sanity_test_with_worker_shutdown(struct rte_distributor_v20 *d,
  * one worker shuts down..
  */
 static int
-test_flush_with_worker_shutdown(struct rte_distributor_v20 *d,
+test_flush_with_worker_shutdown(struct worker_params *wp,
 		struct rte_mempool *p)
 {
+	struct rte_distributor_v20 *d = wp->dist;
 	struct rte_mbuf *bufs[BURST];
 	unsigned i;
 
@@ -481,8 +494,9 @@  int test_error_distributor_create_numworkers(void)
 
 /* Useful function which ensures that all worker functions terminate */
 static void
-quit_workers(struct rte_distributor_v20 *d, struct rte_mempool *p)
+quit_workers(struct worker_params *wp, struct rte_mempool *p)
 {
+	struct rte_distributor_v20 *d = wp->dist;
 	const unsigned num_workers = rte_lcore_count() - 1;
 	unsigned i;
 	struct rte_mbuf *bufs[RTE_MAX_LCORE];
@@ -538,28 +552,34 @@  test_distributor(void)
 		}
 	}
 
-	rte_eal_mp_remote_launch(handle_work, d, SKIP_MASTER);
-	if (sanity_test(d, p) < 0)
+	worker_params.dist = d;
+	sprintf(worker_params.name, "single");
+
+	rte_eal_mp_remote_launch(handle_work, &worker_params, SKIP_MASTER);
+	if (sanity_test(&worker_params, p) < 0)
 		goto err;
-	quit_workers(d, p);
+	quit_workers(&worker_params, p);
 
-	rte_eal_mp_remote_launch(handle_work_with_free_mbufs, d, SKIP_MASTER);
-	if (sanity_test_with_mbuf_alloc(d, p) < 0)
+	rte_eal_mp_remote_launch(handle_work_with_free_mbufs, &worker_params,
+				SKIP_MASTER);
+	if (sanity_test_with_mbuf_alloc(&worker_params, p) < 0)
 		goto err;
-	quit_workers(d, p);
+	quit_workers(&worker_params, p);
 
 	if (rte_lcore_count() > 2) {
-		rte_eal_mp_remote_launch(handle_work_for_shutdown_test, d,
+		rte_eal_mp_remote_launch(handle_work_for_shutdown_test,
+				&worker_params,
 				SKIP_MASTER);
-		if (sanity_test_with_worker_shutdown(d, p) < 0)
+		if (sanity_test_with_worker_shutdown(&worker_params, p) < 0)
 			goto err;
-		quit_workers(d, p);
+		quit_workers(&worker_params, p);
 
-		rte_eal_mp_remote_launch(handle_work_for_shutdown_test, d,
+		rte_eal_mp_remote_launch(handle_work_for_shutdown_test,
+				&worker_params,
 				SKIP_MASTER);
-		if (test_flush_with_worker_shutdown(d, p) < 0)
+		if (test_flush_with_worker_shutdown(&worker_params, p) < 0)
 			goto err;
-		quit_workers(d, p);
+		quit_workers(&worker_params, p);
 
 	} else {
 		printf("Not enough cores to run tests for worker shutdown\n");
@@ -574,7 +594,7 @@  test_distributor(void)
 	return 0;
 
 err:
-	quit_workers(d, p);
+	quit_workers(&worker_params, p);
 	return -1;
 }