[ovs-dev] [PATCH v2] netdev-dpdk: Remove vhost send retries when no packets have been sent.

[Daniele Di Proietto]

Thanks for the patch, it looks good to me.

If everybody agrees (Ilya?) I can push this to master.

Thanks,

Daniele

2016-06-10 9:49 GMT-07:00 Kevin Traynor <kevin.traynor at intel.com>:

> If the guest is connected but not servicing the virt queue, this leads
> to vhost send retries until timeout. This is fine in isolation but if
> there are other high rate queues also being serviced by the same PMD
> it can lead to a performance hit on those queues. Change to only retry
> when at least some packets have been successfully sent on the previous
> attempt.
>
> Also, limit retries to avoid a similar delays if packets are being sent
> at a very low rate due to few available descriptors.
>
> Reported-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
> Signed-off-by: Kevin Traynor <kevin.traynor at intel.com>
> Acked-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
> ---
>
>  RFC->v2
>  - Change to PATCH after ML discussion.
>  - Rebase.
>  - Add retry limit when packets are being sent.
>  - Add Ack from Bhanu.
>
>  lib/netdev-dpdk.c |   34 ++++++----------------------------
>  1 files changed, 6 insertions(+), 28 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 19d355f..582569c 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -141,10 +141,7 @@ static char *cuse_dev_name = NULL;    /* Character
> device cuse_dev_name. */
>  #endif
>  static char *vhost_sock_dir = NULL;   /* Location of vhost-user sockets */
>
> -/*
> - * Maximum amount of time in micro seconds to try and enqueue to vhost.
> - */
> -#define VHOST_ENQ_RETRY_USECS 100
> +#define VHOST_ENQ_RETRY_NUM 8
>
>  static const struct rte_eth_conf port_conf = {
>      .rxmode = {
> @@ -1377,7 +1374,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int
> qid,
>      struct rte_mbuf **cur_pkts = (struct rte_mbuf **) pkts;
>      unsigned int total_pkts = cnt;
>      unsigned int qos_pkts = cnt;
> -    uint64_t start = 0;
> +    int retries = 0;
>
>      qid = dev->tx_q[qid % dev->real_n_txq].map;
>
> @@ -1404,32 +1401,13 @@ __netdev_dpdk_vhost_send(struct netdev *netdev,
> int qid,
>          if (OVS_LIKELY(tx_pkts)) {
>              /* Packets have been sent.*/
>              cnt -= tx_pkts;
> -            /* Prepare for possible next iteration.*/
> +            /* Prepare for possible retry.*/
>              cur_pkts = &cur_pkts[tx_pkts];
>          } else {
> -            uint64_t timeout = VHOST_ENQ_RETRY_USECS * rte_get_timer_hz()
> / 1E6;
> -            unsigned int expired = 0;
> -
> -            if (!start) {
> -                start = rte_get_timer_cycles();
> -            }
> -
> -            /*
> -             * Unable to enqueue packets to vhost interface.
> -             * Check available entries before retrying.
> -             */
> -            while (!rte_vring_available_entries(virtio_dev, vhost_qid)) {
> -                if (OVS_UNLIKELY((rte_get_timer_cycles() - start) >
> timeout)) {
> -                    expired = 1;
> -                    break;
> -                }
> -            }
> -            if (expired) {
> -                /* break out of main loop. */
> -                break;
> -            }
> +            /* No packets sent - do not retry.*/
> +            break;
>          }
> -    } while (cnt);
> +    } while (cnt && (retries++ < VHOST_ENQ_RETRY_NUM));
>
>      rte_spinlock_unlock(&dev->tx_q[qid].tx_lock);
>
> --
> 1.7.4.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>