[dpdk-dev] [dpdk-dev, v1, 2/5] eventdev: add crypto adapter implementation
Jerin Jacob
jerin.jacob at caviumnetworks.com
Fri Apr 20 15:14:46 CEST 2018
-----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?
>
> Akhil
>
More information about the dev
mailing list