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

[Kevin Traynor]

On 24/06/2019 22:47, Flavio Leitner wrote:
> On Fri, Jun 21, 2019 at 02:41:57PM +0100, 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>
>> ---
>>  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 d1682fd05..e79ed082e 100644
>> --- a/Documentation/topics/dpdk/vhost-user.rst
>> +++ b/Documentation/topics/dpdk/vhost-user.rst
>> @@ -107,4 +107,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.*/
>>  
>>      /* Detailed receive errors. */
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 0f3b9c6f4..03a8de380 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -2298,5 +2298,6 @@ netdev_dpdk_vhost_update_tx_counters(struct netdev_stats *stats,
>>                                       struct dp_packet **packets,
>>                                       int attempted,
>> -                                     int dropped)
>> +                                     int dropped,
>> +                                     int retries)
>>  {
>>      int i;
>> @@ -2305,4 +2306,5 @@ netdev_dpdk_vhost_update_tx_counters(struct netdev_stats *stats,
>>      stats->tx_packets += sent;
>>      stats->tx_dropped += dropped;
>> +    stats->tx_retries += MIN(retries, VHOST_ENQ_RETRY_NUM);
> 
> Why MIN(, VHOST_ENQ_RETRY_NUM)?
> The retrans loop is limited to that, so it seems redundant.
> 

Yes, you are right. It's not needed now, but will be needed in 3/3 when
the max can be zero. I will move there and add a comment.

> fbl
>  
>>      for (i = 0; i < sent; i++) {
>> @@ -2359,5 +2361,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
>>      rte_spinlock_lock(&dev->stats_lock);
>>      netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts, total_pkts,
>> -                                         cnt + dropped);
>> +                                         cnt + dropped, retries);
>>      rte_spinlock_unlock(&dev->stats_lock);
>>  
>> @@ -2597,4 +2599,5 @@ netdev_dpdk_vhost_get_stats(const struct netdev *netdev,
>>      stats->rx_errors = dev->stats.rx_errors;
>>      stats->rx_length_errors = dev->stats.rx_length_errors;
>> +    stats->tx_retries = dev->stats.tx_retries;
>>  
>>      stats->rx_1_to_64_packets = dev->stats.rx_1_to_64_packets;
>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index 2976771ae..3dab8d4c7 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -2409,5 +2409,6 @@ iface_refresh_stats(struct iface *iface)
>>      IFACE_STAT(rx_oversize_errors,      "rx_oversize_errors")       \
>>      IFACE_STAT(rx_fragmented_errors,    "rx_fragmented_errors")     \
>> -    IFACE_STAT(rx_jabber_errors,        "rx_jabber_errors")
>> +    IFACE_STAT(rx_jabber_errors,        "rx_jabber_errors")         \
>> +    IFACE_STAT(tx_retries,              "tx_retries")
>>  
>>  #define IFACE_STAT(MEMBER, NAME) + 1
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev