diff mbox

[dpdk-dev,2/2] crypto/aesni_mb: add new option to select SIMD mode

Message ID 1480671985-3677-3-git-send-email-declan.doherty@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Pablo de Lara Guarch
Headers

Checks

Context Check Description
checkpatch/checkpatch warning coding style issues

Commit Message

Doherty, Declan Dec. 2, 2016, 9:46 a.m. UTC 
Add new initialisation option to the aesni_mb_pmd to allow the user to specify
which set of SIMD functions to load from the AESNI Multi-Buffer Crypto for IPsec
library.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c         | 141 ++++++++++++++++++---
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h |   7 +
 2 files changed, 129 insertions(+), 19 deletions(-)

Comments

Thomas Monjalon Dec. 2, 2016, 10:37 a.m. UTC | #1 
2016-12-02 09:46, Declan Doherty:
> Add new initialisation option to the aesni_mb_pmd to allow the user to specify
> which set of SIMD functions to load from the AESNI Multi-Buffer Crypto for IPsec
> library.

Why let user choose? Isn't the most recent the better?

This PMD and other software crypto PMDs could (should?) override the CFLAGS
like rte_acl do, in order to be able to use recent SIMD functions even if it
was globally disabled by the compilation target.
See my comment in the rte_memset thread.
Doherty, Declan Dec. 2, 2016, 4:05 p.m. UTC | #2 
On 02/12/16 10:37, Thomas Monjalon wrote:
> 2016-12-02 09:46, Declan Doherty:
>> Add new initialisation option to the aesni_mb_pmd to allow the user to specify
>> which set of SIMD functions to load from the AESNI Multi-Buffer Crypto for IPsec
>> library.
>
> Why let user choose? Isn't the most recent the better?
>
> This PMD and other software crypto PMDs could (should?) override the CFLAGS
> like rte_acl do, in order to be able to use recent SIMD functions even if it
> was globally disabled by the compilation target.
> See my comment in the rte_memset thread.
>


In general yes, I was mainly using this to allow quick performance 
comparisons between different platforms and different instruction sets 
on the same platform without recompilation, but I admit that this is 
probably not a normal end user use case.

I'll look at the CFLAGS options you mention and address in a V2.
diff mbox

Patch

diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index c400b17..70b1d20 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -594,17 +594,16 @@  aesni_mb_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 	return nb_dequeued;
 }
 
-
 static int cryptodev_aesni_mb_remove(const char *name);
 
 static int
 cryptodev_aesni_mb_create(const char *name,
-		struct rte_crypto_vdev_init_params *init_params)
+		struct rte_crypto_vdev_init_params *init_params,
+		struct aesni_mb_init_params *aesni_mb_init_params)
 {
 	struct rte_cryptodev *dev;
 	char crypto_dev_name[RTE_CRYPTODEV_NAME_MAX_LEN];
 	struct aesni_mb_private *internals;
-	enum aesni_mb_vector_mode vector_mode;
 
 	/* Check CPU for support for AES instruction set */
 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
@@ -612,18 +611,50 @@  cryptodev_aesni_mb_create(const char *name,
 		return -EFAULT;
 	}
 
-	/* Check CPU for supported vector instruction set */
-	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F))
-		vector_mode = RTE_AESNI_MB_AVX512;
-	else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
-		vector_mode = RTE_AESNI_MB_AVX2;
-	else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX))
-		vector_mode = RTE_AESNI_MB_AVX;
-	else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
-		vector_mode = RTE_AESNI_MB_SSE;
-	else {
-		MB_LOG_ERR("Vector instructions are not supported by CPU");
-		return -EFAULT;
+
+	switch(aesni_mb_init_params->mode) {
+	case RTE_AESNI_MB_AVX512:
+		if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) {
+			MB_LOG_ERR("specified instruction set AVX512 "
+					"not supported by CPU");
+			return -EFAULT;
+		}
+		break;
+	case RTE_AESNI_MB_AVX2:
+		if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2)) {
+			MB_LOG_ERR("specified instruction set AVX2 "
+					"not supported by CPU");
+			return -EFAULT;
+		}
+		break;
+	case RTE_AESNI_MB_AVX:
+		if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX)) {
+			MB_LOG_ERR("specified instruction set AVX "
+					"not supported by CPU");
+			return -EFAULT;
+		}
+		break;
+	case RTE_AESNI_MB_SSE:
+		if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1)) {
+			MB_LOG_ERR("specified instruction set SSE "
+					"not supported by CPU");
+			return -EFAULT;
+		}
+		break;
+	default:
+		/* Check CPU for supported vector instruction set */
+		if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F))
+			aesni_mb_init_params->mode = RTE_AESNI_MB_AVX512;
+		else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
+			aesni_mb_init_params->mode = RTE_AESNI_MB_AVX2;
+		else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX))
+			aesni_mb_init_params->mode = RTE_AESNI_MB_AVX;
+		else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
+			aesni_mb_init_params->mode = RTE_AESNI_MB_SSE;
+		else {
+			MB_LOG_ERR("Vector instructions are not supported by CPU");
+			return -EFAULT;
+		}
 	}
 
 	/* create a unique device name */
