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

[Gujjar, Abhinandan S]

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> Sent: Monday, April 23, 2018 12:09 PM
> To: Jerin Jacob <jerin.jacob at caviumnetworks.com>
> Cc: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>;
> hemant.agrawal at nxp.com; dev at dpdk.org; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>; Doherty, Declan
> <declan.doherty at intel.com>; Vangati, Narender
> <narender.vangati at intel.com>; Rao, Nikhil <nikhil.rao at intel.com>; Eads, Gage
> <gage.eads at intel.com>
> Subject: Re: [dpdk-dev] [dpdk-dev, v1, 2/5] eventdev: add crypto adapter
> implementation
> 
> On 4/20/2018 6:44 PM, Jerin Jacob wrote:
> > -----Original Message-----
> >> Date: Fri, 20 Apr 2018 17:04:36 +0530
> >> From: Akhil Goyal <akhil.goyal at nxp.com>
> >> To: "Gujjar, Abhinandan S" <abhinandan.gujjar at intel.com>,
> >> "jerin.jacob at caviumnetworks.com" <jerin.jacob at caviumnetworks.com>,
> >> "hemant.agrawal at nxp.com" <hemant.agrawal at nxp.com>, "dev at dpdk.org"
> >>  <dev at dpdk.org>
> >> CC: "De Lara Guarch, Pablo" <pablo.de.lara.guarch at intel.com>,
> >> "Doherty,  Declan" <declan.doherty at intel.com>, "Vangati, Narender"
> >>  <narender.vangati at intel.com>, "Rao, Nikhil" <nikhil.rao at intel.com>,
> >> "Eads,  Gage" <gage.eads at intel.com>
> >> Subject: Re: [dpdk-dev, v1, 2/5] eventdev: add crypto adapter
> >> implementation
> >> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0)
> >> Gecko/20100101
> >>  Thunderbird/45.8.0
> >>
> >> Hi Abhinandan/ Jerin,
> >> On 4/18/2018 11:51 AM, Gujjar, Abhinandan S wrote:
> >>> Hi Akhil,
> >>>
> >>> Please find the comments inline.
> >>>
> >>>> -----Original Message-----
> >>>> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> >>>> Sent: Tuesday, April 17, 2018 7:48 PM
> >>>> To: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>;
> >>>> jerin.jacob at caviumnetworks.com; hemant.agrawal at nxp.com;
> >>>> dev at dpdk.org
> >>>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>;
> >>>> Doherty, Declan <declan.doherty at intel.com>; Vangati, Narender
> >>>> <narender.vangati at intel.com>; Rao, Nikhil <nikhil.rao at intel.com>;
> >>>> Eads, Gage <gage.eads at 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 at intel.com>
> >>>>> Signed-off-by: Nikhil Rao <nikhil.rao at intel.com>
> >>>>> Signed-off-by: Gage Eads <gage.eads at 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.
> >>>
> >>
> >> I think the intent of that discussion was misunderstood from our end.
> >> But this is not going to work for hardware devices.
> >>
> >> Because in case of hardware implementation, the scheduling is done in
> >> hardware and hardware cannot call the get API to get the event
> >> information then map to event device. Actually the scheduling has
> >> happened before the crypto_op is dequeued from the event port. So
> >> there is no point of set/get private data in our case.
> >>
> >> We need to map the crypto queues to the event queue_ids at the time
> >> of queue_pair add API. In hardware scheduler, we map n(may be 1-8)
> >> crypto queues to m event queues(<= n). We can assign multiple
> >> sessions to any crypto queue pair, and after the crypto op is
> >> received by event queue, they are appropriately scheduled by hardware to
> event ports.
> >>
> >> Session based mapping to event queue cannot be supported. Our design
> >> is same as that of eth rx adapter.
> >
> > Crypto queue pair to eventdev queue mapping should be supported. But
> > That's a limited set. meaning if an application needs millions of
> > IPSec SA sessions then we can not map it. So, IMO, If an HW/SW can not
> > support session based mapping then it needs to be exposed/abstracted
> through capabilities.
> >
> > crypto qp to event queue mapping will be supported in all adapter
> > implementation.
> >
> > Does that sounds OK?
> >
> 
> Yes, that will be fine for us.
Ok. I will include this as caps in v2 patch.

Regards
Abhinandan

> 
> Thanks,
> Akhil