Re: [dpdk-dev] [PATCH v2 3/6] eventdev: implement the northbound APIs

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Dec 06, 2016 at 09:22:17AM +0530, Jerin Jacob wrote:
> This patch implements northbound eventdev API interface using
> southbond driver interface
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
>  config/common_base                           |    6 +
>  lib/Makefile                                 |    1 +
>  lib/librte_eal/common/include/rte_log.h      |    1 +
>  lib/librte_eventdev/Makefile                 |   57 ++
>  lib/librte_eventdev/rte_eventdev.c           | 1001 ++++++++++++++++++++++++++
>  lib/librte_eventdev/rte_eventdev.h           |  108 ++-
>  lib/librte_eventdev/rte_eventdev_pmd.h       |  109 +++
>  lib/librte_eventdev/rte_eventdev_version.map |   33 +
>  mk/rte.app.mk                                |    1 +
>  9 files changed, 1311 insertions(+), 6 deletions(-)
>  create mode 100644 lib/librte_eventdev/Makefile
>  create mode 100644 lib/librte_eventdev/rte_eventdev.c
>  create mode 100644 lib/librte_eventdev/rte_eventdev_version.map
> 
<snip>
> +
> +static inline int
> +rte_event_dev_queue_config(struct rte_eventdev *dev, uint8_t nb_queues)
> +{
> +	uint8_t old_nb_queues = dev->data->nb_queues;
> +	void **queues;
> +	uint8_t *queues_prio;
> +	unsigned int i;
> +
> +	RTE_EDEV_LOG_DEBUG("Setup %d queues on device %u", nb_queues,
> +			 dev->data->dev_id);
> +
> +	/* First time configuration */
> +	if (dev->data->queues == NULL && nb_queues != 0) {
> +		dev->data->queues = rte_zmalloc_socket("eventdev->data->queues",
> +				sizeof(dev->data->queues[0]) * nb_queues,
> +				RTE_CACHE_LINE_SIZE, dev->data->socket_id);
> +		if (dev->data->queues == NULL) {
> +			dev->data->nb_queues = 0;
> +			RTE_EDEV_LOG_ERR("failed to get memory for queue meta,"
> +					"nb_queues %u", nb_queues);
> +			return -(ENOMEM);
> +		}
> +		/* Allocate memory to store queue priority */
> +		dev->data->queues_prio = rte_zmalloc_socket(
> +				"eventdev->data->queues_prio",
> +				sizeof(dev->data->queues_prio[0]) * nb_queues,
> +				RTE_CACHE_LINE_SIZE, dev->data->socket_id);
> +		if (dev->data->queues_prio == NULL) {
> +			dev->data->nb_queues = 0;
> +			RTE_EDEV_LOG_ERR("failed to get mem for queue priority,"
> +					"nb_queues %u", nb_queues);
> +			return -(ENOMEM);
> +		}
> +
> +	} else if (dev->data->queues != NULL && nb_queues != 0) {/* re-config */
> +		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_release, -ENOTSUP);
> +
> +		queues = dev->data->queues;
> +		for (i = nb_queues; i < old_nb_queues; i++)
> +			(*dev->dev_ops->queue_release)(queues[i]);
> +
> +		queues = rte_realloc(queues, sizeof(queues[0]) * nb_queues,
> +				RTE_CACHE_LINE_SIZE);
> +		if (queues == NULL) {
> +			RTE_EDEV_LOG_ERR("failed to realloc queue meta data,"
> +						" nb_queues %u", nb_queues);
> +			return -(ENOMEM);
> +		}
> +		dev->data->queues = queues;
> +
> +		/* Re allocate memory to store queue priority */
> +		queues_prio = dev->data->queues_prio;
> +		queues_prio = rte_realloc(queues_prio,
> +				sizeof(queues_prio[0]) * nb_queues,
> +				RTE_CACHE_LINE_SIZE);
> +		if (queues_prio == NULL) {
> +			RTE_EDEV_LOG_ERR("failed to realloc queue priority,"
> +						" nb_queues %u", nb_queues);
> +			return -(ENOMEM);
> +		}
> +		dev->data->queues_prio = queues_prio;
> +
> +		if (nb_queues > old_nb_queues) {
> +			uint8_t new_qs = nb_queues - old_nb_queues;
> +
> +			memset(queues + old_nb_queues, 0,
> +				sizeof(queues[0]) * new_qs);
> +			memset(queues_prio + old_nb_queues, 0,
> +				sizeof(queues_prio[0]) * new_qs);
> +		}
> +	} else if (dev->data->queues != NULL && nb_queues == 0) {
> +		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_release, -ENOTSUP);
> +
> +		queues = dev->data->queues;
> +		for (i = nb_queues; i < old_nb_queues; i++)
> +			(*dev->dev_ops->queue_release)(queues[i]);
> +	}
> +
> +	dev->data->nb_queues = nb_queues;
> +	return 0;
> +}
> +
While the ports array makes sense to have available at the top level of
the API and allocated from rte_eventdev.c, I'm not seeing what the value
of having the queues allocated at that level is. The only time the queue
array is indexed by eventdev layer is when releasing a queue. Therefore,
I suggest just saving the number of queues for sanity checking and let
the queue array allocation and freeing be handled entirely in the
drivers themselves.

/Bruce