Re: [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler

[Daniel P. Berrange]

On Tue, Sep 13, 2016 at 11:52:08AM +0800, Gonglei wrote:
> crypto queue is a gallery used for executing crypto
> operation, which supports both synchronization and
> asynchronization. The thoughts stolen from net/queue.c
> 
> Signed-off-by: Gonglei <address@hidden>
> ---
>  crypto/Makefile.objs          |   1 +
>  crypto/crypto-queue.c         | 206 
> ++++++++++++++++++++++++++++++++++++++++++
>  crypto/crypto.c               |  28 ++++++
>  include/crypto/crypto-queue.h |  69 ++++++++++++++
>  include/crypto/crypto.h       |  12 +++
>  5 files changed, 316 insertions(+)
>  create mode 100644 crypto/crypto-queue.c
>  create mode 100644 include/crypto/crypto-queue.h
> 

> diff --git a/include/crypto/crypto-queue.h b/include/crypto/crypto-queue.h
> new file mode 100644
> index 0000000..6fba64d
> --- /dev/null
> +++ b/include/crypto/crypto-queue.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * Authors:
> + *    Gonglei <address@hidden>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */

Again, wrong license header.

> +#ifndef QEMU_CRYPTO_QUEUE_H
> +#define QEMU_CRYPTO_QUEUE_H
> +
> +#include "qemu-common.h"
> +
> +typedef struct CryptoPacket CryptoPacket;
> +typedef struct CryptoQueue CryptoQueue;
> +typedef struct CryptoPacketBuf CryptoPacketBuf;
> +
> +typedef void (CryptoPacketSent) (CryptoClientState *, int);

As previously, I'd expect naming of

 QCryptoCryptodevPacket
 QCryptoCryptodevPacketBuf
 QCryptoCryptodevQueue

> +
> +
> +/* Returns:
> + *   >0 - success
> + *    0 - queue packet for future redelivery
> + *   <0 - failure (discard packet)
> + */
> +typedef int (CryptoQueueDeliverFunc)(CryptoClientState *sender,
> +                                     unsigned flags,
> +                                     void *header_opaque,
> +                                     void *opaque);
> +
> +CryptoQueue *
> +qemu_new_crypto_queue(CryptoQueueDeliverFunc *deliver, void *opaque);
> +
> +void qemu_crypto_queue_cache(CryptoQueue *queue,
> +                               unsigned flags,
> +                               CryptoClientState *sender,
> +                               void *opaque,
> +                               CryptoPacketSent *sent_cb);
> +
> +void qemu_del_crypto_queue(CryptoQueue *queue);
> +
> +int qemu_crypto_queue_send(CryptoQueue *queue,
> +                                unsigned flags,
> +                                CryptoClientState *sender,
> +                                void *opaque,
> +                                CryptoPacketSent *sent_cb);
> +
> +void qemu_crypto_queue_purge(CryptoQueue *queue, CryptoClientState *from);
> +bool qemu_crypto_queue_flush(CryptoQueue *queue);

And naming of

  qcrypto_cryptodev_queue_purge
  qcrypto_cryptodev_queue_flush
  etc.

Also missing docs for this file

> +#endif /* QEMU_CRYPTO_QUEUE_H */
> diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
> index f93f6f9..46b3b9e 100644
> --- a/include/crypto/crypto.h
> +++ b/include/crypto/crypto.h
> @@ -29,6 +29,8 @@
>  
>  #include "qemu/queue.h"
>  #include "qapi-types.h"
> +#include "crypto/crypto-queue.h"
> +
>  
>  typedef void (CryptoPoll)(CryptoClientState *, bool);
>  typedef void (CryptoCleanup) (CryptoClientState *);
> @@ -52,6 +54,8 @@ struct CryptoClientState {
>      char *model;
>      char *name;
>      char info_str[256];
> +    CryptoQueue *incoming_queue;
> +    unsigned int queue_index;
>      CryptoClientDestructor *destructor;
>  };
>  
> @@ -62,5 +66,13 @@ CryptoClientState *new_crypto_client(CryptoClientInfo 
> *info,
>                                      CryptoClientState *peer,
>                                      const char *model,
>                                      const char *name);
> +int qemu_deliver_crypto_packet(CryptoClientState *sender,
> +                              unsigned flags,
> +                              void *header_opqaue,
> +                              void *opaque);
> +int qemu_send_crypto_packet_async(CryptoClientState *sender,
> +                                unsigned flags,
> +                                void *opaque,
> +                                CryptoPacketSent *sent_cb);

Missing docs for these API additions.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|