[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [Qemu-devel] [virtio-dev] Re: [PATCH v15 0/2] virtio-crypto: virtio crypto device specification
On 01/16/2017 01:43 PM, Gonglei (Arei) wrote: > Hi Michael and others, > > I'd like to redefine struct virtio_crypto_op_data_req is as below: > > struct virtio_crypto_op_data_req { > struct virtio_crypto_op_header header; > > 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_aead_data_req aead_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; > struct virtio_crypto_aead_data_req_non_sess aead_non_sess_req; > __u8 padding[72]; > } 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. > You mean virtio_crypto_op_data_req.virtio_crypto_op_header.flags are conceptually meant for something else and using that field woulb be misuse? > 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. > I do not understand what are you trying to say here. I think you should tell us what is the new feature and how it is guarded. Would this mean that session or non-session mode will be tied to the whole device, or to one data-queue. If it's data-queue level how is it controlled (e.g. control queue)? I guess virtio_crypto_op_data_req.virtio_crypto_op_header.session_id would become meaningless in case of non_sess? > 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? > In general and AFAIU any new behavior is possible, iff it is appropriately guarded by some negotiation mechanism and does not break per-existing code which knows nothing about the new stuff. I would not mind seeing a spec re-spin with a proper proposal for session-less or stateless or whatever mode. Cheers, Halil > 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]