Hi Ahmed,

-----Original Message-----
From: Ahmed Mansour [mailto:ahmed.mansour@nxp.com]
Sent: Monday, December 18, 2017 9:44 PM
To: dev@dpdk.org; Shally.Verma@cavium.com
Cc: Mahipal.Challa@cavium.com; NarayanaPrasad.Athreya@cavium.com; De Lara Guarch, Pablo
[off-list ref]; Trahe, Fiona [off-list ref]; Roy Pledge
[off-list ref]; Youri Querry [off-list ref]; Hemant Agrawal
[off-list ref]
Subject: Re: [RFC v3 1/1] lib: add compressdev API

On 12/15/2017 11:19 PM, Trahe, Fiona wrote:
.. <snip>

quoted

+
+/** Compression Algorithms */
+enum rte_comp_algorithm {
+	RTE_COMP_NULL = 0,
+	/**< No compression.
+	 * Pass-through, data is copied unchanged from source buffer to
+	 * destination buffer.
+	 */
+	RTE_COMP_DEFLATE,
+	/**< DEFLATE compression algorithm
+	 *

https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftools.ietf.org%2Fhtml%2Frfc195
1&data=02%7C01%7Cahmed.mansour%40nxp.com%7Cf3edbd70b38b49eb1f0308d54634e444%7C686ea
1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636492115333128845&sdata=B3G0aIncVAK17dXlnSivXi0
e56h2D7pEQZ9gK%2Fh3qZQ%3D&reserved=0

quoted

+	 */
+	RTE_COMP_LZS,
+	/**< LZS compression algorithm
+	 *

https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftools.ietf.org%2Fhtml%2Frfc239
5&data=02%7C01%7Cahmed.mansour%40nxp.com%7Cf3edbd70b38b49eb1f0308d54634e444%7C686ea
1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636492115333128845&sdata=aNRFIfkelXlCUUgpp%2BzC
YaTu28tp6fF0m6k7F13w1Ps%3D&reserved=0

quoted

+	 */
+	RTE_COMP_ALGO_LIST_END
+};
+
+/**< Compression Level.
+ * The number is interpreted by each PMD differently. However, lower numbers
+ * give fastest compression, at the expense of compression ratio while
+ * higher numbers may give better compression ratios but are likely slower.
+ */
+#define	RTE_COMP_LEVEL_PMD_DEFAULT	(-1)
+/** Use PMD Default */
+#define	RTE_COMP_LEVEL_NONE		(0)
+/** Output uncompressed blocks if supported by the specified algorithm */
+#define RTE_COMP_LEVEL_MIN		(1)
+/** Use minimum compression level supported by the PMD */
+#define RTE_COMP_LEVEL_MAX		(9)
+/** Use maximum compression level supported by the PMD */
+
+/** Compression checksum types */
+enum rte_comp_checksum_type {
+	RTE_COMP_NONE,
+	/**< No checksum generated */
+	RTE_COMP_CRC32,
+	/**< Generates a CRC32 checksum, as used by gzip */
+	RTE_COMP_ADLER32,
+	/**< Generates an Adler-32 checksum, as used by zlib */
+	RTE_COMP_CRC32_ADLER32,
+	/**< Generates both Adler-32 and CRC32 checksums, concatenated.
+	 * CRC32 is in the lower 32bits, Adler-32 in the upper 32 bits.
+	 */

What would be a real life use case for returning both CRC32 and ADLER32?
Packaging the data once as Gzip and once as zlib?

[Fiona] We've had requests for this from customers.

quoted

+};
+
+/*
+ * enum rte_comp_hash_algo {
+ *   RTE_COMP_HASH_NONE,
+ *   RTE_COMP_HASH_SHA1,
+ *   RTE_COMP_HASH_SHA256,
+ * };
+ * Need further input from cavium on this
+ * xform will need a flag with above enum value
+ * op will need to provide a virt/phys ptr to a data buffer of appropriate size.
+ * And via capability PMD can say whether supported or not.
+ */
+
+/** Compression Huffman Type - used by DEFLATE algorithm */
+enum rte_comp_huffman {
+	RTE_COMP_DEFAULT,
+	/**< PMD may choose which Huffman codes to use */
+	RTE_COMP_FIXED,
+	/**< Use Fixed Huffman codes */
+	RTE_COMP_DYNAMIC,
+	/**< Use Dynamic Huffman codes */
+};
+
+
+enum rte_comp_flush_flag {
+	RTE_COMP_FLUSH_NONE,
+	/**< Data is not flushed. Output may remain in the compressor and be
+	 * processed during a following op. It may not be possible to decompress
+	 * output until a later op with some other flush flag has been sent.
+	 */
+	RTE_COMP_FLUSH_SYNC,
+	/**< All data should be flushed to output buffer. Output data can be
+	 * decompressed. However state and history is not cleared, so future
+	 * ops may use history from this op */
+	RTE_COMP_FLUSH_FULL,
+	/**< All data should be flushed to output buffer. Output data can be
+	 * decompressed. State and history data is cleared, so future
+	 * ops will be independent of ops processed before this.
+	 */
+	RTE_COMP_FLUSH_FINAL
+	/**< Same as RTE_COMP_FLUSH_FULL but also bfinal bit is set in last block
+	 */
+	 /* TODO:
+	  * describe flag meanings for decompression.
+	  * describe behavous in OUT_OF_SPACE case.
+	  * At least the last flag is specific to deflate algo. Should this be
+	  * called rte_comp_deflate_flush_flag? And should there be
+	  * comp_op_deflate_params in the op? */

What about Z_BLOCK and Z_TREES? Those are needed for sfwr zlib.net
replacements.

[Fiona] We haven't seen a need for those. I would suggest proposing a patch later once the initial API is stabilised. 

quoted

+};
+
+/** Compression transform types */
+enum rte_comp_xform_type {
+	RTE_COMP_COMPRESS,
+	/**< Compression service - compress */
+	RTE_COMP_DECOMPRESS,
+	/**< Compression service - decompress */
+};
+

