[dpdk-dev] [PATCH v1 0/6] bbdev update related to CRC usage

[dpdk-dev] [PATCH v1 0/6] bbdev update related to CRC usage

[Nicolas Chautru]

Capturing in this serie minor change to bbdev to support a more 
comprehensive set of options for CRC used for 4G and 5G (exposed 
as optional capabilities).
This also includes clarifications in documention and minor 
update to log print.

Nicolas Chautru (6):
  bbdev: add capability for CRC16 check
  baseband/turbo_sw: add support for CRC16
  bbdev: add capability for 4G CB CRC DROP
  baseband/acc100: add support for 4G CRC drop
  doc: clarification of usage of HARQ in bbdev doc
  bbdev: reduce warning level for one scenario

 app/test-bbdev/test_bbdev_vector.c               |  4 +++
 doc/guides/bbdevs/acc100.rst                     |  1 +
 doc/guides/prog_guide/bbdev.rst                  | 26 ++++++++++++++++
 doc/guides/rel_notes/release_21_11.rst           |  8 +++++
 drivers/baseband/acc100/rte_acc100_pmd.c         | 12 ++++++--
 drivers/baseband/turbo_sw/bbdev_turbo_software.c | 17 +++++++++++
 lib/bbdev/rte_bbdev.c                            |  7 +++--
 lib/bbdev/rte_bbdev_op.h                         | 39 +++++++++++++-----------
 8 files changed, 91 insertions(+), 23 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16 check

[Nicolas Chautru]

Adding a missing operation when CRC16
is being used for TB CRC check.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 app/test-bbdev/test_bbdev_vector.c     |  2 ++
 doc/guides/prog_guide/bbdev.rst        |  3 +++
 doc/guides/rel_notes/release_21_11.rst |  1 +
 lib/bbdev/rte_bbdev_op.h               | 34 ++++++++++++++++++----------------
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
index 614dbd1..8d796b1 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -167,6 +167,8 @@
 		*op_flag_value = RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK;
 	else if (!strcmp(token, "RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP"))
 		*op_flag_value = RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP;
+	else if (!strcmp(token, "RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK"))
+		*op_flag_value = RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK;
 	else if (!strcmp(token, "RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS"))
 		*op_flag_value = RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS;
 	else if (!strcmp(token, "RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE"))
diff --git a/doc/guides/prog_guide/bbdev.rst b/doc/guides/prog_guide/bbdev.rst
index 9619280..8bd7cba 100644
--- a/doc/guides/prog_guide/bbdev.rst
+++ b/doc/guides/prog_guide/bbdev.rst
@@ -891,6 +891,9 @@ given below.
 |RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP                                    |
 | Set to drop the last CRC bits decoding output                      |
 +--------------------------------------------------------------------+
+|RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK                                    |
+| Set for code block CRC-16 checking                                 |
++--------------------------------------------------------------------+
 |RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS                                 |
 | Set for bit-level de-interleaver bypass on input stream            |
 +--------------------------------------------------------------------+
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index d707a55..69dd518 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -84,6 +84,7 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* bbdev: Added capability related to more comprehensive CRC options.
 
 ABI Changes
 -----------
