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

Ian Stokes ian.stokes at intel.com
Wed Jul 3 15:03:31 UTC 2019


On 7/2/2019 1:32 AM, Kevin Traynor wrote:
> vhost tx retries may occur, and it can be a sign that
> the guest is not optimally configured.
> 
> Add a custom stat so a user will know if vhost tx retries are
> occurring and hence give a hint that guest config should be
> examined.
> 

Thanks Kevin, tests ok for me.

Just a general comment on the design. In comparison to the previous 
approach proposed there seems to be more required with the custom stat 
approach below.

This may be ok as the retry is very OVS DPDK specific and doesn;t come 
dor lets say DPDK (unlike the XTATS).

@Ilya, whats your thoughts? Is this approach preferable for you rather 
than adding it to the general stats for all devices? (in which case I 
agree, they will not be used for dpdk types so will not be of use).

One other minor comment below.

> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
> ---
>   Documentation/topics/dpdk/vhost-user.rst |  5 ++++
>   lib/netdev-dpdk.c                        | 38 +++++++++++++++++++++++-
>   2 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst
> index 5f393aced..368f44520 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -521,4 +521,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
> +
>   vhost-user Dequeue Zero Copy (experimental)
>   -------------------------------------------
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index a380b6ffe..d3e02d389 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -139,4 +139,10 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / ROUND_DOWN_POW2(MAX_NB_MBUF / MIN_NB_MBUF))
>   #define XSTAT_RX_JABBER_ERRORS           "rx_jabber_errors"
>   
> +/* Size of vHost custom stats. */
> +#define VHOST_STAT_TX_RETRIES_SIZE    1
> +
> +/* Name of vHost custom stats. */
> +#define VHOST_STAT_TX_RETRIES    "tx_retries" > +

The alignment of the defines above look odd as they are not aligned with 
the existing DPDK XSTAT name definitions. It's only minor, as Kevin is 
on leave I think this will be ok to change on commit if there are no 
objections.

Regards
Ian
>   #define SOCKET0              0
>   
> @@ -434,7 +440,9 @@ struct netdev_dpdk {
>       PADDED_MEMBERS(CACHE_LINE_SIZE,
>           struct netdev_stats stats;
> +        /* Custom stat for retries when unable to transmit. */
> +        uint64_t tx_retries;
>           /* Protects stats */
>           rte_spinlock_t stats_lock;
> -        /* 44 pad bytes here. */
> +        /* 4 pad bytes here. */
>       );
>   
> @@ -1190,4 +1198,6 @@ common_construct(struct netdev *netdev, dpdk_port_t port_no,
>       dev->rte_xstats_ids_size = 0;
>   
> +    dev->tx_retries = 0;
> +
>       return 0;
>   }
> @@ -2381,4 +2391,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
>       netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts, total_pkts,
>                                            cnt + dropped);
> +    dev->tx_retries += MIN(retries, VHOST_ENQ_RETRY_NUM);
>       rte_spinlock_unlock(&dev->stats_lock);
>   
> @@ -2818,4 +2829,27 @@ netdev_dpdk_get_custom_stats(const struct netdev *netdev,
>   }
>   
> +static int
> +netdev_dpdk_vhost_get_custom_stats(const struct netdev *netdev,
> +                                   struct netdev_custom_stats *custom_stats)
> +{
> +    struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +
> +    ovs_mutex_lock(&dev->mutex);
> +
> +    custom_stats->size = VHOST_STAT_TX_RETRIES_SIZE;
> +    custom_stats->counters = (struct netdev_custom_counter *)
> +                                 xzalloc(sizeof(struct netdev_custom_counter));
> +    ovs_strlcpy(custom_stats->counters->name, VHOST_STAT_TX_RETRIES,
> +                NETDEV_CUSTOM_STATS_NAME_SIZE);
> +
> +    rte_spinlock_lock(&dev->stats_lock);
> +    custom_stats->counters->value = dev->tx_retries;
> +    rte_spinlock_unlock(&dev->stats_lock);
> +
> +    ovs_mutex_unlock(&dev->mutex);
> +
> +    return 0;
> +}
> +
>   static int
>   netdev_dpdk_get_features(const struct netdev *netdev,
> @@ -4398,4 +4432,5 @@ static const struct netdev_class dpdk_vhost_class = {
>       .get_carrier = netdev_dpdk_vhost_get_carrier,
>       .get_stats = netdev_dpdk_vhost_get_stats,
> +    .get_custom_stats = netdev_dpdk_vhost_get_custom_stats,
>       .get_status = netdev_dpdk_vhost_user_get_status,
>       .reconfigure = netdev_dpdk_vhost_reconfigure,
> @@ -4413,4 +4448,5 @@ static const struct netdev_class dpdk_vhost_client_class = {
>       .get_carrier = netdev_dpdk_vhost_get_carrier,
>       .get_stats = netdev_dpdk_vhost_get_stats,
> +    .get_custom_stats = netdev_dpdk_vhost_get_custom_stats,
>       .get_status = netdev_dpdk_vhost_user_get_status,
>       .reconfigure = netdev_dpdk_vhost_client_reconfigure,
> 



More information about the dev mailing list