[dpdk-dev] [PATCH] compressdev: implement API

Verma, Shally Shally.Verma at cavium.com
Thu Mar 1 07:58:03 CET 2018


Hi Fiona

>-----Original Message-----
>From: Trahe, Fiona [mailto:fiona.trahe at intel.com]
>Sent: 01 March 2018 00:09
>To: Verma, Shally <Shally.Verma at cavium.com>; Ahmed Mansour <ahmed.mansour at nxp.com>; dev at dpdk.org
>Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>; Athreya, Narayana Prasad <NarayanaPrasad.Athreya at cavium.com>;
>Gupta, Ashish <Ashish.Gupta at cavium.com>; Sahu, Sunila <Sunila.Sahu at cavium.com>; Challa, Mahipal
><Mahipal.Challa at cavium.com>; Jain, Deepak K <deepak.k.jain at intel.com>; Hemant Agrawal <hemant.agrawal at nxp.com>; Roy
>Pledge <roy.pledge at nxp.com>; Youri Querry <youri.querry_1 at nxp.com>; Trahe, Fiona <fiona.trahe at intel.com>
>Subject: RE: [dpdk-dev] [PATCH] compressdev: implement API
>
>Hi Ahmed, Shally,
>
>So just to capture what we concluded in the call today:
>
> - There's no requirement for a device-agnostic session to facilitate load-balancing.
> - For stateful data a stream is compulsory. Xform is passed to stream on creation.
>    So no need for a session in stateful op.
>
>Re session data for stateless ops:
> - All PMDs could cope with just passing in a xform with a stateless op. But it might
>   not be performant.
> - Some PMDs need to allocate some resources which can only be used by one op
>   at a time. For stateful ops these resources can be attached to a stream. For stateless
>   they could allocate the resources on each op enqueue, but it would be better if
>   the resources were setup once based on the xform and could be re-used on ops,
>   though only by one op at a time.
> - Some PMDs don't need to allocate such resources, but could benefit by
>   setting up some pmd data based on the xform. This data would not be
>   constrained, could be used in parallel by any op or qp of the device.
> - The name pmd_stateless_data was not popular, maybe something like
>    xform_private_data can be used. On creation of this data, the PMD can return
>    an indication of whether it should be used by one op at a time or shared.
>
>So I'll
> - remove the session completely from the API.
> - add an initialiser API for the data to be attached to stateless ops
> - add a union to the op:
>
> union {
>        void *pmd_private_xform;
>        /**< Stateless private PMD data derived from an rte_comp_xform
>         * rte_comp_xform_init() must be called on a device
>         * before sending any STATELESS operations. The PMD returns a handle
>         * which must be attached to subsequent STATELESS operations.
>         * The PMD also returns a flag, if this is COMP_PRIVATE_XFORM_SHAREABLE
>         * then the xform can be attached to multiple ops at the same time,
>         * if it's COMP_PRIVATE_XFORM_SINGLE_OP then it can only be
>         * be used on one op at a time, other private xforms must be initialised
>         * to send other ops in parallel.
>         */
>        void *stream;
>        /* Private PMD data derived initially from an rte_comp_xform, which holds state
>         * and history data and evolves as operations are processed.
>         * rte_comp_stream_create() must be called on a device for all STATEFUL
>         * data streams and the resulting stream attached
>         * to the one or more operations associated with the data stream.
>         * All operations in a stream must be sent to the same device.
>         */
>    }
>
>Previous startup flow before sending a stateful op:
>rte_comp_get_private_size(devid)
>rte_comp_mempool_create() - returns sess_pool
>rte_comp_session_create(sess_pool)
>rte_comp_session_init(devid, sess, sess_pool, xform)
>rte_comp_stream_create(devid, sess, **stream, op_type)
>
>simplified to:
>rte_comp_xform_init(devid, xform, **priv_xform, *flag) - returns handle and flag
>(pool is within the PMD)
>
>Note, I don't think we bottomed out on removing the xform from the union, but I don't
>think we need it with above solution.

