Skip to content

Commit

Permalink
vhost: introduce API to start a specific driver
Browse files Browse the repository at this point in the history
We used to use rte_vhost_driver_session_start() to trigger the vhost-user
session. It takes no argument, thus it's a global trigger. And it could
be problematic.

The issue is, currently, rte_vhost_driver_register(path, flags) actually
tries to put it into the session loop (by fdset_add). However, it needs
a set of APIs to set a vhost-user driver properly:
  * rte_vhost_driver_register(path, flags);
  * rte_vhost_driver_set_features(path, features);
  * rte_vhost_driver_callback_register(path, vhost_device_ops);

If a new vhost-user driver is registered after the trigger (think OVS-DPDK
that could add a port dynamically from cmdline), the current code will
effectively starts the session for the new driver just after the first
API rte_vhost_driver_register() is invoked, leaving later calls taking
no effect at all.

To handle the case properly, this patch introduce a new API,
rte_vhost_driver_start(path), to trigger a specific vhost-user driver.
To do that, the rte_vhost_driver_register(path, flags) is simplified
to create the socket only and let rte_vhost_driver_start(path) to
actually put it into the session loop.

Meanwhile, the rte_vhost_driver_session_start is removed: we could hide
the session thread internally (create the thread if it has not been
created). This would also simplify the application.

NOTE: the API order in prog guide is slightly adjusted for showing the
correct invoke order.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
  • Loading branch information
Yuanhan Liu committed Apr 1, 2017
1 parent 52f8091 commit af14759
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 103 deletions.
24 changes: 11 additions & 13 deletions doc/guides/prog_guide/vhost_lib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,6 @@ The following is an overview of some key Vhost API functions:
vhost-user driver could be vhost-user net, yet it could be something else,
say, vhost-user SCSI.

* ``rte_vhost_driver_session_start()``

This function starts the vhost session loop to handle vhost messages. It
starts an infinite loop, therefore it should be called in a dedicated
thread.

* ``rte_vhost_driver_callback_register(path, vhost_device_ops)``

This function registers a set of callbacks, to let DPDK applications take
Expand Down Expand Up @@ -149,6 +143,17 @@ The following is an overview of some key Vhost API functions:
``VHOST_F_LOG_ALL`` will be set/cleared at the start/end of live
migration, respectively.

* ``rte_vhost_driver_disable/enable_features(path, features))``

This function disables/enables some features. For example, it can be used to
disable mergeable buffers and TSO features, which both are enabled by
default.

* ``rte_vhost_driver_start(path)``

This function triggers the vhost-user negotiation. It should be invoked at
the end of initializing a vhost-user driver.

* ``rte_vhost_enqueue_burst(vid, queue_id, pkts, count)``

Transmits (enqueues) ``count`` packets from host to guest.
Expand All @@ -157,13 +162,6 @@ The following is an overview of some key Vhost API functions:

Receives (dequeues) ``count`` packets from guest, and stored them at ``pkts``.

* ``rte_vhost_driver_disable/enable_features(path, features))``

This function disables/enables some features. For example, it can be used to
disable mergeable buffers and TSO features, which both are enabled by
default.


Vhost-user Implementations
--------------------------

Expand Down
4 changes: 4 additions & 0 deletions doc/guides/rel_notes/release_17_05.rst
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ API Changes
* The vhost struct ``virtio_net_device_ops`` is renamed to
``vhost_device_ops``

* The vhost API ``rte_vhost_driver_session_start`` is removed. Instead,
``rte_vhost_driver_start`` should be used, and no need to create a
thread to call it.


ABI Changes
-----------
Expand Down
50 changes: 4 additions & 46 deletions drivers/net/vhost/rte_eth_vhost.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ static struct internal_list_head internal_list =

static pthread_mutex_t internal_list_lock = PTHREAD_MUTEX_INITIALIZER;

static rte_atomic16_t nb_started_ports;
static pthread_t session_th;

static struct rte_eth_link pmd_link = {
.link_speed = 10000,
.link_duplex = ETH_LINK_FULL_DUPLEX,
Expand Down Expand Up @@ -769,42 +766,6 @@ rte_eth_vhost_get_vid_from_port_id(uint8_t port_id)
return vid;
}

static void *
vhost_driver_session(void *param __rte_unused)
{
/* start event handling */
rte_vhost_driver_session_start();

return NULL;
}

static int
vhost_driver_session_start(void)
{
int ret;

ret = pthread_create(&session_th,
NULL, vhost_driver_session, NULL);
if (ret)
RTE_LOG(ERR, PMD, "Can't create a thread\n");

return ret;
}

static void
vhost_driver_session_stop(void)
{
int ret;

ret = pthread_cancel(session_th);
if (ret)
RTE_LOG(ERR, PMD, "Can't cancel the thread\n");

ret = pthread_join(session_th, NULL);
if (ret)
RTE_LOG(ERR, PMD, "Can't join the thread\n");
}

