Skip to content

Commit

Permalink
vhost: remove copy threshold for async path
Browse files Browse the repository at this point in the history
Copy threshold has been introduced in async vhost data
path to select the appropriate copy engine to do copies
for higher efficiency.

However, it may cause packets ordering issues and also
introduces performance unpredictability.

Therefore, this patch removes copy threshold support in
async vhost data path.

Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Signed-off-by: Cheng Jiang <cheng1.jiang@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
  • Loading branch information
humasama authored and mcoquelin committed Sep 14, 2021
1 parent 818ce11 commit abeb865
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 381 deletions.
7 changes: 0 additions & 7 deletions doc/guides/prog_guide/vhost_lib.rst
Expand Up @@ -235,13 +235,6 @@ The following is an overview of some key Vhost API functions:
Currently, only ``RTE_VHOST_ASYNC_INORDER`` capable device is
supported by vhost.

* ``async_threshold``

The copy length (in bytes) below which CPU copy will be used even if
applications call async vhost APIs to enqueue/dequeue data.

Typical value is 256~1024 depending on the async device capability.

Applications must provide following ``ops`` callbacks for vhost lib to
work with the async copy devices:

Expand Down
22 changes: 4 additions & 18 deletions examples/vhost/main.c
Expand Up @@ -882,17 +882,11 @@ drain_vhost(struct vhost_dev *vdev)
if (builtin_net_driver) {
ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
} else if (async_vhost_driver) {
uint32_t cpu_cpl_nr = 0;
uint16_t enqueue_fail = 0;
struct rte_mbuf *m_cpu_cpl[nr_xmit];

complete_async_pkts(vdev);
ret = rte_vhost_submit_enqueue_burst(vdev->vid, VIRTIO_RXQ,
m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
__atomic_add_fetch(&vdev->pkts_inflight, ret - cpu_cpl_nr, __ATOMIC_SEQ_CST);

if (cpu_cpl_nr)
free_pkts(m_cpu_cpl, cpu_cpl_nr);
ret = rte_vhost_submit_enqueue_burst(vdev->vid, VIRTIO_RXQ, m, nr_xmit);
__atomic_add_fetch(&vdev->pkts_inflight, ret, __ATOMIC_SEQ_CST);

enqueue_fail = nr_xmit - ret;
if (enqueue_fail)
Expand Down Expand Up @@ -1213,19 +1207,12 @@ drain_eth_rx(struct vhost_dev *vdev)
enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,
pkts, rx_count);
} else if (async_vhost_driver) {
uint32_t cpu_cpl_nr = 0;
uint16_t enqueue_fail = 0;
struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];

complete_async_pkts(vdev);
enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
VIRTIO_RXQ, pkts, rx_count,
m_cpu_cpl, &cpu_cpl_nr);
__atomic_add_fetch(&vdev->pkts_inflight, enqueue_count - cpu_cpl_nr,
__ATOMIC_SEQ_CST);

if (cpu_cpl_nr)
free_pkts(m_cpu_cpl, cpu_cpl_nr);
VIRTIO_RXQ, pkts, rx_count);
__atomic_add_fetch(&vdev->pkts_inflight, enqueue_count, __ATOMIC_SEQ_CST);

enqueue_fail = rx_count - enqueue_count;
if (enqueue_fail)
Expand Down Expand Up @@ -1486,7 +1473,6 @@ new_device(int vid)
ioat_check_completed_copies_cb;

config.features = RTE_VHOST_ASYNC_INORDER;
config.async_threshold = 256;

return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
config, &channel_ops);
Expand Down
22 changes: 5 additions & 17 deletions lib/vhost/rte_vhost_async.h
Expand Up @@ -103,7 +103,6 @@ enum {
* async channel configuration
*/
struct rte_vhost_async_config {
uint32_t async_threshold;
uint32_t features;
uint32_t rsvd[2];
};
Expand Down Expand Up @@ -182,13 +181,9 @@ int rte_vhost_async_channel_unregister_thread_unsafe(int vid,
uint16_t queue_id);

/**
* This function submits enqueue data to async engine. Successfully
* enqueued packets can be transfer completed or being occupied by DMA
* engines, when this API returns. Transfer completed packets are returned
* in comp_pkts, so users need to guarantee its size is greater than or
* equal to the size of pkts; for packets that are successfully enqueued
* but not transfer completed, users should poll transfer status by
* rte_vhost_poll_enqueue_completed().
* This function submits enqueue packets to async copy engine. Users
* need to poll transfer status by rte_vhost_poll_enqueue_completed()
* for successfully enqueued packets.
*
* @param vid
* id of vhost device to enqueue data
Expand All @@ -198,19 +193,12 @@ int rte_vhost_async_channel_unregister_thread_unsafe(int vid,
* array of packets to be enqueued
* @param count
* packets num to be enqueued
* @param comp_pkts
* empty array to get transfer completed packets. Users need to
* guarantee its size is greater than or equal to that of pkts
* @param comp_count
* num of packets that are transfer completed, when this API returns.
* If no packets are transfer completed, its value is set to 0.
* @return
* num of packets enqueued, including in-flight and transfer completed
* num of packets enqueued
*/
__rte_experimental
uint16_t rte_vhost_submit_enqueue_burst(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count,
struct rte_mbuf **comp_pkts, uint32_t *comp_count);
struct rte_mbuf **pkts, uint16_t count);

/**
* This function checks async completion status for a specific vhost
Expand Down
6 changes: 2 additions & 4 deletions lib/vhost/vhost.c
Expand Up @@ -1621,7 +1621,6 @@ int rte_vhost_extern_callback_register(int vid,

static __rte_always_inline int
async_channel_register(int vid, uint16_t queue_id,
struct rte_vhost_async_config config,
struct rte_vhost_async_channel_ops *ops)
{
struct virtio_net *dev = get_device(vid);
Expand Down Expand Up @@ -1693,7 +1692,6 @@ async_channel_register(int vid, uint16_t queue_id,

vq->async_ops.check_completed_copies = ops->check_completed_copies;
vq->async_ops.transfer_data = ops->transfer_data;
vq->async_threshold = config.async_threshold;

vq->async_registered = true;

Expand Down Expand Up @@ -1732,7 +1730,7 @@ rte_vhost_async_channel_register(int vid, uint16_t queue_id,
return -1;

rte_spinlock_lock(&vq->access_lock);
ret = async_channel_register(vid, queue_id, config, ops);
ret = async_channel_register(vid, queue_id, ops);
rte_spinlock_unlock(&vq->access_lock);

return ret;
Expand Down Expand Up @@ -1768,7 +1766,7 @@ rte_vhost_async_channel_register_thread_unsafe(int vid, uint16_t queue_id,
ops->transfer_data == NULL))
return -1;

return async_channel_register(vid, queue_id, config, ops);
return async_channel_register(vid, queue_id, ops);
}

int
Expand Down
1 change: 0 additions & 1 deletion lib/vhost/vhost.h
Expand Up @@ -219,7 +219,6 @@ struct vhost_virtqueue {

/* vq async features */
bool async_registered;
uint32_t async_threshold;

int notif_enable;
#define VIRTIO_UNINITIALIZED_NOTIF (-1)
Expand Down

0 comments on commit abeb865

Please sign in to comment.