[ovs-dev] [PATCH v3 3/4] netdev-dpdk: Add stats for vhost tx retries.

[Kevin Traynor]

On 28/06/2019 14:35, Flavio Leitner wrote:
> On Fri, Jun 28, 2019 at 04:29:36PM +0300, Ilya Maximets wrote:
>> On 28.06.2019 16:17, Flavio Leitner wrote:
>>> On Fri, Jun 28, 2019 at 03:21:07PM +0300, Ilya Maximets wrote:
>>>> On 27.06.2019 14:12, Kevin Traynor wrote:
>>>>> vhost tx retries may occur, and it can be a sign that
>>>>> the guest is not optimally configured.
>>>>>
>>>>> Add some stats so a user will know if vhost tx retries are
>>>>> occurring and hence give a hint that guest config should be
>>>>> examined.
>>>>>
>>>>> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
>>>>> Acked-by: Flavio Leitner <fbl at sysclose.org>
>>>>> Acked-by: Eelco Chaudron <echaudro at redhat.com>
>>>>> ---
>>>>>  Documentation/topics/dpdk/vhost-user.rst | 5 +++++
>>>>>  include/openvswitch/netdev.h             | 1 +
>>>>>  lib/netdev-dpdk.c                        | 7 +++++--
>>>>>  vswitchd/bridge.c                        | 3 ++-
>>>>>  4 files changed, 13 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst
>>>>> index 1dd02b8b6..3caa88231 100644
>>>>> --- a/Documentation/topics/dpdk/vhost-user.rst
>>>>> +++ b/Documentation/topics/dpdk/vhost-user.rst
>>>>> @@ -112,4 +112,9 @@ The guest should also have sufficient cores dedicated for consuming and
>>>>>  processing packets at the required rate.
>>>>>  
>>>>> +The amount of Tx retries on a vhost-user or vhost-user-client interface can be
>>>>> +shown with::
>>>>> +
>>>>> +  ovs-vsctl get Interface dpdkvhostclient0 statistics:tx_retries
>>>>> +
>>>>>  .. _dpdk-vhost-user:
>>>>>  
>>>>> diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
>>>>> index 0c10f7b48..4d18b9f66 100644
>>>>> --- a/include/openvswitch/netdev.h
>>>>> +++ b/include/openvswitch/netdev.h
>>>>> @@ -46,4 +46,5 @@ struct netdev_stats {
>>>>>      uint64_t multicast;         /* Multicast packets received. */
>>>>>      uint64_t collisions;
>>>>> +    uint64_t tx_retries;        /* Retries when unable to transmit.*/
>>>>
>>>>
>>>> This seems very vhost specific counter.
>>>> Maybe it's better to report it in custom_stats?
>>>
>>> I thought we could implement retry for nics and rings if we want to
>>> not drop any packets. Not sure if it will ever be implemented
>>> though.
>>
>> For HW NICs, I think, it's unlikely that we'll push packets faster than
>> HW will send them. For ring ports it makes more sense, but do you know
>> if anyone uses them? Since ivshmem deprecation this functionality looks
>> not very interesting. I don't know if anyone uses ring ports at all.
> 
> Things might change when we mix with HWOL, but it's hard to tell
> today.
>  
>> Anyway, with custom stats where will be no difference for the user.
>> The only difference is in OVS code. At least, there is no re-try
>> functionality for system interfaces in both datapaths.
> 
> Just wanted to point out that possibility, but that's not the case
> today so I have no strong opinion either way.
> 
> fbl
> 

Thanks for the comment and discussion, will look into the custom stats
suggestion