Petri Savolainen(psavol) replied on github web page:
include/odp/api/spec/comp.h
line 397
@@ -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;
Comment:
Should is not must. So, "Implementation should not support hash only operation
on data" does not specify anything concrete. Implementation can still offer
hash with NULL.
I think we can leave it as a capability (we have the API for that), but say
that NULL algo is for testing purposes only == not performance optimized etc.
> Petri Savolainen(psavol) wrote:
> E.g. move to odp_comp_op() documentation (in this file) and then refer to
> that from other comp operation functions.
>> Petri Savolainen(psavol) wrote:
>> It looks like OUT_OF_SPACE causes too much controversity ATM. I would
>> suggest to make it a hard error at this point, merge API w/o out-of-space,
>> then rethink this particular item. Partially processed packets need
>> additional thought with respect to state being saved/passed back/etc.
>>> Petri Savolainen(psavol) wrote:
>>> Does the partially processed packet contain valid data or not == does
>>> application keep or discard the packet?
>>> Shally > It will be a valid data and thus application is supposed to
>>> consume it.
>>> Is there any way for application to prepare large enough packet in advance?
>>> Shally > Its more of application dependent such as ipcomp it knows packet
>>> cannot go beyond a maximum length of IP Packet.
>>> E.g. a per algo capability of worst case decompression ratio?
>>> Shally > No. it is not guaranteed. Its more of data type property and
>>> algorithm capability.
>>> Should "pkt_out = INVALID" be supported?
>>> Shally > No.
>>> implementation allocates large enough packet.
>>> Shally > we refrained from this approach as in such case implementation may
>>> end up eating whole of user pool. Better approach is pass back what ever is
>>> available at output. Now, application may consume it and reuse same packet
>>> for next o/p OR extend the packet by some length (say double it up).
>>> Single packet handle should be defined as const.
>>> Shally > It is const in API param list ... odp_comp_op(const odp_packet_t
>>> pkt_in....) .. what am i missing here?
>>> There is out_data_range defined in both op_param_t and op_result_t. Which
>>> one is updated on output?
>>> Shally> One in op_result.
>>> May be operation output comments should be removed from op_param_t
>>> documentation.
>>> Shally > Yea. Did that already.
>>>
>>> Please mark your approval to these.
>>>> Petri Savolainen(psavol) wrote:
>>>> ... 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)."
>>>>> Petri Savolainen(psavol) 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.
>>>>>> Petri Savolainen(psavol) 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.
>>>>>>> Petri Savolainen(psavol) 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.
>>>>>>> *
>>>>>>> */
>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>> Ok.
>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>> "then we should document that the value is ignored."
>>>>>>>>> >> Sorry, I didn't get that. You referring to which value to be
>>>>>>>>> >> ignored?
>>>>>>>>>> Petri Savolainen(psavol) 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.
>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>> accepted.
>>>>>>>>>>>> Petri Savolainen(psavol) 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".
>>>>>>>>>>>>> Petri Savolainen(psavol) 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."
>>>>>>>>>>>>>> Petri Savolainen(psavol) 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.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 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_r138057465
updated_at 2017-09-11 12:42:57
