From mboxrd@z Thu Jan  1 00:00:00 1970
From: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
Subject: Re: [PATCH v7 5/7] test: add event timer adapter
	auto-test
Date: Wed, 14 Mar 2018 19:01:04 +0530
Message-ID: <20180314133103.GB27234@ltp-pvn>
References: <1515630074-29020-1-git-send-email-erik.g.carrillo@intel.com>
 <1520546046-6973-1-git-send-email-erik.g.carrillo@intel.com>
 <1520546046-6973-6-git-send-email-erik.g.carrillo@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org
To: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>,
 jerin.jacob@caviumnetworks.com, nipun.gupta@nxp.com, hemant.agrawal@nxp.com
Return-path: <dev-bounces@dpdk.org>
Received: from NAM03-DM3-obe.outbound.protection.outlook.com
 (mail-dm3nam03on0048.outbound.protection.outlook.com [104.47.41.48])
 by dpdk.org (Postfix) with ESMTP id CE36C5B18
 for <dev@dpdk.org>; Wed, 14 Mar 2018 14:31:15 +0100 (CET)
Content-Disposition: inline
In-Reply-To: <1520546046-6973-6-git-send-email-erik.g.carrillo@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Gabriel,

Please make sure that the unit tests are generic, I could see that some places
it is verifying whether event port is used or service cores are used, but
doesn't verify if actually event port/service core are needed i.e.
INTERNAL_PORT capability.

On Thu, Mar 08, 2018 at 03:54:04PM -0600, Erik Gabriel Carrillo wrote:
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---
>  test/test/Makefile                   |    1 +
>  test/test/test_event_timer_adapter.c | 1234 ++++++++++++++++++++++++++++++++++
>  2 files changed, 1235 insertions(+)
>  create mode 100644 test/test/test_event_timer_adapter.c
>
<snip>
> +
> +/* This port conf callback is used by the max adapter instance creation test.
> + * Because that test may be limited by the number of ports available in the
> + * event device, this callback allocates just one port and returns it each
> + * time a port is requested.
> + */
> +static int
> +test_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
> +		  void *conf_arg)
> +{
> +	struct rte_eventdev *dev;
> +	struct rte_event_dev_config dev_conf;
> +	struct rte_event_port_conf *port_conf, def_port_conf = {0};
> +	int started;
> +	static int port_allocated;
> +	static uint8_t port_id;
> +	uint8_t dev_id;
> +	int ret;
> +
> +	if (port_allocated) {
> +		*event_port_id = port_id;
> +		return 0;
> +	}
> +
> +	RTE_SET_USED(id);
> +
> +	dev = &rte_eventdevs[event_dev_id];

I don't think this is the correct way of accessing event dev information i.e.
accessing the global rte_eventdevs structure from a application.

> +	dev_id = dev->data->dev_id;
> +	dev_conf = dev->data->dev_conf;
> +
> +	started = dev->data->dev_started;
> +	if (started)
> +		rte_event_dev_stop(dev_id);
> +
> +	port_id = dev_conf.nb_event_ports;
> +	dev_conf.nb_event_ports += 1;
> +	ret = rte_event_dev_configure(dev_id, &dev_conf);
> +	if (ret < 0) {
> +		if (started)
> +			rte_event_dev_start(dev_id);
Shouldn't this be !started ?. The same pattern repeats a few places.

> +
> +		return ret;
> +	}
> +
> +	if (conf_arg != NULL)
> +		port_conf = conf_arg;
> +	else {
> +		port_conf = &def_port_conf;
> +		ret = rte_event_port_default_conf_get(dev_id, port_id,
> +						      port_conf);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = rte_event_port_setup(dev_id, port_id, port_conf);
> +	if (ret < 0)
> +		return ret;
> +
> +	*event_port_id = port_id;
> +
> +	if (started)
> +		rte_event_dev_start(dev_id);
> +
> +	port_allocated = 1;
> +
> +	return 0;
> +}
<snip>

Thanks,
Pavan.