All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: zhenwei pi <pizhenwei@bytedance.com>
Cc: arei.gonglei@huawei.com, dgilbert@redhat.com,
	pbonzini@redhat.com, berrange@redhat.com, armbru@redhat.com,
	qemu-devel@nongnu.org
Subject: Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
Date: Tue, 28 Feb 2023 07:56:14 -0500	[thread overview]
Message-ID: <20230228075511-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20230129025747.682282-12-pizhenwei@bytedance.com>

On Sun, Jan 29, 2023 at 10:57:46AM +0800, zhenwei pi wrote:
> Now we can use "query-stats" QMP command to query statistics of
> crypto devices. (Originally this was designed to show statistics
> by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
> querying configuration info by "query-cryptodev", and querying
> runtime performance info by "query-stats". This makes sense!)
> 
> Example:
> ~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
>    "arguments": {"target": "cryptodev"} }' | jq
> {
>   "return": [
>     {
>       "provider": "cryptodev",
>       "stats": [
>         {
>           "name": "asym-verify-bytes",
>           "value": 7680
>         },
>         ...
>         {
>           "name": "asym-decrypt-ops",
>           "value": 32
>         },
>         {
>           "name": "asym-encrypt-ops",
>           "value": 48
>         }
>       ],
>       "qom-path": "/objects/cryptodev0" # support asym only
>     },
>     {
>       "provider": "cryptodev",
>       "stats": [
>         {
>           "name": "asym-verify-bytes",
>           "value": 0
>         },
>         ...
>         {
>           "name": "sym-decrypt-bytes",
>           "value": 5376
>         },
>         ...
>       ],
>       "qom-path": "/objects/cryptodev1" # support asym/sym
>     }
>   ],
>   "id": "libvirt-422"
> }
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>

I assume since this has been out a long time and no
comments by maintainers it's ok from QAPI POV.


