Re: [dpdk-dev] [PATCH 1/8] app/eventdev: add ethernet device producer option

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Wed, 18 Oct 2017 18:09:01 +0530
> From: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> To: bruce.richardson@intel.com, harry.van.haaren@intel.com,
>  gage.eads@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>  nikhil.rao@intel.com, santosh.shukla@caviumnetworks.com,
>  jerin.jacob@caviumnetworks.com
> Cc: dev@dpdk.org, Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Subject: [PATCH 1/8] app/eventdev: add ethernet device producer option
> X-Mailer: git-send-email 2.7.4
> 
> Add command line option --prod_type_ethdev to specify that the events
> are generated by ethernet device.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> ---
>  app/test-eventdev/evt_options.c      | 11 +++++++++++
>  app/test-eventdev/evt_options.h      | 29 +++++++++++++++++++++++++++++
>  app/test-eventdev/test_perf_common.c | 34 ++++++++++++++++++++--------------
>  3 files changed, 60 insertions(+), 14 deletions(-)
> 
> @@ -272,6 +282,7 @@ evt_opts_parse_long(int opt_idx, struct evt_options *opt)
>  		{ EVT_SCHED_TYPE_LIST, evt_parse_sched_type_list},
>  		{ EVT_FWD_LATENCY, evt_parse_fwd_latency},
>  		{ EVT_QUEUE_PRIORITY, evt_parse_queue_priority},
> +		{ EVT_PROD_ETHDEV, evt_parse_prod_type},

There could be more producer type like timer or crypto in future. So,
IMO, the name can be like "evt_parse_eth_prod_type" or something like
similar.

>  	};
>  
>  	for (i = 0; i < RTE_DIM(parsermap); i++) {
> diff --git a/app/test-eventdev/evt_options.h b/app/test-eventdev/evt_options.h
> index a9a9125..a6607e1 100644
> --- a/app/test-eventdev/evt_options.h
> +++ b/app/test-eventdev/evt_options.h
> @@ -58,8 +58,16 @@
>  #define EVT_SCHED_TYPE_LIST      ("stlist")
>  #define EVT_FWD_LATENCY          ("fwd_latency")
>  #define EVT_QUEUE_PRIORITY       ("queue_priority")
> +#define EVT_PROD_ETHDEV          ("prod_type_ethdev")
>  #define EVT_HELP                 ("help")
>  
> +enum evt_prod_type {
> +	EVT_PROD_TYPE_NONE,
> +	EVT_PROD_TYPE_SYNT,          /* Producer type Synthetic i.e. CPU. */
> +	EVT_PROD_TYPE_ETH_RX_ADPTR,  /* Producer type Eth Rx Adapter. */
> +	EVT_PROD_TYPE_MAX,
> +};
> +
>  struct evt_options {
>  #define EVT_TEST_NAME_MAX_LEN     32
>  	char test_name[EVT_TEST_NAME_MAX_LEN];
> @@ -76,6 +84,7 @@ struct evt_options {
>  	uint8_t dev_id;
>  	uint32_t fwd_latency:1;
>  	uint32_t q_priority:1;
> +	enum evt_prod_type prod_type;
>  };
>  
>  void evt_options_default(struct evt_options *opt);
> @@ -266,4 +275,24 @@ evt_dump_sched_type_list(struct evt_options *opt)
>  	evt_dump_end;
>  }
>  
> +#define EVT_PROD_MAX_NAME_LEN 50
> +static inline void
> +evt_dump_producer_type(struct evt_options *opt)
> +{
> +	char name[EVT_PROD_MAX_NAME_LEN];
> +
> +	switch (opt->prod_type) {
> +		default:
> +		case EVT_PROD_TYPE_SYNT:
> +			snprintf(name, EVT_PROD_MAX_NAME_LEN,
> +					"Synthetic producer lcores");
> +			break;
> +		case EVT_PROD_TYPE_ETH_RX_ADPTR:
> +			snprintf(name, EVT_PROD_MAX_NAME_LEN,
> +					"Rx Adapter producers");

How about the name as "Ethdev Rx Adapter producers"

> +			break;
> +	}
> +	evt_dump("prod_type", "%s", name);
> +}
> +

Other than that, it looks good to me

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

> -- 
> 2.7.4
>