DPDK patches and discussions
 help / color / mirror / Atom feed
From: Akhil Goyal <akhil.goyal@nxp.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>,
	"Doherty, Declan" <declan.doherty@intel.com>
Cc: Abhinandan Gujjar <abhinandan.gujjar@intel.com>, <dev@dpdk.org>,
	<narender.vangati@intel.com>, Nikhil Rao <nikhil.rao@intel.com>,
	Gage Eads <gage.eads@intel.com>, <hemant.agrawal@nxp.com>,
	<nidadavolu.murthy@cavium.com>, <nithin.dabilpuram@cavium.com>,
	<narayanaprasad.athreya@cavium.com>
Subject: Re: [dpdk-dev] [RFC] eventdev: add crypto adapter API header
Date: Wed, 13 Dec 2017 19:52:24 +0530	[thread overview]
Message-ID: <b28f2e29-a4e4-0102-f609-a948c68fb7fb@nxp.com> (raw)
In-Reply-To: <20171213112606.GA5166@jerin>

Hi Jerin,
On 12/13/2017 4:56 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Wed, 13 Dec 2017 11:03:06 +0000
>> From: "Doherty, Declan" <declan.doherty@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Abhinandan Gujjar
>>   <abhinandan.gujjar@intel.com>
>> CC: dev@dpdk.org, narender.vangati@intel.com, Nikhil Rao
>>   <nikhil.rao@intel.com>, Gage Eads <gage.eads@intel.com>,
>>   hemant.agrawal@nxp.com, nidadavolu.murthy@cavium.com,
>>   nithin.dabilpuram@cavium.com, narayanaprasad.athreya@cavium.com
>> Subject: Re: [RFC] eventdev: add crypto adapter API header
>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>>   Thunderbird/52.5.0
>>
>> On 29/11/2017 11:41 AM, Jerin Jacob wrote:
>>> -----Original Message-----
>>
>> ...
>>
>>>
>>> Adding Declan and Hemant.
>>>> IMO, RTE_EVENT_CRYPTO_ENQ_MULTI_EVENTQ may not be very useful
>>> from application perceptive as the scope is very limited.
>>> In real world use cases, we will be attaching destination event queue information
>>> to the session, not to the queue pair.
>>>
>>>
>>> IMO, RTE_EVENT_CRYPTO_ENQ_MBUF_MULTI_EVENTQ scheme may not very
>>> convenient for application writers as
>>> # it relies on mbuf private area memory. So it has additional memory alloc/free
>>> requirements.
>>> # additional overhead for application/PMD to write/read the event queue metadata
>>> information per packet.
>>>
>>> Since we already have meta data structure in the crypto
>>> area, How about adding the destination event queue attributes
>>> in the PMD crypto session area and for, _session less_, we can add it
>>> in rte_crypto_op stricture? This will help in:
>>>
>>> # Offloading HW specific meta data generation for event queue attributes
>>> to slow path.
>>> # From the application perspective, most likely the event queue parameters
>>> will be different for each session not per packet nor per event queue
>>> pair.
>>>
>>
>> Hey Jerin,
> 
> Hey Declan,
> 
>>
>> given my limited experience with eventdev, your proposed approach in general
>> makes sense to me, in that a packet flow having crypto processing done will
>> always be forwarded the same next stage event queue. So storing this state
>> in the crypto session, or crypto op in the case of sessionless ops, seems
>> sensible.
>>
>>> Something like below to share my view. Exact details may be we can work it out.
>>>
>>
>> I terms of your proposed changes below, my main concern would be introducing
>> dependencies on the eventdev library into cryptodev, as with this new crypto
>> adpater you will have a dependency on cryptodev in eventdev.
> 
> I agree with your dependency concern.
> 
>>
>> I think the best approach would be to support opaque metadata in both the
>> crypto op and crypto session structures, as this would allow other uses
>> cases to be supported which aren't specific to eventdev to also store
>> metadata across cryptodev processing.
> 
> Make sense. Just to add, adding a pointer would be overhead. I think, we
> can reserve a few bytes as byte array and then later typecast with
> eventdev api in eventdev library.
> 
> uint8_t eventdev_metadata[SOMEBYTES];
> 
> Thoughts?
I believe only 1 uint64 is sufficient. The metadata that we need here is 
rte_event which is 2 uint64 and the second one is mbuf. Since mbuf is 
already part of rte_crypto_sym_op, we do not need it.

So only a pointer/uint64 is required.

