diff mbox series

[v2] examples/vhost_crypto: fix zero copy

Message ID 20181114111642.5526-1-roy.fan.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] examples/vhost_crypto: fix zero copy |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Fan Zhang Nov. 14, 2018, 11:16 a.m. UTC 
  This patch fixes the zero copy enable problem for vhost crypto
sample application.

For some Crypto PMDs such as AESNI-MB and AESNI-GCM the data to
be processed will be made a copy in the same buffer but next to the
data. For example, to encrypt 64 bytes data the PMD will copy this
data from offset 64 to offset 123. This requires the application
provides the buffer with at least double of the data size.

However there is no way for VMs to know this limitation. When
zero-copy is enabled in Vhost the PMD may overwrite the buffer
next to the VM data to be processed, and further cause problems
such as Segmentation Fault or even worse, crashes the VM.

To fix the problem the user should avoid enabling the zero copy
for these Crypto PMDs. This patch adds the checking of the PMD
names to see if zero copy can be applied.

Fixes: 709521f4c2cd ("examples/vhost_crypto: support multi-core")

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
v2:
- removed unecessary checks
- Changed log message when zero-copy is not applicable.

 examples/vhost_crypto/main.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Maxime Coquelin Nov. 16, 2018, 10:48 a.m. UTC | #1 
On 11/14/18 12:16 PM, Fan Zhang wrote:
> This patch fixes the zero copy enable problem for vhost crypto
> sample application.
> 
> For some Crypto PMDs such as AESNI-MB and AESNI-GCM the data to
> be processed will be made a copy in the same buffer but next to the
> data. For example, to encrypt 64 bytes data the PMD will copy this
> data from offset 64 to offset 123. This requires the application
> provides the buffer with at least double of the data size.
> 
> However there is no way for VMs to know this limitation. When
> zero-copy is enabled in Vhost the PMD may overwrite the buffer
> next to the VM data to be processed, and further cause problems
> such as Segmentation Fault or even worse, crashes the VM.
> 
> To fix the problem the user should avoid enabling the zero copy
> for these Crypto PMDs. This patch adds the checking of the PMD
> names to see if zero copy can be applied.
> 
> Fixes: 709521f4c2cd ("examples/vhost_crypto: support multi-core")
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
> v2:
> - removed unecessary checks
> - Changed log message when zero-copy is not applicable.
> 
>   examples/vhost_crypto/main.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks!
Maxime
Ferruh Yigit Nov. 16, 2018, 3:23 p.m. UTC | #2 
On 11/16/2018 10:48 AM, Maxime Coquelin wrote:
> 
> 
> On 11/14/18 12:16 PM, Fan Zhang wrote:
>> This patch fixes the zero copy enable problem for vhost crypto
>> sample application.
>>
>> For some Crypto PMDs such as AESNI-MB and AESNI-GCM the data to
>> be processed will be made a copy in the same buffer but next to the
>> data. For example, to encrypt 64 bytes data the PMD will copy this
>> data from offset 64 to offset 123. This requires the application
>> provides the buffer with at least double of the data size.
>>
>> However there is no way for VMs to know this limitation. When
>> zero-copy is enabled in Vhost the PMD may overwrite the buffer
>> next to the VM data to be processed, and further cause problems
>> such as Segmentation Fault or even worse, crashes the VM.
>>
>> To fix the problem the user should avoid enabling the zero copy
>> for these Crypto PMDs. This patch adds the checking of the PMD
>> names to see if zero copy can be applied.
>>
>> Fixes: 709521f4c2cd ("examples/vhost_crypto: support multi-core")
>>
>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
>> ---
>> v2:
>> - removed unecessary checks
>> - Changed log message when zero-copy is not applicable.
>>
>>   examples/vhost_crypto/main.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Applied to dpdk-next-net/master, thanks.
diff mbox series

Patch

diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
index cbb5e49d2..f08babd97 100644
--- a/examples/vhost_crypto/main.c
+++ b/examples/vhost_crypto/main.c
@@ -4,6 +4,7 @@ 
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
 #include <stdbool.h>
 #include <assert.h>
@@ -442,6 +443,9 @@  free_resource(void)
 		struct lcore_option *lo = &options.los[i];
 		struct vhost_crypto_info *info = options.infos[i];
 
+		if (!info)
+			continue;
+
 		rte_mempool_free(info->cop_pool);
 		rte_mempool_free(info->sess_pool);
 
@@ -493,6 +497,19 @@  main(int argc, char *argv[])
 		info->nb_vids = lo->nb_sockets;
 
 		rte_cryptodev_info_get(info->cid, &dev_info);
+		if (options.zero_copy == RTE_VHOST_CRYPTO_ZERO_COPY_ENABLE) {
+#define VHOST_CRYPTO_CDEV_NAME_AESNI_MB_PMD	crypto_aesni_mb
+#define VHOST_CRYPTO_CDEV_NAME_AESNI_GCM_PMD	crypto_aesni_gcm
+			if (strstr(dev_info.driver_name,
+				RTE_STR(VHOST_CRYPTO_CDEV_NAME_AESNI_MB_PMD)) ||
+				strstr(dev_info.driver_name,
+				RTE_STR(VHOST_CRYPTO_CDEV_NAME_AESNI_GCM_PMD)))
+			RTE_LOG(ERR, USER1, "Cannot enable zero-copy in %s\n",
+					dev_info.driver_name);
+			ret = -EPERM;
+			goto error_exit;
+		}
+
 		if (dev_info.max_nb_queue_pairs < info->qid + 1) {
 			RTE_LOG(ERR, USER1, "Number of queues cannot over %u",
 					dev_info.max_nb_queue_pairs);