DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Matan Azrad <matan@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>,
	Xiao Wang <xiao.w.wang@intel.com>
Cc: Slava Ovsiienko <viacheslavo@mellanox.com>,
	Shahaf Shuler <shahafs@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH 1/4] vhost: inroduce operation to get vDPA queue stats
Date: Thu, 16 Apr 2020 15:19:38 +0200
Message-ID: <bc3c7b57-ff2f-0314-7e0a-83a757206be7@redhat.com> (raw)
In-Reply-To: <AM0PR0502MB401975AEE41BFC00933D35F4D2D80@AM0PR0502MB4019.eurprd05.prod.outlook.com>

Hi Matan,

On 4/16/20 11:06 AM, Matan Azrad wrote:
> Hi Maxime
> 
> Can you point on specific vendor specific counter I suggested?

No, I can't, but I think we can expect that other vendors may have other
counters they would be interested to dump.

Maybe Intel has some counters in the IFC that they could dump.
Xiao, any thoughts?

> I think all of them come directly from virtio protocols.

exceed_max_chain, for example. Doesn't the spec specify that a
descriptors chain can be as long as the size of the virtqueue?

Here it seems to indicate the device could support less.

Also, as the spec evolves, we may have new counters that comes up,
so better to have something flexible from the start IMHO to avoid ABI
breakages.

Maybe we can have some common xstats names for the Virtio related
counters define in vdpa lib, and then the vendors can specify more
vendor-specific counters if they wish?

