Re: [dpdk-dev] [dpdk-dev, v1, 2/5] eventdev: add crypto adapter implementation

["Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>]

Hi Akhil,

Please find the comments inline.

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Tuesday, April 17, 2018 7:48 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> jerin.jacob@caviumnetworks.com; hemant.agrawal@nxp.com; dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>; Eads, Gage
> <gage.eads@intel.com>
> Subject: Re: [dpdk-dev, v1, 2/5] eventdev: add crypto adapter implementation
> 
> Hi Abhinandan,
> 
> I have not reviewed the patch completely. But I have below query for further
> review.
> On 4/4/2018 12:26 PM, Abhinandan Gujjar wrote:
> > Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> > Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> > Signed-off-by: Gage Eads <gage.eads@intel.com>
> > ---
> 
> [..snip..]
> > +
> > +int __rte_experimental
> > +rte_event_crypto_adapter_queue_pair_add(uint8_t id,
> > +					uint8_t cdev_id,
> > +					int32_t queue_pair_id)
> > +{
> > +	struct rte_event_crypto_adapter *adapter;
> > +	struct rte_eventdev *dev;
> > +	struct crypto_device_info *dev_info;
> > +	uint32_t cap;
> > +	int ret;
> > +
> > +	RTE_EVENT_CRYPTO_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> > +
> > +	if (!rte_cryptodev_pmd_is_valid_dev(cdev_id)) {
> > +		RTE_EDEV_LOG_ERR("Invalid dev_id=%" PRIu8, cdev_id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	adapter = eca_id_to_adapter(id);
> > +	if (adapter == NULL)
> > +		return -EINVAL;
> > +
> > +	dev = &rte_eventdevs[adapter->eventdev_id];
> > +	ret = rte_event_crypto_adapter_caps_get(adapter->eventdev_id,
> > +						cdev_id,
> > +						&cap);
> > +	if (ret) {
> > +		RTE_EDEV_LOG_ERR("Failed to get adapter caps dev %" PRIu8
> > +			"cdev %" PRIu8, id, cdev_id);
> > +		return ret;
> > +	}
> > +
> > +	dev_info = &adapter->cdevs[cdev_id];
> > +
> > +	if (queue_pair_id != -1 &&
> > +	    (uint16_t)queue_pair_id >= dev_info->dev->data->nb_queue_pairs) {
> > +		RTE_EDEV_LOG_ERR("Invalid queue_pair_id %" PRIu16,
> > +				 (uint16_t)queue_pair_id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (cap & RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT) {
> > +		RTE_FUNC_PTR_OR_ERR_RET(
> > +			*dev->dev_ops->crypto_adapter_queue_pair_add,
> > +			-ENOTSUP);
> > +		if (dev_info->qpairs == NULL) {
> > +			dev_info->qpairs =
> > +			    rte_zmalloc_socket(adapter->mem_name,
> > +					dev_info->dev->data->nb_queue_pairs
> *
> > +					sizeof(struct crypto_queue_pair_info),
> > +					0, adapter->socket_id);
> > +			if (dev_info->qpairs == NULL)
> > +				return -ENOMEM;
> > +		}
> > +
> > +		ret = (*dev->dev_ops->crypto_adapter_queue_pair_add)(dev,
> > +				dev_info->dev,
> > +				queue_pair_id);
> 
> crypto_adapter_queue_pair_add is supposed to attach a queue
> (queue_pair_id) of cryptodev(dev_info->dev) to event device (dev).
> But how will the underlying implementation attach it to event device without
> knowing the eventdev queue_id. This information was coming in the RFC
> patches with the parameter (rte_event_crypto_queue_pair_conf).
> Why is this removed and if removed how will the driver attach the queue.
> I can see that rte_event is passed in the session private data but how can we
> attach the crypto queue with event dev queue?

Yes, this was present in the first version of the RFC which is similar to eth rx adapter.
After couple of discussions, thread http://dpdk.org/dev/patchwork/patch/31752/),
it was changed. In eth rx adapter, eth queues are mapped to eventdev, whereas in crypto
adapter the sessions are mapped to eventdev. Since event info is present along with the
session, the get API has to be called in respective API to get the event information and
then map to eventdev.

Regards
Abhinandan
> 
> Regards,
> Akhil