> ---
>  backends/cryptodev.c | 141 +++++++++++++++++++++++++++++++++++++++++++
>  monitor/hmp-cmds.c   |   5 ++
>  monitor/qmp-cmds.c   |   2 +
>  qapi/stats.json      |  10 ++-
>  4 files changed, 156 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index 09ffdd345f..9d52220772 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -22,9 +22,11 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "monitor/stats.h"
>  #include "sysemu/cryptodev.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-cryptodev.h"
> +#include "qapi/qapi-types-stats.h"
>  #include "qapi/visitor.h"
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
> @@ -32,6 +34,14 @@
>  #include "qom/object_interfaces.h"
>  #include "hw/virtio/virtio-crypto.h"
>  
> +typedef struct StatsArgs {
> +    union StatsResultsType {
> +        StatsResultList **stats;
> +        StatsSchemaList **schema;
> +    } result;
> +    strList *names;
> +    Error **errp;
> +} StatsArgs;
>  
>  static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
>  
> @@ -435,6 +445,134 @@ static void cryptodev_backend_finalize(Object *obj)
>      }
>  }
>  
> +static StatsList *cryptodev_backend_stats_add(const char *name, int64_t *val,
> +                                              StatsList *stats_list)
> +{
> +    Stats *stats = g_new0(Stats, 1);
> +
> +    stats->name = g_strdup(name);
> +    stats->value = g_new0(StatsValue, 1);
> +    stats->value->type = QTYPE_QNUM;
> +    stats->value->u.scalar = *val;
> +
> +    QAPI_LIST_PREPEND(stats_list, stats);
> +    return stats_list;
> +}
> +
> +static int cryptodev_backend_stats_query(Object *obj, void *data)
> +{
> +    StatsArgs *stats_args = data;
> +    StatsResultList **stats_results = stats_args->result.stats;
> +    StatsList *stats_list = NULL;
> +    StatsResult *entry;
> +    CryptoDevBackend *backend;
> +    QCryptodevBackendSymStat *sym_stat;
> +    QCryptodevBackendAsymStat *asym_stat;
> +
> +    if (!object_dynamic_cast(obj, TYPE_CRYPTODEV_BACKEND)) {
> +        return 0;
> +    }
> +
> +    backend = CRYPTODEV_BACKEND(obj);
> +    sym_stat = backend->sym_stat;
> +    if (sym_stat) {
> +        stats_list = cryptodev_backend_stats_add("sym-encrypt-ops",
> +                         &sym_stat->encrypt_ops, stats_list);
> +        stats_list = cryptodev_backend_stats_add("sym-decrypt-ops",
> +                         &sym_stat->decrypt_ops, stats_list);
> +        stats_list = cryptodev_backend_stats_add("sym-encrypt-bytes",
> +                         &sym_stat->encrypt_bytes, stats_list);
> +        stats_list = cryptodev_backend_stats_add("sym-decrypt-bytes",
> +                         &sym_stat->decrypt_bytes, stats_list);
> +    }
> +
> +    asym_stat = backend->asym_stat;
> +    if (asym_stat) {
> +        stats_list = cryptodev_backend_stats_add("asym-encrypt-ops",
> +                         &asym_stat->encrypt_ops, stats_list);
> +        stats_list = cryptodev_backend_stats_add("asym-decrypt-ops",
> +                         &asym_stat->decrypt_ops, stats_list);
> +        stats_list = cryptodev_backend_stats_add("asym-sign-ops",
> +                         &asym_stat->sign_ops, stats_list);
> +        stats_list = cryptodev_backend_stats_add("asym-verify-ops",
> +                         &asym_stat->verify_ops, stats_list);
> +        stats_list = cryptodev_backend_stats_add("asym-encrypt-bytes",
> +                         &asym_stat->encrypt_bytes, stats_list);
> +        stats_list = cryptodev_backend_stats_add("asym-decrypt-bytes",
> +                         &asym_stat->decrypt_bytes, stats_list);
> +        stats_list = cryptodev_backend_stats_add("asym-sign-bytes",
> +                         &asym_stat->sign_bytes, stats_list);
> +        stats_list = cryptodev_backend_stats_add("asym-verify-bytes",
> +                         &asym_stat->verify_bytes, stats_list);
> +    }
> +
> +    entry = g_new0(StatsResult, 1);
> +    entry->provider = STATS_PROVIDER_CRYPTODEV;
> +    entry->qom_path = g_strdup(object_get_canonical_path(obj));
> +    entry->stats = stats_list;
> +    QAPI_LIST_PREPEND(*stats_results, entry);
> +
> +    return 0;
> +}
> +
> +static void cryptodev_backend_stats_cb(StatsResultList **result,
> +                                       StatsTarget target,
> +                                       strList *names, strList *targets,
> +                                       Error **errp)
> +{
> +    switch (target) {
> +    case STATS_TARGET_CRYPTODEV:
> +    {
> +        Object *objs = container_get(object_get_root(), "/objects");
> +        StatsArgs stats_args;
> +        stats_args.result.stats = result;
> +        stats_args.names = names;
> +        stats_args.errp = errp;
> +
> +        object_child_foreach(objs, cryptodev_backend_stats_query, &stats_args);
> +        break;
> +    }
> +    default:
> +        break;
> +    }
> +}
> +
> +static StatsSchemaValueList *cryptodev_backend_schemas_add(const char *name,
> +                                 StatsSchemaValueList *list)
> +{
> +    StatsSchemaValueList *schema_entry = g_new0(StatsSchemaValueList, 1);
> +
> +    schema_entry->value = g_new0(StatsSchemaValue, 1);
> +    schema_entry->value->type = STATS_TYPE_CUMULATIVE;
> +    schema_entry->value->name = g_strdup(name);
> +    schema_entry->next = list;
> +
> +    return schema_entry;
> +}
> +
> +static void cryptodev_backend_schemas_cb(StatsSchemaList **result,
> +                                         Error **errp)
> +{
> +    StatsSchemaValueList *stats_list = NULL;
> +    const char *sym_stats[] = {"sym-encrypt-ops", "sym-decrypt-ops",
> +                               "sym-encrypt-bytes", "sym-decrypt-bytes"};
> +    const char *asym_stats[] = {"asym-encrypt-ops", "asym-decrypt-ops",
> +                                "asym-sign-ops", "asym-verify-ops",
> +                                "asym-encrypt-bytes", "asym-decrypt-bytes",
> +                                "asym-sign-bytes", "asym-verify-bytes"};
> +
> +    for (int i = 0; i < ARRAY_SIZE(sym_stats); i++) {
> +        stats_list = cryptodev_backend_schemas_add(sym_stats[i], stats_list);
> +    }
> +
> +    for (int i = 0; i < ARRAY_SIZE(asym_stats); i++) {
> +        stats_list = cryptodev_backend_schemas_add(asym_stats[i], stats_list);
> +    }
> +
> +    add_stats_schema(result, STATS_PROVIDER_CRYPTODEV, STATS_TARGET_CRYPTODEV,
> +                     stats_list);
> +}
> +
>  static void
>  cryptodev_backend_class_init(ObjectClass *oc, void *data)
>  {
> @@ -456,6 +594,9 @@ cryptodev_backend_class_init(ObjectClass *oc, void *data)
>                                cryptodev_backend_get_ops,
>                                cryptodev_backend_set_ops,
>                                NULL, NULL);
> +
> +    add_stats_callbacks(STATS_PROVIDER_CRYPTODEV, cryptodev_backend_stats_cb,
> +                        cryptodev_backend_schemas_cb);
>  }
>  
>  static const TypeInfo cryptodev_backend_info = {
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index cda52c2526..dda69a1098 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1883,6 +1883,8 @@ static StatsFilter *stats_filter(StatsTarget target, const char *names,
>          filter->u.vcpu.vcpus = vcpu_list;
>          break;
>      }
> +    case STATS_TARGET_CRYPTODEV:
> +        break;
>      default:
>          break;
>      }
> @@ -1954,6 +1956,9 @@ void hmp_info_stats(Monitor *mon, const QDict *qdict)
>          int cpu_index = monitor_get_cpu_index(mon);
>          filter = stats_filter(target, names, cpu_index, provider);
>          break;
> +    case STATS_TARGET_CRYPTODEV:
> +        filter = stats_filter(target, names, -1, provider);
> +        break;
>      default:
>          abort();
>      }
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index bf22a8c5a6..dd31936f6a 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -385,6 +385,8 @@ static bool invoke_stats_cb(StatsCallbacks *entry,
>              targets = filter->u.vcpu.vcpus;
>          }
>          break;
> +    case STATS_TARGET_CRYPTODEV:
> +        break;
>      default:
>          abort();
>      }
> diff --git a/qapi/stats.json b/qapi/stats.json
> index 57db5b1c74..f9dec18066 100644
> --- a/qapi/stats.json
> +++ b/qapi/stats.json
> @@ -50,10 +50,14 @@
>  #
>  # Enumeration of statistics providers.
>  #
> +# @kvm: since 7.1
> +#
> +# @cryptodev: since 8.0
> +#
>  # Since: 7.1
>  ##
>  { 'enum': 'StatsProvider',
> -  'data': [ 'kvm' ] }
> +  'data': [ 'kvm', 'cryptodev' ] }
>  
>  ##
>  # @StatsTarget:
> @@ -65,10 +69,12 @@
>  #
>  # @vcpu: statistics that apply to a single virtual CPU.
>  #
> +# @cryptodev: statistics that apply to a crypto device.
> +#
>  # Since: 7.1
>  ##
>  { 'enum': 'StatsTarget',
> -  'data': [ 'vm', 'vcpu' ] }
> +  'data': [ 'vm', 'vcpu', 'cryptodev' ] }
>  
>  ##
>  # @StatsRequest:
> -- 
> 2.34.1



  reply	other threads:[~2023-02-28 12:57 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-29  2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
2023-01-29  2:57 ` [PATCH v4 01/12] cryptodev: Introduce cryptodev.json zhenwei pi
2023-01-29  2:57 ` [PATCH v4 02/12] cryptodev: Remove 'name' & 'model' fields zhenwei pi
2023-01-29  2:57 ` [PATCH v4 03/12] cryptodev: Introduce cryptodev alg type in QAPI zhenwei pi
2023-01-29  2:57 ` [PATCH v4 04/12] cryptodev: Introduce server " zhenwei pi
2023-01-29  2:57 ` [PATCH v4 05/12] cryptodev: Introduce 'query-cryptodev' QMP command zhenwei pi
2023-01-29  2:57 ` [PATCH v4 06/12] cryptodev-builtin: Detect akcipher capability zhenwei pi
2023-01-29  2:57 ` [PATCH v4 07/12] hmp: add cryptodev info command zhenwei pi
2023-01-29  2:57 ` [PATCH v4 08/12] cryptodev: Use CryptoDevBackendOpInfo for operation zhenwei pi
2023-01-29  2:57 ` [PATCH v4 09/12] cryptodev: Account statistics zhenwei pi
2023-01-29  2:57 ` [PATCH v4 10/12] cryptodev: support QoS zhenwei pi
2023-01-29  2:57 ` [PATCH v4 11/12] cryptodev: Support query-stats QMP command zhenwei pi
2023-02-28 12:56   ` Michael S. Tsirkin [this message]
2023-02-28 13:17     ` Daniel P. Berrangé
2023-02-28 14:06       ` Markus Armbruster
2023-02-28 14:13       ` Michael S. Tsirkin
2023-02-28 14:21         ` Daniel P. Berrangé
2023-02-28 14:58           ` Michael S. Tsirkin
2023-03-01  2:53             ` zhenwei pi
2023-01-29  2:57 ` [PATCH v4 12/12] MAINTAINERS: add myself as the maintainer for cryptodev zhenwei pi
2023-02-06  0:23 ` PING: [PATCH v4 00/12] Refactor cryptodev zhenwei pi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230228075511-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=arei.gonglei@huawei.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=pizhenwei@bytedance.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.