DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v1 1/3] cryptodev: add queue pair reset API
@ 2024-08-22 10:28 Vidya Sagar Velumuri
  2024-08-22 10:28 ` [PATCH v1 2/3] crypto/cnxk: add queue pair reset support Vidya Sagar Velumuri
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Vidya Sagar Velumuri @ 2024-08-22 10:28 UTC (permalink / raw)
  To: Akhil Goyal, Fan Zhang
  Cc: jerinj, anoobj, vvelumuri, asasidharan, ktejasree, gmuthukrishn, dev

The API will reset the specific queue pair of a cryptodev.
The current API, cryptodev_queue_pair_setup(), requires the cryptodev
to be stopped before reconfiguring any queue pair. Stopping the
cryptodev in one thread can result in a segmentation fault when
multiple queues are used for enqueue and dequeue operations.

On supported PMDs, the cryptodev_queue_pair_reset() will
reconfigure/reset the queue pair without affecting other queues or
the cryptodev state.

The caller should ensure that there are no enqueue or dequeue operations
ongoing on that queue and that there are no inflight packets before
calling this API.

Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>

diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
index 6c114f7181..311ae63abb 100644
--- a/lib/cryptodev/cryptodev_pmd.h
+++ b/lib/cryptodev/cryptodev_pmd.h
@@ -290,6 +290,22 @@ typedef int (*cryptodev_queue_pair_setup_t)(struct rte_cryptodev *dev,
 typedef int (*cryptodev_queue_pair_release_t)(struct rte_cryptodev *dev,
 		uint16_t qp_id);
 
+/**
+ * Reset or reconfigure a queue pair for a device.
+ *
+ * @param	dev		Crypto device pointer
+ * @param	qp_id		Queue pair index
+ * @param	qp_conf		Queue configuration structure
+ * @param	socket_id	Socket index
+ *
+ * @return
+ *  - 0: on success.
+ *  - ENOTSUP: if crypto device does not support the operation.
+ */
+typedef int (*cryptodev_queue_pair_reset_t)(struct rte_cryptodev *dev,
+		uint16_t qp_id,	const struct rte_cryptodev_qp_conf *qp_conf,
+		int socket_id);
+
 /**
  * Create a session mempool to allocate sessions from
  *
@@ -476,6 +492,8 @@ struct rte_cryptodev_ops {
 	/**< Set up a device queue pair. */
 	cryptodev_queue_pair_release_t queue_pair_release;
 	/**< Release a queue pair. */
+	cryptodev_queue_pair_reset_t queue_pair_reset;
+	/**< Reset a queue pair. */
 
 	cryptodev_sym_get_session_private_size_t sym_session_get_size;
 	/**< Return private session. */
diff --git a/lib/cryptodev/cryptodev_trace.h b/lib/cryptodev/cryptodev_trace.h
index 935f0d564b..633f17afe1 100644
--- a/lib/cryptodev/cryptodev_trace.h
+++ b/lib/cryptodev/cryptodev_trace.h
@@ -58,6 +58,16 @@ RTE_TRACE_POINT(
 	rte_trace_point_emit_ptr(conf->mp_session);
 )
 
+RTE_TRACE_POINT(
+	rte_cryptodev_trace_queue_pair_reset,
+	RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_pair_id,
+		const struct rte_cryptodev_qp_conf *conf, int socket_id),
+	rte_trace_point_emit_u8(dev_id);
+	rte_trace_point_emit_u16(queue_pair_id);
+	rte_trace_point_emit_u32(conf->nb_descriptors);
+	rte_trace_point_emit_int(socket_id);
+)
+
 RTE_TRACE_POINT(
 	rte_cryptodev_trace_sym_session_pool_create,
 	RTE_TRACE_POINT_ARGS(const char *name, uint32_t nb_elts,
diff --git a/lib/cryptodev/cryptodev_trace_points.c b/lib/cryptodev/cryptodev_trace_points.c
index 7403412553..6f37780595 100644
--- a/lib/cryptodev/cryptodev_trace_points.c
+++ b/lib/cryptodev/cryptodev_trace_points.c
@@ -21,6 +21,9 @@ RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_close,
 RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_setup,
 	lib.cryptodev.queue.pair.setup)
 
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_reset,
+	lib.cryptodev.queue.pair.reset)
+
 RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_pool_create,
 	lib.cryptodev.sym.pool.create)
 
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index 682c9f49d0..281ca51cf3 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -1222,6 +1222,30 @@ rte_cryptodev_queue_pairs_config(struct rte_cryptodev *dev, uint16_t nb_qpairs,
 	return 0;
 }
 
+int
+rte_cryptodev_queue_pair_reset(uint8_t dev_id, uint16_t queue_pair_id,
+		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id)
+{
+	struct rte_cryptodev *dev;
+
+	if (!rte_cryptodev_is_valid_dev(dev_id)) {
+		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
+		return -EINVAL;
+	}
+
+	dev = &rte_crypto_devices[dev_id];
+	if (queue_pair_id >= dev->data->nb_queue_pairs) {
+		CDEV_LOG_ERR("Invalid queue_pair_id=%d", queue_pair_id);
+		return -EINVAL;
+	}
+
+	if (*dev->dev_ops->queue_pair_reset == NULL)
+		return -ENOTSUP;
+
+	rte_cryptodev_trace_queue_pair_reset(dev_id, queue_pair_id, qp_conf, socket_id);
+	return (*dev->dev_ops->queue_pair_reset)(dev, queue_pair_id, qp_conf, socket_id);
+}
+
 int
 rte_cryptodev_configure(uint8_t dev_id, struct rte_cryptodev_config *config)
 {
diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index bec947f6d5..e0fc35db2a 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -840,6 +840,35 @@ int
 rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
 		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Reset a queue pair for a device.
+ * The caller of this API must ensure that, there are no enqueues to the queue and there are no
+ * pending/inflight packets in the queue when the API is called.
+ * The API can reconfigure the queue pair when the queue pair configuration data is provided.
+ *
+ * @param	dev_id		The identifier of the device.
+ * @param	queue_pair_id	The index of the queue pairs to set up. The value must be in the
+ *				range [0, nb_queue_pair - 1] previously supplied to
+ *				rte_cryptodev_configure().
+ * @param	qp_conf		The pointer to configuration data to be used for the queue pair.
+ *				It should be NULL, if the API is called from an interrupt context.
+ * @param	socket_id	The *socket_id* argument is the socket identifier in case of NUMA.
+ *				The value can be *SOCKET_ID_ANY* if there is no NUMA constraint
+ *				for the DMA memory allocated for the queue pair.
+ *
+ * @return
+ *   - 0:  Queue pair is reset successfully.
+ *   - ENOTSUP: If the operation is not supported by the PMD.
+ *   - <0: Queue pair reset failed
+ */
+__rte_experimental
+int
+rte_cryptodev_queue_pair_reset(uint8_t dev_id, uint16_t queue_pair_id,
+		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
+
 /**
  * Get the status of queue pairs setup on a specific crypto device
  *
diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
index fdac0d876e..eec06d9939 100644
--- a/lib/cryptodev/version.map
+++ b/lib/cryptodev/version.map
@@ -87,6 +87,9 @@ EXPERIMENTAL {
 
 	# added in 24.03
 	__rte_cryptodev_trace_qp_depth_used;
+
+	# added in 24.07
+	rte_cryptodev_queue_pair_reset;
 };
 
 INTERNAL {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v1 2/3] crypto/cnxk: add queue pair reset support
  2024-08-22 10:28 [PATCH v1 1/3] cryptodev: add queue pair reset API Vidya Sagar Velumuri
@ 2024-08-22 10:28 ` Vidya Sagar Velumuri
  2024-09-18  5:40   ` Akhil Goyal
  2024-08-22 10:28 ` [PATCH v1 3/3] test/crypto: add support for error recovery Vidya Sagar Velumuri
  2024-09-18  5:40 ` [PATCH v1 1/3] cryptodev: add queue pair reset API Akhil Goyal
  2 siblings, 1 reply; 7+ messages in thread
From: Vidya Sagar Velumuri @ 2024-08-22 10:28 UTC (permalink / raw)
  To: Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao,
	Harman Kalra, Ankur Dwivedi, Anoob Joseph, Tejasree Kondoj
  Cc: gakhil, jerinj, vvelumuri, asasidharan, gmuthukrishn, dev

Add support for reset of a specific queue pair on cnxk platform.

Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>

diff --git a/drivers/common/cnxk/roc_cpt.c b/drivers/common/cnxk/roc_cpt.c
index aba2a49d19..3855d5e7a5 100644
--- a/drivers/common/cnxk/roc_cpt.c
+++ b/drivers/common/cnxk/roc_cpt.c
@@ -953,6 +953,20 @@ cpt_lf_fini(struct roc_cpt_lf *lf)
 	lf->iq_vaddr = NULL;
 }
 
+void
+roc_cpt_lf_reset(struct roc_cpt_lf *lf)
+{
+	if (lf == NULL)
+		return;
+
+	cpt_lf_misc_intr_enb_dis(lf, false);
+	cpt_lf_done_intr_enb_dis(lf, false);
+	roc_cpt_iq_disable(lf);
+	roc_cpt_iq_reset(lf);
+	cpt_lf_misc_intr_enb_dis(lf, true);
+	cpt_lf_done_intr_enb_dis(lf, true);
+}
+
 void
 roc_cpt_lf_fini(struct roc_cpt_lf *lf)
 {
diff --git a/drivers/common/cnxk/roc_cpt.h b/drivers/common/cnxk/roc_cpt.h
index e2e919f80f..0b9c933925 100644
--- a/drivers/common/cnxk/roc_cpt.h
+++ b/drivers/common/cnxk/roc_cpt.h
@@ -187,6 +187,7 @@ int __roc_api roc_cpt_dev_configure(struct roc_cpt *roc_cpt, int nb_lf, bool rxc
 void __roc_api roc_cpt_dev_clear(struct roc_cpt *roc_cpt);
 int __roc_api roc_cpt_lf_init(struct roc_cpt *roc_cpt, struct roc_cpt_lf *lf);
 void __roc_api roc_cpt_lf_fini(struct roc_cpt_lf *lf);
+void __roc_api roc_cpt_lf_reset(struct roc_cpt_lf *lf);
 int __roc_api roc_cpt_lf_ctx_flush(struct roc_cpt_lf *lf, void *cptr,
 				   bool inval);
 int __roc_api roc_cpt_lf_ctx_reload(struct roc_cpt_lf *lf, void *cptr);
diff --git a/drivers/common/cnxk/version.map b/drivers/common/cnxk/version.map
index f98738d07e..88e59f4161 100644
--- a/drivers/common/cnxk/version.map
+++ b/drivers/common/cnxk/version.map
@@ -74,6 +74,7 @@ INTERNAL {
 	roc_cpt_lf_ctx_reload;
 	roc_cpt_lf_init;
 	roc_cpt_lf_fini;
+	roc_cpt_lf_reset;
 	roc_cpt_lfs_print;
 	roc_cpt_lmtline_init;
 	roc_cpt_parse_hdr_dump;
diff --git a/drivers/crypto/cnxk/cn10k_cryptodev_ops.c b/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
index 780785d656..f18ed69014 100644
--- a/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
@@ -1976,6 +1976,7 @@ struct rte_cryptodev_ops cn10k_cpt_ops = {
 	.stats_reset = NULL,
 	.queue_pair_setup = cnxk_cpt_queue_pair_setup,
 	.queue_pair_release = cnxk_cpt_queue_pair_release,
+	.queue_pair_reset = cnxk_cpt_queue_pair_reset,
 
 	/* Symmetric crypto ops */
 	.sym_session_get_size = cnxk_cpt_sym_session_get_size,
diff --git a/drivers/crypto/cnxk/cn9k_cryptodev_ops.c b/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
index f443cb9563..ae00af5019 100644
--- a/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
@@ -707,6 +707,7 @@ struct rte_cryptodev_ops cn9k_cpt_ops = {
 	.stats_reset = NULL,
 	.queue_pair_setup = cnxk_cpt_queue_pair_setup,
 	.queue_pair_release = cnxk_cpt_queue_pair_release,
+	.queue_pair_reset = cnxk_cpt_queue_pair_reset,
 
 	/* Symmetric crypto ops */
 	.sym_session_get_size = cnxk_cpt_sym_session_get_size,
diff --git a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
index cfcfa79fdf..09ce98db88 100644
--- a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
@@ -496,6 +496,27 @@ cnxk_cpt_queue_pair_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return ret;
 }
 
+int
+cnxk_cpt_queue_pair_reset(struct rte_cryptodev *dev, uint16_t qp_id,
+			  const struct rte_cryptodev_qp_conf *conf, int socket_id)
+{
+	if (conf == NULL) {
+		struct cnxk_cpt_vf *vf = dev->data->dev_private;
+		struct roc_cpt_lf *lf;
+
+		if (vf == NULL)
+			return -ENOTSUP;
+
+		lf = vf->cpt.lf[qp_id];
+		roc_cpt_lf_reset(lf);
+		roc_cpt_iq_enable(lf);
+
+		return 0;
+	}
+
+	return cnxk_cpt_queue_pair_setup(dev, qp_id, conf, socket_id);
+}
+
 uint32_t
 cnxk_cpt_qp_depth_used(void *qptr)
 {
diff --git a/drivers/crypto/cnxk/cnxk_cryptodev_ops.h b/drivers/crypto/cnxk/cnxk_cryptodev_ops.h
index caf6ac35e5..e4d85801d7 100644
--- a/drivers/crypto/cnxk/cnxk_cryptodev_ops.h
+++ b/drivers/crypto/cnxk/cnxk_cryptodev_ops.h
@@ -120,6 +120,9 @@ int cnxk_cpt_queue_pair_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 			      const struct rte_cryptodev_qp_conf *conf,
 			      int socket_id __rte_unused);
 
+int cnxk_cpt_queue_pair_reset(struct rte_cryptodev *dev, uint16_t qp_id,
+			      const struct rte_cryptodev_qp_conf *conf, int socket_id);
+
 int cnxk_cpt_queue_pair_release(struct rte_cryptodev *dev, uint16_t qp_id);
 
 unsigned int cnxk_cpt_sym_session_get_size(struct rte_cryptodev *dev);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v1 3/3] test/crypto: add support for error recovery
  2024-08-22 10:28 [PATCH v1 1/3] cryptodev: add queue pair reset API Vidya Sagar Velumuri
  2024-08-22 10:28 ` [PATCH v1 2/3] crypto/cnxk: add queue pair reset support Vidya Sagar Velumuri
