DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Shahaf Shuler <shahafs@mellanox.com>,
	Matan Azrad <matan@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>,
	Xiao Wang <xiao.w.wang@intel.com>
Cc: Slava Ovsiienko <viacheslavo@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH 1/4] vhost: inroduce operation to get vDPA queue stats
Date: Mon, 20 Apr 2020 18:18:47 +0200	[thread overview]
Message-ID: <956f4b42-ecb3-a1f7-4e57-4bfc31c4047a@redhat.com> (raw)
In-Reply-To: <AM0PR0502MB37958605390A2F1406DC4A37C3D40@AM0PR0502MB3795.eurprd05.prod.outlook.com>



On 4/20/20 5:57 PM, Shahaf Shuler wrote:
> Monday, April 20, 2020 10:13 AM, Maxime Coquelin:
>> Subject: Re: [PATCH 1/4] vhost: inroduce operation to get vDPA queue stats
>>
>> Hi Shahaf,
>>
>> On 4/19/20 8:18 AM, Shahaf Shuler wrote:
>>> Thursday, April 16, 2020 4:20 PM, Maxime Coquelin:
>>>> Subject: Re: [PATCH 1/4] vhost: inroduce operation to get vDPA queue
>>>> stats
>>>>
>>>> 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.
>>>
>>> Spec allows device to limit the max supported chain (see [1]).
>>
>> Ha ok, I missed that. Please note that this is only allowed for packed ring, it is
>> not in the split ring part.
> 
> On my version of spec (csprd01) it is also for split, however it was removed on the latest version not sure why. 

Problem is that older drivers may assume max chain size is the virtio
ring size.

By the way, how is the guest Virtio driver made aware of the max
chain size? Isn't that missing in the spec?

>>
>>>>
>>>> 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.
>>>
>>> I think there are better ways to address that, e.g.:
>>> 1. have some reserved fields for future 2. have the option to point to
>>> next item, and by that link chain of stat structures
>>>
>>>>
>>>> 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?
>>>
>>> xstats are good, and we should have it to expose the vendor specific
>> counters. The basic counters though, should be simple and vendor agnostic
>> so that any SW/scripting layer on top of the DPDK can easily use and expose
>> it.
>>> Hence I think it will be good to have the basic counters with well-defined
>> stats structure as part of the vdpa stats API. Is the exceed_max_chain is the
>> only counter you find odd or there are more?
>>
>> Problem is that not all the vDPA NIC will implement these counters, so with
>> only proposed implementation, the user cannot know whether counter
>> value is 0 or counter is just not implemented. For example, the Virtio
>> specification does not specify counters, so a full Virtio HW offload device
>> won't have them.
> 
> Yeah, full virtio emulated device is a good example. 
> I think it is odd virtio doesn’t provide any statistics, e.g. how would the Netdev on top of the virtio device report anything? How will ppl debug? 
> I think sooner or later we will need a way to expose stats. 

I agree.
These missing counters were not blocking when using a SW backend, but
with HW Offload I agree such counters would be welcome.

>>
>> So I think the xstat is the right thing to do, with standardized names for the
>> standard  counters.
> 
> Yeah tend to agree on this one due to the lack of spec statistics. 
> 
>>
>> Regards,
>> Maxime
>>>>
>>>> Thanks,
>>>> Maxime
>>>
>>> [1]
>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
>>> ub.com%2Foasis-tcs%2Fvirtio-spec%2Fblob%2Fmaster%2Fpacked-
>> ring.tex%23L
>>>
>> 498&amp;data=02%7C01%7Cshahafs%40mellanox.com%7C2fbf00c6e115488f
>> 483e08
>>>
>> d7e4fa4940%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6372296
>> 3594175
>>>
>> 9512&amp;sdata=m2rPPMM%2Fen9Vkbp%2Fg5xz0MSTWYURh7woI7w5%2B
>> b2Zjy8%3D&am
>>> p;reserved=0
>>>
> 


  reply	other threads:[~2020-04-20 16: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
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 [this message]
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=956f4b42-ecb3-a1f7-4e57-4bfc31c4047a@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).