Petri Savolainen(psavol) replied on github web page:

include/odp/api/spec/comp.h
line 689
@@ -0,0 +1,873 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:    BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP Compression
+ */
+
+#ifndef ODP_API_COMP_H_
+#define ODP_API_COMP_H_
+
+#include <odp/visibility_begin.h>
+#include <odp/api/support.h>
+#include <odp/api/packet.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** @defgroup odp_compression ODP COMP
+ *  ODP Compression is an API set to do compression+hash or decompression+hash
+ *  operations on data. Hash is calculated on plaintext.
+ *
+ *  if opcode = ODP_COMP_COMPRESS, then it will apply hash and then compress,
+ *  if opcode = ODP_COMP_DECOMPRESS, then it will decompress and then apply
+ *  hash.
+ *  Independent hash-only operations are not supported. Implementation should
+ *  perform hash along with valid compression algo.
+ *  Macros, enums, types and operations to utilize compression interface.
+ *  @{
+ */
+
+/**
+ * @def ODP_COMP_SESSION_INVALID
+ * Invalid session handle
+ */
+
+/**
+ * @typedef odp_comp_session_t
+ * Compression/Decompression session handle
+ */
+
+/**
+ * Compression API operation mode
+ */
+typedef enum {
+       /** Synchronous, return results immediately */
+       ODP_COMP_SYNC,
+       /** Asynchronous, return results via event queue */
+       ODP_COMP_ASYNC
+} odp_comp_op_mode_t;
+
+/**
+ * Comp API operation type.
+ *
+ */
+typedef enum {
+       /** Compress */
+       ODP_COMP_OP_COMPRESS,
+       /** Decompress */
+       ODP_COMP_OP_DECOMPRESS
+} odp_comp_op_t;
+
+/**
+ * Comp API hash algorithm
+ *
+ */
+typedef enum {
+       /** ODP_COMP_HASH_ALG_NONE - No hash algorithm selected. */
+       ODP_COMP_HASH_ALG_NONE,
+       /** ODP_COMP_HASH_ALG_SHA1 - SHA-1 hash algorithm. */
+       ODP_COMP_HASH_ALG_SHA1,
+       /** ODP_COMP_HASH_ALG_SHA256 - SHA-2 hash algorithm
+        * 256-bit digest length.
+        */
+       ODP_COMP_HASH_ALG_SHA256
+} odp_comp_hash_alg_t;
+
+/**
+ * Comp API compression algorithm
+ *
+ */
+typedef enum {
+       /** No algorithm specified.
+        * Means no compression, output == input.
+        * if provided, no operation (compression/decompression or hash)
+        * applied on input. Added for testing purpose.
+        */
+       ODP_COMP_ALG_NULL,
+       /** DEFLATE - RFC1951 */
+       ODP_COMP_ALG_DEFLATE,
+       /** ZLIB - RFC1950 */
+       ODP_COMP_ALG_ZLIB,
+       /** LZS */
+       ODP_COMP_ALG_LZS
+} odp_comp_alg_t;
+
+/**
+ * Comp API session creation return code
+ *
+ */
+typedef enum {
+       /** Session created */
+       ODP_COMP_SES_CREATE_ERR_NONE,
+       /** Creation failed, no resources */
+       ODP_COMP_SES_CREATE_ERR_ENOMEM,
+       /** Creation failed, bad compression params */
+       ODP_COMP_SES_CREATE_ERR_INV_COMP,
+       /** Creation failed, bad hash params */
+       ODP_COMP_SES_CREATE_ERR_INV_HASH,
+       /** Creation failed,requested configuration not supported*/
+       ODP_COMP_SES_CREATE_ERR_NOT_SUPPORTED
+} odp_comp_ses_create_err_t;
+
+/**
+ * Comp API operation return codes
+ *
+ */
+typedef enum {
+       /** Operation completed successfully*/
+       ODP_COMP_ERR_NONE,
+       /** Operation paused due to insufficient output buffer.
+        *
+        * This is not an error condition. On seeing this situation,
+        * Implementation should maintain context of in-progress operation and
+        * application should call packet processing API again with valid
+        * output buffer but no other alteration to operation params
+        * (odp_comp_op_param_t).
+        *
+        * if using async mode, application should either make sure to
+        * provide sufficient output buffer size OR maintain relevant
+        * context (or ordering) information with respect to each input packet
+        * en-queued for processing.
+        *
+        */
+       ODP_COMP_ERR_OUT_OF_SPACE,
+       /** Invalid user data pointers */
+       ODP_COMP_ERR_DATA_PTR,
+       /** Invalid input data size */
+       ODP_COMP_ERR_DATA_SIZE,
+       /**  Compression Algo failure */
+       ODP_COMP_ERR_ALGO_FAIL,
+       /** Error if operation has been requested in an invalid state */
+       ODP_COMP_ERR_INV_STATE,
+       /** Error if API does not support any of the operational parameter. */
+       ODP_COMP_ERR_NOT_SUPPORTED,
+       /** Error if session is invalid. */
+       ODP_COMP_ERR_INV_SESS
+} odp_comp_err_t;
+
+/**
+ * Comp API enumeration for preferred compression level/speed. Applicable
+ * only for compression operation not decompression.
+ * Value provided defines a trade-off between speed and compression ratio.
+ *
+ * If compression level == ODP_COMP_LEVEL_MIN, output will be produced at
+ * fastest possible rate,
+ *
+ * If compression level == ODP_COMP_LEVEL_MAX, output will be highest possible
+ * compression,
+ *
+ * compression level == ODP_COMP_LEVEL_DEFAULT means implementation will use
+ * its default choice of compression level.
+ *
+ */
+typedef enum {
+       /* Use implementation default */
+       ODP_COMP_LEVEL_DEFAULT = 0,
+       /** Minimum compression (fastest in speed) */
+       ODP_COMP_LEVEL_MIN,
+       /** Maximum compression (slowest in speed) */
+       ODP_COMP_LEVEL_MAX,
+} odp_comp_level_t;
+
+/**
+ * Comp API enumeration for huffman encoding. Valid for compression operation.
+ *
+ */
+typedef enum {
+       /** use implementation default to choose between compression codes  */
+       ODP_COMP_HUFFMAN_CODE_DEFAULT = 0,
+       /** use fixed huffman codes */
+       ODP_COMP_HUFFMAN_CODE_FIXED,
+       /** use dynamic huffman coding */
+       ODP_COMP_HUFFMAN_CODE_DYNAMIC,
+} odp_comp_huffman_code_t;
+
+/**
+ * Hash algorithms in a bit field structure
+ *
+ */
+typedef union odp_comp_hash_algos_t {
+       /** hash algorithms */
+       struct {
+               /** ODP_COMP_HASH_ALG_SHA1 */
+               uint32_t sha1  : 1;
+
+               /** ODP_COMP_HASH_ALG_SHA256 */
+               uint32_t sha256 : 1;
+
+       } bit;
+
+       /** All bits of the bit field structure
+        *
+        * This field can be used to set/clear all flags, or bitwise
+        * operations over the entire structure.
+        */
+       uint32_t all_bits;
+} odp_comp_hash_algos_t;
+
+/**
+ * Comp algorithms in a bit field structure
+ *
+ */
+typedef union odp_comp_algos_t {
+       /** Compression algorithms */
+       struct {
+               /** ODP_COMP_ALG_NULL */
+               uint32_t null       : 1;
+
+               /** ODP_COMP_ALG_DEFLATE */
+               uint32_t deflate    : 1;
+
+               /** ODP_COMP_ALG_ZLIB */
+               uint32_t zlib       : 1;
+
+               /** ODP_COMP_ALG_LZS */
+               uint32_t lzs        :1;
+       } bit;
+
+       /** All bits of the bit field structure
+        * This field can be used to set/clear all flags, or bitwise
+        * operations over the entire structure.
+        */
+       uint32_t all_bits;
+} odp_comp_algos_t;
+
+/**
+ * Compression Interface Capabilities
+ *
+ */
+typedef struct odp_comp_capability_t {
+       /** Maximum number of  sessions */
+       uint32_t max_sessions;
+
+       /** Supported compression algorithms */
+       odp_comp_algos_t comp_algos;
+
+       /** Supported hash algorithms. */
+       odp_comp_hash_algos_t hash_algos;
+
+       /** Support type for synchronous operation mode (ODP_COMP_SYNC).
+        *  User should set odp_comp_session_param_t:mode based on
+        *  support level as indicated by this param.
+        */
+       odp_support_t sync;
+
+       /** Support type for asynchronous operation mode (ODP_COMP_ASYNC).
+        *  User should set odp_comp_session_param_t:mode param based on
+        *  support level as indicated by this param.
+        */
+       odp_support_t async;
+} odp_comp_capability_t;
+
+/**
+ * Hash algorithm capabilities
+ *
+ */
+typedef struct odp_comp_hash_alg_capability_t {
+       /** Digest length in bytes */
+       uint32_t digest_len;
+} odp_comp_hash_alg_capability_t;
+
+/**
+ * Compression algorithm capabilities structure for each algorithm.
+ *
+ */
+typedef struct odp_comp_alg_capability_t {
+       /** Enumeration indicating algorithm support for dictionary load */
+       odp_support_t support_dict;
+
+       /** Optional Maximum length of dictionary supported
+        *  by implementation of an algorithm.
+        *
+        *  Invalid if support_dict == ODP_SUPPORT_NO.
+        *
+        *  Implementation use dictionary of length less than or equal to value
+        *  indicated by dict_len. if set to 0 and if support_dict ==
+        *  ODP_SUPPORT_YES, then implementation will use dictionary length
+        *  less than or equal to user input length in odp_comp_set_dict()
+        *  and update used dictionary length at output of the call.
+        *
+        */
+       uint32_t dict_len;
+
+       /** Maximum compression level supported by implementation of this algo.
+        * Indicates number of compression levels supported by implementation,
+        *
+        * where,
+        *
+        * 1 means fastest compression i.e. output produced at
+        * best possible speed at the expense of compression quality, and
+        *
+        * max_level means best compression i.e.output produced is best possible
+        * compressed content at the expense of speed.
+        *
+        * Example, if max_level = 4 , it means algorithm supports four levels
+        * of compression from value 1 up to 4. User can set this value from
+        * 1 (fastest compression) to 4 (best compression).
+        * See RFC1950 for an example explanation to level.
+        *
+        * Value 0 mean implementation use its default value.
+        *
+        */
+       uint32_t max_level;
+
+       /* Supported hash algorithms */
+       odp_comp_hash_algos_t hash_algo;
+} odp_comp_alg_capability_t;
+
+/**
+ * Comp API dictionary type
+ * Consists of pointer to byte buffer. length of dictionary
+ * indicated by length parameter.
+ */
+typedef struct odp_comp_dict_t {
+       /** pointer to byte array */
+       uint8_t *buf;
+       /** length of the dictionary. */
+       uint32_t len;
+} odp_comp_dict_t;
+
+/**
+ * Comp API algorithm specific parameters
+ *
+ */
+typedef struct odp_comp_algo_param_t {
+       /** struct for defining deflate algorithm parameters.
+       * Also initialized by other deflate based algorithms , ex. ZLIB
+       */
+       struct comp_alg_def_param {
+               /** compression level where
+                * ODP_COMP_LEVEL_MIN <= level <= ODP_COMP_LEVEL_MAX
+                */
+               odp_comp_level_t level;
+               /** huffman code to use */
+               odp_comp_huffman_code_t comp_code;
+       } deflate;
+
+       /** struct for defining zlib algorithm parameters.
+        */
+       struct comp_alg_zlib_param {
+                       /** deflate algo params */
+                       struct comp_alg_def_param def;
+       } zlib;
+} odp_comp_alg_param_t;
+
+ /**
+ * Comp API session creation parameters
+ *
+ */
+typedef struct odp_comp_session_param_t {
+       /** Compress vs. Decompress operation */
+       odp_comp_op_t op;
+
+       /** Sync vs Async mode
+        *
+        * When mode = ODP_COMP_SYNC, odp_comp_xxx()
+        * should be called.
+        *
+        * When mode = ODP_COMP_ASYNC, odp_comp_xxx_enq()
+        * should be called.
+        *
+        * Use odp_comp_capability() for supported mode.
+        *
+        */
+       odp_comp_op_mode_t mode;
+
+       /** Compression algorithm
+        *
+        *  Use odp_comp_capability() for supported algorithms.
+        */
+       odp_comp_alg_t comp_algo;
+
+       /** Hash algorithm
+        *
+        *  Use odp_comp_alg_capability() for supported hash algo for
+        *  compression algo given as comp_alg. Implementation should not
+        *  support hash only operation on data. output should always contain
+        *  data + hash.
+        *
+        */
+       odp_comp_hash_alg_t hash_algo;
+
+       /** parameters specific to compression */
+       odp_comp_alg_param_t algo_param;
+
+       /** Async mode completion event queue
+        *
+        * When mode = ODP_COMP_ASYNC, user should wait on ODP_EVENT_PACKET
+        * with subtype ODP_EVENT_PACKET_COMP on this queue.
+        *
+        * By default, implementation enques completion events in-order-of
+        * request submission and thus queue is considered ordered.
+        *
+        * Please note, behavior could be changed or enhanced
+        * to queue event in-order-of their completion to enable
+        * performance-oriented application to leverage hw offered parallelism.
+        * However, this will be subject to application requirement and more
+        * explicit defined use-case.
+        *
+        */
+       odp_queue_t compl_queue;
+} odp_comp_session_param_t;
+
+/**
+ * Comp API per packet operation result
+ *
+ */
+typedef struct odp_comp_packet_op_result_t {
+       /** Operation Return Code */
+       odp_comp_err_t err;
+
+       /** Output packet data range. Where,
+        *
+        *  offset = base offset within packet, current data is written at.
+        *
+        *  length = length of data written.
+        *
+        */
+       odp_packet_data_range_t out_data_range;
+} odp_comp_packet_op_result_t;
+
+/**
+ * Comp packet API per packet operation parameters
+ */
+typedef struct odp_comp_packet_op_param_t {
+       /** Session handle from creation */
+       odp_comp_session_t session;
+
+       /** Boolean indicating if current input is last chunk of data, where
+        *
+        *   true : last chunk
+        *
+        *   false: more to follow
+        *
+        * If set to true, indicates this is the last chunk of
+        * data. After processing of last chunk of data is complete i.e.
+        * call returned with any error code except ODP_COMP_ERR_OUT_OF_SPACE,
+        * implementation should move algorithm to stateless mode
+        * for next of batch of operation i.e. reset history,
+        * insert 'End of Block' marker into compressed data stream(if
+        * supported by algo).See deflate/zlib for interpretation of
+        * stateless/stateful.
+        *
+        * For stateless compressions (ex ipcomp), last should be set to 'true'
+        * for every input packet processing call.
+        *
+        * For compression + hash, digest will be available after
+        * last chunk is processed completely. In case of
+        * ODP_COMP_ERR_OUT_OF_SPACE, application should keep on calling
+        * odp_comp_xxx() API with more output buffer unless call returns
+        * with ODP_COMP_ERR_NONE or other failure code except
+        *  ODP_COMP_ERR_OUT_OF_SPACE.
+        */
+       odp_bool_t last;
+
+       /** Input data range to process */
+       odp_packet_data_range_t in_data_range;
+
+       /** Output packet data range.
+        * Indicates where processed packet will be written
+        * where length specifies,
+        * length of available packet buffer at input, and
+        * length of data written at output
+        */
+       odp_packet_data_range_t out_data_range;
+} odp_comp_packet_op_param_t;
+
+/**
+ * Query comp capabilities
+ *
+ * Output comp capabilities on success.
+ *
+ * @param[out] capa   Pointer to capability structure for output
+ *
+ * @retval 0 on success
+ * @retval <0 on failure
+ */
+int odp_comp_capability(odp_comp_capability_t *capa);
+
+/**
+ * Query supported compression algorithm capabilities
+ *
+ * Output algorithm capabilities.
+ *
+ * @param      comp     Compression algorithm
+ * @param[out] capa     Array of capability structures for output
+ * @param      num     Maximum number of capability structures to output
+ *
+ * @return Number of capability structures for the algorithm. If this is larger
+ *         than 'num', only 'num' first structures were output and application
+ *         may call the function again with a larger value of 'num'.
+ * @retval <0 on failure
+ */
+int odp_comp_alg_capability(odp_comp_alg_t comp,
+                           odp_comp_alg_capability_t capa[], int num);
+
+/**
+ * Query supported hash algorithm capabilities
+ *
+ * Outputs all supported configuration options for the algorithm.
+ *
+ * @param      hash     Hash algorithm
+ * @param[out] capa     Array of capability structures for output
+ * @param      num      Maximum number of capability structures to output
+ *
+ * @return Number of capability structures for the algorithm. If this is larger
+ *         than 'num', only 'num' first structures were output and application
+ *         may call the function again with a larger value of 'num'.
+ * @retval <0 on failure
+ */
+int odp_comp_hash_alg_capability(odp_comp_hash_alg_t hash,
+                                odp_comp_hash_alg_capability_t capa[],
+                                int num);
+
+/**
+ * Initialize comp session parameters
+ *
+ * Initialize an odp_comp_session_param_t to its default values for
+ * all fields.
+ *
+ * @param param   Pointer to odp_comp_session_param_t to be initialized
+ */
+void odp_comp_session_param_init(odp_comp_session_param_t *param);
+
+/**
+ * Compression session creation
+ *
+ * Create a comp session according to the session parameters. Use
+ * odp_comp_session_param_init() to initialize parameters into their
+ * default values.
+ *
+ * @param param             Session parameters
+ * @param session           Created session else ODP_COMP_SESSION_INVALID
+ * @param status            Failure code if unsuccessful
+ *
+ * @retval 0 on success
+ * @retval <0 on failure
+ */
+int odp_comp_session_create(odp_comp_session_param_t *param,
+                           odp_comp_session_t *session,
+                           odp_comp_ses_create_err_t *status);
+
+/**
+ * Comp session destroy
+ *
+ * Destroy an unused session. Result is undefined if session is being used
+ * (i.e. asynchronous operation is in progress).
+ *
+ * @param session           Session handle
+ *
+ * @retval 0 on success
+ * @retval <0 on failure
+ */
+int odp_comp_session_destroy(odp_comp_session_t session);
+
+/**
+ * Comp set dictionary
+ *
+ * Should be called when there is no operation in progress i.e.
+ * before initiating processing of first chunk of data and
+ * after processing of last chunk of data is complete.
+ *
+ * @param session           Session handle
+ * @param dict[in,out]      Pointer to dictionary.
+ *                          implementation should update length of dictionary
+ *                          used at output.
+ * @retval 0 on success
+ * @retval <0 on failure
+ *
+ * @note:
+ * Application should call odp_comp_alg_capability() to query 'support_dict'
+ * before making call to this API.
+ */
+int odp_comp_set_dict(odp_comp_session_t session,
+                     odp_comp_dict_t *dict);
+
+/**
+ * Comp API to process a packet synchronously
+ *
+ * If session is created in ODP_COMP_SYNC mode, this call wait for operation
+ * to complete.
+ *
+ * If session is created in ODP_COMP_ASYNC mode, behavior is undefined.
+ *
+ * For compression + hash, call returns with hash appended at the end of
+ * packet which has last field set in corresponding param.
+ *
+ * User should compute processed data len = output_data_range.len - digest_len,
+ * where digest_len queried through odp_comp_hash_alg_capability().
+ *
+ * For every input packet, user should pass corresponding valid output
+ * packet.
+ *
+ * For every successful or partially processed packet, api update result param
+ * with out_data_range indicating length of the data written in output packet
+ * and error code indicating operation status.
+ *
+ * A packet is said to be partially processed if it runs out of buffer during
+ * processing and need more buffer from user to continue. Under such condition,
+ * API fails with error code code in result param set to
+ * ODP_COMP_ERR_OUT_OF_SPACE.
+ *
+ * In case of ODP_COMP_ERR_OUT_OF_SPACE error, application should
+ * keep on calling odp_comp_op() API with more output buffer until call returns
+ * with success or any other error code.
+ *
+ * @param         pkt_in   Input Packet
+ * @param         pkt_out  Output Packet
+ * @param         param    Operation parameter
+ * @param[out] result   Result parameter, containing after operation result.
+ *
+ * @return 0 on success
+ * @retval <0 on failure
+ */
+int odp_comp_op(const odp_packet_t pkt_in,
+               const odp_packet_t pkt_out,
+               const odp_comp_packet_op_param_t *param,
+               odp_comp_packet_op_result_t *result);
+
+/**
+ * Comp API to process a packet asynchronously
+ *
+ * If session is created in ODP_COMP_ASYNC mode, result will be queued
+ * to completion queue. Application should monitor ODP_EVENT_PACKET
+ * with subtype ODP_EVENT_PACKET_COMP on queue.
+ *
+ * If session is created in ODP_COMP_SYNC mode, behavior is undefined.
+ *
+ * For compression + hash, call returns with hash appended at the end of
+ * packet which has last field set in corresponding param.
+ *
+ * User should compute processed data len = output_data_range.len - digest_len,
+ * where digest_len queried through odp_comp_hash_alg_capability().
+ *
+ * For every input packet, user should pass corresponding valid output
+ * packet.
+ *
+ * If API returns with success, application should monitor for
+ * ODP_EVENT_PACKET with subtype set to ODP_EVENT_PACKET_COMP on
+ * completion queue provided during session create.
+ * Once event is received, application should call odp_comp_result() to
+ * retrieve result corresponding to operation done on packet.
+ *
+ * For every successful or partially processed packet, api update result param
+ * with out_data_range indicating length of the data written in output packet
+ * and error code indicating operation status.
+ *
+ * A packet is said to be partially processed if it runs out of buffer during
+ * processing and need more buffer from user to continue. Under such condition,
+ * API fails with error code code in result param set to
+ * ODP_COMP_ERR_OUT_OF_SPACE.
+ *
+ * In case of ODP_COMP_ERR_OUT_OF_SPACE error, application should
+ * keep on calling odp_comp_op_enq() API with more output buffer until call
+ * returns with success or any other error code.
+ *
+ * Please note it is always recommended that application using async mode,
+ * provide sufficiently large buffer size to avoid ODP_COMP_ERR_OUT_OF_SPACE.
+ * Else it is recommended that application set required user context on packet
+ * before calling each odp_comp_op_enq() to correctly retrieve required
+ * information associated with packet.
+ *
+ * @param         pkt_in   Input Packet
+ * @param         pkt_out  Output Packet
+ * @param         param    Operation parameter
+ *
+ * @return 0 on success
+ * @retval <0 on failure
+ */
+int odp_comp_op_enq(const odp_packet_t pkt_in,
+                   const odp_packet_t pkt_out,
+                   const odp_comp_packet_op_param_t *param);
+


Comment:
First, async call can reuse sync call spec by saying: "... processes packets 
otherwise identically to odp_comp_op(), but outputs resulting packets as 
ODP_EVENT_PACKET events ...". Then the remaining text can concentrate on what 
is unique on async vs sync operation.

I think OUT_OF_SPACE spec might need re-thinking:
1) user sees a partial packet (error on a received packet)
2) implementation waits that user calls op_enq() again
3) what input packet should be given on the new call? I think the previous 
operation already consumed the input packet which contains the input data for 
operation. Did implementation store it?
4) how long implementation waits for new output packet to arrive from user, 
what if user don't send new output packets.