@ 2024-08-22 10:28 ` Vidya Sagar Velumuri
  2024-09-18  5:42   ` Akhil Goyal
  2024-09-18  5:40 ` [PATCH v1 1/3] cryptodev: add queue pair reset API Akhil Goyal
  2 siblings, 1 reply; 7+ messages in thread
From: Vidya Sagar Velumuri @ 2024-08-22 10:28 UTC (permalink / raw)
  To: Akhil Goyal, Fan Zhang
  Cc: jerinj, anoobj, vvelumuri, asasidharan, ktejasree, gmuthukrishn, dev

Add a callback for error recovery and register it with cryptodev.
Add a unit test to verify the error recovery of cryptodev.
The unit test generates an error by passing an mbuf to cryptodev
allocated from heap memory. The registered callback will be called as
part of error recovery.
The unit test verifies the cryptodev recovery by testing a simple
crypto operation.

Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index c846b26ed1..bfbdb8b10a 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -71,6 +71,9 @@
 #define IN_PLACE 0
 #define OUT_OF_PLACE 1
 
+#define QP_DRAIN_TIMEOUT 100
+#define HW_ERR_RECOVER_TIMEOUT 500
+
 static int gbl_driver_id;
 
 static enum rte_security_session_action_type gbl_action_type =
@@ -202,6 +205,81 @@ static struct crypto_unittest_params unittest_params;
 static bool enq_cb_called;
 static bool deq_cb_called;
 
+enum cryptodev_err_state {
+	CRYPTODEV_ERR_CLEARED,
+	CRYPTODEV_ERR_TRIGGERED,
+	CRYPTODEV_ERR_UNRECOVERABLE,
+};
+
+static enum cryptodev_err_state crypto_err = CRYPTODEV_ERR_CLEARED;
+
+static void
+test_cryptodev_error_cb(uint8_t dev_id, enum rte_cryptodev_event_type event, void *cb_arg)
+{
+	struct crypto_testsuite_params *ts_params = &testsuite_params;
+	uint16_t qp_id;
+	int ticks = 0;
+	int ret = 0;
+
+	RTE_SET_USED(event);
+	RTE_SET_USED(cb_arg);
+
+	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
+		ret = rte_cryptodev_queue_pair_event_error_query(dev_id, qp_id);
+		if (ret)
+			break;
+	}
+	if (ret == 1) {
+		/* Wait for the queue to be completely drained */
+		while (rte_cryptodev_qp_depth_used(dev_id, qp_id) != 0) {
+			rte_delay_ms(10);
+			ticks++;
+			if (ticks > QP_DRAIN_TIMEOUT) {
+				crypto_err = CRYPTODEV_ERR_UNRECOVERABLE;
+				return;
+			}
+		}
+		if (rte_cryptodev_queue_pair_reset(dev_id, qp_id, NULL, 0)) {
+			crypto_err = CRYPTODEV_ERR_UNRECOVERABLE;
+			return;
+		}
+	}
+
+	crypto_err = CRYPTODEV_ERR_CLEARED;
+}
+
+static struct rte_mbuf *
+create_mbuf_from_heap(int pkt_len, uint8_t pattern)
+{
+	struct rte_mbuf *m = NULL;
+	uint8_t *dst;
+
+	m = calloc(1, MBUF_SIZE);
+	if (m == NULL) {
+		printf("Cannot create mbuf from heap");
+		return NULL;
+	}
+
+	/* Set the default values to the mbuf */
+	m->nb_segs = 1;
+	m->port = RTE_MBUF_PORT_INVALID;
+	m->buf_len = MBUF_SIZE - sizeof(struct rte_mbuf) - RTE_PKTMBUF_HEADROOM;
+	rte_pktmbuf_reset_headroom(m);
+	__rte_mbuf_sanity_check(m, 1);
+
+	m->buf_addr = (char *)m + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM;
+
+	memset(m->buf_addr, pattern, m->buf_len);
+	dst = (uint8_t *)rte_pktmbuf_append(m, pkt_len);
+	if (dst == NULL) {
+		printf("Cannot append %d bytes to the mbuf\n", pkt_len);
+		free(m);
+		return NULL;
+	}
+
+	return m;
+}
+
 int
 process_sym_raw_dp_op(uint8_t dev_id, uint16_t qp_id,
 		struct rte_crypto_op *op, uint8_t is_cipher, uint8_t is_auth,
@@ -12868,6 +12946,172 @@ test_tls_1_3_record_proto_sgl_oop(void)
 
 #endif
 
+static int
+test_cryptodev_error_recover_helper_check(void)
+{
+	struct crypto_testsuite_params *ts_params = &testsuite_params;
+	struct rte_cryptodev_info dev_info;
+	uint64_t feat_flags;
+	int ret;
+
+	rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+	feat_flags = dev_info.feature_flags;
+
+	/* Skip the test if queue pair reset is not supported */
+	ret = rte_cryptodev_queue_pair_reset(ts_params->valid_devs[0], 0, NULL, 0);
+	if (ret == -ENOTSUP)
+		return TEST_SKIPPED;
+
+	ret = rte_cryptodev_qp_depth_used(ts_params->valid_devs[0], 0);
+	if (ret == -ENOTSUP)
+		return TEST_SKIPPED;
+
+	if (!(feat_flags & RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO) ||
+	    ((global_api_test_type == CRYPTODEV_RAW_API_TEST) &&
+	     !(dev_info.feature_flags & RTE_CRYPTODEV_FF_SYM_RAW_DP))) {
+		RTE_LOG(INFO, USER1, "Feature flag req for AES Cipheronly, testsuite not met\n");
+		return TEST_SKIPPED;
+	}
+
+	return 0;
+}
+
+static int
+test_cryptodev_error_recover_helper(uint8_t dev_id, const void *test_data, bool generate_err)
+{
+	struct crypto_testsuite_params *ts_params = &testsuite_params;
+	struct crypto_unittest_params *ut_params = &unittest_params;
+	const struct blockcipher_test_data *tdata = test_data;
+	uint8_t cipher_key[tdata->cipher_key.len];
+	struct rte_crypto_sym_op *sym_op = NULL;
+	struct rte_crypto_op *op = NULL;
+	char *dst;
+
+	memcpy(cipher_key, tdata->cipher_key.data, tdata->cipher_key.len);
+	ut_params->cipher_xform.next = NULL;
+	ut_params->cipher_xform.type = RTE_CRYPTO_SYM_XFORM_CIPHER;
+	ut_params->cipher_xform.cipher.algo = tdata->crypto_algo;
+	ut_params->cipher_xform.cipher.op = RTE_CRYPTO_CIPHER_OP_ENCRYPT;
+	ut_params->cipher_xform.cipher.key.data = cipher_key;
+	ut_params->cipher_xform.cipher.key.length = tdata->cipher_key.len;
+	ut_params->cipher_xform.cipher.iv.offset = IV_OFFSET;
+	ut_params->cipher_xform.cipher.iv.length = tdata->iv.len;
+
+	ut_params->sess = rte_cryptodev_sym_session_create(dev_id, &ut_params->cipher_xform,
+							   ts_params->session_mpool);
+	TEST_ASSERT_NOT_NULL(ut_params->sess, "Session creation failed");
+
+	ut_params->op = rte_crypto_op_alloc(ts_params->op_mpool, RTE_CRYPTO_OP_TYPE_SYMMETRIC);
+	TEST_ASSERT_NOT_NULL(ut_params->op, "Failed to allocate symmetric crypto op");
+
+	memcpy(rte_crypto_op_ctod_offset(ut_params->op, uint8_t *, IV_OFFSET), tdata->iv.data,
+	       tdata->iv.len);
+	sym_op = ut_params->op->sym;
+	sym_op->cipher.data.offset = tdata->cipher_offset;
+	sym_op->cipher.data.length = tdata->ciphertext.len - tdata->cipher_offset;
+
+	rte_crypto_op_attach_sym_session(ut_params->op, ut_params->sess);
+
+	if (generate_err) {
+		ut_params->ibuf = create_mbuf_from_heap(tdata->ciphertext.len, 0);
+		if (ut_params->ibuf == NULL)
+			return TEST_FAILED;
+		crypto_err = CRYPTODEV_ERR_TRIGGERED;
+	} else {
+		ut_params->ibuf = rte_pktmbuf_alloc(ts_params->mbuf_pool);
+	}
+
+	/* clear mbuf payload */
+	memset(rte_pktmbuf_mtod(ut_params->ibuf, uint8_t *), 0,
+	       rte_pktmbuf_tailroom(ut_params->ibuf));
+
+	dst = rte_pktmbuf_mtod_offset(ut_params->ibuf, char *, 0);
+	memcpy(dst, tdata->plaintext.data, tdata->plaintext.len);
+
+	sym_op->m_src = ut_params->ibuf;
+	sym_op->m_dst = NULL;
+
+	op = process_crypto_request(ts_params->valid_devs[0], ut_params->op);
+
+	if (generate_err) {
+		free(ut_params->ibuf);
+		ut_params->ibuf = NULL;
+		if (op == NULL) {
+			rte_cryptodev_sym_session_free(ts_params->valid_devs[0], ut_params->sess);
+			ut_params->sess = NULL;
+			return TEST_SUCCESS;
+		} else {
+			return TEST_FAILED;
+		}
+	}
+
+	TEST_ASSERT_EQUAL(ut_params->op->status, RTE_CRYPTO_OP_STATUS_SUCCESS,
+			  "crypto op processing failed");
+
+	TEST_ASSERT_BUFFERS_ARE_EQUAL(dst, tdata->ciphertext.data + tdata->cipher_offset,
+				      tdata->ciphertext.len - tdata->cipher_offset,
+				      "Data not as expected");
+
+	rte_cryptodev_sym_session_free(ts_params->valid_devs[0], ut_params->sess);
+	ut_params->sess = NULL;
+
+	return TEST_SUCCESS;
+}
+
+/*
+ * This unit test verifies the recovery of the cryptodev from any fatal error.
+ * It verifies a single test data multiple times in a iteration. It uses a flag and flips its value
+ * for every call to helper function.
+ *
+ * When the flag is set to 0, the helper function verifies the test data without generating any
+ * errors, i.e: verifies the default behaviour of the cryptodev.
+ *
+ * When the flag is set to 1, the helper function allocates a pointer from heap or non-DMAble
+ * memory and passes the pointer to cryptodev PMD inorder to generate a fatal error. Once the error
+ * is generated, it waits till the cryptodev is recoverd from the error.
+ *
+ * Iterates the above steps multiple times, to verify the error recovery of cryptodev and behaviour
+ * of cryptodev after the recovery.
+ */
+static int
+test_cryptodev_verify_error_recover(const void *test_data)
+{
+	int ret = TEST_FAILED;
+	int i, num_itr = 5;
+
+	ret = test_cryptodev_error_recover_helper_check();
+	if (ret)
+		return ret;
+
+	TEST_ASSERT_SUCCESS(rte_cryptodev_callback_register(p_testsuite_params->valid_devs[0],
+							    RTE_CRYPTODEV_EVENT_ERROR,
+							    test_cryptodev_error_cb, NULL),
+			    "Failed to register Cryptodev callback");
+
+	for (i = 0; i < num_itr; i++) {
+		int ticks = 0;
+
+		ret = test_cryptodev_error_recover_helper(p_testsuite_params->valid_devs[0],
+							  test_data, false);
+		TEST_ASSERT_EQUAL(ret, TEST_SUCCESS, "encryption failed");
+
+		/* Generate Error */
+		ret = test_cryptodev_error_recover_helper(p_testsuite_params->valid_devs[0],
+							  test_data, true);
+		TEST_ASSERT_EQUAL(ret, TEST_SUCCESS, "encryption failed");
+
+		/* Wait till cryptodev recovered from error */
+		while (crypto_err == CRYPTODEV_ERR_TRIGGERED) {
+			rte_delay_ms(10);
+			if (ticks++ > HW_ERR_RECOVER_TIMEOUT)
+				return TEST_FAILED;
+		}
+	}
+	TEST_ASSERT_EQUAL(crypto_err, CRYPTODEV_ERR_CLEARED, "cryptodev error recovery failed");
+
+	return ret;
+}
+
 static int
 test_AES_GCM_authenticated_encryption_test_case_1(void)
 {
@@ -18442,6 +18686,8 @@ static struct unit_test_suite cryptodev_gen_testsuite  = {
 		TEST_CASE_ST(ut_setup, ut_teardown, test_stats),
 		TEST_CASE_ST(ut_setup, ut_teardown, test_enq_callback_setup),
 		TEST_CASE_ST(ut_setup, ut_teardown, test_deq_callback_setup),
+		TEST_CASE_NAMED_WITH_DATA("Verify cryptodev error recover", ut_setup, ut_teardown,
+					  test_cryptodev_verify_error_recover, &aes_test_data_4),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
 };
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v1 1/3] cryptodev: add queue pair reset API
  2024-08-22 10:28 [PATCH v1 1/3] cryptodev: add queue pair reset API Vidya Sagar Velumuri
  2024-08-22 10:28 ` [PATCH v1 2/3] crypto/cnxk: add queue pair reset support Vidya Sagar Velumuri
  2024-08-22 10:28 ` [PATCH v1 3/3] test/crypto: add support for error recovery Vidya Sagar Velumuri
@ 2024-09-18  5:40 ` Akhil Goyal
  2024-10-09 14:29   ` Akhil Goyal
  2 siblings, 1 reply; 7+ messages in thread
From: Akhil Goyal @ 2024-09-18  5:40 UTC (permalink / raw)
  To: Vidya Sagar Velumuri, Fan Zhang
  Cc: Jerin Jacob, Anoob Joseph, Vidya Sagar Velumuri,
	Aakash Sasidharan, Tejasree Kondoj, Gowrishankar Muthukrishnan,
	dev


> Subject: [PATCH v1 1/3] cryptodev: add queue pair reset API
> 
> The API will reset the specific queue pair of a cryptodev.
> The current API, cryptodev_queue_pair_setup(), requires the cryptodev
> to be stopped before reconfiguring any queue pair. Stopping the
> cryptodev in one thread can result in a segmentation fault when
> multiple queues are used for enqueue and dequeue operations.
> 
> On supported PMDs, the cryptodev_queue_pair_reset() will
> reconfigure/reset the queue pair without affecting other queues or
> the cryptodev state.
> 
> The caller should ensure that there are no enqueue or dequeue operations
> ongoing on that queue and that there are no inflight packets before
> calling this API.
> 
> Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>

Acked-by: Akhil Goyal <gakhil@marvell.com>

Please update release notes.

> 
> diff --git a/lib/cryptodev/cryptodev_pmd.h b/lib/cryptodev/cryptodev_pmd.h
> index 6c114f7181..311ae63abb 100644
> --- a/lib/cryptodev/cryptodev_pmd.h
> +++ b/lib/cryptodev/cryptodev_pmd.h
> @@ -290,6 +290,22 @@ typedef int (*cryptodev_queue_pair_setup_t)(struct
> rte_cryptodev *dev,
>  typedef int (*cryptodev_queue_pair_release_t)(struct rte_cryptodev *dev,
>  		uint16_t qp_id);
> 
> +/**
> + * Reset or reconfigure a queue pair for a device.
> + *
> + * @param	dev		Crypto device pointer
> + * @param	qp_id		Queue pair index
> + * @param	qp_conf		Queue configuration structure
> + * @param	socket_id	Socket index
> + *
> + * @return
> + *  - 0: on success.
> + *  - ENOTSUP: if crypto device does not support the operation.
> + */
> +typedef int (*cryptodev_queue_pair_reset_t)(struct rte_cryptodev *dev,
> +		uint16_t qp_id,	const struct rte_cryptodev_qp_conf *qp_conf,
> +		int socket_id);
> +
>  /**
>   * Create a session mempool to allocate sessions from
>   *
> @@ -476,6 +492,8 @@ struct rte_cryptodev_ops {
>  	/**< Set up a device queue pair. */
>  	cryptodev_queue_pair_release_t queue_pair_release;
>  	/**< Release a queue pair. */
> +	cryptodev_queue_pair_reset_t queue_pair_reset;
> +	/**< Reset a queue pair. */
> 
>  	cryptodev_sym_get_session_private_size_t sym_session_get_size;
>  	/**< Return private session. */
> diff --git a/lib/cryptodev/cryptodev_trace.h b/lib/cryptodev/cryptodev_trace.h
> index 935f0d564b..633f17afe1 100644
> --- a/lib/cryptodev/cryptodev_trace.h
> +++ b/lib/cryptodev/cryptodev_trace.h
> @@ -58,6 +58,16 @@ RTE_TRACE_POINT(
>  	rte_trace_point_emit_ptr(conf->mp_session);
>  )
> 
> +RTE_TRACE_POINT(
> +	rte_cryptodev_trace_queue_pair_reset,
> +	RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint16_t queue_pair_id,
> +		const struct rte_cryptodev_qp_conf *conf, int socket_id),
> +	rte_trace_point_emit_u8(dev_id);
> +	rte_trace_point_emit_u16(queue_pair_id);
> +	rte_trace_point_emit_u32(conf->nb_descriptors);
> +	rte_trace_point_emit_int(socket_id);
> +)
> +
>  RTE_TRACE_POINT(
>  	rte_cryptodev_trace_sym_session_pool_create,
>  	RTE_TRACE_POINT_ARGS(const char *name, uint32_t nb_elts,
> diff --git a/lib/cryptodev/cryptodev_trace_points.c
> b/lib/cryptodev/cryptodev_trace_points.c
> index 7403412553..6f37780595 100644
> --- a/lib/cryptodev/cryptodev_trace_points.c
> +++ b/lib/cryptodev/cryptodev_trace_points.c
> @@ -21,6 +21,9 @@ RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_close,
>  RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_setup,
>  	lib.cryptodev.queue.pair.setup)
> 
> +RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_reset,
> +	lib.cryptodev.queue.pair.reset)
> +
>  RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_pool_create,
>  	lib.cryptodev.sym.pool.create)
> 
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index 682c9f49d0..281ca51cf3 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -1222,6 +1222,30 @@ rte_cryptodev_queue_pairs_config(struct
> rte_cryptodev *dev, uint16_t nb_qpairs,
>  	return 0;
>  }
> 
> +int
> +rte_cryptodev_queue_pair_reset(uint8_t dev_id, uint16_t queue_pair_id,
> +		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id)
> +{
> +	struct rte_cryptodev *dev;
> +
> +	if (!rte_cryptodev_is_valid_dev(dev_id)) {
> +		CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_crypto_devices[dev_id];
> +	if (queue_pair_id >= dev->data->nb_queue_pairs) {
> +		CDEV_LOG_ERR("Invalid queue_pair_id=%d", queue_pair_id);
> +		return -EINVAL;
> +	}
> +
> +	if (*dev->dev_ops->queue_pair_reset == NULL)
> +		return -ENOTSUP;
> +
> +	rte_cryptodev_trace_queue_pair_reset(dev_id, queue_pair_id, qp_conf,
> socket_id);
> +	return (*dev->dev_ops->queue_pair_reset)(dev, queue_pair_id, qp_conf,
> socket_id);
> +}
> +
>  int
>  rte_cryptodev_configure(uint8_t dev_id, struct rte_cryptodev_config *config)
>  {
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index bec947f6d5..e0fc35db2a 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -840,6 +840,35 @@ int
>  rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
>  		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Reset a queue pair for a device.
> + * The caller of this API must ensure that, there are no enqueues to the queue
> and there are no
> + * pending/inflight packets in the queue when the API is called.
> + * The API can reconfigure the queue pair when the queue pair configuration
> data is provided.
> + *
> + * @param	dev_id		The identifier of the device.
> + * @param	queue_pair_id	The index of the queue pairs to set up. The
> value must be in the
> + *				range [0, nb_queue_pair - 1] previously supplied
> to
> + *				rte_cryptodev_configure().
> + * @param	qp_conf		The pointer to configuration data to be used for
> the queue pair.
> + *				It should be NULL, if the API is called from an
> interrupt context.
> + * @param	socket_id	The *socket_id* argument is the socket
> identifier in case of NUMA.
> + *				The value can be *SOCKET_ID_ANY* if there is
> no NUMA constraint
> + *				for the DMA memory allocated for the queue
> pair.
> + *
> + * @return
> + *   - 0:  Queue pair is reset successfully.
> + *   - ENOTSUP: If the operation is not supported by the PMD.
> + *   - <0: Queue pair reset failed
> + */
> +__rte_experimental
> +int
> +rte_cryptodev_queue_pair_reset(uint8_t dev_id, uint16_t queue_pair_id,
> +		const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
> +
>  /**
>   * Get the status of queue pairs setup on a specific crypto device
>   *
> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> index fdac0d876e..eec06d9939 100644
> --- a/lib/cryptodev/version.map
> +++ b/lib/cryptodev/version.map
> @@ -87,6 +87,9 @@ EXPERIMENTAL {
> 
>  	# added in 24.03
>  	__rte_cryptodev_trace_qp_depth_used;
> +
> +	# added in 24.07
> +	rte_cryptodev_queue_pair_reset;
>  };
> 
>  INTERNAL {
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v1 2/3] crypto/cnxk: add queue pair reset support
  2024-08-22 10:28 ` [PATCH v1 2/3] crypto/cnxk: add queue pair reset support Vidya Sagar Velumuri
