[ovs-dev] [RFC] dpdk: support multiple queues in vhost

Karthick, A.R. kramanar at ciena.com
Thu Aug 6 17:54:16 UTC 2015


Just to top-post briefly:
 qemu reverted the vhost-user multiqueue support sometime back.
 So this patch isn't really usable.

Regards,
-Karthick

 commit f73ca7363440240b7ee5ee7f7ddb1c64751efb54
Merge: 7135847 f9d6dbf
Author: Peter Maydell <peter.maydell at linaro.org>
Date:   Mon Jul 20 13:25:28 2015 +0100

    Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into
staging

    virtio, vhost, pc fixes for 2.4

    The only notable thing here is vhost-user multiqueue
    revert. We'll work on making it stable in 2.5,
    reverting now means we won't have to maintain
    bug for bug compability forever.

    Signed-off-by: Michael S. Tsirkin <mst at redhat.com>

    # gpg: Signature made Mon Jul 20 12:24:00 2015 BST using RSA key ID
D28D5469
    # gpg: Good signature from "Michael S. Tsirkin <mst at kernel.org>"
    # gpg:                 aka "Michael S. Tsirkin <mst at redhat.com>"

    * remotes/mst/tags/for_upstream:
      virtio-net: remove virtio queues if the guest doesn't support
multiqueue
      virtio-net: Flush incoming queues when DRIVER_OK is being set
      pci_add_capability: remove duplicate comments
      virtio-net: unbreak any layout
      Revert "vhost-user: add multi queue support"
      ich9: fix skipped vmstate_memhp_state subsection

    Signed-off-by: Peter Maydell <peter.maydell at linaro.org>


On Thu, Aug 6, 2015 at 10:40 AM, Flavio Leitner <fbl at sysclose.org> wrote:

> On Thu, Aug 06, 2015 at 12:39:29PM -0400, Thomas F Herbert wrote:
> > On 7/31/15 6:30 PM, Flavio Leitner wrote:
> > >This RFC is based on the vhost multiple queues work on
> > >dpdk-dev: http://dpdk.org/ml/archives/dev/2015-June/019345.html
> > >
> > >Signed-off-by: Flavio Leitner <fbl at redhat.com>
> > >---
> > >  lib/netdev-dpdk.c | 61
> ++++++++++++++++++++++++++++++++++++-------------------
> > >  1 file changed, 40 insertions(+), 21 deletions(-)
> > >
> > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > >index 5ae805e..493172c 100644
> > >--- a/lib/netdev-dpdk.c
> > >+++ b/lib/netdev-dpdk.c
> > >@@ -215,12 +215,9 @@ struct netdev_dpdk {
> > >       * If the numbers match, 'txq_needs_locking' is false, otherwise
> it is
> > >       * true and we will take a spinlock on transmission */
> > >      int real_n_txq;
> > >+    int real_n_rxq;
> > >      bool txq_needs_locking;
> > >
> > >-    /* Spinlock for vhost transmission.  Other DPDK devices use
> spinlocks in
> > >-     * dpdk_tx_queue */
> > >-    rte_spinlock_t vhost_tx_lock;
> > >-
> > >      /* virtio-net structure for vhost device */
> > >      OVSRCU_TYPE(struct virtio_net *) virtio_dev;
> > >
> > >@@ -602,13 +599,10 @@ dpdk_dev_parse_name(const char dev_name[], const
> char prefix[],
> > >  static int
> > >  vhost_construct_helper(struct netdev *netdev_)
> OVS_REQUIRES(dpdk_mutex)
> > >  {
> > >-    struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
> > >-
> > >      if (rte_eal_init_ret) {
> > >          return rte_eal_init_ret;
> > >      }
> > >
> > >-    rte_spinlock_init(&netdev->vhost_tx_lock);
> > >      return netdev_dpdk_init(netdev_, -1, DPDK_DEV_VHOST);
> > >  }
> > >
> > >@@ -791,9 +785,16 @@ netdev_dpdk_vhost_set_multiq(struct netdev
> *netdev_, unsigned int n_txq,
> > >      ovs_mutex_lock(&dpdk_mutex);
> > >      ovs_mutex_lock(&netdev->mutex);
> > >
> > >+    rte_free(netdev->tx_q);
> > >+    /* FIXME: the number of vqueues needs to match */
> > >      netdev->up.n_txq = n_txq;
> > >-    netdev->real_n_txq = 1;
> > >-    netdev->up.n_rxq = 1;
> > >+    netdev->up.n_rxq = n_rxq;
> > >+
> > >+    /* vring has txq = rxq */
> > >+    netdev->real_n_txq = n_rxq;
> > >+    netdev->real_n_rxq = n_rxq;
> > >+    netdev->txq_needs_locking = netdev->real_n_txq != netdev->up.n_txq;
> > >+    netdev_dpdk_alloc_txq(netdev, netdev->up.n_txq);
> > >
> > >      ovs_mutex_unlock(&netdev->mutex);
> > >      ovs_mutex_unlock(&dpdk_mutex);
> > >@@ -904,14 +905,14 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq
> *rxq_,
> > >      struct netdev *netdev = rx->up.netdev;
> > >      struct netdev_dpdk *vhost_dev = netdev_dpdk_cast(netdev);
> > >      struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(vhost_dev);
> > >-    int qid = 1;
> > >+    int qid = rxq_->queue_id;
> > >      uint16_t nb_rx = 0;
> > >
> > >      if (OVS_UNLIKELY(!is_vhost_running(virtio_dev))) {
> > >          return EAGAIN;
> > >      }
> > >
> > >-    nb_rx = rte_vhost_dequeue_burst(virtio_dev, qid,
> > >+    nb_rx = rte_vhost_dequeue_burst(virtio_dev, VIRTIO_TXQ + qid * 2,
> > >                                      vhost_dev->dpdk_mp->mp,
> > >                                      (struct rte_mbuf **)packets,
> > >                                      NETDEV_MAX_BURST);
> > >@@ -958,8 +959,9 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,
> struct dp_packet **packets,
> > >  }
> > >
> > >  static void
> > >-__netdev_dpdk_vhost_send(struct netdev *netdev, struct dp_packet
> **pkts,
> > >-                         int cnt, bool may_steal)
> > >+__netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
> > >+                         struct dp_packet **pkts, int cnt,
> > >+                         bool may_steal)
> > >  {
> > >      struct netdev_dpdk *vhost_dev = netdev_dpdk_cast(netdev);
> > >      struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(vhost_dev);
> > >@@ -974,13 +976,16 @@ __netdev_dpdk_vhost_send(struct netdev *netdev,
> struct dp_packet **pkts,
> > >          goto out;
> > >      }
> > >
> > >-    /* There is vHost TX single queue, So we need to lock it for TX. */
> > >-    rte_spinlock_lock(&vhost_dev->vhost_tx_lock);
> > >+    if (vhost_dev->txq_needs_locking) {
> > >+        qid = qid % vhost_dev->real_n_txq;
> > >+        rte_spinlock_lock(&vhost_dev->tx_q[qid].tx_lock);
> > >+    }
> > >
> > >      do {
> > >+        int vhost_qid = VIRTIO_RXQ + qid * VIRTIO_QNUM;
> > >          unsigned int tx_pkts;
> > >
> > >-        tx_pkts = rte_vhost_enqueue_burst(virtio_dev, VIRTIO_RXQ,
> > >+        tx_pkts = rte_vhost_enqueue_burst(virtio_dev, vhost_qid,
> > >                                            cur_pkts, cnt);
> > >          if (OVS_LIKELY(tx_pkts)) {
> > >              /* Packets have been sent.*/
> > >@@ -999,7 +1004,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev,
> struct dp_packet **pkts,
> > >               * Unable to enqueue packets to vhost interface.
> > >               * Check available entries before retrying.
> > >               */
> > >-            while (!rte_vring_available_entries(virtio_dev,
> VIRTIO_RXQ)) {
> > >+            while (!rte_vring_available_entries(virtio_dev,
> vhost_qid)) {
> > >                  if (OVS_UNLIKELY((rte_get_timer_cycles() - start) >
> timeout)) {
> > >                      expired = 1;
> > >                      break;
> > >@@ -1011,7 +1016,10 @@ __netdev_dpdk_vhost_send(struct netdev *netdev,
> struct dp_packet **pkts,
> > >              }
> > >          }
> > >      } while (cnt);
> > >-    rte_spinlock_unlock(&vhost_dev->vhost_tx_lock);
> > >+
> > >+    if (vhost_dev->txq_needs_locking) {
> > >+        rte_spinlock_unlock(&vhost_dev->tx_q[qid].tx_lock);
> > >+    }
> > >
> > >      rte_spinlock_lock(&vhost_dev->stats_lock);
> > >      vhost_dev->stats.tx_packets += (total_pkts - cnt);
> > >@@ -1116,7 +1124,7 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,
> struct dp_packet **pkts,
> > >      }
> > >
> > >      if (dev->type == DPDK_DEV_VHOST) {
> > >-        __netdev_dpdk_vhost_send(netdev, (struct dp_packet **) mbufs,
> newcnt, true);
> > >+        __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **)
> mbufs, newcnt, true);
> > >      } else {
> > >          dpdk_queue_pkts(dev, qid, mbufs, newcnt);
> > >          dpdk_queue_flush(dev, qid);
> > >@@ -1128,7 +1136,7 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,
> struct dp_packet **pkts,
> > >  }
> > >
> > >  static int
> > >-netdev_dpdk_vhost_send(struct netdev *netdev, int qid OVS_UNUSED,
> struct dp_packet **pkts,
> > >+netdev_dpdk_vhost_send(struct netdev *netdev, int qid, struct
> dp_packet **pkts,
> > >                   int cnt, bool may_steal)
> > >  {
> > >      if (OVS_UNLIKELY(pkts[0]->source != DPBUF_DPDK)) {
> > >@@ -1141,7 +1149,7 @@ netdev_dpdk_vhost_send(struct netdev *netdev, int
> qid OVS_UNUSED, struct dp_pack
> > >              }
> > >          }
> > >      } else {
> > >-        __netdev_dpdk_vhost_send(netdev, pkts, cnt, may_steal);
> > >+        __netdev_dpdk_vhost_send(netdev, qid, pkts, cnt, may_steal);
> > >      }
> > >      return 0;
> > >  }
> > >@@ -1656,7 +1664,18 @@ new_device(struct virtio_net *dev)
> > >      /* Add device to the vhost port with the same name as that passed
> down. */
> > >      LIST_FOR_EACH(netdev, list_node, &dpdk_list) {
> > >          if (strncmp(dev->ifname, netdev->vhost_id, IF_NAME_SZ) == 0) {
> > >+            int qp_num = rte_vhost_qp_num_get(dev);
> > Hi Flavio.
> >
> > rte_vhost_qp_num_get() is in the multiple queue patch for upstream DPDK,
> > referenced above, http://dpdk.org/ml/archives/dev/2015-June/019345.html.
> It
> > gets the max number of virt queues for the device or 0. Is this
> unnecessary
> > locking of the netdev dev, for each queue associated with the device?
> Should
> > the test for qp_num below be before the netdev lock?
>
> It only compares the number of queues if the device is the
> one we are looking for, so it happens only a single time, not per queue.
> And the real_n_rxq is a property of the datapath which can change,
> hence the need for the lock.
>
> fbl
>
> > >              ovs_mutex_lock(&netdev->mutex);
> > >+            if (qp_num != netdev->real_n_rxq) {
> > >+                VLOG_INFO("vHost Device '%s' (%ld) can't be added - "
> > >+                          "unmatched number of queues %d and %d",
> > >+                          dev->ifname, dev->device_fh, qp_num,
> > >+                          netdev->real_n_rxq);
> > >+                ovs_mutex_unlock(&netdev->mutex);
> > >+                ovs_mutex_unlock(&dpdk_mutex);
> > >+                return -1;
> > >+            }
> > >+
> > >              ovsrcu_set(&netdev->virtio_dev, dev);
> > >              ovs_mutex_unlock(&netdev->mutex);
> > >              exists = true;
> > >
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list