@@ -652,7 +683,7 @@  cryptodev_aesni_mb_create(const char *name,
 			RTE_CRYPTODEV_FF_SYM_OPERATION_CHAINING |
 			RTE_CRYPTODEV_FF_CPU_AESNI;
 
-	switch (vector_mode) {
+	switch (aesni_mb_init_params->mode) {
 	case RTE_AESNI_MB_SSE:
 		dev->feature_flags |= RTE_CRYPTODEV_FF_CPU_SSE;
 		break;
@@ -672,7 +703,7 @@  cryptodev_aesni_mb_create(const char *name,
 	/* Set vector instructions mode supported */
 	internals = dev->data->dev_private;
 
-	internals->vector_mode = vector_mode;
+	internals->vector_mode = aesni_mb_init_params->mode;
 	internals->max_nb_queue_pairs = init_params->max_nb_queue_pairs;
 	internals->max_nb_sessions = init_params->max_nb_sessions;
 
@@ -684,18 +715,90 @@  cryptodev_aesni_mb_create(const char *name,
 	return -EFAULT;
 }
 
+static int
+parse_simd_mode_option(const char *key __rte_unused,
+		const char *value, void *extra_args)
+{
+	int retval = -1;
+
+	enum aesni_mb_vector_mode *mode =
+			(enum aesni_mb_vector_mode *)extra_args;
+
+	if (strcmp("avx512", value) == 0) {
+		*mode = RTE_AESNI_MB_AVX512;
+		retval = 0;
+	}
+	else if (strcmp("avx2", value) == 0) {
+		*mode = RTE_AESNI_MB_AVX2;
+		retval = 0;
+	}
+	else if (strcmp("avx", value) == 0) {
+		*mode = RTE_AESNI_MB_AVX;
+		retval = 0;
+	}
+	else if (strcmp("sse", value) == 0) {
+		*mode = RTE_AESNI_MB_SSE;
+		retval = 0;
+	}
+
+	return retval;
+}
+
+static const char *cryptodev_vdev_valid_params[] = {
+		AESNI_MB_PMD_SIMD_MODE_ARG
+};
+
+static int
+aesni_mb_parse_init_params(struct aesni_mb_init_params *params,
+		const char *input_args)
+{
+	struct rte_kvargs *kvlist = NULL;
+	int ret = 0;
+
+	if (params == NULL)
+		return -EINVAL;
+
+	if (input_args) {
+		kvlist = rte_kvargs_parse(input_args,
+				cryptodev_vdev_valid_params);
+		if (kvlist == NULL)
+			return -1;
+
+		ret = rte_kvargs_process(kvlist,
+				AESNI_MB_PMD_SIMD_MODE_ARG,
+					&parse_simd_mode_option,
+					&params->mode);
+		if (ret < 0)
+			goto free_kvlist;
+	}
+
+free_kvlist:
+	rte_kvargs_free(kvlist);
+	return ret;
+}
+
 
 static int
 cryptodev_aesni_mb_probe(const char *name,
 		const char *input_args)
 {
+	int retval;
+	struct aesni_mb_init_params aesni_mb_init_params = {
+			RTE_AESNI_MB_NOT_SUPPORTED
+	};
 	struct rte_crypto_vdev_init_params init_params = {
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_QUEUE_PAIRS,
 		RTE_CRYPTODEV_VDEV_DEFAULT_MAX_NB_SESSIONS,
 		rte_socket_id()
 	};
 
-	rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
+	retval = rte_cryptodev_parse_vdev_init_params(&init_params, input_args);
+	if (retval)
+		RTE_LOG(ERR, PMD, "Failed to parse default init params\n");
+
+	retval = aesni_mb_parse_init_params(&aesni_mb_init_params, input_args);
+	if (retval)
+		RTE_LOG(ERR, PMD, "Failed to parse aesni mb init params\n");
 
 	RTE_LOG(INFO, PMD, "Initialising %s on NUMA node %d\n", name,
 			init_params.socket_id);
@@ -704,7 +807,7 @@  cryptodev_aesni_mb_probe(const char *name,
 	RTE_LOG(INFO, PMD, "  Max number of sessions = %d\n",
 			init_params.max_nb_sessions);
 
-	return cryptodev_aesni_mb_create(name, &init_params);
+	return cryptodev_aesni_mb_create(name, &init_params, &aesni_mb_init_params);
 }
 
 static int
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
index 17f367f..aea5da1 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
@@ -225,5 +225,12 @@  aesni_mb_set_session_parameters(const struct aesni_mb_ops *mb_ops,
 extern struct rte_cryptodev_ops *rte_aesni_mb_pmd_ops;
 
 
+#define AESNI_MB_PMD_SIMD_MODE_ARG		("simd-mode")
+
+/** Device specific initialisation parameters */
+struct aesni_mb_init_params {
+	enum aesni_mb_vector_mode mode;
+};
+
 
 #endif /* _RTE_AESNI_MB_PMD_PRIVATE_H_ */