DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address
@ 2018-06-14 11:02 Fan Zhang
  2018-06-14 12:38 ` Akhil Goyal
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Fan Zhang @ 2018-06-14 11:02 UTC (permalink / raw)
  To: dev; +Cc: pablo.de.lara.guarch, jianjay.zhou, stable

The physical address of IV required by Virtio was computed using
crypto operations' physical address plus the offset. However not
all crypto ops will have physical address field initialized and
compute it runtimely is costly. This patch fixes this problem by
adding iv field in virtio_crypto_op_cookie and does a memcpy of
iv instead.

Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
Cc: stable@dpdk.org

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 drivers/crypto/virtio/virtio_cryptodev.c | 6 ++++++
 drivers/crypto/virtio/virtio_cryptodev.h | 3 +++
 drivers/crypto/virtio/virtio_rxtx.c      | 8 +++++++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/virtio/virtio_cryptodev.c b/drivers/crypto/virtio/virtio_cryptodev.c
index df88953f6..6ffa7619c 100644
--- a/drivers/crypto/virtio/virtio_cryptodev.c
+++ b/drivers/crypto/virtio/virtio_cryptodev.c
@@ -1223,6 +1223,12 @@ virtio_crypto_sym_pad_op_ctrl_req(
 	/* Get cipher xform from crypto xform chain */
 	cipher_xform = virtio_crypto_get_cipher_xform(xform);
 	if (cipher_xform) {
+		if (cipher_xform->iv.length > VIRTIO_CRYPTO_MAX_IV_SIZE) {
+			VIRTIO_CRYPTO_SESSION_LOG_ERR(
+				"cipher IV cannot longer than %u",
+				VIRTIO_CRYPTO_MAX_IV_SIZE);
+			return -1;
+		}
 		if (is_chainned)
 			ret = virtio_crypto_sym_pad_cipher_param(
 				&ctrl->u.sym_create_session.u.chain.para
diff --git a/drivers/crypto/virtio/virtio_cryptodev.h b/drivers/crypto/virtio/virtio_cryptodev.h
index e402c0309..676e008d9 100644
--- a/drivers/crypto/virtio/virtio_cryptodev.h
+++ b/drivers/crypto/virtio/virtio_cryptodev.h
@@ -16,6 +16,8 @@
 
 #define NUM_ENTRY_VIRTIO_CRYPTO_OP 7
 
+#define VIRTIO_CRYPTO_MAX_IV_SIZE 32
+
 extern uint8_t cryptodev_virtio_driver_id;
 
 enum virtio_crypto_cmd_id {
@@ -29,6 +31,7 @@ struct virtio_crypto_op_cookie {
 	struct virtio_crypto_op_data_req data_req;
 	struct virtio_crypto_inhdr inhdr;
 	struct vring_desc desc[NUM_ENTRY_VIRTIO_CRYPTO_OP];
+	uint8_t iv[VIRTIO_CRYPTO_MAX_IV_SIZE];
 };
 
 /*
diff --git a/drivers/crypto/virtio/virtio_rxtx.c b/drivers/crypto/virtio/virtio_rxtx.c
index 450392843..1b16221d0 100644
--- a/drivers/crypto/virtio/virtio_rxtx.c
+++ b/drivers/crypto/virtio/virtio_rxtx.c
@@ -203,6 +203,8 @@ virtqueue_crypto_sym_enqueue_xmit(
 	uint16_t req_data_len = sizeof(struct virtio_crypto_op_data_req);
 	uint32_t indirect_vring_addr_offset = req_data_len +
 		sizeof(struct virtio_crypto_inhdr);
+	uint32_t indirect_iv_addr_offset = indirect_vring_addr_offset +
+			sizeof(struct vring_desc) * NUM_ENTRY_VIRTIO_CRYPTO_OP;
 	struct rte_crypto_sym_op *sym_op = cop->sym;
 	struct virtio_crypto_session *session =
 		(struct virtio_crypto_session *)get_session_private_data(
@@ -259,7 +261,11 @@ virtqueue_crypto_sym_enqueue_xmit(
 
 	/* indirect vring: iv of cipher */
 	if (session->iv.length) {
-		desc[idx].addr = cop->phys_addr + session->iv.offset;
+		rte_memcpy(crypto_op_cookie->iv, rte_crypto_op_ctod_offset(cop,
+				uint8_t *, session->iv.offset),
+				session->iv.length);
+		desc[idx].addr = indirect_op_data_req_phys_addr +
+				indirect_iv_addr_offset;
 		desc[idx].len = session->iv.length;
 		desc[idx++].flags = VRING_DESC_F_NEXT;
 	}
-- 
2.13.6

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

* Re: [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address
  2018-06-14 11:02 [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address Fan Zhang
@ 2018-06-14 12:38 ` Akhil Goyal
  2018-06-15  4:45   ` Zhoujian (jay)
  2018-06-15  4:41 ` Zhoujian (jay)
  2018-06-26  2:10 ` [dpdk-dev] [PATCH v2] " Fan Zhang
  2 siblings, 1 reply; 8+ messages in thread
From: Akhil Goyal @ 2018-06-14 12:38 UTC (permalink / raw)
  To: Fan Zhang, dev; +Cc: pablo.de.lara.guarch, jianjay.zhou, stable

On 6/14/2018 4:32 PM, Fan Zhang wrote:
> The physical address of IV required by Virtio was computed using
> crypto operations' physical address plus the offset. However not
> all crypto ops will have physical address field initialized and
> compute it runtimely is costly. This patch fixes this problem by
> adding iv field in virtio_crypto_op_cookie and does a memcpy of
> iv instead.
>
> Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
> Cc: stable@dpdk.org
>
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  drivers/crypto/virtio/virtio_cryptodev.c | 6 ++++++
>  drivers/crypto/virtio/virtio_cryptodev.h | 3 +++
>  drivers/crypto/virtio/virtio_rxtx.c      | 8 +++++++-
>  3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/virtio/virtio_cryptodev.c b/drivers/crypto/virtio/virtio_cryptodev.c
> index df88953f6..6ffa7619c 100644
> --- a/drivers/crypto/virtio/virtio_cryptodev.c
> +++ b/drivers/crypto/virtio/virtio_cryptodev.c
> @@ -1223,6 +1223,12 @@ virtio_crypto_sym_pad_op_ctrl_req(
>  	/* Get cipher xform from crypto xform chain */
>  	cipher_xform = virtio_crypto_get_cipher_xform(xform);
>  	if (cipher_xform) {
> +		if (cipher_xform->iv.length > VIRTIO_CRYPTO_MAX_IV_SIZE) {
> +			VIRTIO_CRYPTO_SESSION_LOG_ERR(
> +				"cipher IV cannot longer than %u",
> +				VIRTIO_CRYPTO_MAX_IV_SIZE);
> +			return -1;
> +		}
>  		if (is_chainned)
>  			ret = virtio_crypto_sym_pad_cipher_param(
>  				&ctrl->u.sym_create_session.u.chain.para
> diff --git a/drivers/crypto/virtio/virtio_cryptodev.h b/drivers/crypto/virtio/virtio_cryptodev.h
> index e402c0309..676e008d9 100644
> --- a/drivers/crypto/virtio/virtio_cryptodev.h
> +++ b/drivers/crypto/virtio/virtio_cryptodev.h
> @@ -16,6 +16,8 @@
>
>  #define NUM_ENTRY_VIRTIO_CRYPTO_OP 7
>
> +#define VIRTIO_CRYPTO_MAX_IV_SIZE 32
> +

max iv size supported in capability is 16 and here it is 32.

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

* Re: [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address
  2018-06-14 11:02 [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address Fan Zhang
  2018-06-14 12:38 ` Akhil Goyal