static int
eth_dev_start(struct rte_eth_dev *dev)
{
Expand Down Expand Up @@ -1120,10 +1081,10 @@ eth_dev_vhost_create(const char *name, char *iface_name, int16_t queues,
goto error;
}

/* We need only one message handling thread */
if (rte_atomic16_add_return(&nb_started_ports, 1) == 1) {
if (vhost_driver_session_start())
goto error;
if (rte_vhost_driver_start(iface_name) < 0) {
RTE_LOG(ERR, PMD, "Failed to start driver for %s\n",
iface_name);
goto error;
}

return data->port_id;
Expand Down Expand Up @@ -1250,9 +1211,6 @@ rte_pmd_vhost_remove(const char *name)

eth_dev_close(eth_dev);

if (rte_atomic16_sub_return(&nb_started_ports, 1) == 0)
vhost_driver_session_stop();

rte_free(vring_states[eth_dev->data->port_id]);
vring_states[eth_dev->data->port_id] = NULL;

Expand Down
8 changes: 7 additions & 1 deletion examples/tep_termination/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1263,7 +1263,13 @@ main(int argc, char *argv[])
"failed to register vhost driver callbacks.\n");
}

rte_vhost_driver_session_start();
if (rte_vhost_driver_start(dev_basename) < 0) {
rte_exit(EXIT_FAILURE,
"failed to start vhost driver.\n");
}

RTE_LCORE_FOREACH_SLAVE(lcore_id)
rte_eal_wait_lcore(lcore_id);

return 0;
}
9 changes: 8 additions & 1 deletion examples/vhost/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1545,9 +1545,16 @@ main(int argc, char *argv[])
rte_exit(EXIT_FAILURE,
"failed to register vhost driver callbacks.\n");
}

if (rte_vhost_driver_start(file) < 0) {
rte_exit(EXIT_FAILURE,
"failed to start vhost driver.\n");
}
}

rte_vhost_driver_session_start();
RTE_LCORE_FOREACH_SLAVE(lcore_id)
rte_eal_wait_lcore(lcore_id);

return 0;

}
9 changes: 6 additions & 3 deletions lib/librte_vhost/fd_man.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ fdset_del(struct fdset *pfdset, int fd)
* will wait until the flag is reset to zero(which indicates the callback is
* finished), then it could free the context after fdset_del.
*/
void
fdset_event_dispatch(struct fdset *pfdset)
void *
fdset_event_dispatch(void *arg)
{
int i;
struct pollfd *pfd;
Expand All @@ -221,9 +221,10 @@ fdset_event_dispatch(struct fdset *pfdset)
int fd, numfds;
int remove1, remove2;
int need_shrink;
struct fdset *pfdset = arg;

if (pfdset == NULL)
return;
return NULL;

while (1) {

Expand Down Expand Up @@ -294,4 +295,6 @@ fdset_event_dispatch(struct fdset *pfdset)
if (need_shrink)
fdset_shrink(pfdset);
}

return NULL;
}
2 changes: 1 addition & 1 deletion lib/librte_vhost/fd_man.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,6 @@ int fdset_add(struct fdset *pfdset, int fd,

void *fdset_del(struct fdset *pfdset, int fd);

void fdset_event_dispatch(struct fdset *pfdset);
void *fdset_event_dispatch(void *arg);

#endif
2 changes: 1 addition & 1 deletion lib/librte_vhost/rte_vhost_version.map
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ DPDK_2.0 {
rte_vhost_dequeue_burst;
rte_vhost_driver_callback_register;
rte_vhost_driver_register;
rte_vhost_driver_session_start;
rte_vhost_enable_guest_notification;
rte_vhost_enqueue_burst;

Expand Down Expand Up @@ -35,6 +34,7 @@ DPDK_17.05 {
rte_vhost_driver_enable_features;
rte_vhost_driver_get_features;
rte_vhost_driver_set_features;
rte_vhost_driver_start;
rte_vhost_get_mem_table;
rte_vhost_get_mtu;
rte_vhost_get_negotiated_features;
Expand Down
15 changes: 13 additions & 2 deletions lib/librte_vhost/rte_virtio_net.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,19 @@ int rte_vhost_get_negotiated_features(int vid, uint64_t *features);
/* Register callbacks. */
int rte_vhost_driver_callback_register(const char *path,
struct vhost_device_ops const * const ops);
/* Start vhost driver session blocking loop. */
int rte_vhost_driver_session_start(void);

/**
*
* Start the vhost-user driver.
*
* This function triggers the vhost-user negotiation.
*
* @param path
* The vhost-user socket file path
* @return
* 0 on success, -1 on failure
*/
int rte_vhost_driver_start(const char *path);

/**
* Get the MTU value of the device if set in QEMU.
Expand Down
Loading

0 comments on commit af14759

Please sign in to comment.