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

Thomas F Herbert thomasfherbert at gmail.com
Thu Aug 6 19:24:29 UTC 2015


On 8/6/15 1:40 PM, Flavio Leitner 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 */
Do you still need this "FIXME?" Isn't the code you added below freeing 
and re-allocating the correct number of tx queues?
>>>       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
OK, Thanks!
>
>>>               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
>
>




More information about the dev mailing list