Shally Verma(1234sv) replied on github web page:
include/odp/api/spec/comp.h
line 620
@@ -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
Comment:
... with valid pkt_out and pkt_in being ODP_PACKET_INVALID...
>> No. pkt_in too has to be valid and untouched. It is quite possible, that
>> algorithm may or may not consume full input from input buffer when it hit
>> out of buffer situation. Thus, none of the parameters except output packet
>> should be updated by application until fully process and that's why
>> description says " 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)."
> Shally Verma(1234sv) wrote:
> default values are implementation dependent. There's no standard what they
> could be set to. Example, one implementation may set default comp_alg to zlib
> and other to deflate or possibly any other to zlib+hash. So, we cannot
> document here what default values should be.
>> Shally Verma(1234sv) wrote:
>> Can this return more than one hash_alg_capa (== digest_len) per hash_alg?
>> >>No. ideally, an implementation should return only 1 single capability per
>> >>hash algorithm. I cannot think why a valid implementation would return
>> >>more than 1 capability according to capability structure we have here.
>> This API signature has been retained to keep consistency with other
>> interface capability API.
>>> Shally Verma(1234sv) wrote:
>>> offset is not updated by spec as compression does not support
>>> auto-allocation of packet and starts data write at given offset only. For
>>> description, I rephrased it as below.
>>> " /** Output packet data range.
>>> *
>>> * out_data_range.offset = offset in output packet data buffer to start
>>> * write at,
>>> *
>>> * out_data_range.length = length of available space for output
>>> * from given offset.
>>> *
>>> */
>>>> Shally Verma(1234sv) wrote:
>>>> Ok.
>>>>> Shally Verma(1234sv) wrote:
>>>>> "then we should document that the value is ignored."
>>>>> >> Sorry, I didn't get that. You referring to which value to be ignored?
>>>>>> Shally Verma(1234sv) wrote:
>>>>>> Did you refer to note starting here " * Please note, behavior could be
>>>>>> changed or enhanced........"? if yes, okay I will remove that text.
>>>>>> Please confirm.
>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>> accepted.
>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>> 1.comp_algo is always set. It should be e.g. ALG_NULL by default.
>>>>>>>> >> I am bit confused. Are you suggesting adding a description to "comp
>>>>>>>> >> ALG_NULL+hash"? We agreed in past that " user must use crypto for
>>>>>>>> >> standalone hashing and compression algo NULL is only for testing
>>>>>>>> >> purpose" and current description on usage of hash and ALG_NULL
>>>>>>>> >> abide that. So, we can leave current description as is.
>>>>>>>> 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.
>>>>>>>> >> I will rephrase it to "output *will* always contain data + hash".
>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>> " This should be specified at operation documentation" means which
>>>>>>>>> place? user guide Or ?
>>>>>>>>>
>>>>>>>>> Do you mean I should remove this description from here? Or
>>>>>>>>> "+ * 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."
>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>> 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.
>>>>>>>>>>
>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>> Simply say that:
>>>>>>>>>>>> This operation does de/-compression in synchronous mode
>>>>>>>>>>>> (ODP_COMP_SYNC).
>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>> Shally Verma(1234sv) 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"
>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>> If only option for a stateless algorithm is true, then we
>>>>>>>>>>>>>>>>>>>> should document that the value is ignored.
>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>> Typo: "enques" -> enqueues"
>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>> Remove this block text since it's speculation of what
>>>>>>>>>>>>>>>>>>>>>> could be specified.
>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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).
>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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?
>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> @psavol Hmm. I thought that the user should use this
>>>>>>>>>>>>>>>>>>>>>>>>>> value. And `odp_comp_level_t` should be dropped from
>>>>>>>>>>>>>>>>>>>>>>>>>> the spec.
>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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 ?
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> odp_comp_alg_deflate_param, please.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Please check the see list. It mentions
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> non-existing functions.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> no result here
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> s/Else/Otherwise/
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ... an error....
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ... with valid pkt_out and pkt_in being
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ODP_PACKET_INVALID...
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> It is an error to call this api when
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> session was created in ODP_COMP_ASYNC
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mode.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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 ...
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shally Verma(1234sv) 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_r137739940
updated_at 2017-09-08 08:49:13
