From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id CF3825F59 for ; Wed, 14 Mar 2018 13:42:31 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Mar 2018 05:42:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,470,1515484800"; d="scan'208";a="208135309" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by orsmga005.jf.intel.com with ESMTP; 14 Mar 2018 05:41:55 -0700 Received: from irsmsx111.ger.corp.intel.com (10.108.20.4) by IRSMSX154.ger.corp.intel.com (163.33.192.96) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 14 Mar 2018 12:41:53 +0000 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.5]) by irsmsx111.ger.corp.intel.com ([169.254.2.246]) with mapi id 14.03.0319.002; Wed, 14 Mar 2018 12:41:53 +0000 From: "Trahe, Fiona" To: Shally Verma CC: "pathreya@caviumnetworks.com" , "mchalla@caviumnetworks.com" , "agupta@caviumnetworks.com" , "dev@dpdk.org" , "ahmed.mansour@nxp.com" , "Trahe, Fiona" , "De Lara Guarch, Pablo" , "Jain, Deepak K" Thread-Topic: [RFC v4 1/1] lib/compressdev: Adding hash support Thread-Index: AQHTm0z2d1qAEvD+okqrqh1FeH/w06POlQdw Date: Wed, 14 Mar 2018 12:41:53 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B4358934AD24@IRSMSX101.ger.corp.intel.com> References: <1517483232-28756-1-git-send-email-shally.verma@caviumnetworks.com> In-Reply-To: <1517483232-28756-1-git-send-email-shally.verma@caviumnetworks.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNGJjZmI3YjktMmU2Ni00MDIwLTk0MmYtNTQ3NjExYzUwNmQwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkpjRlFoRHlpeWJ3cVVDSVg2Y2ZjUUR4NzBnRGNXekVaSHZwYk82dEtxWVk9In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC v4 1/1] lib/compressdev: Adding hash support 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: Wed, 14 Mar 2018 12:42:34 -0000 Hi Shally, Sorry about the delay in responding. Comments below. If you want to push next update to github we can continue dev there. Regards, Fiona > -----Original Message----- > From: Shally Verma [mailto:shally.verma@caviumnetworks.com] > Sent: Thursday, February 1, 2018 11:07 AM > To: Trahe, Fiona > Cc: pathreya@caviumnetworks.com; mchalla@caviumnetworks.com; agupta@caviu= mnetworks.com; > dev@dpdk.org; ahmed.mansour@nxp.com > Subject: [RFC v4 1/1] lib/compressdev: Adding hash support >=20 > Added hash support in lib compressdev. It's an incremental patch to > compression lib RFC v3 https://dpdk.org/dev/patchwork/patch/32331/ >=20 > Changes from RFC v3: > - Added hash algo enumeration and associated capability stucture > and params in xform and rte_comp_op > - Rearranged rte_compresdev_capability structure to have > separate rte_comp_algo_capability and moved > algo specific capabilities: window_size, dictionary support, > and hash as part of it > - Added RTE_COMP_UNSPECIFIED=3D0 in enum rte_comp_algorithm > - Redefined RTE_COMP_END_OF_CAPABILITIES_LIST to use RTE_COMP_UNSPECIFIED > to resolve missing-field-initializer compiler warning > - Updated compress/decompress xform to input hash algorithm > during session init > - Updated struct rte_comp_op to input hash buffer > - Fixed checkpatch reported errors on RFCv3 >=20 > Every compression algorithm can indicate its capability to > perform alongside hash in its associate rte_comp_algo_capa structure. > If none is supported, can terminate array with > hash_algo =3D RTE_COMP_HASH_ALGO_UNSPECIFIED. > if supported, application can initialize session with desired > algorithm enumeration in xform structure and pass valid hash buffer > pointer during enqueue_burst(). >=20 > Signed-off-by: Shally Verma > --- > lib/librte_compressdev/rte_comp.h | 83 ++++++++++++++++= +----- > lib/librte_compressdev/rte_compressdev.c | 19 +++++ > lib/librte_compressdev/rte_compressdev.h | 59 ++++++++++++--- > lib/librte_compressdev/rte_compressdev_version.map | 1 + > 4 files changed, 137 insertions(+), 25 deletions(-) >=20 > diff --git a/lib/librte_compressdev/rte_comp.h b/lib/librte_compressdev/r= te_comp.h > index ca8cbb4..341f59f 100644 > --- a/lib/librte_compressdev/rte_comp.h > +++ b/lib/librte_compressdev/rte_comp.h > @@ -75,10 +75,11 @@ enum rte_comp_op_status { > */ > }; >=20 > - > /** Compression Algorithms */ > enum rte_comp_algorithm { > - RTE_COMP_NULL =3D 0, > + RTE_COMP_UNSPECIFIED =3D 0, > + /** No Compression algo */ > + RTE_COMP_NULL, > /**< No compression. > * Pass-through, data is copied unchanged from source buffer to > * destination buffer. > @@ -94,6 +95,18 @@ enum rte_comp_algorithm { > RTE_COMP_ALGO_LIST_END > }; >=20 > + > +/** Compression hash algorithms */ > +enum rte_comp_hash_algorithm { > + RTE_COMP_HASH_ALGO_UNSPECIFIED =3D 0, > + /**< No hash */ > + RTE_COMP_HASH_ALGO_SHA1, > + /**< SHA1 hash algorithm */ > + RTE_COMP_HASH_ALGO_SHA256, > + /**< SHA256 hash algorithm */ > + RTE_COMP_HASH_ALGO_LIST_END, > +}; > + > /**< Compression Level. > * The number is interpreted by each PMD differently. However, lower num= bers > * give fastest compression, at the expense of compression ratio while > @@ -154,21 +167,24 @@ enum rte_comp_flush_flag { > RTE_COMP_FLUSH_SYNC, > /**< All data should be flushed to output buffer. Output data can be > * decompressed. However state and history is not cleared, so future > - * ops may use history from this op */ > + * ops may use history from this op > + */ > RTE_COMP_FLUSH_FULL, > /**< All data should be flushed to output buffer. Output data can be > * decompressed. State and history data is cleared, so future > * ops will be independent of ops processed before this. > */ > RTE_COMP_FLUSH_FINAL > - /**< Same as RTE_COMP_FLUSH_FULL but also bfinal bit is set in last blo= ck > + /**< Same as RTE_COMP_FLUSH_FULL but also bfinal bit is set in > + * last block > */ > /* TODO: > * describe flag meanings for decompression. > * describe behavous in OUT_OF_SPACE case. > * At least the last flag is specific to deflate algo. Should this be > * called rte_comp_deflate_flush_flag? And should there be > - * comp_op_deflate_params in the op? */ > + * comp_op_deflate_params in the op? > + */ > }; >=20 > /** Compression transform types */ > @@ -180,17 +196,17 @@ enum rte_comp_xform_type { > }; >=20 > enum rte_comp_op_type { > - RTE_COMP_OP_STATELESS, > - /**< All data to be processed is submitted in the op, no state or hi= story > - * from previous ops is used and none will be stored for future ops. > - * flush must be set to either FLUSH_FULL or FLUSH_FINAL > - */ > - RTE_COMP_OP_STATEFUL > - /**< There may be more data to be processed after this op, it's part= of a > - * stream of data. State and history from previous ops can be used > - * and resulting state and history can be stored for future ops, > - * depending on flush_flag. > - */ > + RTE_COMP_OP_STATELESS, > + /**< All data to be processed is submitted in the op, no state or > + * history from previous ops is used and none will be stored for > + * future ops.flush must be set to either FLUSH_FULL or FLUSH_FINAL > + */ > + RTE_COMP_OP_STATEFUL > + /**< There may be more data to be processed after this op, it's > + * part of a stream of data. State and history from previous ops > + * can be usedand resulting state and history can be stored for > + * future ops, depending on flush_flag. > + */ > }; >=20 >=20 > @@ -207,6 +223,13 @@ struct rte_comp_deflate_params { > struct rte_comp_compress_common_params { > enum rte_comp_algorithm algo; > /**< Algorithm to use for compress operation */ > + enum rte_comp_hash_algorithm hash_algo; > + /**< Hash algorithm to be used with compress operation. Hash is always > + * done on plaintext. application can query > + * rte_compressdev_capability_get() and parse hash_capa array per each > + * algorithm to know supported hash algos and associated params until > + * it find an entry with RTE_COMP_HASH_ALGO_UNSPECIFIED > + */ > union { > struct rte_comp_deflate_params deflate; > /**< Parameters specific to the deflate algorithm */ > @@ -240,6 +263,13 @@ struct rte_comp_compress_xform { > struct rte_comp_decompress_common_params { > enum rte_comp_algorithm algo; > /**< Algorithm to use for decompression */ > + enum rte_comp_hash_algorithm hash_algo; > + /**< Hash algorithm to be used with decompress operation. Hash is alway= s > + * done on plaintext. application can query > + * rte_compressdev_capability_get() and parse hash_capa array per each > + * algorithm to know supported hash algos and associated params until > + * it find an entry with RTE_COMP_HASH_ALGO_UNSPECIFIED > + */ > enum rte_comp_checksum_type chksum; > /**< Type of checksum to generate on the decompressed data. */ > uint16_t window_size; > @@ -301,7 +331,7 @@ struct rte_comp_xform { > struct rte_comp_op { >=20 > enum rte_comp_op_type op_type; > - void * stream_private; > + void *stream_private; > /* location where PMD maintains stream state > * only required if op_type is STATEFUL, else should be NULL > */ > @@ -344,6 +374,25 @@ struct rte_comp_op { > * decompress direction. > */ > } dst; > + struct { > + uint8_t *hash_buf; > + /**< Pointer to output hash calculated on plaintext if session > + * is initialized with Hash algorithm RTE_COMP_HASH_ALGO_SHA1 / > + * RTE_COMP_HASH_ALGO_SHA256. Buffer would contain valid value > + * only after an op with > + * flush flag =3D RTE_COMP_FLUSH_FULL/FLUSH_FINAL is processed > + * successfully. > + * > + * Length of buffer should be large enough to accommodate digest > + * produced by specific hash algo. Application can know > + * digest_size via parsing hash capability array in struct > + * rte_compressdev_capabilities > + */ > + uint32_t offset; > + /**< Starting offset for writing hash bytes, specified as > + * number of bytes from start of hash buffer pointer. > + */ [Fiona] Is offset really necessary? If it is then need to update description of length of hash buffer above to = be digest_size + offset. Also I expect an iova address is necessary for a hw accelerator to produce = this hash. > + } hash; > enum rte_comp_flush_flag flush_flag; > /**< defines flush characteristics for the output data. > * Only applicable in compress direction > diff --git a/lib/librte_compressdev/rte_compressdev.c b/lib/librte_compre= ssdev/rte_compressdev.c > index 6186ce5..b62b1ce 100644 > --- a/lib/librte_compressdev/rte_compressdev.c > +++ b/lib/librte_compressdev/rte_compressdev.c > @@ -113,6 +113,25 @@ struct rte_compressdev_callback { >=20 > }; >=20 > +const struct rte_compressdev_capabilities * > +rte_compressdev_capability_get(uint8_t dev_id, > + const struct rte_compressdev_capability_idx *idx) > +{ > + const struct rte_compressdev_capabilities *capability; > + struct rte_compressdev_info dev_info; > + int i =3D 0; > + > + memset(&dev_info, 0, sizeof(struct rte_compressdev_info)); > + rte_compressdev_info_get(dev_id, &dev_info); > + > + while ((capability =3D &dev_info.capabilities[i++])->algo !=3D > + RTE_COMP_UNSPECIFIED){ > + if (capability->algo =3D=3D idx->algo) > + return capability; > + } > + > + return NULL; > +} >=20 > #define param_range_check(x, y) \ > (((x < y.min) || (x > y.max)) || \ > diff --git a/lib/librte_compressdev/rte_compressdev.h b/lib/librte_compre= ssdev/rte_compressdev.h > index 9a86603..7d1b9b1 100644 > --- a/lib/librte_compressdev/rte_compressdev.h > +++ b/lib/librte_compressdev/rte_compressdev.h > @@ -125,22 +125,65 @@ struct rte_comp_param_range { > */ > }; >=20 > +/** > + * Compression hash algo Capability > + */ > +struct rte_comp_hash_algo_capability { > + enum rte_comp_hash_algorithm hash_alg; > + /**< hash algo enumeration */ > + uint32_t digest_size; > + /**< length of digest produced by hash */ [Fiona] op doesn't give an option to specify digest_length. So if it's a fixed size per hash algo then no need for=20 length in capabilities. And so can maybe use a bitmask for hash capabilitie= s. If a range is needed then this should be a range and add digest_length to o= p or xform > +}; > + > +/** > + * Compression algo Capability > + */ > +struct rte_comp_algo_capability { > + struct rte_comp_param_range window_size; > + /**< window size range in bytes */ > + int support_dict; > + /** Indicate algo support for dictionary load */ [Fiona] I need more info on this - What else is needed on the API to suppor= t it? > + struct rte_comp_hash_algo_capability *hash_capa; > + /** pointer to an array of hash capability struct */ [Fiona] Does the hash capability depend on the compression algorithm? I'd have thought it's completely independent as it's computed=20 on the uncompressed data? I suppose the same can be said of checksum. This all seems a bit awkward. Would something like this be better: enum rte_comp_capability_type { RTE_COMP_CAPA_TYPE_ALGO, RTE_COMP_CAPA_TYPE_CHKSUM, RTE_COMP_CAPA_TYPE_HASH } struct rte_comp_algo_capability { enum rte_comp_algorithm algo; /** Compression Algorithm */ uint64_t comp_feature_flags; /**< bitmap of flags for compression service features supported with this = algo */ struct rte_comp_param_range window_size; /**< window size range in bytes supported with this algo */ /* int support_dict; */ /** Indicate algo support for dictionary load */ }; struct rte_compressdev_capabilities { { rte_comp_capability_type capa_type; union { struct rte_comp_algo_capability algo_capa, int checksum_capa, /* bitmask of supported checksums */ int hash_capa, /* bitmask of supported hashes - this could be a struct= if more data needed */ } }; >=20 > /** Structure used to capture a capability of a comp device */ > struct rte_compressdev_capabilities { > - /* TODO */ > enum rte_comp_algorithm algo; > + /** Compression Algorithm */ > + struct rte_comp_algo_capability alg_capa; > + /**< Compression algo capabilities */ > uint64_t comp_feature_flags; > - /**< bitmap of flags for compression service features*/ > - struct rte_comp_param_range window_size; > - /**< window size range in bytes */ > + /**< bitmap of flags for compression service features */ > }; >=20 > +/** Structure used to index compression device capability array */ > +struct rte_compressdev_capability_idx { > + enum rte_comp_algorithm algo; > +}; > + > +/** > + * Provide device capability for requested algorithm > + * > + * @param dev_id The identifier of the device. > + * @param idx Index to capability array > + * > + * @return > + * - Return pointer to capability structure, if exist. > + * - Return NULL, if does not exist. > + */ > +const struct rte_compressdev_capabilities * > +rte_compressdev_capability_get(uint8_t dev_id, > + const struct rte_compressdev_capability_idx *idx); > + >=20 > /** Macro used at end of comp PMD list */ > #define RTE_COMP_END_OF_CAPABILITIES_LIST() \ > - { RTE_COMP_ALGO_LIST_END } > + { RTE_COMP_UNSPECIFIED } >=20 > +/** Macro used at end of comp hash capability list */ > +#define RTE_COMP_HASH_END_OF_CAPABILITIES_LIST() \ > + { RTE_COMP_HASH_ALG_UNSPECIFIED } >=20 > /** > * compression device supported feature flags > @@ -833,7 +876,7 @@ struct rte_comp_session * > rte_compressdev_queue_pair_detach_session(uint8_t dev_id, uint16_t qp_id= , > struct rte_comp_session *session); > /** > - * This should alloc a stream from the device's mempool and initialise i= t. > + * This should alloc a stream from the device's mempool and initialise i= t. > * This handle will be passed to the PMD with every op in the stream. > * > * @param dev_id The identifier of the device. > @@ -850,7 +893,7 @@ struct rte_comp_session * > */ > int > rte_comp_stream_create(uint8_t dev_id, struct rte_comp_session *sess, > - void ** stream); > + void **stream); >=20 > /** > * This should clear the stream and return it to the device's mempool. > @@ -863,7 +906,7 @@ struct rte_comp_session * > * @return > */ > int > -rte_comp_stream_free(uint8_t dev_id, void * stream); > +rte_comp_stream_free(uint8_t dev_id, void *stream); >=20 > /** > * Provide driver identifier. > diff --git a/lib/librte_compressdev/rte_compressdev_version.map > b/lib/librte_compressdev/rte_compressdev_version.map > index 665f0bf..3114949 100644 > --- a/lib/librte_compressdev/rte_compressdev_version.map > +++ b/lib/librte_compressdev/rte_compressdev_version.map > @@ -5,6 +5,7 @@ EXPERIMENTAL { > rte_compressdev_allocate_driver; > rte_compressdev_callback_register; > rte_compressdev_callback_unregister; > + rte_compressdev_capability_get; > rte_compressdev_close; > rte_compressdev_configure; > rte_compressdev_count; > -- > 1.9.1