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

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

[Nicolas Chautru]

v2: typo missed in documentation.
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 v2 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 v2 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 v2 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 v2 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 v2 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..f39b62f 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. typically 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 v2 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 v2 6/6] bbdev: reduce warning level for one scenario

[Zhang, Mingshan]

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>
Acked-by: Mingshan Zhang <mingshan.zhang@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 v2 1/6] bbdev: add capability for CRC16 check

[Tom Rix]

On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> 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),

Changing these enums will break the abi backwards.

Why not add the new one at the end ?

Tom

>   	/** 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 */

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

[Tom Rix]

On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> 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) {

The 'else if' assumes the new flag is mutually exclusive wrt the other 
crc flags.

At least a debug check should be added to verify.

> +		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)) {

The series of 'if-statements' means the new flag is not mutually 
exclusive wrt the other crc flags.

doing both 24a and 16 would create a mess.

this should likely change to an else-if-statement similar to above.

Tom

> +		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;

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

[Tom Rix]

On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> 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)
These two if-statements can be combined.
> +			&& 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,

crc24_overlap had been set before this patch in the above if-statement 
for crc_24b_keep.

so this looks like a bug.

If it is a bug, it should be separated out as its own patch.

Tom

> +			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",

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

[Tom Rix]

On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> 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);

Earlier code tears down the exiting plumbing.

If it is ok to fail, should this block move earlier before the teardown ?

Tom

>   		return ret;
>   	}
>   

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

[Chautru, Nicolas]

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Wednesday, September 1, 2021 6:37 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com
> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
> Subject: Re: [PATCH v2 1/6] bbdev: add capability for CRC16 check
> 
> 
> On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> > 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),
> 
> Changing these enums will break the abi backwards.
> 
> Why not add the new one at the end ?

To keep all the CRC related flags next to each other for better readability and logical clarity. The ABI is still marked as experimental. 

> 
> Tom
> 
> >   	/** 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 */

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

[Chautru, Nicolas]

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Wednesday, September 1, 2021 7:01 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com
> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
> Subject: Re: [PATCH v2 2/6] baseband/turbo_sw: add support for CRC16
> 
> 
> On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> > 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) {
> 
> The 'else if' assumes the new flag is mutually exclusive wrt the other crc flags.
> 
> At least a debug check should be added to verify.

There is typically not a validation of the input API in these PMD to report invalid operations mix. 
There may be other combination of operation which may be invalid. 

> 
> > +		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)) {
> 
> The series of 'if-statements' means the new flag is not mutually exclusive wrt
> the other crc flags.
> 
> doing both 24a and 16 would create a mess.
> 
> this should likely change to an else-if-statement similar to above.

Ok, I will change this as this is indeed mutually exclusive and to match similar snippet above. Thanks
> 
> Tom
> 
> > +		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;

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

[Chautru, Nicolas]

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Wednesday, September 1, 2021 7:20 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com
> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
> Subject: Re: [PATCH v2 4/6] baseband/acc100: add support for 4G CRC drop
> 
> 
> On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> > 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)
> These two if-statements can be combined.

They could but really when I tried  that became arguably unreadable, I thought it was still better this way. 

> > +			&& 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,
> 
> crc24_overlap had been set before this patch in the above if-statement for
> crc_24b_keep.
> 
> so this looks like a bug.
> 
> If it is a bug, it should be separated out as its own patch.

Ok fair enough, will do. Thanks

> 
> Tom
> 
> > +			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",

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

[Chautru, Nicolas]

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Wednesday, September 1, 2021 7:30 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com
> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
> Subject: Re: [PATCH v2 6/6] bbdev: reduce warning level for one scenario
> 
> 
> On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> > 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);
> 
> Earlier code tears down the exiting plumbing.
> 
> If it is ok to fail, should this block move earlier before the teardown ?

I may miss your point Tom.
If we reached that point at least there was nothing fundamental bad with the request which would have been reported as a formal error.
 At that point this may still be rejected because the provided priority bucket may genuinely be empty.
The code will handle it the same way as before (based on unchanged API) but at least we don't print an error string for something which may be reported as a failure on a routine sunny day scenario. 
Thanks

> 
> Tom
> 
> >   		return ret;
> >   	}
> >

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

[Tom Rix]

On 9/1/21 8:00 AM, Chautru, Nicolas wrote:
>
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Wednesday, September 1, 2021 6:37 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com
>> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
>> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
>> Subject: Re: [PATCH v2 1/6] bbdev: add capability for CRC16 check
>>
>>
>> On 8/19/21 2:10 PM, Nicolas Chautru wrote:
>>> 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),
>> Changing these enums will break the abi backwards.
>>
>> Why not add the new one at the end ?
> To keep all the CRC related flags next to each other for better readability and logical clarity. The ABI is still marked as experimental.

Ok

>
>> Tom
>>
>>>    	/** 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 */

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

[Tom Rix]

On 9/7/21 6:04 PM, Chautru, Nicolas wrote:
>
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Wednesday, September 1, 2021 7:20 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com
>> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
>> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
>> Subject: Re: [PATCH v2 4/6] baseband/acc100: add support for 4G CRC drop
>>
>>
>> On 8/19/21 2:10 PM, Nicolas Chautru wrote:
>>> 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)
>> These two if-statements can be combined.
> They could but really when I tried  that became arguably unreadable, I thought it was still better this way.

Ok

Tom

>
>>> +			&& 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,
>> crc24_overlap had been set before this patch in the above if-statement for
>> crc_24b_keep.
>>
>> so this looks like a bug.
>>
>> If it is a bug, it should be separated out as its own patch.
> Ok fair enough, will do. Thanks
>
>> Tom
>>
>>> +			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",