@ 2024-09-18  5:40   ` Akhil Goyal
  0 siblings, 0 replies; 7+ messages in thread
From: Akhil Goyal @ 2024-09-18  5:40 UTC (permalink / raw)
  To: Vidya Sagar Velumuri, Nithin Kumar Dabilpuram,
	Kiran Kumar Kokkilagadda, Sunil Kumar Kori,
	Satha Koteswara Rao Kottidi, Harman Kalra, Ankur Dwivedi,
	Anoob Joseph, Tejasree Kondoj
  Cc: Jerin Jacob, Vidya Sagar Velumuri, Aakash Sasidharan,
	Gowrishankar Muthukrishnan, dev

> Subject: [PATCH v1 2/3] crypto/cnxk: add queue pair reset support
> 
> Add support for reset of a specific queue pair on cnxk platform.
> 
> Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>



^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v1 3/3] test/crypto: add support for error recovery
  2024-08-22 10:28 ` [PATCH v1 3/3] test/crypto: add support for error recovery Vidya Sagar Velumuri
@ 2024-09-18  5:42   ` Akhil Goyal
  0 siblings, 0 replies; 7+ messages in thread
From: Akhil Goyal @ 2024-09-18  5:42 UTC (permalink / raw)
  To: Vidya Sagar Velumuri, Fan Zhang
  Cc: Jerin Jacob, Anoob Joseph, Vidya Sagar Velumuri,
	Aakash Sasidharan, Tejasree Kondoj, Gowrishankar Muthukrishnan,
	dev

> Subject: [PATCH v1 3/3] test/crypto: add support for error recovery
> 
> Add a callback for error recovery and register it with cryptodev.
> Add a unit test to verify the error recovery of cryptodev.
> The unit test generates an error by passing an mbuf to cryptodev
> allocated from heap memory. The registered callback will be called as
> part of error recovery.
> The unit test verifies the cryptodev recovery by testing a simple
> crypto operation.
> 
> Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v1 1/3] cryptodev: add queue pair reset API
  2024-09-18  5:40 ` [PATCH v1 1/3] cryptodev: add queue pair reset API Akhil Goyal
