From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Trahe, Fiona" Subject: Re: [PATCH] compressdev: implement API Date: Wed, 21 Feb 2018 19:11:58 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B43589325187@IRSMSX101.ger.corp.intel.com> References: <1517595924-25963-1-git-send-email-fiona.trahe@intel.com> <12544144.czVLKRyaz4@xps> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "De Lara Guarch, Pablo" , "Athreya, Narayana Prasad" , "Gupta, Ashish" , "Sahu, Sunila" , "Challa, Mahipal" , "Jain, Deepak K" , Hemant Agrawal , "Roy Pledge" , Youri Querry To: "dev@dpdk.org" , "ahmed.mansour@nxp.com" , "Shally.Verma@cavium.com" Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 1FFBD7CE7 for ; Wed, 21 Feb 2018 20:12:05 +0100 (CET) In-Reply-To: <12544144.czVLKRyaz4@xps> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" We've been struggling with the idea of session in compressdev. Is it really a session?=20 - It's not in the same sense as cryptodev where it's used to hold a key, a= nd maps to a Security Association. - It's a set of immutable data that is needed with the op and stream to pe= rform the operation. - It inherited from cryptodev the ability to be set up for multiple driver= types and used across any=20 devices of those types. For stateful ops this facility can't be used.=20 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, s= o there's less work to be done on the data path. Initially we didn't have a strea= m, 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.=20 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 probl= em with setting=20 up some private PMD data to use with stateless operations, just calling= it a stream doesn't seem right. 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 =20 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=20 * before sending any STATELESS operations. If the PMD returns a no= n-NULL * value the handle must be attached to subsequent STATELESS operat= ions. * If a PMD returns NULL, then the xform should be passed directly = to each op=20 */ 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 STAT= EFUL=20 * 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. */ } Notes:=20 1. Internally if a PMD wants to use the exact same data structure for both = it can do,=20 just on the API I think it's better if they're named differently with= =20 different comments. 2. I'm not clear of the constraints if any, which attach to the pmd_statele= ss_data For our PMD it would only hold immutable data as the session did, and = so could be attached to many ops in parallel.=20 Is this true for all PMDs or are there constraints which should be cal= led out? Is it limited to a specific device, qp, or to be used on one op at a t= ime? 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 pe= ople are in agreement, just want to see if the concept flies before going furthe= r? Fiona =20