@ 2018-06-15  4:41 ` Zhoujian (jay)
  2018-06-26  2:10 ` [dpdk-dev] [PATCH v2] " Fan Zhang
  2 siblings, 0 replies; 8+ messages in thread
From: Zhoujian (jay) @ 2018-06-15  4:41 UTC (permalink / raw)
  To: Fan Zhang, dev; +Cc: pablo.de.lara.guarch, stable

Hi Fan,

> -----Original Message-----
> From: Fan Zhang [mailto:roy.fan.zhang@intel.com]
> Sent: Thursday, June 14, 2018 7:03 PM
> To: dev@dpdk.org
> Cc: pablo.de.lara.guarch@intel.com; Zhoujian (jay) <jianjay.zhou@huawei.com>;
> stable@dpdk.org
> Subject: [PATCH] crypto/virtio: fix iv physical address
> 
> The physical address of IV required by Virtio was computed using crypto
> operations' physical address plus the offset. However not all crypto ops will
> have physical address field initialized and compute it runtimely is costly.
> This patch fixes this problem by adding iv field in virtio_crypto_op_cookie
> and does a memcpy of iv instead.
> 
> Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  drivers/crypto/virtio/virtio_cryptodev.c | 6 ++++++
> drivers/crypto/virtio/virtio_cryptodev.h | 3 +++
>  drivers/crypto/virtio/virtio_rxtx.c      | 8 +++++++-
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/virtio/virtio_cryptodev.c
> b/drivers/crypto/virtio/virtio_cryptodev.c
> index df88953f6..6ffa7619c 100644
> --- a/drivers/crypto/virtio/virtio_cryptodev.c
> +++ b/drivers/crypto/virtio/virtio_cryptodev.c
> @@ -1223,6 +1223,12 @@ virtio_crypto_sym_pad_op_ctrl_req(
>  	/* Get cipher xform from crypto xform chain */
>  	cipher_xform = virtio_crypto_get_cipher_xform(xform);
>  	if (cipher_xform) {
> +		if (cipher_xform->iv.length > VIRTIO_CRYPTO_MAX_IV_SIZE) {

Considering the consistency with iv.length, could we use VIRTIO_CRYPTO_MAX_IV_LENGTH
instead of VIRTIO_CRYPTO_MAX_IV_SIZE?

> +			VIRTIO_CRYPTO_SESSION_LOG_ERR(
> +				"cipher IV cannot longer than %u",

s/cipher IV/cipher IV length/ would be better, I think.

> +				VIRTIO_CRYPTO_MAX_IV_SIZE);
> +			return -1;
> +		}
>  		if (is_chainned)
>  			ret = virtio_crypto_sym_pad_cipher_param(
>  				&ctrl->u.sym_create_session.u.chain.para
> diff --git a/drivers/crypto/virtio/virtio_cryptodev.h
> b/drivers/crypto/virtio/virtio_cryptodev.h
> index e402c0309..676e008d9 100644
> --- a/drivers/crypto/virtio/virtio_cryptodev.h
> +++ b/drivers/crypto/virtio/virtio_cryptodev.h
> @@ -16,6 +16,8 @@
> 
>  #define NUM_ENTRY_VIRTIO_CRYPTO_OP 7
> 
> +#define VIRTIO_CRYPTO_MAX_IV_SIZE 32
> +
>  extern uint8_t cryptodev_virtio_driver_id;
> 
>  enum virtio_crypto_cmd_id {
> @@ -29,6 +31,7 @@ struct virtio_crypto_op_cookie {
>  	struct virtio_crypto_op_data_req data_req;
>  	struct virtio_crypto_inhdr inhdr;
>  	struct vring_desc desc[NUM_ENTRY_VIRTIO_CRYPTO_OP];
> +	uint8_t iv[VIRTIO_CRYPTO_MAX_IV_SIZE];
>  };
> 
>  /*
> diff --git a/drivers/crypto/virtio/virtio_rxtx.c
> b/drivers/crypto/virtio/virtio_rxtx.c
> index 450392843..1b16221d0 100644
> --- a/drivers/crypto/virtio/virtio_rxtx.c
> +++ b/drivers/crypto/virtio/virtio_rxtx.c
> @@ -203,6 +203,8 @@ virtqueue_crypto_sym_enqueue_xmit(
>  	uint16_t req_data_len = sizeof(struct virtio_crypto_op_data_req);
>  	uint32_t indirect_vring_addr_offset = req_data_len +
>  		sizeof(struct virtio_crypto_inhdr);
> +	uint32_t indirect_iv_addr_offset = indirect_vring_addr_offset +
> +			sizeof(struct vring_desc) * NUM_ENTRY_VIRTIO_CRYPTO_OP;
>  	struct rte_crypto_sym_op *sym_op = cop->sym;
>  	struct virtio_crypto_session *session =
>  		(struct virtio_crypto_session *)get_session_private_data( @@ -
> 259,7 +261,11 @@ virtqueue_crypto_sym_enqueue_xmit(
> 
>  	/* indirect vring: iv of cipher */
>  	if (session->iv.length) {
> -		desc[idx].addr = cop->phys_addr + session->iv.offset;
> +		rte_memcpy(crypto_op_cookie->iv, rte_crypto_op_ctod_offset(cop,
> +				uint8_t *, session->iv.offset),
> +				session->iv.length);
> +		desc[idx].addr = indirect_op_data_req_phys_addr +
> +				indirect_iv_addr_offset;

This ops of rte_memcpy could be avoided if cop->phys_addr is initialized,
so how about:

    if (cop->phys_addr) {
        desc[idx].addr = cop->phys_addr + session->iv.offset;
    } else {
        ...
    }


Regards,
Jay

>  		desc[idx].len = session->iv.length;
>  		desc[idx++].flags = VRING_DESC_F_NEXT;
>  	}
> --
> 2.13.6

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

* Re: [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address
  2018-06-14 12:38 ` Akhil Goyal
@ 2018-06-15  4:45   ` Zhoujian (jay)
  0 siblings, 0 replies; 8+ messages in thread
From: Zhoujian (jay) @ 2018-06-15  4:45 UTC (permalink / raw)
  To: Akhil Goyal, Fan Zhang, dev; +Cc: pablo.de.lara.guarch, stable

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Thursday, June 14, 2018 8:39 PM
> To: Fan Zhang <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: pablo.de.lara.guarch@intel.com; Zhoujian (jay) <jianjay.zhou@huawei.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address
> 
> On 6/14/2018 4:32 PM, Fan Zhang wrote:
> > The physical address of IV required by Virtio was computed using
> > crypto operations' physical address plus the offset. However not all
> > crypto ops will have physical address field initialized and compute it
> > runtimely is costly. This patch fixes this problem by adding iv field
> > in virtio_crypto_op_cookie and does a memcpy of iv instead.
> >
> > Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> >  drivers/crypto/virtio/virtio_cryptodev.c | 6 ++++++
> > drivers/crypto/virtio/virtio_cryptodev.h | 3 +++
> >  drivers/crypto/virtio/virtio_rxtx.c      | 8 +++++++-
> >  3 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/crypto/virtio/virtio_cryptodev.c
> > b/drivers/crypto/virtio/virtio_cryptodev.c
> > index df88953f6..6ffa7619c 100644
> > --- a/drivers/crypto/virtio/virtio_cryptodev.c
> > +++ b/drivers/crypto/virtio/virtio_cryptodev.c
> > @@ -1223,6 +1223,12 @@ virtio_crypto_sym_pad_op_ctrl_req(
> >  	/* Get cipher xform from crypto xform chain */
> >  	cipher_xform = virtio_crypto_get_cipher_xform(xform);
> >  	if (cipher_xform) {
> > +		if (cipher_xform->iv.length > VIRTIO_CRYPTO_MAX_IV_SIZE) {
> > +			VIRTIO_CRYPTO_SESSION_LOG_ERR(
> > +				"cipher IV cannot longer than %u",
> > +				VIRTIO_CRYPTO_MAX_IV_SIZE);
> > +			return -1;
> > +		}
> >  		if (is_chainned)
> >  			ret = virtio_crypto_sym_pad_cipher_param(
> >  				&ctrl->u.sym_create_session.u.chain.para
> > diff --git a/drivers/crypto/virtio/virtio_cryptodev.h
> > b/drivers/crypto/virtio/virtio_cryptodev.h
> > index e402c0309..676e008d9 100644
> > --- a/drivers/crypto/virtio/virtio_cryptodev.h
> > +++ b/drivers/crypto/virtio/virtio_cryptodev.h
> > @@ -16,6 +16,8 @@
> >
> >  #define NUM_ENTRY_VIRTIO_CRYPTO_OP 7
> >
> > +#define VIRTIO_CRYPTO_MAX_IV_SIZE 32
> > +
> 
> max iv size supported in capability is 16 and here it is 32.

Yes, I agree. We could use 16 for now.

Regards,
Jay

> 

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

* [dpdk-dev] [PATCH v2] crypto/virtio: fix iv physical address
  2018-06-14 11:02 [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address Fan Zhang
  2018-06-14 12:38 ` Akhil Goyal
  2018-06-15  4:41 ` Zhoujian (jay)
@ 2018-06-26  2:10 ` Fan Zhang
  2018-07-03 12:29   ` Zhoujian (jay)
  2 siblings, 1 reply; 8+ messages in thread
From: Fan Zhang @ 2018-06-26  2:10 UTC (permalink / raw)
  To: dev; +Cc: pablo.de.lara.guarch, jianjay.zhou, stable

The physical address of IV required by Virtio was computed using
crypto operations' physical address plus the offset. However not
all crypto ops will have physical address field initialized and
compute it runtimely is costly. This patch fixes this problem by
adding iv field in virtio_crypto_op_cookie and does a memcpy of
iv instead.

Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
Cc: stable@dpdk.org

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---

v2:
- change max iv size to 16
- use branch to avoid unnecessary memcpy

 drivers/crypto/virtio/virtio_cryptodev.c |  6 ++++++
 drivers/crypto/virtio/virtio_cryptodev.h |  3 +++
 drivers/crypto/virtio/virtio_rxtx.c      | 14 +++++++++++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/virtio/virtio_cryptodev.c b/drivers/crypto/virtio/virtio_cryptodev.c
index df88953f6..6ffa7619c 100644
--- a/drivers/crypto/virtio/virtio_cryptodev.c
+++ b/drivers/crypto/virtio/virtio_cryptodev.c
@@ -1223,6 +1223,12 @@ virtio_crypto_sym_pad_op_ctrl_req(
 	/* Get cipher xform from crypto xform chain */
 	cipher_xform = virtio_crypto_get_cipher_xform(xform);
 	if (cipher_xform) {
+		if (cipher_xform->iv.length > VIRTIO_CRYPTO_MAX_IV_SIZE) {
+			VIRTIO_CRYPTO_SESSION_LOG_ERR(
+				"cipher IV cannot longer than %u",
+				VIRTIO_CRYPTO_MAX_IV_SIZE);
+			return -1;
+		}
 		if (is_chainned)
 			ret = virtio_crypto_sym_pad_cipher_param(
 				&ctrl->u.sym_create_session.u.chain.para
diff --git a/drivers/crypto/virtio/virtio_cryptodev.h b/drivers/crypto/virtio/virtio_cryptodev.h
index e402c0309..0fd7b722e 100644
--- a/drivers/crypto/virtio/virtio_cryptodev.h
+++ b/drivers/crypto/virtio/virtio_cryptodev.h
@@ -16,6 +16,8 @@
 
 #define NUM_ENTRY_VIRTIO_CRYPTO_OP 7
 
+#define VIRTIO_CRYPTO_MAX_IV_SIZE 16
+
 extern uint8_t cryptodev_virtio_driver_id;
 
 enum virtio_crypto_cmd_id {
@@ -29,6 +31,7 @@ struct virtio_crypto_op_cookie {
 	struct virtio_crypto_op_data_req data_req;
 	struct virtio_crypto_inhdr inhdr;
 	struct vring_desc desc[NUM_ENTRY_VIRTIO_CRYPTO_OP];
+	uint8_t iv[VIRTIO_CRYPTO_MAX_IV_SIZE];
 };
 
 /*
diff --git a/drivers/crypto/virtio/virtio_rxtx.c b/drivers/crypto/virtio/virtio_rxtx.c
index 450392843..4f695f3e6 100644
--- a/drivers/crypto/virtio/virtio_rxtx.c
+++ b/drivers/crypto/virtio/virtio_rxtx.c
@@ -203,6 +203,8 @@ virtqueue_crypto_sym_enqueue_xmit(
 	uint16_t req_data_len = sizeof(struct virtio_crypto_op_data_req);
 	uint32_t indirect_vring_addr_offset = req_data_len +
 		sizeof(struct virtio_crypto_inhdr);
+	uint32_t indirect_iv_addr_offset = indirect_vring_addr_offset +
+			sizeof(struct vring_desc) * NUM_ENTRY_VIRTIO_CRYPTO_OP;
 	struct rte_crypto_sym_op *sym_op = cop->sym;
 	struct virtio_crypto_session *session =
 		(struct virtio_crypto_session *)get_session_private_data(
@@ -259,7 +261,17 @@ virtqueue_crypto_sym_enqueue_xmit(
 
 	/* indirect vring: iv of cipher */
 	if (session->iv.length) {
-		desc[idx].addr = cop->phys_addr + session->iv.offset;
+		if (cop->phys_addr)
+			desc[idx].addr = cop->phys_addr + session->iv.offset;
+		else {
+			rte_memcpy(crypto_op_cookie->iv,
+					rte_crypto_op_ctod_offset(cop,
+					uint8_t *, session->iv.offset),
+					session->iv.length);
+			desc[idx].addr = indirect_op_data_req_phys_addr +
+				indirect_iv_addr_offset;
+		}
+
 		desc[idx].len = session->iv.length;
 		desc[idx++].flags = VRING_DESC_F_NEXT;
 	}
-- 
2.13.6

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

* Re: [dpdk-dev] [PATCH v2] crypto/virtio: fix iv physical address
  2018-06-26  2:10 ` [dpdk-dev] [PATCH v2] " Fan Zhang
@ 2018-07-03 12:29   ` Zhoujian (jay)
  2018-07-05 10:53     ` De Lara Guarch, Pablo
  0 siblings, 1 reply; 8+ messages in thread
From: Zhoujian (jay) @ 2018-07-03 12:29 UTC (permalink / raw)
  To: Fan Zhang, dev; +Cc: pablo.de.lara.guarch, stable

> -----Original Message-----
> From: Fan Zhang [mailto:roy.fan.zhang@intel.com]
> Sent: Tuesday, June 26, 2018 10:11 AM
> To: dev@dpdk.org
> Cc: pablo.de.lara.guarch@intel.com; Zhoujian (jay) <jianjay.zhou@huawei.com>;
> stable@dpdk.org
> Subject: [PATCH v2] crypto/virtio: fix iv physical address
> 
> The physical address of IV required by Virtio was computed using crypto
> operations' physical address plus the offset. However not all crypto ops will
> have physical address field initialized and compute it runtimely is costly.
> This patch fixes this problem by adding iv field in virtio_crypto_op_cookie
> and does a memcpy of iv instead.
> 
> Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
> 
> v2:
> - change max iv size to 16
> - use branch to avoid unnecessary memcpy
> 
>  drivers/crypto/virtio/virtio_cryptodev.c |  6 ++++++
> drivers/crypto/virtio/virtio_cryptodev.h |  3 +++
>  drivers/crypto/virtio/virtio_rxtx.c      | 14 +++++++++++++-
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/virtio/virtio_cryptodev.c
> b/drivers/crypto/virtio/virtio_cryptodev.c
> index df88953f6..6ffa7619c 100644
> --- a/drivers/crypto/virtio/virtio_cryptodev.c
> +++ b/drivers/crypto/virtio/virtio_cryptodev.c
> @@ -1223,6 +1223,12 @@ virtio_crypto_sym_pad_op_ctrl_req(
>  	/* Get cipher xform from crypto xform chain */
>  	cipher_xform = virtio_crypto_get_cipher_xform(xform);
>  	if (cipher_xform) {
> +		if (cipher_xform->iv.length > VIRTIO_CRYPTO_MAX_IV_SIZE) {
> +			VIRTIO_CRYPTO_SESSION_LOG_ERR(
> +				"cipher IV cannot longer than %u",

Hi Fan,

As I mentioned in V1, do you agree to use "cipher IV length" or
"cipher IV size" instead of "cipher IV" here?

Apart from that, 

Reviewed-by: <jianjay.zhou@huawei.com>

> +				VIRTIO_CRYPTO_MAX_IV_SIZE);
> +			return -1;
> +		}
>  		if (is_chainned)
>  			ret = virtio_crypto_sym_pad_cipher_param(
>  				&ctrl->u.sym_create_session.u.chain.para
> diff --git a/drivers/crypto/virtio/virtio_cryptodev.h
> b/drivers/crypto/virtio/virtio_cryptodev.h
> index e402c0309..0fd7b722e 100644
> --- a/drivers/crypto/virtio/virtio_cryptodev.h
> +++ b/drivers/crypto/virtio/virtio_cryptodev.h
> @@ -16,6 +16,8 @@
> 
>  #define NUM_ENTRY_VIRTIO_CRYPTO_OP 7
> 
> +#define VIRTIO_CRYPTO_MAX_IV_SIZE 16
> +
>  extern uint8_t cryptodev_virtio_driver_id;
> 
>  enum virtio_crypto_cmd_id {
> @@ -29,6 +31,7 @@ struct virtio_crypto_op_cookie {
>  	struct virtio_crypto_op_data_req data_req;
>  	struct virtio_crypto_inhdr inhdr;
>  	struct vring_desc desc[NUM_ENTRY_VIRTIO_CRYPTO_OP];
> +	uint8_t iv[VIRTIO_CRYPTO_MAX_IV_SIZE];
>  };
> 
>  /*
> diff --git a/drivers/crypto/virtio/virtio_rxtx.c
> b/drivers/crypto/virtio/virtio_rxtx.c
> index 450392843..4f695f3e6 100644
> --- a/drivers/crypto/virtio/virtio_rxtx.c
> +++ b/drivers/crypto/virtio/virtio_rxtx.c
> @@ -203,6 +203,8 @@ virtqueue_crypto_sym_enqueue_xmit(
>  	uint16_t req_data_len = sizeof(struct virtio_crypto_op_data_req);
>  	uint32_t indirect_vring_addr_offset = req_data_len +
>  		sizeof(struct virtio_crypto_inhdr);
> +	uint32_t indirect_iv_addr_offset = indirect_vring_addr_offset +
> +			sizeof(struct vring_desc) * NUM_ENTRY_VIRTIO_CRYPTO_OP;
>  	struct rte_crypto_sym_op *sym_op = cop->sym;
>  	struct virtio_crypto_session *session =
>  		(struct virtio_crypto_session *)get_session_private_data( @@ -
> 259,7 +261,17 @@ virtqueue_crypto_sym_enqueue_xmit(
> 
>  	/* indirect vring: iv of cipher */
>  	if (session->iv.length) {
> -		desc[idx].addr = cop->phys_addr + session->iv.offset;
> +		if (cop->phys_addr)
> +			desc[idx].addr = cop->phys_addr + session->iv.offset;
> +		else {
> +			rte_memcpy(crypto_op_cookie->iv,
> +					rte_crypto_op_ctod_offset(cop,
> +					uint8_t *, session->iv.offset),
> +					session->iv.length);
> +			desc[idx].addr = indirect_op_data_req_phys_addr +
> +				indirect_iv_addr_offset;
> +		}
> +
>  		desc[idx].len = session->iv.length;
>  		desc[idx++].flags = VRING_DESC_F_NEXT;
>  	}
> --
> 2.13.6

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

* Re: [dpdk-dev] [PATCH v2] crypto/virtio: fix iv physical address
  2018-07-03 12:29   ` Zhoujian (jay)
@ 2018-07-05 10:53     ` De Lara Guarch, Pablo
  2018-07-06  1:12       ` Zhoujian (jay)
  0 siblings, 1 reply; 8+ messages in thread
From: De Lara Guarch, Pablo @ 2018-07-05 10:53 UTC (permalink / raw)
  To: Zhoujian (jay), Zhang, Roy Fan, dev; +Cc: stable

Hi Jianjay,


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhoujian (jay)
> Sent: Tuesday, July 3, 2018 1:29 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] crypto/virtio: fix iv physical address
> 
> > -----Original Message-----
> > From: Fan Zhang [mailto:roy.fan.zhang@intel.com]
> > Sent: Tuesday, June 26, 2018 10:11 AM
> > To: dev@dpdk.org
> > Cc: pablo.de.lara.guarch@intel.com; Zhoujian (jay)
> > <jianjay.zhou@huawei.com>; stable@dpdk.org
> > Subject: [PATCH v2] crypto/virtio: fix iv physical address
> >
> > The physical address of IV required by Virtio was computed using
> > crypto operations' physical address plus the offset. However not all
> > crypto ops will have physical address field initialized and compute it runtimely
> is costly.
> > This patch fixes this problem by adding iv field in
> > virtio_crypto_op_cookie and does a memcpy of iv instead.
> >
> > Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> >
> > v2:
> > - change max iv size to 16
> > - use branch to avoid unnecessary memcpy
> >
> >  drivers/crypto/virtio/virtio_cryptodev.c |  6 ++++++
> > drivers/crypto/virtio/virtio_cryptodev.h |  3 +++
> >  drivers/crypto/virtio/virtio_rxtx.c      | 14 +++++++++++++-
> >  3 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/crypto/virtio/virtio_cryptodev.c
> > b/drivers/crypto/virtio/virtio_cryptodev.c
> > index df88953f6..6ffa7619c 100644
> > --- a/drivers/crypto/virtio/virtio_cryptodev.c
> > +++ b/drivers/crypto/virtio/virtio_cryptodev.c
> > @@ -1223,6 +1223,12 @@ virtio_crypto_sym_pad_op_ctrl_req(
> >  	/* Get cipher xform from crypto xform chain */
> >  	cipher_xform = virtio_crypto_get_cipher_xform(xform);
> >  	if (cipher_xform) {
> > +		if (cipher_xform->iv.length > VIRTIO_CRYPTO_MAX_IV_SIZE) {
> > +			VIRTIO_CRYPTO_SESSION_LOG_ERR(
> > +				"cipher IV cannot longer than %u",
> 
> Hi Fan,
> 
> As I mentioned in V1, do you agree to use "cipher IV length" or "cipher IV size"
> instead of "cipher IV" here?

Fan is out of office this week. Since this is a minor change,
I can apply it and make that change on the fly.

Thanks,
Pablo

> 
> Apart from that,
> 
> Reviewed-by: <jianjay.zhou@huawei.com>

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

* Re: [dpdk-dev] [PATCH v2] crypto/virtio: fix iv physical address
  2018-07-05 10:53     ` De Lara Guarch, Pablo
@ 2018-07-06  1:12       ` Zhoujian (jay)
  0 siblings, 0 replies; 8+ messages in thread
From: Zhoujian (jay) @ 2018-07-06  1:12 UTC (permalink / raw)
  To: De Lara Guarch, Pablo, Zhang, Roy Fan, dev; +Cc: stable, wangxin (U)



> -----Original Message-----
> From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
> Sent: Thursday, July 05, 2018 6:54 PM
> To: Zhoujian (jay) <jianjay.zhou@huawei.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: RE: [PATCH v2] crypto/virtio: fix iv physical address
> 
> Hi Jianjay,
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhoujian (jay)
> > Sent: Tuesday, July 3, 2018 1:29 PM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> > Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> > stable@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2] crypto/virtio: fix iv physical
> > address
> >
> > > -----Original Message-----
> > > From: Fan Zhang [mailto:roy.fan.zhang@intel.com]
> > > Sent: Tuesday, June 26, 2018 10:11 AM
> > > To: dev@dpdk.org
> > > Cc: pablo.de.lara.guarch@intel.com; Zhoujian (jay)
> > > <jianjay.zhou@huawei.com>; stable@dpdk.org
> > > Subject: [PATCH v2] crypto/virtio: fix iv physical address
> > >
> > > The physical address of IV required by Virtio was computed using
> > > crypto operations' physical address plus the offset. However not all
> > > crypto ops will have physical address field initialized and compute
> > > it runtimely
> > is costly.
> > > This patch fixes this problem by adding iv field in
> > > virtio_crypto_op_cookie and does a memcpy of iv instead.
> > >
> > > Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > > ---
> > >
> > > v2:
> > > - change max iv size to 16
> > > - use branch to avoid unnecessary memcpy
> > >
> > >  drivers/crypto/virtio/virtio_cryptodev.c |  6 ++++++
> > > drivers/crypto/virtio/virtio_cryptodev.h |  3 +++
> > >  drivers/crypto/virtio/virtio_rxtx.c      | 14 +++++++++++++-
> > >  3 files changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/crypto/virtio/virtio_cryptodev.c
> > > b/drivers/crypto/virtio/virtio_cryptodev.c
> > > index df88953f6..6ffa7619c 100644
> > > --- a/drivers/crypto/virtio/virtio_cryptodev.c
> > > +++ b/drivers/crypto/virtio/virtio_cryptodev.c
> > > @@ -1223,6 +1223,12 @@ virtio_crypto_sym_pad_op_ctrl_req(
> > >  	/* Get cipher xform from crypto xform chain */
> > >  	cipher_xform = virtio_crypto_get_cipher_xform(xform);
> > >  	if (cipher_xform) {
> > > +		if (cipher_xform->iv.length > VIRTIO_CRYPTO_MAX_IV_SIZE) {
> > > +			VIRTIO_CRYPTO_SESSION_LOG_ERR(
> > > +				"cipher IV cannot longer than %u",
> >
> > Hi Fan,
> >
> > As I mentioned in V1, do you agree to use "cipher IV length" or "cipher IV
> size"
> > instead of "cipher IV" here?
> 
> Fan is out of office this week. Since this is a minor change, I can apply it
> and make that change on the fly.

Hi Pablo,
Thanks for doing this, :)

Regards,
Jay

> 
> Thanks,
> Pablo
> 
> >
> > Apart from that,
> >
> > Reviewed-by: <jianjay.zhou@huawei.com>

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

end of thread, other threads:[~2018-07-06  1:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-14 11:02 [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address Fan Zhang
2018-06-14 12:38 ` Akhil Goyal
2018-06-15  4:45   ` Zhoujian (jay)
2018-06-15  4:41 ` Zhoujian (jay)
2018-06-26  2:10 ` [dpdk-dev] [PATCH v2] " Fan Zhang
2018-07-03 12:29   ` Zhoujian (jay)
2018-07-05 10:53     ` De Lara Guarch, Pablo
2018-07-06  1:12       ` Zhoujian (jay)

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