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

[Ilya Maximets]

Looks good to me.
Acked-by: Ilya Maximets <i.maximets at samsung.com>


On 11.06.2016 02:08, Daniele Di Proietto wrote:
> 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 <mailto: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 <mailto:bhanuprakash.bodireddy at intel.com>>
>     Signed-off-by: Kevin Traynor <kevin.traynor at intel.com <mailto:kevin.traynor at intel.com>>
>     Acked-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com <mailto: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 <mailto:dev at openvswitch.org>
>     http://openvswitch.org/mailman/listinfo/dev
> 
>