OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

# virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: RE: [virtio-dev] Re: [Qemu-devel] [PATCH v15 0/2] virtio-crypto: virtio crypto device specification

• From: "Gonglei (Arei)" <arei.gonglei@huawei.com>
• To: "Gonglei (Arei)" <arei.gonglei@huawei.com>, "Michael S. Tsirkin" <mst@redhat.com>
• Date: Mon, 16 Jan 2017 12:43:10 +0000

Hi Michael and others,

I'd like to redefine struct virtio_crypto_op_data_req is as below:

struct virtio_crypto_op_data_req {

union {
struct virtio_crypto_sym_data_req   sym_req;
struct virtio_crypto_hash_data_req  hash_req;
struct virtio_crypto_mac_data_req   mac_req;
struct virtio_crypto_sym_data_req_non_sess   sym_non_sess_req;
struct virtio_crypto_hash_data_req_non_sess  hash_non_sess_req;
struct virtio_crypto_mac_data_req_non_sess   mac_non_sess_req;
} u;
};

The length of union in the structure will be changed, which from current 48 byte to 72 byte.

We couldn't redefine a structure named virtio_crypto_op_data_req_non_sess,
Because the feature bits are for crypto services, not for the wrapper packet request.

It's impossible to mixed use struct virtio_crypto_op_data_req and
struct named virtio_crypto_op_data_req_non_sess for one data virtqueue.

For driver compabity, I can submit patches for linux dirver and Qemu to change the length
of struct virtio_crypto_op_data_req.u

Is the approach available?