@ 2024-10-09 14:29   ` Akhil Goyal
  0 siblings, 0 replies; 7+ messages in thread
From: Akhil Goyal @ 2024-10-09 14:29 UTC (permalink / raw)
  To: Vidya Sagar Velumuri, Fan Zhang
  Cc: Jerin Jacob, Anoob Joseph, Vidya Sagar Velumuri,
	Aakash Sasidharan, Tejasree Kondoj, Gowrishankar Muthukrishnan,
	dev

> > Subject: [PATCH v1 1/3] cryptodev: add queue pair reset API
> >
> > The API will reset the specific queue pair of a cryptodev.
> > The current API, cryptodev_queue_pair_setup(), requires the cryptodev
> > to be stopped before reconfiguring any queue pair. Stopping the
> > cryptodev in one thread can result in a segmentation fault when
> > multiple queues are used for enqueue and dequeue operations.
> >
> > On supported PMDs, the cryptodev_queue_pair_reset() will
> > reconfigure/reset the queue pair without affecting other queues or
> > the cryptodev state.
> >
> > The caller should ensure that there are no enqueue or dequeue operations
> > ongoing on that queue and that there are no inflight packets before
> > calling this API.
> >
> > Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> 
> Acked-by: Akhil Goyal <gakhil@marvell.com>
> 
> Please update release notes.

> > diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> > index fdac0d876e..eec06d9939 100644
> > --- a/lib/cryptodev/version.map
> > +++ b/lib/cryptodev/version.map
> > @@ -87,6 +87,9 @@ EXPERIMENTAL {
> >
> >  	# added in 24.03
> >  	__rte_cryptodev_trace_qp_depth_used;
> > +
> > +	# added in 24.07
Changed this to 24.11

Added release note updated for new API.

Applied to dpdk-next-crypto
Thanks.

> > +	rte_cryptodev_queue_pair_reset;
> >  };


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-10-09 14:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-22 10:28 [PATCH v1 1/3] cryptodev: add queue pair reset API Vidya Sagar Velumuri
2024-08-22 10:28 ` [PATCH v1 2/3] crypto/cnxk: add queue pair reset support Vidya Sagar Velumuri
2024-09-18  5:40   ` Akhil Goyal
2024-08-22 10:28 ` [PATCH v1 3/3] test/crypto: add support for error recovery Vidya Sagar Velumuri
2024-09-18  5:42   ` Akhil Goyal
2024-09-18  5:40 ` [PATCH v1 1/3] cryptodev: add queue pair reset API Akhil Goyal
2024-10-09 14:29   ` Akhil Goyal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).