... <snip>

quoted

+struct rte_comp_session;
+/**
+ * Compression Operation.
+ *
+ * This structure contains data relating to performing a compression
+ * operation on the referenced mbuf data buffers.
+ *
+ * All compression operations are Out-of-place (OOP) operations,
+ * as the size of the output data is different to the size of the input data.
+ *
+ * Comp operations are enqueued and dequeued in comp PMDs using the
+ * rte_compressdev_enqueue_burst() / rte_compressdev_dequeue_burst() APIs
+ */
+struct rte_comp_op {
+
+	enum rte_comp_op_type op_type;
+	void * stream_private;
+	/* location where PMD maintains stream state
+	 * only required if op_type is STATEFUL, else should be NULL
+	 */
+	struct rte_comp_session *session;
+	/**< Handle for the initialised session context */
+	struct rte_mempool *mempool;
+	/**< mempool from which operation is allocated */
+	phys_addr_t phys_addr;
+	/**< physical address of this operation */

iova_addr?

[Fiona] Yes, this should be rte_iova_t.  will fix this in the v1.

quoted

+	struct rte_mbuf *m_src;
+	/**< source mbuf
+	 * The total size of the input buffer(s) can be retrieved using
+	 * rte_pktmbuf_data_len(m_src)
+	 */
+	struct rte_mbuf *m_dst;
+	/**< destination mbuf
+	 * The total size of the output buffer(s) can be retrieved using
+	 * rte_pktmbuf_data_len(m_dst)
+	 */
+
+	struct {
+		uint32_t offset;
+		/**< Starting point for compression or decompression,
+		 * specified as number of bytes from start of packet in
+		 * source buffer.
+		 * Starting point for checksum generation in compress direction.
+		 */

Why should offset have two different meanings for compression and
decompression. It seems that the use case of offset on input is
applicable to both use modes 

[Fiona] It does have the same meaning for compression/decompression?
Just for checksum it's different. Checksum is usually required on the uncompressed data.
In decompression the input buffer contains compressed data.
  

<snip>
 ...