Thanks,
Maxime
> 
> השג את Outlook עבור Android <https://aka.ms/ghei36>
> ------------------------------------------------------------------------
> *From:* Maxime Coquelin <maxime.coquelin@redhat.com>
> *Sent:* Wednesday, April 15, 2020 5:36:59 PM
> *To:* Matan Azrad <matan@mellanox.com>; dev@dpdk.org <dev@dpdk.org>
> *Cc:* Slava Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>
> *Subject:* Re: [PATCH 1/4] vhost: inroduce operation to get vDPA queue
> stats
>  
> Hi Matan,
> 
> On 4/2/20 1:26 PM, Matan Azrad wrote:
>> The vDPA device offloads all the datapath of the vhost device to the HW
>> device.
>> 
>> In order to expose to the user traffic information this patch introduce
>> new API to get traffic statistics per virtio queue.
>> 
>> The statistics are taken directly from the vDPA driver managing the HW
>> device.
>> 
>> Signed-off-by: Matan Azrad <matan@mellanox.com>
>> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>> ---
>>  doc/guides/rel_notes/release_20_05.rst    |  4 +++
>>  doc/guides/vdpadevs/features/default.ini  |  1 +
>>  doc/guides/vdpadevs/features_overview.rst |  3 +++
>>  lib/librte_vhost/rte_vdpa.h               | 45 ++++++++++++++++++++++++++++++-
>>  lib/librte_vhost/rte_vhost_version.map    |  1 +
>>  lib/librte_vhost/vdpa.c                   | 14 ++++++++++
>>  6 files changed, 67 insertions(+), 1 deletion(-)
> 
> ...
> 
>> diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
>> index 9a3deb3..d6cbf48 100644
>> --- a/lib/librte_vhost/rte_vdpa.h
>> +++ b/lib/librte_vhost/rte_vdpa.h
>> @@ -37,6 +37,27 @@ struct rte_vdpa_dev_addr {
>>        };
>>  };
>>  
>> +struct rte_vdpa_queue_stats {
>> +     /** Number of descriptors received by device */
>> +     uint64_t received_desc;
>> +     /** Number of descriptors completed by the device */
>> +     uint64_t completed_desc;
>> +     /** Number of bad descriptors received by device */
>> +     uint32_t bad_desc;
>> +     /**
>> +      * Number of chained descriptors received that exceed the max allowed
>> +      * chain by device
>> +      */
>> +     uint32_t exceed_max_chain;
>> +     /**
>> +      * Number of times device tried to read or write buffer that is not
>> +      * registered to the device
>> +      */
>> +     uint32_t invalid_buffer;
>> +     /** Number of errors detected by the device */
>> +     uint32_t errors;
>> +};
>> +
> 
> I think doing it like that, we risk to keep the rte_vdpa_get_stats API
> always experimental, as every vendor will want to add their own counters
> and so break the ABI.
> 
> How about implementing something similar to rte_eth_xstat?
> As these stats are for debugging purpose, it would give you much more
> flexibility in adding new counters as HW or firmwares evolves.
> 
> What do you think?
> 
> Thanks,
> Maxime
> 
>>  /**
>>   * vdpa device operations
>>   */
>> @@ -73,8 +94,11 @@ struct rte_vdpa_dev_ops {
>>        int (*get_notify_area)(int vid, int qid,
>>                        uint64_t *offset, uint64_t *size);
>>  
>> +     /** Get statistics of the queue */
>> +     int (*get_stats)(int did, int qid, struct rte_vdpa_queue_stats *stats);
>> +
>>        /** Reserved for future extension */
>> -     void *reserved[5];
>> +     void *reserved[4];
>>  };
>>  
>>  /**
>> @@ -200,4 +224,23 @@ struct rte_vdpa_device *
>>  __rte_experimental
>>  int
>>  rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * Get vDPA device queue statistics.
>> + *
>> + * @param did
>> + *  device id
>> + * @param qid
>> + *  queue id
>> + * @param stats
>> + *  queue statistics pointer.
>> + * @return
>> + *  0 on success, non-zero on failure.
>> + */
>> +__rte_experimental
>> +int
>> +rte_vdpa_get_stats(int did, uint16_t qid, struct rte_vdpa_queue_stats *stats);
>>  #endif /* _RTE_VDPA_H_ */
>> diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
>> index 051d08c..c9dcff4 100644
>> --- a/lib/librte_vhost/rte_vhost_version.map
>> +++ b/lib/librte_vhost/rte_vhost_version.map
>> @@ -38,6 +38,7 @@ EXPERIMENTAL {
>>        rte_vdpa_find_device_id;
>>        rte_vdpa_get_device;
>>        rte_vdpa_get_device_num;
>> +     rte_vdpa_get_stats;
>>        rte_vhost_driver_attach_vdpa_device;
>>        rte_vhost_driver_detach_vdpa_device;
>>        rte_vhost_driver_get_vdpa_device_id;
>> diff --git a/lib/librte_vhost/vdpa.c b/lib/librte_vhost/vdpa.c
>> index 2b86708..57900fc 100644
>> --- a/lib/librte_vhost/vdpa.c
>> +++ b/lib/librte_vhost/vdpa.c
>> @@ -227,3 +227,17 @@ struct rte_vdpa_device *
>>                free_ind_table(idesc);
>>        return -1;
>>  }
>> +
>> +int
>> +rte_vdpa_get_stats(int did, uint16_t qid, struct rte_vdpa_queue_stats *stats)
>> +{
>> +     struct rte_vdpa_device *vdpa_dev;
>> +
>> +     vdpa_dev = rte_vdpa_get_device(did);
>> +     if (!vdpa_dev)
>> +             return -ENODEV;
>> +
>> +     RTE_FUNC_PTR_OR_ERR_RET(vdpa_dev->ops->get_stats, -ENOTSUP);
>> +
>> +     return vdpa_dev->ops->get_stats(did, qid, stats);
>> +}
>> 
> 
> 
> השג את Outlook עבור Android <https://aka.ms/ghei36>


  reply	other threads:[~2020-04-16 13:19 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-02 11:26 [dpdk-dev] [PATCH 0/4] vhost: support vDPA virtio queue statistics Matan Azrad
2020-04-02 11:26 ` [dpdk-dev] [PATCH 1/4] vhost: inroduce operation to get vDPA queue stats Matan Azrad
2020-04-15 14:36   ` Maxime Coquelin
2020-04-16  9:06     ` Matan Azrad
2020-04-16 13:19       ` Maxime Coquelin [this message]
2020-04-19  6:18         ` Shahaf Shuler
2020-04-20  7:13           ` Maxime Coquelin
2020-04-20 15:57             ` Shahaf Shuler
2020-04-20 16:18               ` Maxime Coquelin
2020-04-21  5:02                 ` Shahaf Shuler
2020-04-02 11:26 ` [dpdk-dev] [PATCH 2/4] common/mlx5: support DevX virtq stats operations Matan Azrad
2020-04-02 11:26 ` [dpdk-dev] [PATCH 3/4] vdpa/mlx5: support virtio queue statistics get Matan Azrad
2020-04-02 11:26 ` [dpdk-dev] [PATCH 4/4] examples/vdpa: add statistics show command Matan Azrad
2020-05-05 15:54 ` [dpdk-dev] [PATCH v2 0/4] vhost: support vDPA virtio queue statistics Matan Azrad
2020-05-05 15:54   ` [dpdk-dev] [PATCH v2 1/4] vhost: inroduce operation to get vDPA queue stats Matan Azrad
2020-05-05 15:54   ` [dpdk-dev] [PATCH v2 2/4] common/mlx5: support DevX virtq stats operations Matan Azrad
2020-05-05 15:54   ` [dpdk-dev] [PATCH v2 3/4] vdpa/mlx5: support virtio queue statistics get Matan Azrad
2020-05-05 15:54   ` [dpdk-dev] [PATCH v2 4/4] examples/vdpa: add statistics show command Matan Azrad
2020-05-07 11:35   ` [dpdk-dev] [PATCH v2 0/4] vhost: support vDPA virtio queue statistics Matan Azrad
2020-06-02 15:47   ` [dpdk-dev] [PATCH v3 " Matan Azrad
2020-06-02 15:47     ` [dpdk-dev] [PATCH v3 1/4] vhost: inroduce operation to get vDPA queue stats Matan Azrad
2020-06-03  8:58       ` Maxime Coquelin
2020-06-04 10:36         ` Wang, Xiao W
2020-06-09  9:18           ` Maxime Coquelin
2020-06-02 15:47     ` [dpdk-dev] [PATCH v3 2/4] common/mlx5: support DevX virtq stats operations Matan Azrad
2020-06-18 10:58       ` Maxime Coquelin
2020-06-02 15:47     ` [dpdk-dev] [PATCH v3 3/4] vdpa/mlx5: support virtio queue statistics get Matan Azrad
2020-06-18 11:05       ` Maxime Coquelin
2020-06-02 15:47     ` [dpdk-dev] [PATCH v3 4/4] examples/vdpa: add statistics show command Matan Azrad
2020-06-18 12:13       ` Maxime Coquelin
2020-06-18 16:29     ` [dpdk-dev] [PATCH v3 0/4] vhost: support vDPA virtio queue statistics Maxime Coquelin
2020-06-19  6:01       ` Maxime Coquelin

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=bc3c7b57-ff2f-0314-7e0a-83a757206be7@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=dev@dpdk.org \
    --cc=matan@mellanox.com \
    --cc=shahafs@mellanox.com \
    --cc=viacheslavo@mellanox.com \
    --cc=xiao.w.wang@intel.com \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git