From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-by2nam03on0069.outbound.protection.outlook.com [104.47.42.69]) by dpdk.org (Postfix) with ESMTP id B647A10B7 for ; Thu, 1 Mar 2018 07:58:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=CAVIUMNETWORKS.onmicrosoft.com; s=selector1-cavium-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=HYYhEggaw8H6lD5HcUFD4OEa4WgkwDH96cSa9CVG7Q0=; b=R2WWlTFs0wfAtlo1ad2wWQ8LsIuzlaqMhSe5Nn8TbYDHZzUekRW4TqjMnx5dlsdNihO/kaD+k5llOa+mGc4MlVj0v3YjPSBTI35BPoL0SsADd//tGmHvIbWg5x0bftoVtVNBG6kbp07MISVYqJXkcUnZ3IjYuNsLtrOowvSg0NM= Received: from CY4PR0701MB3634.namprd07.prod.outlook.com (52.132.101.164) by CY4PR0701MB3826.namprd07.prod.outlook.com (52.132.102.160) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.548.13; Thu, 1 Mar 2018 06:58:05 +0000 Received: from CY4PR0701MB3634.namprd07.prod.outlook.com ([fe80::e842:2eb2:2ff5:5dd6]) by CY4PR0701MB3634.namprd07.prod.outlook.com ([fe80::e842:2eb2:2ff5:5dd6%13]) with mapi id 15.20.0548.013; Thu, 1 Mar 2018 06:58:03 +0000 From: "Verma, Shally" To: "Trahe, Fiona" , Ahmed Mansour , "dev@dpdk.org" CC: "De Lara Guarch, Pablo" , "Athreya, Narayana Prasad" , "Gupta, Ashish" , "Sahu, Sunila" , "Challa, Mahipal" , "Jain, Deepak K" , Hemant Agrawal , Roy Pledge , Youri Querry Thread-Topic: [dpdk-dev] [PATCH] compressdev: implement API Thread-Index: AQHTnFM4Vpm7OG1Ltk6wDJVFHhqAyaO32QHQgAJ1P4CAAMfVEA== Date: Thu, 1 Mar 2018 06:58:03 +0000 Message-ID: References: <1517595924-25963-1-git-send-email-fiona.trahe@intel.com> <12544144.czVLKRyaz4@xps> <348A99DA5F5B7549AA880327E580B43589325187@IRSMSX101.ger.corp.intel.com> <348A99DA5F5B7549AA880327E580B4358932983C@IRSMSX101.ger.corp.intel.com> In-Reply-To: <348A99DA5F5B7549AA880327E580B4358932983C@IRSMSX101.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Shally.Verma@cavium.com; x-originating-ip: [115.113.156.2] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; CY4PR0701MB3826; 7:0bkhNtHyCP56cnZh/0weAXloBMW4Jaz+83ixu0XFfFJ+4Np4o7NK2+x4mwePTqmXYteNfVMufrxlt0tNaxEuov0zxsHgrToSFgkn780cao5iYiIb5ailS3Vb4xUctiUyAhghSfVCJfmlTjCr0MQ0eYGHBdCAOOs+aaFuL6gfu+fWZAL2QtrcGUl7DlY6h2ELl59Qn/IOvmHcs4wpSNxr/sUVhYrzJD/QDyyoWTq5hk3SBA4QPTZjzujcxdZhMD8n x-ms-exchange-antispam-srfa-diagnostics: SSOS;SSOR; x-forefront-antispam-report: SFV:SKI; SCL:-1; SFV:NSPM; SFS:(10009020)(39860400002)(39380400002)(366004)(346002)(376002)(396003)(13464003)(199004)(189003)(7696005)(229853002)(26005)(110136005)(8936002)(3280700002)(86362001)(316002)(6306002)(9686003)(81166006)(2900100001)(54906003)(33656002)(55016002)(81156014)(5250100002)(5890100001)(2501003)(14454004)(97736004)(5660300001)(93886005)(186003)(8676002)(105586002)(102836004)(68736007)(966005)(305945005)(55236004)(25786009)(66066001)(74316002)(59450400001)(4326008)(8656006)(6506007)(53546011)(561944003)(2950100002)(7736002)(72206003)(99286004)(3660700001)(2906002)(76176011)(6246003)(6116002)(6436002)(478600001)(106356001)(53936002)(3846002)(53946003); DIR:OUT; SFP:1101; SCL:1; SRVR:CY4PR0701MB3826; H:CY4PR0701MB3634.namprd07.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; x-ms-office365-filtering-correlation-id: f05c15b3-d3f2-4c52-95bc-08d57f41c787 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(4534165)(7168020)(4627221)(201703031133081)(201702281549075)(5600026)(4604075)(3008032)(2017052603307)(7153060)(7193020); SRVR:CY4PR0701MB3826; x-ms-traffictypediagnostic: CY4PR0701MB3826: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(192374486261705)(131327999870524)(185117386973197)(21532816269658)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040501)(2401047)(8121501046)(5005006)(3231220)(944501224)(52105095)(3002001)(93006095)(93001095)(10201501046)(6041288)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123560045)(20161123558120)(20161123564045)(6072148)(201708071742011); SRVR:CY4PR0701MB3826; BCL:0; PCL:0; RULEID:; SRVR:CY4PR0701MB3826; x-forefront-prvs: 05986C03E0 received-spf: None (protection.outlook.com: cavium.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: DnUR8EAMLDMseiBLNPbvWzAPE3QVUQOIuxQPfCR8PpZU0Y1CO6FKCQ2WzUKbEfAeT9UPchgWymEbzS1lGG6wVUpjNgiAEiZGA2kCuyjDhLfouOrUKgJB01G/4UzwR9Sk8O5dL+LkUNYA2/cUTemY5YZTL/I7l1udFvfpgWN69JU= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: cavium.com X-MS-Exchange-CrossTenant-Network-Message-Id: f05c15b3-d3f2-4c52-95bc-08d57f41c787 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Mar 2018 06:58:03.7759 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR0701MB3826 Subject: Re: [dpdk-dev] [PATCH] compressdev: implement API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Mar 2018 06:58:09 -0000 Hi Fiona >-----Original Message----- >From: Trahe, Fiona [mailto:fiona.trahe@intel.com] >Sent: 01 March 2018 00:09 >To: Verma, Shally ; Ahmed Mansour ; dev@dpdk.org >Cc: De Lara Guarch, Pablo ; Athreya, Naray= ana Prasad ; >Gupta, Ashish ; Sahu, Sunila ; Challa, Mahipal >; Jain, Deepak K ; Hem= ant Agrawal ; Roy >Pledge ; Youri Querry ; Trahe,= Fiona >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. B= ut 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 b= etter if > the resources were setup once based on the xform and could be re-used o= n 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 shar= ed. > >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 hand= le > * which must be attached to subsequent STATELESS operations. > * The PMD also returns a flag, if this is COMP_PRIVATE_XFORM_SHAR= EABLE > * 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 initi= alised > * to send other ops in parallel. > */ > void *stream; > /* Private PMD data derived initially from an rte_comp_xform, whic= h holds state > * and history data and evolves as operations are processed. > * rte_comp_stream_create() must be called on a device for all STA= TEFUL > * 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 an= d 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_in= it() 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 m= ax qps the device can support. > - dev_config() - application specifies how many qps it intends to use - t= ypically 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@cavium.com] >> Sent: Tuesday, February 27, 2018 5:54 AM >> To: Ahmed Mansour ; Trahe, Fiona ; >> dev@dpdk.org >> Cc: De Lara Guarch, Pablo ; Athreya, Nar= ayana Prasad >> ; Gupta, Ashish ; Sahu, Sunila >> ; Challa, Mahipal ; J= ain, Deepak K >> ; Hemant Agrawal ; Roy = Pledge >> ; Youri Querry >> Subject: RE: [dpdk-dev] [PATCH] compressdev: implement API >> >> >> >> >-----Original Message----- >> >From: Ahmed Mansour [mailto:ahmed.mansour@nxp.com] >> >Sent: 27 February 2018 03:05 >> >To: Verma, Shally ; Trahe, Fiona ; dev@dpdk.org >> >Cc: De Lara Guarch, Pablo ; Athreya, Na= rayana Prasad >> ; >> >Gupta, Ashish ; Sahu, Sunila ; Challa, >> Mahipal >> >; Jain, Deepak K ; = Hemant Agrawal >> ; Roy >> >Pledge ; Youri Querry >> >Subject: Re: [dpdk-dev] [PATCH] compressdev: implement API >> > >> >> Hi Fiona, Ahmed >> >>> Hi Fiona, >> >>> >> >>> Thanks for starting this discussion. In the current API the user mus= t >> >>> make 12 API calls just to get information to compress. Maybe there i= s 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 th= e >> >>> 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 stre= am to perform the operation. >> >>>> - It inherited from cryptodev the ability to be set up for multipl= e 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 unl= ikely 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 ses= sion >> >>> holds many properties like the op-type, instead of having this >> >>> information in the op itself. This way we lower the per op setup co= st. >> >>> This also allows rapid reuse of stateful infrastructure, once a stre= am >> >>> is closed on a stateful session, the next op (stream) on this sessio= n >> >>> 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 rel= y on >> >>> their properties to simplify op setup, so why the hassle. >> >> [Shally] As per my knowledge, session came with idea in DPDK, if sys= tem has multiple devices setup >> to do similar jobs then >> >application can fan out ops to any of them for load-balancing. Though i= t 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 di= scussion 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 notio= n too it looks achievable to me, >> if so is desired. >> >> >> In current proposal, stream logically represent data and hold its spe= cific information and session is >> generic information that can be >> >applied on multiple data. If we want to combine stream and session. The= n one way to look at this is: >> >> >> >> "let application only allocate and initialize session with rte_comp_x= form (and possibly op type) >> information so that PMD can do one- >> >time setup and allocate enough resources. Once attached to op, cannot b= e 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 assum= e 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 ke= ep map of device & a session >> map. >> >> >> >> If this sound feasible, then I too believe we can rather get rid of e= ither and keep one (possibly session >> but am open with stream as >> >well). >> >> However, regardless of case whether we live with name stream or sessi= on, 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 po= ol 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_xf= orm >> >>>> * rte_comp_stateless_data_init() must be called on a devic= e >> >>>> * before sending any STATELESS operations. If the PMD retu= rns a non-NULL >> >>>> * value the handle must be attached to subsequent STATELES= S operations. >> >>>> * If a PMD returns NULL, then the xform should be passed d= irectly to each op >> >>>> */ >> >> [Shally] It sounds like stateless_data_init() nothing more than a rep= lacement of session_init(). >> >> So, this is needed neither if we retain session concept nor if we re= tain stream concept ( >> rte_comp_stream_create() with >> >op_type: stateless can serve same purpose). >> >> It should be sufficient to provide either stream (or session) pointe= r. >> >> >> >>>> void *stream; >> >>>> /* Private PMD data derived initially from an rte_comp_xfor= m, 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 s= tream. >> >>>> * All operations in a stream must be sent to the same devi= ce. >> >>>> */ >> >>>> } >> >>> [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 inter= nally managed stream(or >> session data structure). And then we >> >can remove STATEFUL reference here and just say stream(or session) it b= elongs 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 f= or both it can do, >> >>>> just on the API I think it's better if they're named different= ly 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 d= id, and so >> >>>> could be attached to many ops in parallel. >> >>>> Is this true for all PMDs or are there constraints which shoul= d be called out? >> >>>> Is it limited to a specific device, qp, or to be used on one o= p 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 co= de if people >> >>>> are in agreement, just want to see if the concept flies before goin= g further? >> >>>> >> >>>> Fiona >> >>>> >> >>>> >> >>>> >> >>