Async spec (and also sync) spec would be more robust if implementation would 
allocate output packets. 
 


> Petri Savolainen(psavol) wrote:
> Application does not allocate buffers but packets.
> 
> Does the partially processed packet contain valid data or not == does 
> application keep or discard the packet?
> 
> Is there any way for application to prepare large enough packet in advance? 
> E.g. a per algo capability of worst case decompression ratio? Should "pkt_out 
> = INVALID" be supported? ==> implementation allocates large enough packet.
> 
> Single packet handle should **not** be defined as const.
> 
> There is out_data_range defined in both op_param_t and op_result_t. Which one 
> is updated on output? May be operation output comments should be removed from 
> op_param_t documentation.
>> Petri Savolainen(psavol) wrote:
>> Simply say that:
>> This operation does de/-compression in synchronous mode (ODP_COMP_SYNC).
>>> Petri Savolainen(psavol) wrote:
>>> "implementation should update length of dictionary used at output."
>>> This is too vague: either length is updated or it's not. In which cases the 
>>> update would be done and what that would mean? Also, I assume that 
>>> implementation copies the data, so that application is free to overwrite 
>>> the memory after the call. That (copies data) should be mentioned also.
>>> 
>>> Is there a benefit on output param here, especially if it updates only the 
>>> length and not the pointer. Maybe API would be cleaner, if also *dict is 
>>> input only param (and thus const*). Function may return e.g. number of 
>>> bytes copied/used.
>>>> Petri Savolainen(psavol) wrote:
>>>> This can be simply
>>>> 
>>>> odp_comp_session_t odp_comp_session_create(const odp_comp_session_param_t 
>>>> *param)
>>>> 
>>>> Extra error codes may be returned through odp_errno(). Usually, we have 
>>>> not defined error codes, since it's implementation defines what all things 
>>>> can possibly go wrong, and typically application logic is just interested 
>>>> on success vs fail - not so much why we failed.
>>>> 
>>>> This function prototype is copy-paste from crypto API, but all other APIs 
>>>> are the type I suggested above.
>>>> 
>>>> Note: const pointer for "param"
>>>>> Petri Savolainen(psavol) wrote:
>>>>> It would be good to define default values for odp_comp_session_param_t 
>>>>> (at the type documentation). Otherwise, application needs to always set 
>>>>> every param.
>>>>>> Petri Savolainen(psavol) wrote:
>>>>>> Can this return more than one hash_alg_capa (== digest_len) per 
>>>>>> hash_alg? Currently, application cannot select digest length. So, what 
>>>>>> application should do if this returns more than one capa.
>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>> Is array of capabilities needed with comp algorithms? It would be 
>>>>>>> needed if user would need to select between multiple discrete options - 
>>>>>>> e.g. if only some dict_len values would make sense (1kB, 4kB, 16kB), 
>>>>>>> but nothing in between.
>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>> "where length specifies,
>>>>>>>> length of available packet buffer at input, and
>>>>>>>> length of data written at output"
>>>>>>>> 
>>>>>>>> This specification should be clarified. "Input" refers refers to 
>>>>>>>> out_data_range.length as operation input param and  "output" refers to 
>>>>>>>> the same as operation output parameter.
>>>>>>>> 
>>>>>>>> In addition to length, offset may be updated at operation output (at 
>>>>>>>> least when new packet was allocated). That should be mentioned also.
>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>> This last chuck: 
>>>>>>>>> "In case of
>>>>>>>>> ODP_COMP_ERR_OUT_OF_SPACE, application should keep on calling
>>>>>>>>> odp_comp_xxx() API with more output buffer unless call returns
>>>>>>>>> with ODP_COMP_ERR_NONE or other failure code except
>>>>>>>>> ODP_COMP_ERR_OUT_OF_SPACE. "
>>>>>>>>> 
>>>>>>>>> Documents operation return / error codes rather than "last" field 
>>>>>>>>> usage. So, I'd remove it from here.
>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>> If only option for a stateless algorithm is true, then we should 
>>>>>>>>>> document that the value is ignored.
>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>> Typo: "enques" -> enqueues"
>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>> Remove this block text since it's speculation of what could be 
>>>>>>>>>>>> specified.
>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>> Typo: "... given as comp_alg" -> "... given as comp_algo"
>>>>>>>>>>>>> 
>>>>>>>>>>>>> There are two SHOULDs, but I think those are not needed:
>>>>>>>>>>>>> 1) comp_algo is always set. It should be e.g. ALG_NULL by 
>>>>>>>>>>>>> default. We have defined that NULL does not have hash capability. 
>>>>>>>>>>>>> It would be also possible to leave NULL + hash to be 
>>>>>>>>>>>>> implementation defined capability. When we say that ALG_NULL is 
>>>>>>>>>>>>> only for test, it should be enough to discourage people from 
>>>>>>>>>>>>> misusing this API for hashing.
>>>>>>>>>>>>> 2) What this means: "output should always contain data + hash" ? 
>>>>>>>>>>>>> If we enable hashing, hash is always calculated. It's not 
>>>>>>>>>>>>> optional for implementation to calculate it - right.
>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>> Field name should match the type == "hash_algos" (many 
>>>>>>>>>>>>>> algorithms in a bit field). Underneath "hash_algo" is used with 
>>>>>>>>>>>>>> enum odp_comp_hash_alg_t (one algo).
>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>> I did comment the same a month back:
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Is the data input only? If it is, then this should be "const 
>>>>>>>>>>>>>>> uint8_t *buf"
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> What is expected about the format of the data? Null terminated 
>>>>>>>>>>>>>>> strings? If it's strings, then e.g. "const char *str" would be 
>>>>>>>>>>>>>>> better.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Also the type documentation should be updated:
>>>>>>>>>>>>>>> * This is not dictionary type, but pointer into dictionary data 
>>>>>>>>>>>>>>> - right?
>>>>>>>>>>>>>>> * "Consists of pointer to byte buffer. length of dictionary 
>>>>>>>>>>>>>>> indicated by length parameter. ", this needs clarification.  
>>>>>>>>>>>>>>> What is expected about the data behind the pointer?
>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>> @psavol Hmm. I thought that the user should use this value. 
>>>>>>>>>>>>>>>> And `odp_comp_level_t` should be dropped from the spec.
>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>> How this relates to odp_comp_level_t enumeration ? Shouldn't 
>>>>>>>>>>>>>>>>> user use those levels and not a number from 0 to this one ?
>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>> This is description of how de/-compression operation work. 
>>>>>>>>>>>>>>>>>> This should be specified at operation documentation, and 
>>>>>>>>>>>>>>>>>> optimally only there. Otherwise, we are in danger of this 
>>>>>>>>>>>>>>>>>> spec getting deprecated (== in conflict with operation 
>>>>>>>>>>>>>>>>>> documentation) without noticing. 
>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>> True, all types need to be odp_ prefixed. Also this type 
>>>>>>>>>>>>>>>>>>> would be better to typedef and pull out (from inside of 
>>>>>>>>>>>>>>>>>>> algo_param_t) since it's used in two places already.
>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>> As a side note: it might be logical, we might want to add 
>>>>>>>>>>>>>>>>>>>> `odp_support_t` field to notify user if implementation 
>>>>>>>>>>>>>>>>>>>> supports stateful compression. I can easily assume that 
>>>>>>>>>>>>>>>>>>>> there might be implementations implementing only stateless 
>>>>>>>>>>>>>>>>>>>> compression. 
>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>> odp_comp_alg_deflate_param, please.
>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>> Please check the see list. It mentions non-existing 
>>>>>>>>>>>>>>>>>>>>>> functions.
>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>> no result here
>>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>>> s/Else/Otherwise/
>>>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> ... an error....
>>>>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> ... with valid  pkt_out and pkt_in being 
>>>>>>>>>>>>>>>>>>>>>>>>>> ODP_PACKET_INVALID...
>>>>>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>> It is an error to call this api when session was 
>>>>>>>>>>>>>>>>>>>>>>>>>>> created in ODP_COMP_ASYNC mode.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> As said, single and multi versions are possible, 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> but not as proposed in this patch. Both versions 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> should have the same param structure and 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> specification. Only difference would be pkt_in[] 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> vs pkt_in, pkt_out[] vs pkt_out, param[] vs. param 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Then I propose to add a new API like
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> odp_comp_compress_multi(const odp_packet_t 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> pkt_in[], odp_packet_t pkt_out[], const 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> odp_comp_compress_param_t param[], int num_pkt);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> which takes of handling multiple packets and 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> retain current version for single packet handling.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> @bala-manoharan 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Results for both sync and async through result 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> function:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> int odp_comp_result(odp_comp_packet_result_t 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> *result, odp_packet_t packet);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> OR have the results as output parameter. Results 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> function is needed any way for the async 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> operation.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> int odp_comp_compress(const odp_packet_t 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> pkt_in[], odp_packet_t pkt_out[], const 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> odp_comp_compress_param_t param[], int num_pkt);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Single packet version may be also added, but is 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> not necessary since couple of additional 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> if-clauses does not significant overhead when 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> compare CPU cycles of the entire comp operation.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> int odp_comp_compress_single(odp_packet_t 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> pkt_in, odp_packet_t pkt_out, const 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> odp_comp_compress_param_t param);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The two prototypes would be the same except [] 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> and num_pkt.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
https://github.com/Linaro/odp/pull/156#discussion_r137543160
updated_at 2017-09-07 13:48:44

Reply via email to