Shally Verma(1234sv) replied on github web page:

include/odp/api/spec/comp.h
line 461
@@ -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.


Comment:
"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_r137722674
updated_at 2017-09-08 07:11:14

Reply via email to