>-----Original Message----- >From: Ahmed Mansour [mailto:ahmed.mans...@nxp.com] >Sent: 27 February 2018 03:05 >To: Verma, Shally <shally.ve...@cavium.com>; Trahe, Fiona ><fiona.tr...@intel.com>; dev@dpdk.org >Cc: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com>; Athreya, Narayana >Prasad <narayanaprasad.athr...@cavium.com>; >Gupta, Ashish <ashish.gu...@cavium.com>; Sahu, Sunila ><sunila.s...@cavium.com>; Challa, Mahipal ><mahipal.cha...@cavium.com>; Jain, Deepak K <deepak.k.j...@intel.com>; Hemant >Agrawal <hemant.agra...@nxp.com>; Roy >Pledge <roy.ple...@nxp.com>; Youri Querry <youri.querr...@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 >>>> >>>> >>>> >>
