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

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

[Nicolas Chautru]

v3: 2 updates based on review comment by tom rix, thanks
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 | 16 ++++++++++
 lib/bbdev/rte_bbdev.c                            |  7 +++--
 lib/bbdev/rte_bbdev_op.h                         | 39 +++++++++++++-----------
 8 files changed, 90 insertions(+), 23 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v3 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 v3 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 | 16 ++++++++++++++++
 2 files changed, 19 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..e1db2bf 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);
 
@@ -1491,6 +1499,14 @@ struct turbo_sw_queue {
 			bblib_lte_crc24a_check(&crc_req, &crc_resp);
 		if (!crc_resp.check_passed)
 			op->status |= 1 << RTE_BBDEV_CRC_ERROR;
+	} else 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
-- 
1.8.3.1

[dpdk-dev] [PATCH v3 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 v3 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..2e9ce92 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 >> 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 v3 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 v3 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 v3 1/6] bbdev: add capability for CRC16 check

[Tom Rix]

On 9/7/21 6:15 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),
>   	/** 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)

The fiddling in the middle is fine since bbdev api is experimental

Look good.

Reviewed-by: Tom Rix <trix@redhat.com>

>   };
>   
>   /** Flags for LDPC encoder operation and capability structure */

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

[Tom Rix]

On 9/7/21 6:15 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 | 16 ++++++++++++++++
>   2 files changed, 19 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..e1db2bf 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);
>   
> @@ -1491,6 +1499,14 @@ struct turbo_sw_queue {
>   			bblib_lte_crc24a_check(&crc_req, &crc_resp);
>   		if (!crc_resp.check_passed)
>   			op->status |= 1 << RTE_BBDEV_CRC_ERROR;
> +	} else 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;
>   	}

Thanks for the else-if() change.

Looks good.

Reviewed-by: Tom Rix <trix@redhat.com>

>   
>   #ifdef RTE_BBDEV_OFFLOAD_COST

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

[Tom Rix]

On 9/7/21 6:15 PM, Nicolas Chautru wrote:
> 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)
>   };
>   
>   

This is ok, thanks.

Reviewed-by: Tom Rix <trix@redhat.com>

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

[Tom Rix]

On 9/7/21 6:15 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..2e9ce92 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;

ok if this isn't combined.

the bug moved to its own patch.

Looks good.

Reviewed-by: Tom Rix <trix@redhat.com>

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

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

[Tom Rix]

On 9/7/21 6:15 PM, Nicolas Chautru wrote:
> 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.
>   

Fine.

Reviewed-by: Tom Rix <trix@redhat.com>

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

[Tom Rix]

On 9/7/21 6:15 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);

This change is just changing the log level, which is fine.

I am looking at how the error handling is done for the function.

It seems like the bailing is done in the middle of change the queue state.

ex/ the block above this one

/* Release existing queue ... */

Does this leave the queue in a bad state ?

Tom

>   		return ret;
>   	}
>   

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

[Chautru, Nicolas]

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Sunday, September 12, 2021 5:55 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 v3 6/6] bbdev: reduce warning level for one scenario
> 
> 
> On 9/7/21 6:15 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);
> 
> This change is just changing the log level, which is fine.
> 
> I am looking at how the error handling is done for the function.
> 
> It seems like the bailing is done in the middle of change the queue state.
> 
> ex/ the block above this one
> 
> /* Release existing queue ... */
> 
> Does this leave the queue in a bad state ?

Hi Tom, 
That would not be related to that change indeed. 

The queue would end up in a not configured when rte_bbdev_queue_configure() fails but then can still  be configured again without limitation (worst thing than can happen is that queue_release is called, hence leaves the queue in a deterministic state, unconfigured but ready to be configured).
Note that queue_release() just removes the configuration of the queue, but the queue is still there and can be configured again (actual total number of queues unchanged, based on number previously set with rte_bbdev_setup_queues()).

Thanks
Nic

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

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

[Tom Rix]

