From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from huawei.com (szxga02-in.huawei.com [45.249.212.188]) by dpdk.org (Postfix) with ESMTP id 7F4591D647; Fri, 15 Jun 2018 06:41:48 +0200 (CEST) Received: from DGGEMM405-HUB.china.huawei.com (unknown [172.30.72.56]) by Forcepoint Email with ESMTP id 0F4D984931B96; Fri, 15 Jun 2018 12:41:33 +0800 (CST) Received: from DGGEMM508-MBX.china.huawei.com ([169.254.2.165]) by DGGEMM405-HUB.china.huawei.com ([10.3.20.213]) with mapi id 14.03.0382.000; Fri, 15 Jun 2018 12:41:31 +0800 From: "Zhoujian (jay)" To: Fan Zhang , "dev@dpdk.org" CC: "pablo.de.lara.guarch@intel.com" , "stable@dpdk.org" Thread-Topic: [PATCH] crypto/virtio: fix iv physical address Thread-Index: AQHUA9CUjn2zPX7njk2ctvLNb00YmqRgumRg Date: Fri, 15 Jun 2018 04:41:30 +0000 Message-ID: References: <20180614110257.10967-1-roy.fan.zhang@intel.com> In-Reply-To: <20180614110257.10967-1-roy.fan.zhang@intel.com> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.177.19.14] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH] crypto/virtio: fix iv physical address X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Jun 2018 04:41:50 -0000 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) ; > stable@dpdk.org > Subject: [PATCH] crypto/virtio: fix iv physical address >=20 > 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 costl= y. > This patch fixes this problem by adding iv field in virtio_crypto_op_cook= ie > and does a memcpy of iv instead. >=20 > Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue") > Cc: stable@dpdk.org >=20 > Signed-off-by: Fan Zhang > --- > 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(-) >=20 > 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 =3D 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 =3D 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 @@ >=20 > #define NUM_ENTRY_VIRTIO_CRYPTO_OP 7 >=20 > +#define VIRTIO_CRYPTO_MAX_IV_SIZE 32 > + > extern uint8_t cryptodev_virtio_driver_id; >=20 > 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]; > }; >=20 > /* > 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 =3D sizeof(struct virtio_crypto_op_data_req); > uint32_t indirect_vring_addr_offset =3D req_data_len + > sizeof(struct virtio_crypto_inhdr); > + uint32_t indirect_iv_addr_offset =3D indirect_vring_addr_offset + > + sizeof(struct vring_desc) * NUM_ENTRY_VIRTIO_CRYPTO_OP; > struct rte_crypto_sym_op *sym_op =3D cop->sym; > struct virtio_crypto_session *session =3D > (struct virtio_crypto_session *)get_session_private_data( @@ - > 259,7 +261,11 @@ virtqueue_crypto_sym_enqueue_xmit( >=20 > /* indirect vring: iv of cipher */ > if (session->iv.length) { > - desc[idx].addr =3D 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 =3D 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 =3D cop->phys_addr + session->iv.offset; } else { ... } Regards, Jay > desc[idx].len =3D session->iv.length; > desc[idx++].flags =3D VRING_DESC_F_NEXT; > } > -- > 2.13.6