Thanks,
-Gonglei

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Saturday, January 14, 2017 9:21 AM
> To: 'Michael S. Tsirkin'
> Cc: Halil Pasic; qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> Huangweidong (C); john.griffin@intel.com; cornelia.huck@de.ibm.com;
> Zhoujian (jay, Euler); Varun.Sethi@freescale.com; denglingli@chinamobile.com;
> arei.gonglei@hotmail.com; agraf@suse.de; nmorey@kalray.eu;
> vincent.jardin@6wind.com; Ola.Liljedahl@arm.com; Luonengjun;
> xin.zeng@intel.com; liang.j.ma@intel.com; stefanha@redhat.com; Shiqing Fan;
> Jani Kokkonen; brian.a.keating@intel.com; Claudio Fontana;
> mike.caraman@nxp.com; Wubin (H)
> Subject: RE: [virtio-dev] Re: [Qemu-devel] [PATCH v15 0/2] virtio-crypto: virtio
> crypto device specification
>
> >
> > On Fri, Jan 13, 2017 at 02:54:29AM +0000, Gonglei (Arei) wrote:
> > >
> > > >
> > > > On Thu, Jan 12, 2017 at 12:26:24PM +0000, Gonglei (Arei) wrote:
> > > > > Hi,
> > > > >
> > > > >
> > > > > >
> > > > > > On 01/04/2017 11:10 AM, Gonglei (Arei) wrote:
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I attach the diff files between v14 and v15 for better review.
> > > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > only had a quick look. Will try to come back to this later.
> > > > > >
> > > > > That's cool.
> > > > >
> > > > > > > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > > > > > > index 9f7faf0..884ee95 100644
> > > > > > > --- a/virtio-crypto.tex
> > > > > > > +++ b/virtio-crypto.tex
> > > > > > > @@ -2,8 +2,8 @@
> > > > > > >
> > > > > > >  The virtio crypto device is a virtual cryptography device as well as a
> > kind
> > > > of
> > > > > > >  virtual hardware accelerator for virtual machines. The encryption
> > and
> > > > > > > -decryption requests are placed in the data queue and are ultimately
> > > > handled
> > > > > > by the
> > > > > > > -backend crypto accelerators. The second queue is the control queue
> > used
> > > > to
> > > > > > create
> > > > > > > +decryption requests are placed in any of the data active queues and
> > are
> > > > > > ultimately handled by the
> > > > > > s/data active/active data/
> > > > > > > +backend crypto accelerators. The second kind of queue is the
> control
> > > > queue
> > > > > > used to create
> > > > > > >  or destroy sessions for symmetric algorithms and will control some
> > > > > > advanced
> > > > > > >  features in the future. The virtio crypto device provides the
> following
> > > > crypto
> > > > > > >  services: CIPHER, MAC, HASH, and AEAD.
> > > > > >
> > > > > > [..]
> > > > > >
> > > > > > > ===============The below diff shows the changes of add
> > non-session
> > > > mode
> > > > > > support:
> > > > > > >
> > > > > > > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > > > > > > index 884ee95..44819f9 100644
> > > > > > > --- a/virtio-crypto.tex
> > > > > > > +++ b/virtio-crypto.tex
> > > > > > > @@ -26,7 +26,10 @@ N is set by \field{max_dataqueues}.
> > > > > > >
> > > > > > >  \subsection{Feature bits}\label{sec:Device Types / Crypto Device /
> > > > Feature
> > > > > > bits}
> > > > > > >
> > > > > > > -None currently defined.
> > > > > > > +VIRTIO_CRYPTO_F_CIPHER_SESSION_MODE (1) Session mode is
> > > > available
> > > > > > for CIPHER service.
> > > > > > > +VIRTIO_CRYPTO_F_HASH_SESSION_MODE (2) Session mode is
> > available
> > > > for
> > > > > > HASH service.
> > > > > > > +VIRTIO_CRYPTO_F_MAC_SESSION_MODE (3) Session mode is
> > available
> > > > for
> > > > > > MAC service.
> > > > > > > +VIRTIO_CRYPTO_F_AEAD_SESSION_MODE (4) Session mode is
> > available
> > > > for
> > > > > > AEAD service.
> > > > > > >
> > > > > > >  \subsection{Device configuration layout}\label{sec:Device Types /
> > > > Crypto
> > > > > > Device / Device configuration layout}
> > > > > > >
> > > > > > > @@ -208,6 +211,9 @@ Operation parameters are
> algorithm-specific
> > > > > > parameters, output data is the
> > > > > > >  data that should be utilized in operations, and input data is equal
> to
> > > > > > >  "operation result + result data".
> > > > > > >
> > > > > > > +The device can support both session mode (See \ref{sec:Device
> Types
> > /
> > > > > > Crypto Device / Device Operation / Control Virtqueue / Session
> > operation})
> > > > and
> > > > > > non-session mode, for example,
> > > > > > > +As VIRTIO_CRYPTO_F_CIPHER_SESSION feature bit is negotiated,
> > the
> > > > driver
> > > > > > can use session mode for CIPHER service, otherwise it can only use
> > > > non-session
> > > > > > mode.
> > > > > > > +
> > > > > >
> > > > > > As far as I understand you are adding non-session mode to the mix but
> > > > > > providing feature bits for session mode. Would this render the the
> > current
> > > > > > implementation non-compliant?
> > > > > >
> > > > > You are right, shall we use feature bits for non-session mode for
> > compliancy?
> > > > > Or because the spec is on the fly, and some structures in the
> > virtio_crypto.h
> > > > need to
> > > > > be modified, can we keep the compliancy completely?
> > > > >
> > > > > Thanks,
> > > > > -Gonglei
> > > >
> > > > Since there's a linux driver upstream you must at least
> > > > keep compatibility with that.
> > > >
> > > Sure. We use feature bits for non-session mode then.
> > > For structures modification, do we need to do some specific
> > > actions for compatibility?  Take CIPHER service as an example:
> > >
> > > The present structure:
> > >
> > > struct virtio_crypto_cipher_para {
> > >     /*
> > >      * Byte Length of valid IV/Counter data pointed to by the below iv
> data.
> > >      *
> > >      * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or
> for
> > >      *   SNOW3G in UEA2 mode, this is the length of the IV (which
> > >      *   must be the same as the block length of the cipher).
> > >      * For block ciphers in CTR mode, this is the length of the counter
> > >      *   (which must be the same as the block length of the cipher).
> > >      */
> > >     le32 iv_len;
> > >     /* length of source data */
> > >     le32 src_data_len;
> > >     /* length of destination data */
> > >     le32 dst_data_len;
> > > };
> > >
> > > The future structure if supporting non-session based operations:
> > >
> > > struct virtio_crypto_cipher_para {
> > >     struct {
> > >         /* See VIRTIO_CRYPTO_CIPHER* above */
> > >         le32 algo;
> > >         /* length of key */
> > >         le32 keylen;
> > >
> > >         /* See VIRTIO_CRYPTO_OP_* above */
> > >         le32 op;
> > >     } sess_para;
> > >
> > >     /*
> > >      * Byte Length of valid IV/Counter data pointed to by the below iv
> data.
> > >      *
> > >      * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or
> for
> > >      *   SNOW3G in UEA2 mode, this is the length of the IV (which
> > >      *   must be the same as the block length of the cipher).
> > >      * For block ciphers in CTR mode, this is the length of the counter
> > >      *   (which must be the same as the block length of the cipher).
> > >      */
> > >     le32 iv_len;
> > >     /* length of source data */
> > >     le32 src_data_len;
> > >     /* length of destination data */
> > >     le32 dst_data_len;
> > > };
> > >
> > > Thanks,
> > > -Gonglei
> >
> > So you will have to maintain both structures for when non-session based
> > feature is and aren't present. You will have to give them different
> > names, too.
> >
> OK, I get your point. :)
>
> Thanks,
> -Gonglei



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]