On 9/13/21 10:03 AM, Chautru, Nicolas wrote:
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Sunday, September 12, 2021 5:55 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 v3 6/6] bbdev: reduce warning level for one scenario
>>
>>
>> On 9/7/21 6:15 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);
>> This change is just changing the log level, which is fine.
>>
>> I am looking at how the error handling is done for the function.
>>
>> It seems like the bailing is done in the middle of change the queue state.
>>
>> ex/ the block above this one
>>
>> /* Release existing queue ... */
>>
>> Does this leave the queue in a bad state ?
> Hi Tom,
> That would not be related to that change indeed.
>
> The queue would end up in a not configured when rte_bbdev_queue_configure() fails but then can still  be configured again without limitation (worst thing than can happen is that queue_release is called, hence leaves the queue in a deterministic state, unconfigured but ready to be configured).
> Note that queue_release() just removes the configuration of the queue, but the queue is still there and can be configured again (actual total number of queues unchanged, based on number previously set with rte_bbdev_setup_queues()).

So its in a bad state, but outside the scope of this commit.

Reviewed-by: Tom Rix <trix@redhat.com>

Tom

>
> Thanks
> Nic
>
>> Tom
>>
>>>    		return ret;
>>>    	}
>>>

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

[Chautru, Nicolas]

Hi Akhil, 
Can this serie below be applied now?
Thanks and regards,
Nic

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Monday, September 13, 2021 1:02 PM
> 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 v3 6/6] bbdev: reduce warning level for one scenario
> 
> 
> On 9/13/21 10:03 AM, Chautru, Nicolas wrote:
> >> -----Original Message-----
> >> From: Tom Rix <trix@redhat.com>
> >> Sent: Sunday, September 12, 2021 5:55 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 v3 6/6] bbdev: reduce warning level for one
> >> scenario
> >>
> >>
> >> On 9/7/21 6:15 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);
> >> This change is just changing the log level, which is fine.
> >>
> >> I am looking at how the error handling is done for the function.
> >>
> >> It seems like the bailing is done in the middle of change the queue state.
> >>
> >> ex/ the block above this one
> >>
> >> /* Release existing queue ... */
> >>
> >> Does this leave the queue in a bad state ?
> > Hi Tom,
> > That would not be related to that change indeed.
> >
> > The queue would end up in a not configured when
> rte_bbdev_queue_configure() fails but then can still  be configured again
> without limitation (worst thing than can happen is that queue_release is
> called, hence leaves the queue in a deterministic state, unconfigured but
> ready to be configured).
> > Note that queue_release() just removes the configuration of the queue,
> but the queue is still there and can be configured again (actual total number
> of queues unchanged, based on number previously set with
> rte_bbdev_setup_queues()).
> 
> So its in a bad state, but outside the scope of this commit.
> 
> Reviewed-by: Tom Rix <trix@redhat.com>
> 
> Tom
> 
> >
> > Thanks
> > Nic
> >
> >> Tom
> >>
> >>>    		return ret;
> >>>    	}
> >>>

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

[Akhil Goyal]

> Can this serie below be applied now?
> Thanks and regards,
> Nic
Series Applied to dpd-next-crypto

Thanks.

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

[Thomas Monjalon]

12/09/2021 14:44, Tom Rix:
> On 9/7/21 6:15 PM, Nicolas Chautru wrote:
> > 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::

blank line missing

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

no space before ":"

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

"*no* explicit"

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

"must *be* set"

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

lines are too long

> > +
> > +
> 
> Fine.
> 
> Reviewed-by: Tom Rix <trix@redhat.com>

No, it isn't fine.
Fixing some typos and formatting while pulling in main.

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

[Chautru, Nicolas]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, October 11, 2021 1:29 PM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>
> Cc: dev@dpdk.org; gakhil@marvell.com; hemant.agrawal@nxp.com; Zhang,
> Mingshan <mingshan.zhang@intel.com>; Joshi, Arun
> <arun.joshi@intel.com>; Tom Rix <trix@redhat.com>
> Subject: Re: [dpdk-dev] [PATCH v3 5/6] doc: clarification of usage of HARQ in
> bbdev doc
> 
> 12/09/2021 14:44, Tom Rix:
> > On 9/7/21 6:15 PM, Nicolas Chautru wrote:
> > > 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::
> 
> blank line missing
> 
> > > +    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 :
> 
> no space before ":"
> 
> > > +
> > > +    - 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.
> 
> "*no* explicit"
> 
> > > +
> > > +    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).
> 
> "must *be* set"
> 
> > > +
> > > +    - 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).
> 
> lines are too long
> 
> > > +
> > > +
> >
> > Fine.
> >
> > Reviewed-by: Tom Rix <trix@redhat.com>
> 
> No, it isn't fine.
> Fixing some typos and formatting while pulling in main.

Thanks Thomas. 

>