Re: [dpdk-dev] [PATCH 1/4] vhost: inroduce operation to get vDPA queue stats

[Shahaf Shuler <shahafs@mellanox.com>]

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. 

> 
> >>
> >> 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. 

> 
> 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&data=02%7C01%7Cshahafs%40mellanox.com%7C2fbf00c6e115488f
> 483e08
> >
> d7e4fa4940%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6372296
> 3594175
> >
> 9512&sdata=m2rPPMM%2Fen9Vkbp%2Fg5xz0MSTWYURh7woI7w5%2B
> b2Zjy8%3D&am
> > p;reserved=0
> >