diff mbox series

[v3,18/30] baseband/acc100: enable input validation by default

Message ID 20221012025346.204394-19-hernan.vargas@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series baseband/acc100: changes for 22.11 |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Hernan Vargas Oct. 12, 2022, 2:53 a.m. UTC 
  Enable validation functions by default and provide a new flag
RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without
validating input to save cycles.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 36 +++++++++++++--------------
 1 file changed, 18 insertions(+), 18 deletions(-)

Comments

Akhil Goyal Oct. 13, 2022, 12:56 p.m. UTC | #1 
> Enable validation functions by default and provide a new flag
> RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without
> validating input to save cycles.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/baseband/acc/rte_acc100_pmd.c | 36 +++++++++++++--------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
How will the user know about this new flag?
Can you update the driver documentation for this?
Maxime Coquelin Oct. 18, 2022, 4:28 p.m. UTC | #2 
On 10/12/22 04:53, Hernan Vargas wrote:
> Enable validation functions by default and provide a new flag
> RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without
> validating input to save cycles.

I would prefer a devarg, so that it can be enabled/disabled at runtime,
instead of build time. The extra if condition would minimal.

> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 36 +++++++++++++--------------
>   1 file changed, 18 insertions(+), 18 deletions(-)
>
Chautru, Nicolas Oct. 19, 2022, 10:12 p.m. UTC | #3 
Hi Maxime, 

> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> On 10/12/22 04:53, Hernan Vargas wrote:
> > Enable validation functions by default and provide a new flag
> > RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without validating
> > input to save cycles.
> 
> I would prefer a devarg, so that it can be enabled/disabled at runtime, instead
> of build time. The extra if condition would minimal.

Could you please review the previous discussion on https://patches.dpdk.org/project/dpdk/cover/20221012025346.204394-1-hernan.vargas@intel.com/
Basically we would keep this build time flag and update documentation for this release. Then for next release consider devarg. 
Let us know what you think?
Maxime Coquelin Oct. 21, 2022, 8:06 a.m. UTC | #4 
Hi Nicolas,

On 10/20/22 00:12, Chautru, Nicolas wrote:
> Hi Maxime,
> 
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> On 10/12/22 04:53, Hernan Vargas wrote:
>>> Enable validation functions by default and provide a new flag
>>> RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without validating
>>> input to save cycles.
>>
>> I would prefer a devarg, so that it can be enabled/disabled at runtime, instead
>> of build time. The extra if condition would minimal.
> 
> Could you please review the previous discussion on https://patches.dpdk.org/project/dpdk/cover/20221012025346.204394-1-hernan.vargas@intel.com/
> Basically we would keep this build time flag and update documentation for this release. Then for next release consider devarg.
> Let us know what you think?
> 
> 

That's not ideal, but if you commit to do it in the next release this is
OK for me.

Thanks,
Maxime
diff mbox series

Patch

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 76667d1293..1ccbe4b8b7 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1784,7 +1784,7 @@  acc100_dma_desc_ld_update(struct rte_bbdev_dec_op *op,
 	desc->op_addr = op;
 }
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 /* Validates turbo encoder parameters */
 static inline int
 validate_enc_op(struct rte_bbdev_enc_op *op, struct acc_queue *q)
@@ -2047,10 +2047,10 @@  enqueue_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 		seg_total_left;
 	struct rte_mbuf *input, *output_head, *output;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_enc_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "Turbo encoder validation failed");
+		rte_bbdev_log(ERR, "Turbo encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2101,10 +2101,10 @@  enqueue_ldpc_enc_n_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ops,
 	uint16_t  in_length_in_bytes;
 	struct rte_bbdev_op_ldpc_enc *enc = &ops[0]->ldpc_enc;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_ldpc_enc_op(ops[0], q) == -1) {
-		rte_bbdev_log(ERR, "LDPC encoder validation failed");
+		rte_bbdev_log(ERR, "LDPC encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2163,10 +2163,10 @@  enqueue_ldpc_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 		seg_total_left;
 	struct rte_mbuf *input, *output_head, *output;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	if (validate_ldpc_enc_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "LDPC encoder validation failed");
+		rte_bbdev_log(ERR, "LDPC encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2220,10 +2220,10 @@  enqueue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 	struct rte_mbuf *input, *output_head, *output;
 	uint16_t current_enqueued_cbs = 0;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_enc_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "Turbo encoder validation failed");
+		rte_bbdev_log(ERR, "Turbo encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2296,7 +2296,7 @@  enqueue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 	return current_enqueued_cbs;
 }
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 /* Validates turbo decoder parameters */
 static inline int
 validate_dec_op(struct rte_bbdev_dec_op *op, struct acc_queue *q)
@@ -2454,10 +2454,10 @@  enqueue_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	struct rte_mbuf *input, *h_output_head, *h_output,
 		*s_output_head, *s_output;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_dec_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "Turbo decoder validation failed");
+		rte_bbdev_log(ERR, "Turbo decoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2678,10 +2678,10 @@  enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		return ret;
 	}
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_ldpc_dec_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "LDPC decoder validation failed");
+		rte_bbdev_log(ERR, "LDPC decoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2783,10 +2783,10 @@  enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	struct rte_mbuf *input, *h_output_head, *h_output;
 	uint16_t current_enqueued_cbs = 0;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_ldpc_dec_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "LDPC decoder validation failed");
+		rte_bbdev_log(ERR, "LDPC decoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2875,10 +2875,10 @@  enqueue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		*s_output_head, *s_output;
 	uint16_t current_enqueued_cbs = 0;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_dec_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "Turbo decoder validation failed");
+		rte_bbdev_log(ERR, "Turbo decoder validation rejected");
 		return -EINVAL;
 	}
 #endif