> 
>>
>>> ➜ [master][dpdk.org] $ git diff
>>> diff --git a/lib/librte_cryptodev/rte_crypto.h
>>> b/lib/librte_cryptodev/rte_crypto.h
>>> index 3d672fe7d..b44ef673b 100644
>>> --- a/lib/librte_cryptodev/rte_crypto.h
>>> +++ b/lib/librte_cryptodev/rte_crypto.h
>>> @@ -115,6 +115,9 @@ struct rte_crypto_op {
>>>           uint8_t reserved[5];
>>>           /**< Reserved bytes to fill 64 bits for future additions */
>>>
>>> +#if 0
>>> + Crypto completion event attribute. For _session less_ crypto enqueue operation,
>>> + The will field shall be used by application to post the crypto completion event
>>> + upon the crypto enqueue operation complete.
>>>
>>> + Note: In the case of session based crypto operation, SW based crypto adapter can use
>>> + this memory to store crypto event completion attributes from the PMD
>>> + specific session area.
>>> +
>>> + Note: ev.event_ptr will point to struct rte_crypto_op *op, So
>>> + that core can free the ops memory on event_dequeue().
>>> +#endif
>>> +
>>> +       struct rte_event ev;
>>>
>>>           struct rte_mempool *mempool;
>>>           /**< crypto operation mempool which operation is allocated from
>>>    * */
>>> diff --git a/lib/librte_cryptodev/rte_cryptodev.h
>>> b/lib/librte_cryptodev/rte_cryptodev.h
>>> index dade5548f..540b29e66 100644
>>> --- a/lib/librte_cryptodev/rte_cryptodev.h
>>> +++ b/lib/librte_cryptodev/rte_cryptodev.h
>>> @@ -945,6 +945,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
>>>                           struct rte_crypto_sym_xform *xforms,
>>>                           struct rte_mempool *mempool);
>>>
>>> +#if 0
>>> + Create PMD specific session meta data for the destination event queue
>>> + attribute to post the crypto completion event on crypto work complete.
>>> +#endif
>>> +int
>>> +rte_cryptodev_sym_session_event_init(uint8_t dev_id,
>>> +                       struct rte_cryptodev_sym_session *sess,
>>> +                       struct rte_crypto_sym_xform *xforms,
>>> +                       struct rte_mempool *mempool,
>>> +                       struct rte_event ev);
>>> +
>>>    /**
>>>     * Frees private data for the device id, based on its device type,
>>>     * returning it to its mempool.
>>>
>>>
>>>> + *
>>>> + * The metadata offset is used to configure the location of the
>>>> + * rte_event_crypto_metadata structure within the mbuf's private metadata area.
>>>> + *
>>>> + * When the application sends crypto operations to the adapter,
>>>> + * the crypto queue pair identifier needs to be specified, similarly eventdev
>>>> + * parameters such as the flow id, scheduling type etc are needed by the
>>>> + * adapter when it enqueues mbufs from completed crypto operations to eventdev.
>>>> + */
>>>> +
>>>> +#ifdef __cplusplus
>>>> +extern "C" {
>>>> +#endif
>>>> +
>>>> +#include <stdint.h>
>>>> +#include <rte_service.h>
>>>> +
>>>> +#include "rte_eventdev.h"
>>>> +
>>>> +#define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
>>>> +
>>>> + /**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this enum may change without prior notice
>>>> + *
>>>> + * Crypto event queue conf type
>>>> + */
>>>> +enum rte_event_crypto_conf_type {
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_EVENT = 1,
>>>> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MULTI_EVENTQ */
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_MBUF,
>>>> +	/**< Refer RTE_EVENT_CRYPTO_ADAPTER_CAP_MBUF_MULTI_EVENTQ */
>>>> +	RTE_EVENT_CRYPTO_CONF_TYPE_MAX
>>>> +};
>>>
>>> See above.
>>>
>>>> +
>>>> + /**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this enum may change without prior notice
>>>> + *
>>>> + * Crypto event adapter type
>>>> + */
>>>> +enum rte_event_crypto_adapter_type {
>>>> +	RTE_EVENT_CRYPTO_ADAPTER_RX_ONLY = 1,
>>>> +	/**< Start only Rx part of crypto adapter.
>>>> +	* Packets dequeued from cryptodev are new to eventdev and
>>>> +	* events will be treated as RTE_EVENT_OP_NEW */
>>>> +	RTE_EVENT_CRYPTO_ADAPTER_RX_TX,
>>>> +	/**< Start both Rx & Tx part of crypto adapter.
>>>> +	* Packet's event context will be retained and
>>>> +	* event will be treated as RTE_EVENT_OP_FORWARD */
>>>> +};
>>>
>>> How about leveraging ev.op based schematics as mentioned above?
>>>
>>
> 

  parent reply	other threads:[~2017-12-13 14:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-09  6:54 Abhinandan Gujjar
2017-11-29 11:41 ` Jerin Jacob
2017-12-13 11:03   ` Doherty, Declan
2017-12-13 11:26     ` Jerin Jacob
2017-12-13 12:28       ` Nithin Dabilpuram
2017-12-13 14:29         ` Akhil Goyal
2017-12-13 14:22       ` Akhil Goyal [this message]
2017-12-13 17:37         ` Jerin Jacob
2017-12-13 23:35   ` Eads, Gage
2017-12-14  2:49     ` Jerin Jacob
2017-12-14 18:52       ` Eads, Gage
2017-12-18  6:30         ` Jerin Jacob
2017-12-18 16:33           ` Eads, Gage
2017-12-19  8:47             ` Jerin Jacob

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b28f2e29-a4e4-0102-f609-a948c68fb7fb@nxp.com \
    --to=akhil.goyal@nxp.com \
    --cc=abhinandan.gujjar@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=gage.eads@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=narayanaprasad.athreya@cavium.com \
    --cc=narender.vangati@intel.com \
    --cc=nidadavolu.murthy@cavium.com \
    --cc=nikhil.rao@intel.com \
    --cc=nithin.dabilpuram@cavium.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).