[Shally] This looks better to me. So it mean app would always call xform_init() for stateless and attach an updated priv_xform to ops (depending upon if there's shareable or not). So it does not need to have  NULL pointer on priv_xform. right?


>
>Other discussion:
> - we should document on API that qp is not thread-safe, so enqueue
>   and dequeue should be performed by same thread.
>
>device and qp flow:
> - dev_info_get() - application reads device capabilities, including the max qps the device can support.
> - dev_config() - application specifies how many qps it intends to use - typically one per thread, must be < device max
> - qp_setup() - called per qp. Creates the qp based on the size indicated by max_inflights
> - dev_start() - once started device can't be reconfigured, must call dev_stop to reconfigure.
>
>
>Regards,
>Fiona
>
>> -----Original Message-----
>> From: Verma, Shally [mailto:Shally.Verma at cavium.com]
>> Sent: Tuesday, February 27, 2018 5:54 AM
>> To: Ahmed Mansour <ahmed.mansour at nxp.com>; Trahe, Fiona <fiona.trahe at intel.com>;
>> dev at dpdk.org
>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>; Athreya, Narayana Prasad
>> <NarayanaPrasad.Athreya at cavium.com>; Gupta, Ashish <Ashish.Gupta at cavium.com>; Sahu, Sunila
>> <Sunila.Sahu at cavium.com>; Challa, Mahipal <Mahipal.Challa at cavium.com>; Jain, Deepak K
>> <deepak.k.jain at intel.com>; Hemant Agrawal <hemant.agrawal at nxp.com>; Roy Pledge
>> <roy.pledge at nxp.com>; Youri Querry <youri.querry_1 at nxp.com>
>> Subject: RE: [dpdk-dev] [PATCH] compressdev: implement API
>>
>>
>>
>> >-----Original Message-----
>> >From: Ahmed Mansour [mailto:ahmed.mansour at nxp.com]
>> >Sent: 27 February 2018 03:05
>> >To: Verma, Shally <Shally.Verma at cavium.com>; Trahe, Fiona <fiona.trahe at intel.com>; dev at dpdk.org
>> >Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>; Athreya, Narayana Prasad
>> <NarayanaPrasad.Athreya at cavium.com>;
>> >Gupta, Ashish <Ashish.Gupta at cavium.com>; Sahu, Sunila <Sunila.Sahu at cavium.com>; Challa,
>> Mahipal
>> ><Mahipal.Challa at cavium.com>; Jain, Deepak K <deepak.k.jain at intel.com>; Hemant Agrawal
>> <hemant.agrawal at nxp.com>; Roy
>> >Pledge <roy.pledge at nxp.com>; Youri Querry <youri.querry_1 at nxp.com>
>> >Subject: Re: [dpdk-dev] [PATCH] compressdev: implement API
>> >
>> >> Hi Fiona, Ahmed
>> >>> Hi Fiona,
>> >>>
>> >>> Thanks for starting this discussion. In the current API the user must
>> >>> make 12 API calls just to get information to compress. Maybe there is a
>> >>> way to simplify. At least for some use cases (stateless). I think a call
>> >>> sometime next week would be good to help clarify coalesce some of the
>> >>> complexity.
>> >>>
>> >>> I added specific comments inline.
>> >>>
>> >>> Thanks,
>> >>>
>> >>> Ahmed
>> >>>
>> >>> On 2/21/2018 2:12 PM, Trahe, Fiona wrote:
>> >>>> We've been struggling with the idea of session in compressdev.
>> >>>>
>> >>>> Is it really a session?
>> >>>>  - It's not in the same sense as cryptodev where it's used to hold a key, and maps to a Security
>> Association.
>> >>>>  - It's a set of immutable data that is needed with the op and stream to perform the operation.
>> >>>>  - It inherited from cryptodev the ability to be set up for multiple driver types and used across any
>> >>>>     devices of those types. For stateful ops this facility can't be used.
>> >>>>     For stateless we don't think it's important, and think it's unlikely to be used.
>> >>>>  - Drivers use it to prepare private data, set up resources, do pre-work, so there's
>> >>>>     less work to be done on the data path. Initially we didn't have a stream, we do now,
>> >>>>     this may be a better alternative place for that work.
>> >>>> So we've been toying with the idea of getting rid of the session.
>> >>> [Ahmed] In our proprietary API the stream and session are one. A session
>> >>> holds many properties like the op-type, instead of having this
>> >>> information in the op itself.  This way we lower the per op setup cost.
>> >>> This also allows rapid reuse of stateful infrastructure, once a stream
>> >>> is closed on a stateful session, the next op (stream) on this session
>> >>> reuses the stateful storage. Obviously if a stream is in "pause mode" on
>> >>> a session, all following ops that may be unrelated to this
>> >>> stream/session must also wait until this current stream is closed or
>> >>> aborted before the infrastructure can be reused.
>> >>>> We also struggle with the idea of setting up a stream for stateless ops.
>> >>>>   - Well, really I just think the name is misleading, i.e. there's no problem with setting
>> >>>>     up some private PMD data to use with stateless operations, just calling it a
>> >>>>     stream doesn't seem right.
>> >>> [Ahmed] I agree. The op has all the necessary information to process it
>> >>> in the current API? Both the stream and the op are one time use. We
>> >>> can't attach multiple similar ops to a single stream/session and rely on
>> >>> their properties to simplify op setup, so why the hassle.
>> >> [Shally]  As per my knowledge, session came with idea in DPDK, if system has multiple devices setup
>> to do similar jobs then
>> >application can fan out ops to any of them for load-balancing. Though it is not possible for stateful ops
>> but it still can work for stateless.
>> >If there's an application which only have stateless ops to process then I see this is still useful feature to
>> support.
>> >[Ahmed] Is there an advantage to exposing load balancing to the user? I
>> >do not see load balancing as a feature within itself. Can the PMD take
>> >care of this? I guess a system that has
>>
>> [Shally] I assume idea was to leverage multiple PMDs that are available in system (say QAT+SW ZLIB)
>> and I believe matter of load-balancing came out of one of the earlier discussion with Fiona on RFC v1.
>> http://dev.dpdk.narkive.com/CHS5l01B/dpdk-dev-rfc-v1-doc-compression-api-for-dpdk#post3
>> So, I wait for her comments on this. But in any case, with changed notion too it looks achievable to me,
>> if so is desired.
>>
>> >> In current proposal, stream logically represent data and hold its specific information and session is
>> generic information that can be
>> >applied on multiple data. If we want to combine stream and session. Then one way to look at this is:
>> >>
>> >> "let application only allocate and initialize session with rte_comp_xform (and possibly op type)
>> information so that PMD can do one-
>> >time setup and allocate enough resources. Once attached to op, cannot be reused until that op is fully
>> processed. So, if app has 16
>> >data elements to process in a burst, it will setup 16 sessions."
>> >[Ahmed] Why not allow multiple inflight stateless ops with the same
>> >session? Stateless by definition guarantees that the resources used to
>> >work on one up will be free after the op is processed. That means that
>> >even if an op fails to process correctly on a session, it will have no
>> >effect on the next op since there is not interdependence. This assumes
>> >that the resources are shareable between hardware instances for
>> >stateless. That is not a bad assumption since hardware should not need
>> >more than the data of the op itself to work on a statelss op.
>>
>> [Shally]  multiple ops in-flight can connect to same session but I assume you agree then they cannot
>> execute in parallel i.e. only one op at-a-time can use session here? And as far as I understand your PMD
>> works this way. Your HW execute one op at-a-time from queue?!
>>
>> >> This is same as what Ahmed suggested. For a particular load-balancing case suggested above, If
>> application want, can initialize
>> >different sessions on multiple devices with same xform so that each is prepared to process ops.
>> Application can then fanout stateless
>> >ops to multiple devices for load-balancing but then it would need to keep map of device & a session
>> map.
>> >>
>> >> If this sound feasible, then I too believe we can rather get rid of either and keep one (possibly session
>> but am open with stream as
>> >well).
>> >> However, regardless of case whether we live with name stream or session, I don't see much deviation
>> from current API spec except
>> >description and few modifications/additions as identified.
>> >> So, then I see it as:
>> >>
>> >> - A stream(or session whichever name is chosen) can be used with only one-op at-a-time
>> >> - It can be re-used when previously attached op is processed
>> >> -  if it is stream then currently it is allocated from PMD managed pool whereas Sessions are allocated
>> from application created
>> >mempool.
>> >>    In either of case, I would expect to review pool management API
>> >>
>> >> With this in mind, below are few of my comments
>> >>
>> >>>> So putting above thoughts together I want to propose:
>> >>>> -	Removal of the session and all associated APIs.
>> >>>> -	Passing in one of three data types in the rte_comp_op
>> >>>>
>> >>>>     union {
>> >>>>         struct rte_comp_xform *xform;
>> >>>>         /**< Immutable compress/decompress params */
>> >>>>         void *pmd_stateless_data;
>> >>>>         /**< Stateless private PMD data derived from an rte_comp_xform
>> >>>>          * rte_comp_stateless_data_init() must be called on a device
>> >>>>          * before sending any STATELESS operations. If the PMD returns a non-NULL
>> >>>>          * value the handle must be attached to subsequent STATELESS operations.
>> >>>>          * If a PMD returns NULL, then the xform should be passed directly to each op
>> >>>>          */
>> >> [Shally] It sounds like stateless_data_init() nothing more than a replacement of session_init().
>> >> 	So, this is needed neither if we retain session concept nor if we retain stream concept (
>> rte_comp_stream_create() with
>> >op_type: stateless can serve same purpose).
>> >> 	It should be sufficient to provide either stream (or session) pointer.
>> >>
>> >>>>         void *stream;
>> >>>>         /* Private PMD data derived initially from an rte_comp_xform, which holds state
>> >>>>          * and history data and evolves as operations are processed.
>> >>>>          * rte_comp_stream_create() must be called on a device for all STATEFUL
>> >>>>          * data streams and the resulting stream attached
>> >>>>          * to the one or more operations associated with the data stream.
>> >>>>          * All operations in a stream must be sent to the same device.
>> >>>>          */
>> >>>>     }
>> >>> [Ahmed] I like this setup, but I am not sure in what cases the xform
>> >>> immutable would be used. I understand the other two.
>> >> [Shally] my understanding is xform will be mapped by PMD to its internally managed stream(or
>> session data structure). And then we
>> >can remove STATEFUL reference here and just say stream(or session) it belongs to. However, This
>> condition still apply:
>> >>        *All operations that belong to same stream must be sent to the same device.*
>> >>
>> >>>> Notes:
>> >>>> 1. Internally if a PMD wants to use the exact same data structure for both it can do,
>> >>>>      just on the API I think it's better if they're named differently with
>> >>>>      different comments.
>> >>>> 2. I'm not clear of the constraints if any, which attach to the pmd_stateless_data
>> >>>>      For our PMD it would only hold immutable data as the session did, and so
>> >>>>      could be attached to many ops in parallel.
>> >>>>      Is this true for all PMDs or are there constraints which should be called out?
>> >>>>      Is it limited to a specific device, qp, or to be used on one op at a time?
>> >>>> 3. Am open to other naming suggestions, just trying to capture the essence
>> >>>>     of these data structs better than our current API does.
>> >>>>
>> >>>> We would put some more helper fns and structure around the above code if people
>> >>>> are in agreement, just want to see if the concept flies before going further?
>> >>>>
>> >>>> Fiona
>> >>>>
>> >>>>
>> >>>>
>> >>



More information about the dev mailing list