diff --git a/lib/bbdev/rte_bbdev_op.h b/lib/bbdev/rte_bbdev_op.h
index f946842..7c44ddd 100644
--- a/lib/bbdev/rte_bbdev_op.h
+++ b/lib/bbdev/rte_bbdev_op.h
@@ -142,51 +142,53 @@ enum rte_bbdev_op_ldpcdec_flag_bitmasks {
 	RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK = (1ULL << 1),
 	/** Set to drop the last CRC bits decoding output */
 	RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP = (1ULL << 2),
+	/** Set for transport block CRC-16 checking */
+	RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK = (1ULL << 3),
 	/** Set for bit-level de-interleaver bypass on Rx stream. */
-	RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS = (1ULL << 3),
+	RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS = (1ULL << 4),
 	/** Set for HARQ combined input stream enable. */
-	RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE = (1ULL << 4),
+	RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE = (1ULL << 5),
 	/** Set for HARQ combined output stream enable. */
-	RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE = (1ULL << 5),
+	RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE = (1ULL << 6),
 	/** Set for LDPC decoder bypass.
 	 *  RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE must be set.
 	 */
-	RTE_BBDEV_LDPC_DECODE_BYPASS = (1ULL << 6),
+	RTE_BBDEV_LDPC_DECODE_BYPASS = (1ULL << 7),
 	/** Set for soft-output stream enable */
-	RTE_BBDEV_LDPC_SOFT_OUT_ENABLE = (1ULL << 7),
+	RTE_BBDEV_LDPC_SOFT_OUT_ENABLE = (1ULL << 8),
 	/** Set for Rate-Matching bypass on soft-out stream. */
-	RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS = (1ULL << 8),
+	RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS = (1ULL << 9),
 	/** Set for bit-level de-interleaver bypass on soft-output stream. */
-	RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS = (1ULL << 9),
+	RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS = (1ULL << 10),
 	/** Set for iteration stopping on successful decode condition
 	 *  i.e. a successful syndrome check.
 	 */
-	RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE = (1ULL << 10),
+	RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE = (1ULL << 11),
 	/** Set if a device supports decoder dequeue interrupts. */
-	RTE_BBDEV_LDPC_DEC_INTERRUPTS = (1ULL << 11),
+	RTE_BBDEV_LDPC_DEC_INTERRUPTS = (1ULL << 12),
 	/** Set if a device supports scatter-gather functionality. */
-	RTE_BBDEV_LDPC_DEC_SCATTER_GATHER = (1ULL << 12),
+	RTE_BBDEV_LDPC_DEC_SCATTER_GATHER = (1ULL << 13),
 	/** Set if a device supports input/output HARQ compression. */
-	RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION = (1ULL << 13),
+	RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION = (1ULL << 14),
 	/** Set if a device supports input LLR compression. */
-	RTE_BBDEV_LDPC_LLR_COMPRESSION = (1ULL << 14),
+	RTE_BBDEV_LDPC_LLR_COMPRESSION = (1ULL << 15),
 	/** Set if a device supports HARQ input from
 	 *  device's internal memory.
 	 */
-	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE = (1ULL << 15),
+	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE = (1ULL << 16),
 	/** Set if a device supports HARQ output to
 	 *  device's internal memory.
 	 */
-	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE = (1ULL << 16),
+	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE = (1ULL << 17),
 	/** Set if a device supports loop-back access to
 	 *  HARQ internal memory. Intended for troubleshooting.
 	 */
-	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_LOOPBACK = (1ULL << 17),
+	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_LOOPBACK = (1ULL << 18),
 	/** Set if a device includes LLR filler bits in the circular buffer
 	 *  for HARQ memory. If not set, it is assumed the filler bits are not
 	 *  in HARQ memory and handled directly by the LDPC decoder.
 	 */
-	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS = (1ULL << 18)
+	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS = (1ULL << 19)
 };
 
 /** Flags for LDPC encoder operation and capability structure */
-- 
1.8.3.1

[dpdk-dev] [PATCH v1 2/6] baseband/turbo_sw: add support for CRC16

[Nicolas Chautru]

This is to support the case for operation
where CRC16 is to be appended or checked.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 doc/guides/rel_notes/release_21_11.rst           |  3 +++
 drivers/baseband/turbo_sw/bbdev_turbo_software.c | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 69dd518..8ca59b7 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -55,6 +55,9 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Updated the turbo_sw bbdev PMD.**
+
+  Added support for more comprehensive CRC options.
 
 Removed Items
 -------------
diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
index 77e9a2e..e570044 100644
--- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
+++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
@@ -199,6 +199,7 @@ struct turbo_sw_queue {
 			.cap.ldpc_enc = {
 				.capability_flags =
 						RTE_BBDEV_LDPC_RATE_MATCH |
+						RTE_BBDEV_LDPC_CRC_16_ATTACH |
 						RTE_BBDEV_LDPC_CRC_24A_ATTACH |
 						RTE_BBDEV_LDPC_CRC_24B_ATTACH,
 				.num_buffers_src =
@@ -211,6 +212,7 @@ struct turbo_sw_queue {
 		.type   = RTE_BBDEV_OP_LDPC_DEC,
 		.cap.ldpc_dec = {
 			.capability_flags =
+					RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK |
 					RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK |
 					RTE_BBDEV_LDPC_CRC_TYPE_24A_CHECK |
 					RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP |
@@ -880,6 +882,12 @@ struct turbo_sw_queue {
 		crc_req.len = in_length_in_bits - 24;
 		crc_resp.data = q->enc_in;
 		bblib_lte_crc24b_gen(&crc_req, &crc_resp);
+	} else if (enc->op_flags & RTE_BBDEV_LDPC_CRC_16_ATTACH) {
+		rte_memcpy(q->enc_in, in, in_length_in_bytes - 2);
+		crc_req.data = in;
+		crc_req.len = in_length_in_bits - 16;
+		crc_resp.data = q->enc_in;
+		bblib_lte_crc16_gen(&crc_req, &crc_resp);
 	} else
 		rte_memcpy(q->enc_in, in, in_length_in_bytes);
 
@@ -1492,6 +1500,15 @@ struct turbo_sw_queue {
 		if (!crc_resp.check_passed)
 			op->status |= 1 << RTE_BBDEV_CRC_ERROR;
 	}
+	if (check_bit(dec->op_flags, RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK)) {
+		crc_req.data = adapter_input;
+		crc_req.len  = K - dec->n_filler - 16;
+		crc_resp.check_passed = false;
+		crc_resp.data = adapter_input;
+		bblib_lte_crc16_check(&crc_req, &crc_resp);
+		if (!crc_resp.check_passed)
+			op->status |= 1 << RTE_BBDEV_CRC_ERROR;
+	}
 
 #ifdef RTE_BBDEV_OFFLOAD_COST
 	q_stats->acc_offload_cycles += rte_rdtsc_precise() - start_time;
-- 
1.8.3.1

[dpdk-dev] [PATCH v1 3/6] bbdev: add capability for 4G CB CRC DROP

[Nicolas Chautru]

Adding option to drop CRC24B to align with existing
feature for 5G

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 app/test-bbdev/test_bbdev_vector.c | 2 ++
 lib/bbdev/rte_bbdev_op.h           | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
index 8d796b1..f020836 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -149,6 +149,8 @@
 		*op_flag_value = RTE_BBDEV_TURBO_DEC_SCATTER_GATHER;
 	else if (!strcmp(token, "RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP"))
 		*op_flag_value = RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP;
+	else if (!strcmp(token, "RTE_BBDEV_TURBO_DEC_CRC_24B_DROP"))
+		*op_flag_value = RTE_BBDEV_TURBO_DEC_CRC_24B_DROP;
 	else {
 		printf("The given value is not a turbo decoder flag\n");
 		return -1;
diff --git a/lib/bbdev/rte_bbdev_op.h b/lib/bbdev/rte_bbdev_op.h
index 7c44ddd..5512859 100644
--- a/lib/bbdev/rte_bbdev_op.h
+++ b/lib/bbdev/rte_bbdev_op.h
@@ -114,7 +114,10 @@ enum rte_bbdev_op_td_flag_bitmasks {
 	/** Set to keep CRC24B bits appended while decoding. Only usable when
 	 * decoding Transport Block mode.
 	 */
-	RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP = (1ULL << 16)
+	RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP = (1ULL << 16),
+	/** Set to drop CRC24B bits not to be appended while decoding.
+	 */
+	RTE_BBDEV_TURBO_DEC_CRC_24B_DROP = (1ULL << 17)
 };
 
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v1 4/6] baseband/acc100: add support for 4G CRC drop

[Nicolas Chautru]

This implements in PMD the option to drop the CB CRC
after 4G decoding to help transport block concatenation.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 doc/guides/bbdevs/acc100.rst             |  1 +
 doc/guides/rel_notes/release_21_11.rst   |  4 ++++
 drivers/baseband/acc100/rte_acc100_pmd.c | 12 +++++++++---
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/doc/guides/bbdevs/acc100.rst b/doc/guides/bbdevs/acc100.rst
index ff0fa4b..9fff6ab 100644
--- a/doc/guides/bbdevs/acc100.rst
+++ b/doc/guides/bbdevs/acc100.rst
@@ -58,6 +58,7 @@ ACC100 5G/4G FEC PMD supports the following BBDEV capabilities:
    - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN`` :  set if negative LLR encoder i/p is supported
    - ``RTE_BBDEV_TURBO_POS_LLR_1_BIT_IN`` :  set if positive LLR encoder i/p is supported
    - ``RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP`` :  keep CRC24B bits appended while decoding
+   - ``RTE_BBDEV_TURBO_DEC_CRC_24B_DROP`` : option to drop the code block CRC after decoding
    - ``RTE_BBDEV_TURBO_EARLY_TERMINATION`` :  set early termination feature
    - ``RTE_BBDEV_TURBO_DEC_SCATTER_GATHER`` :  supports scatter-gather for input/output data
    - ``RTE_BBDEV_TURBO_HALF_ITERATION_EVEN`` :  set half iteration granularity
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 8ca59b7..f7843bc 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -59,6 +59,10 @@ New Features
 
   Added support for more comprehensive CRC options.
 
+* **Updated the ACC100 bbdev PMD.**
+
+  Added support for more comprehensive CRC options.
+
 Removed Items
 -------------
 
diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
index 68ba523..891be81 100644
--- a/drivers/baseband/acc100/rte_acc100_pmd.c
+++ b/drivers/baseband/acc100/rte_acc100_pmd.c
@@ -980,6 +980,7 @@
 					RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
 					RTE_BBDEV_TURBO_MAP_DEC |
 					RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
+					RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
 					RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
 				.max_llr_modulus = INT8_MAX,
 				.num_buffers_src =
@@ -1708,8 +1709,12 @@
 	}
 
 	if ((op->turbo_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
-		&& !check_bit(op->turbo_dec.op_flags,
-		RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
+			&& !check_bit(op->turbo_dec.op_flags,
+			RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
+		crc24_overlap = 24;
+	if ((op->turbo_dec.code_block_mode == RTE_BBDEV_CODE_BLOCK)
+			&& check_bit(op->turbo_dec.op_flags,
+			RTE_BBDEV_TURBO_DEC_CRC_24B_DROP))
 		crc24_overlap = 24;
 
 	/* Calculates circular buffer size.
@@ -1744,7 +1749,8 @@
 
 	next_triplet = acc100_dma_fill_blk_type_out(
 			desc, h_output, *h_out_offset,
-			k >> 3, next_triplet, ACC100_DMA_BLKID_OUT_HARD);
+			(k - crc24_overlap)  >> 3, next_triplet,
+			ACC100_DMA_BLKID_OUT_HARD);
 	if (unlikely(next_triplet < 0)) {
 		rte_bbdev_log(ERR,
 				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
-- 
1.8.3.1

[dpdk-dev] [PATCH v1 5/6] doc: clarification of usage of HARQ in bbdev doc

[Nicolas Chautru]

New paragraph detailing typical VRAN usecase and mapping
to bbdev API usage.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 doc/guides/prog_guide/bbdev.rst | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/doc/guides/prog_guide/bbdev.rst b/doc/guides/prog_guide/bbdev.rst
index 8bd7cba..d48c9a0 100644
--- a/doc/guides/prog_guide/bbdev.rst
+++ b/doc/guides/prog_guide/bbdev.rst
@@ -1054,6 +1054,29 @@ capability RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE is set
 then the HARQ is stored in memory internal to the device and not visible
 to BBDEV.
 
+.. note::
+    More explicitly for a typical usage of HARQ retransmission in a VRAN
+    application using a HW PMD, there will be 2 cases.
+
+    For 1st transmission, only the HARQ output is enabled :
+
+    - the harq_combined_output.offset is provided to a given address. ie. typicallly an integer index * 32K, where the index is tracked by the application based on code block index for a given UE and HARQ process.
+
+    - the related operation flag would notably include RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE and RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION.
+
+    - note that not explicit flush or reset of the memory is required.
+
+    For 2nd transmission, an input is also required to benefit from HARQ combination gain:
+
+    - the changes mentioned above are the same (note that rvIndex may be adjusted).
+
+    - the operation flag would additionally include the LDPC_HQ_COMBINE_IN_ENABLE flag.
+
+    - the harq_combined_input.offset must set to the address of the related code block (ie. same as the harq_combine_output index above for the same code block, HARQ process, UE).
+
+    - the harq_combined_input.length must be set to the length which was provided back in the related harq_combined_output.length when it has processed and dequeued (previous HARQ iteration).
+
+
 The output mbuf data structures are expected to be allocated by the
 application with enough room for the output data.
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v1 6/6] bbdev: reduce warning level for one scenario

[Nicolas Chautru]

Queue setup may genuinely fail when adding incremental queues
for a given priority level. In that case application would
attempt to configure a queue at a different priority level.
Not an actual error.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 lib/bbdev/rte_bbdev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c
index fc37236..defddcf 100644
--- a/lib/bbdev/rte_bbdev.c
+++ b/lib/bbdev/rte_bbdev.c
@@ -528,9 +528,10 @@ struct rte_bbdev *
 	ret = dev->dev_ops->queue_setup(dev, queue_id, (conf != NULL) ?
 			conf : &dev_info.default_queue_conf);
 	if (ret < 0) {
-		rte_bbdev_log(ERR,
-				"Device %u queue %u setup failed", dev_id,
-				queue_id);
+		/* This may happen when trying different priority levels */
+		rte_bbdev_log(INFO,
+				"Device %u queue %u setup failed",
+				dev_id, queue_id);
 		return ret;
 	}
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16 check

[Thomas Monjalon]

13/08/2021 18:51, Nicolas Chautru:
> Adding a missing operation when CRC16
> is being used for TB CRC check.
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -84,6 +84,7 @@ API Changes
>     Also, make sure to start the actual text at the margin.
>     =======================================================
>  
> +* bbdev: Added capability related to more comprehensive CRC options.

That's not an API change, the enum symbols are the same.
Only enum values are changed so it impacts only ABI.

Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16 check

[Chautru, Nicolas]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, October 11, 2021 1:17 PM
> To: gakhil@marvell.com; Chautru, Nicolas <nicolas.chautru@intel.com>
> Cc: dev@dpdk.org; trix@redhat.com; hemant.agrawal@nxp.com; Zhang,
> Mingshan <mingshan.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16
> check
> 
> 13/08/2021 18:51, Nicolas Chautru:
> > Adding a missing operation when CRC16
> > is being used for TB CRC check.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> > --- a/doc/guides/rel_notes/release_21_11.rst
> > +++ b/doc/guides/rel_notes/release_21_11.rst
> > @@ -84,6 +84,7 @@ API Changes
> >     Also, make sure to start the actual text at the margin.
> >     =======================================================
> >
> > +* bbdev: Added capability related to more comprehensive CRC options.
> 
> That's not an API change, the enum symbols are the same.
> Only enum values are changed so it impacts only ABI.

Hi Thomas, 
How is that not a API change when new additional capability are exposed? Ie. new enums defined for new capabilities. 
I think I see other similar cases in the same release notes " * cryptodev: ``RTE_CRYPTO_AEAD_LIST_END`` from ``enum rte_crypto_aead_algo ...".
You know best, just checking the intent, maybe worth clarifying the guideline except in case this is just me. 


> 
> 

Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16 check

[Thomas Monjalon]

11/10/2021 22:38, Chautru, Nicolas:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 13/08/2021 18:51, Nicolas Chautru:
> > > Adding a missing operation when CRC16
> > > is being used for TB CRC check.
> > >
> > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > > ---
> > > --- a/doc/guides/rel_notes/release_21_11.rst
> > > +++ b/doc/guides/rel_notes/release_21_11.rst
> > > @@ -84,6 +84,7 @@ API Changes
> > >     Also, make sure to start the actual text at the margin.
> > >     =======================================================
> > >
> > > +* bbdev: Added capability related to more comprehensive CRC options.
> > 
> > That's not an API change, the enum symbols are the same.
> > Only enum values are changed so it impacts only ABI.
> 
> Hi Thomas, 
> How is that not a API change when new additional capability are exposed? Ie. new enums defined for new capabilities. 

API change is when the app source code has to be updated.
ABI change is when the app binary has to be rebuilt.

> I think I see other similar cases in the same release notes " * cryptodev: ``RTE_CRYPTO_AEAD_LIST_END`` from ``enum rte_crypto_aead_algo ...".

I don't see this one.

> You know best, just checking the intent, maybe worth clarifying the guideline except in case this is just me. 

Given my explanation above, how would you classify your change?

Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16 check

[Chautru, Nicolas]

Hi Thomas, 

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, October 11, 2021 11:53 PM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>
> Cc: gakhil@marvell.com; dev@dpdk.org; trix@redhat.com;
> hemant.agrawal@nxp.com; Zhang, Mingshan <mingshan.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16
> check
> 
> 11/10/2021 22:38, Chautru, Nicolas:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 13/08/2021 18:51, Nicolas Chautru:
> > > > Adding a missing operation when CRC16 is being used for TB CRC
> > > > check.
> > > >
> > > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > > > ---
> > > > --- a/doc/guides/rel_notes/release_21_11.rst
> > > > +++ b/doc/guides/rel_notes/release_21_11.rst
> > > > @@ -84,6 +84,7 @@ API Changes
> > > >     Also, make sure to start the actual text at the margin.
> > > >
> =======================================================
> > > >
> > > > +* bbdev: Added capability related to more comprehensive CRC
> options.
> > >
> > > That's not an API change, the enum symbols are the same.
> > > Only enum values are changed so it impacts only ABI.
> >
> > Hi Thomas,
> > How is that not a API change when new additional capability are exposed?
> Ie. new enums defined for new capabilities.
> 
> API change is when the app source code has to be updated.

Thanks. What you are referring to may be strictly API breakage as opposed to generic API change. I would expect an API change could be either backward compatible (extending API but application only has to change if it wants to use the new functionality) vs an actual API breakage (application needs to change regardless even to keep same functionality as before). 
In case the intent is to use the 2 terms interchangeably (change vs breakage) then I agree that these 2 bbdev changes do not constitute an API breakage (only ABI). 
It might be good to capture this more explicitly except if you believe this is obvious (doc describes ABI change, not API change). Regardless for next time I will use that distinction (change == breakage). 
Thanks

> ABI change is when the app binary has to be rebuilt.
>
> > I think I see other similar cases in the same release notes " * cryptodev:
> ``RTE_CRYPTO_AEAD_LIST_END`` from ``enum rte_crypto_aead_algo ...".
> 
> I don't see this one.
> 
> > You know best, just checking the intent, maybe worth clarifying the
> guideline except in case this is just me.
> 
> Given my explanation above, how would you classify your change?
> 
> 

Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16 check

[Thomas Monjalon]

12/10/2021 18:36, Chautru, Nicolas:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 11/10/2021 22:38, Chautru, Nicolas:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 13/08/2021 18:51, Nicolas Chautru:
> > > > > Adding a missing operation when CRC16 is being used for TB CRC
> > > > > check.
> > > > >
> > > > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > > > > ---
> > > > > --- a/doc/guides/rel_notes/release_21_11.rst
> > > > > +++ b/doc/guides/rel_notes/release_21_11.rst
> > > > > @@ -84,6 +84,7 @@ API Changes
> > > > >     Also, make sure to start the actual text at the margin.
> > > > >
> > =======================================================
> > > > >
> > > > > +* bbdev: Added capability related to more comprehensive CRC
> > options.
> > > >
> > > > That's not an API change, the enum symbols are the same.
> > > > Only enum values are changed so it impacts only ABI.
> > >
> > > Hi Thomas,
> > > How is that not a API change when new additional capability are exposed?
> > Ie. new enums defined for new capabilities.
> > 
> > API change is when the app source code has to be updated.
> 
> Thanks. What you are referring to may be strictly API breakage as opposed to generic API change. I would expect an API change could be either backward compatible (extending API but application only has to change if it wants to use the new functionality) vs an actual API breakage (application needs to change regardless even to keep same functionality as before).

Yes
An API change which does not break is a new feature, so it is referenced
at the beginning of the release notes in general.

> In case the intent is to use the 2 terms interchangeably (change vs breakage) then I agree that these 2 bbdev changes do not constitute an API breakage (only ABI). 
> It might be good to capture this more explicitly except if you believe this is obvious (doc describes ABI change, not API change). Regardless for next time I will use that distinction (change == breakage). 

Yes feel free to send a patch to rename "change" to 'breakage".