diff mbox series

[RFC,4/4] net/virtio: add support for SocketPair Broker

Message ID 20210317202530.4145673-5-i.maximets@ovn.org (mailing list archive)
State New
Delegated to: Maxime Coquelin
Headers show
Series SocketPair Broker support for vhost and virtio-user. | expand

Checks

Context Check Description
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing warning Testing issues
ci/iol-abi-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Ilya Maximets March 17, 2021, 8:25 p.m. UTC
New configuration option 'broker-key' to specify that the socket
path is actually a path to SocketPair Broker's socket.  The value
of a 'broker-key' argument will be used as a broker key, so the
broker will be able to identify which vhost device virtio-user
should be paired with.

Ex.:
  --vdev="net_virtio_user,path=broker.socket,broker-key=<key>,server=1"

libspbroker needed for a build.

Had to implement few tricks to trigger interrupts by alarm for the
case where we have no any sockets open, i.e. broker is disconnected
and vhost connection is dead.  OTOH, this infrastructure might be
used later to implement client reconnection for virtio-user.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 doc/guides/nics/virtio.rst                    |   5 +
 drivers/net/virtio/meson.build                |   6 +
 drivers/net/virtio/virtio_user/vhost_user.c   | 118 +++++++++++++++++-
 .../net/virtio/virtio_user/virtio_user_dev.c  |  92 ++++++++++----
 .../net/virtio/virtio_user/virtio_user_dev.h  |   4 +-
 drivers/net/virtio/virtio_user_ethdev.c       |  30 ++++-
 6 files changed, 227 insertions(+), 28 deletions(-)
diff mbox series

Patch

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index 02e74a6e77..e3ca5b57f8 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -376,6 +376,11 @@  Below devargs are supported by the virtio-user vdev:
 
     It is used to specify a path to connect to vhost backend.
 
+#.  ``broker-key``:
+
+    It is used to specify that ``path`` points to a SocketPair Broker, value
+    is used as a pairing key.
+
 #.  ``mac``:
 
     It is used to specify the MAC address.
diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build
index d595cfdcab..aada19d8da 100644
--- a/drivers/net/virtio/meson.build
+++ b/drivers/net/virtio/meson.build
@@ -7,6 +7,12 @@  if is_windows
 	subdir_done()
 endif
 
+spbroker_dep = dependency('spbroker', required: false)
+if spbroker_dep.found()
+	dpdk_conf.set('VIRTIO_SOCKETPAIR_BROKER', 1)
+	ext_deps += spbroker_dep
+endif
+
 sources += files('virtio.c',
 	'virtio_ethdev.c',
 	'virtio_pci_ethdev.c',
diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index 25f74c625b..9d87cda2f5 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -11,6 +11,11 @@ 
 #include <string.h>
 #include <errno.h>
 
+#ifdef VIRTIO_SOCKETPAIR_BROKER
+#include <socketpair-broker/helper.h>
+#endif
+
+
 #include <rte_alarm.h>
 #include <rte_string_fns.h>
 #include <rte_fbarray.h>
@@ -21,6 +26,7 @@ 
 struct vhost_user_data {
 	int vhostfd;
 	int listenfd;
+	int brokerfd;
 	uint64_t protocol_features;
 };
 
@@ -790,12 +796,78 @@  vhost_user_server_disconnect(struct virtio_user_dev *dev)
 	return 0;
 }
 
+static void
+vhost_user_update_broker_fd(struct virtio_user_dev *dev __rte_unused)
+{
+#ifdef VIRTIO_SOCKETPAIR_BROKER
+	struct vhost_user_data *data = dev->backend_data;
+	char *err = NULL;
+	int flags;
+
+	if (data->brokerfd != -1)
+		return;
+
+	data->brokerfd = sp_broker_connect(dev->path, false, &err);
+	if (data->brokerfd < 0) {
+		PMD_DRV_LOG(WARNING, "failed to connect to broker: %s", err);
+		free(err);
+		data->brokerfd = -1;
+		return;
+	}
+
+	if (sp_broker_send_get_pair(data->brokerfd, dev->broker_key,
+				    dev->is_server, &err)) {
+		PMD_DRV_LOG(WARNING,
+			    "failed to send GET_PAIR request: %s", err);
+		free(err);
+		close(data->brokerfd);
+		data->brokerfd = -1;
+		return;
+	}
+
+	flags = fcntl(data->brokerfd, F_GETFL);
+	if (fcntl(data->brokerfd, F_SETFL, flags | O_NONBLOCK) == -1) {
+		PMD_DRV_LOG(ERR, "error setting O_NONBLOCK flag");
+		close(data->brokerfd);
+		data->brokerfd = -1;
+		return;
+	}
+#endif
+}
+
 static int
 vhost_user_server_reconnect(struct virtio_user_dev *dev)
 {
 	struct vhost_user_data *data = dev->backend_data;
 	int fd;
 
+#ifdef VIRTIO_SOCKETPAIR_BROKER
+	if (dev->broker_key) {
+		char *err;
+
+		vhost_user_update_broker_fd(dev);
+		if (data->brokerfd == -1)
+			return -1;
+
+		fd = sp_broker_receive_set_pair(data->brokerfd, &err);
+		if (fd < 0) {
+			PMD_DRV_LOG(DEBUG,
+				    "failed to receive SET_PAIR: %s", err);
+			free(err);
+			/*
+			 * Unfortunately, since connection is non-blocking
+			 * we can't reliably say if the other end is dead for
+			 * a case where it didn't close the socket gracefully.
+			 * Closing current connection to re-establish later.
+			 */
+			close(data->brokerfd);
+			data->brokerfd = -1;
+			return -1;
+		}
+		data->vhostfd = fd;
+		return 0;
+	}
+#endif
 	fd = accept(data->listenfd, NULL, NULL);
 	if (fd < 0)
 		return -1;
@@ -832,6 +904,29 @@  vhost_user_setup(struct virtio_user_dev *dev)
 
 	data->vhostfd = -1;
 	data->listenfd = -1;
+	data->brokerfd = -1;
+
+	if (dev->broker_key) {
+#ifdef VIRTIO_SOCKETPAIR_BROKER
+		char *err;
+
+		fd = sp_broker_get_pair(dev->path, dev->broker_key,
+					dev->is_server, &err);
+		if (fd < 0) {
+			PMD_DRV_LOG(ERR,
+				"virtio-user failed to connect to broker: %s",
+				err);
+			free(err);
+			goto err_data;
+		}
+		data->vhostfd = fd;
+		return 0;
+#else
+		PMD_DRV_LOG(ERR, "virtio-user broker connection requested "
+				 "but not compiled");
+		goto err_data;
+#endif
+	}
 
 	fd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (fd < 0) {
@@ -890,6 +985,11 @@  vhost_user_destroy(struct virtio_user_dev *dev)
 		data->listenfd = -1;
 	}
 
+	if (data->brokerfd >= 0) {
+		close(data->brokerfd);
+		data->brokerfd = -1;
+	}
+
 	free(data);
 	dev->backend_data = NULL;
 
@@ -983,8 +1083,22 @@  vhost_user_get_intr_fd(struct virtio_user_dev *dev)
 {
 	struct vhost_user_data *data = dev->backend_data;
 
-	if (dev->is_server && data->vhostfd == -1)
-		return data->listenfd;
+	if (data->vhostfd == -1) {
+		if (dev->broker_key) {
+			vhost_user_update_broker_fd(dev);
+			return data->brokerfd;
+		}
+		if (dev->is_server)
+			return data->listenfd;
+	} else if (data->brokerfd != -1) {
+		/*
+		 * Broker not needed anymore and we need to close the socket
+		 * because other end will be closed by the broker anyway.
+		 */
+		PMD_DRV_LOG(DEBUG, "Closing broker fd: %d", data->brokerfd);
+		close(data->brokerfd);
+		data->brokerfd = -1;
+	}
 
 	return data->vhostfd;
 }
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 68cece42d3..8b53af0724 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -414,11 +414,16 @@  virtio_user_mem_event_cb(enum rte_mem_event type __rte_unused,
 static int
 virtio_user_dev_setup(struct virtio_user_dev *dev)
 {
-	if (dev->is_server) {
-		if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
-			PMD_DRV_LOG(ERR, "Server mode only supports vhost-user!");
-			return -1;
-		}
+	if (dev->is_server &&
+	    dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
+		PMD_DRV_LOG(ERR, "Server mode only supports vhost-user!");
+		return -1;
+	}
+
+	if (dev->broker_key &&
+	    dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
+		PMD_DRV_LOG(ERR, "Broker connection only supports vhost-user!");
+		return -1;
 	}
 
 	switch (dev->backend_type) {
@@ -483,10 +488,10 @@  virtio_user_dev_setup(struct virtio_user_dev *dev)
 	 1ULL << VIRTIO_F_RING_PACKED)
 
 int
-virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
-		     int cq, int queue_size, const char *mac, char **ifname,
-		     int server, int mrg_rxbuf, int in_order, int packed_vq,
-		     enum virtio_user_backend_type backend_type)
+virtio_user_dev_init(struct virtio_user_dev *dev, char *path, char **broker_key,
+		     int queues, int cq, int queue_size, const char *mac,
+		     char **ifname, int server, int mrg_rxbuf, int in_order,
+		     int packed_vq, enum virtio_user_backend_type backend_type)
 {
 	uint64_t backend_features;
 	int i;
@@ -511,6 +516,11 @@  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 
 	parse_mac(dev, mac);
 
+	if (*broker_key) {
+		dev->broker_key = *broker_key;
+		*broker_key = NULL;
+	}
+
 	if (*ifname) {
 		dev->ifname = *ifname;
 		*ifname = NULL;
@@ -595,6 +605,10 @@  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 void
 virtio_user_dev_uninit(struct virtio_user_dev *dev)
 {
+	struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->port_id];
+
+	rte_eal_alarm_cancel(virtio_interrupt_handler, eth_dev);
+
 	virtio_user_stop_device(dev);
 
 	rte_mem_event_callback_unregister(VIRTIO_USER_MEM_EVENT_CLB_NAME, dev);
@@ -603,9 +617,11 @@  virtio_user_dev_uninit(struct virtio_user_dev *dev)
 
 	free(dev->ifname);
 
-	if (dev->is_server)
+	if (dev->is_server && !dev->broker_key)
 		unlink(dev->path);
 
+	free(dev->broker_key);
+
 	dev->ops->destroy(dev);
 }
 
@@ -929,21 +945,44 @@  virtio_user_dev_delayed_intr_reconfig_handler(void *param)
 	struct virtio_user_dev *dev = param;
 	struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->port_id];
 
-	PMD_DRV_LOG(DEBUG, "Unregistering intr fd: %d",
-		    eth_dev->intr_handle->fd);
+	if (rte_intr_disable(eth_dev->intr_handle) < 0) {
+		PMD_DRV_LOG(ERR, "interrupt disable failed");
+		return;
+	}
 
-	if (rte_intr_callback_unregister(eth_dev->intr_handle,
-					 virtio_interrupt_handler,
-					 eth_dev) != 1)
-		PMD_DRV_LOG(ERR, "interrupt unregister failed");
+	if (eth_dev->intr_handle->fd != -1) {
+		PMD_DRV_LOG(DEBUG, "Unregistering intr fd: %d",
+			    eth_dev->intr_handle->fd);
+
+		if (rte_intr_callback_unregister(eth_dev->intr_handle,
+						 virtio_interrupt_handler,
+						 eth_dev) != 1)
+			PMD_DRV_LOG(ERR, "interrupt unregister failed");
+	}
 
 	eth_dev->intr_handle->fd = dev->ops->get_intr_fd(dev);
 
-	PMD_DRV_LOG(DEBUG, "Registering intr fd: %d", eth_dev->intr_handle->fd);
+	rte_eal_alarm_cancel(virtio_interrupt_handler, eth_dev);
+	if (eth_dev->intr_handle->fd == -1) {
+		PMD_DRV_LOG(DEBUG, "Scheduling interrupt as alarm");
+		/*
+		 * There is no interrupt source.  Setting alarm
+		 * instead to try to re-connect later.
+		 */
+		rte_eal_alarm_set(US_PER_S, virtio_interrupt_handler,
+				  (void *)eth_dev);
+		return;
+	}
+
+	if (eth_dev->intr_handle->fd != -1) {
+		PMD_DRV_LOG(DEBUG, "Registering intr fd: %d",
+			    eth_dev->intr_handle->fd);
 
-	if (rte_intr_callback_register(eth_dev->intr_handle,
-				       virtio_interrupt_handler, eth_dev))
-		PMD_DRV_LOG(ERR, "interrupt register failed");
+		if (rte_intr_callback_register(eth_dev->intr_handle,
+					       virtio_interrupt_handler,
+					       eth_dev))
+			PMD_DRV_LOG(ERR, "interrupt register failed");
+	}
 
 	if (rte_intr_enable(eth_dev->intr_handle) < 0)
 		PMD_DRV_LOG(ERR, "interrupt enable failed");
@@ -963,6 +1002,15 @@  virtio_user_dev_server_reconnect(struct virtio_user_dev *dev)
 
 	if (dev->ops->server_reconnect(dev)) {
 		PMD_DRV_LOG(ERR, "(%s) Reconnect callback call failed", dev->path);
+		if (dev->broker_key) {
+			/*
+			 * We might need to re-establish broker connection, so
+			 * we need to re-configure interrupts for it.
+			 */
+			rte_eal_alarm_set(1,
+				virtio_user_dev_delayed_intr_reconfig_handler,
+				(void *)dev);
+		}
 		return -1;
 	}
 
@@ -1010,10 +1058,6 @@  virtio_user_dev_server_reconnect(struct virtio_user_dev *dev)
 		}
 	}
 	if (eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC) {
-		if (rte_intr_disable(eth_dev->intr_handle) < 0) {
-			PMD_DRV_LOG(ERR, "interrupt disable failed");
-			return -1;
-		}
 		/*
 		 * This function can be called from the interrupt handler, so
 		 * we can't unregister interrupt handler here.  Setting
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 15d177ccd2..f1d75c76d3 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -47,6 +47,7 @@  struct virtio_user_dev {
 	uint8_t		mac_addr[RTE_ETHER_ADDR_LEN];
 	char		path[PATH_MAX];
 	char		*ifname;
+	char		*broker_key;
 
 	union {
 		struct vring		vrings[VIRTIO_MAX_VIRTQUEUES];
@@ -65,7 +66,8 @@  struct virtio_user_dev {
 int virtio_user_dev_set_features(struct virtio_user_dev *dev);
 int virtio_user_start_device(struct virtio_user_dev *dev);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
-int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
+int virtio_user_dev_init(struct virtio_user_dev *dev, char *path,
+			 char **broker_key, int queues,
 			 int cq, int queue_size, const char *mac, char **ifname,
 			 int server, int mrg_rxbuf, int in_order,
 			 int packed_vq,
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 1810a54694..bace903658 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -315,6 +315,8 @@  static const char *valid_args[] = {
 	VIRTIO_USER_ARG_SPEED,
 #define VIRTIO_USER_ARG_VECTORIZED     "vectorized"
 	VIRTIO_USER_ARG_VECTORIZED,
+#define VIRTIO_USER_ARG_BROKER_KEY     "broker-key"
+	VIRTIO_USER_ARG_BROKER_KEY,
 	NULL
 };
 
@@ -467,6 +469,7 @@  virtio_user_pmd_probe(struct rte_vdev_device *vdev)
 	uint64_t packed_vq = 0;
 	uint64_t vectorized = 0;
 	char *path = NULL;
+	char *broker_key = NULL;
 	char *ifname = NULL;
 	char *mac_addr = NULL;
 	int ret = -1;
@@ -578,6 +581,28 @@  virtio_user_pmd_probe(struct rte_vdev_device *vdev)
 		}
 	}
 
+	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_BROKER_KEY) == 1) {
+#ifndef VIRTIO_SOCKETPAIR_BROKER
+		PMD_INIT_LOG(ERR,
+			"arg %s requested but support for SocketPair Broker "
+			"is not compiled",
+			VIRTIO_USER_ARG_BROKER_KEY);
+		goto end;
+#endif
+		if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
+			PMD_INIT_LOG(ERR,
+				"arg %s applies only to vhost-user backend",
+				VIRTIO_USER_ARG_BROKER_KEY);
+			goto end;
+		}
+		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_BROKER_KEY,
+				       &get_string_arg, &broker_key) < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+				     VIRTIO_USER_ARG_BROKER_KEY);
+			goto end;
+		}
+	}
+
 	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) {
 		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM,
 				       &get_integer_arg, &cq) < 0) {
@@ -645,7 +670,7 @@  virtio_user_pmd_probe(struct rte_vdev_device *vdev)
 
 	dev = eth_dev->data->dev_private;
 	hw = &dev->hw;
-	if (virtio_user_dev_init(dev, path, queues, cq,
+	if (virtio_user_dev_init(dev, path, &broker_key, queues, cq,
 			 queue_size, mac_addr, &ifname, server_mode,
 			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
 		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
@@ -682,6 +707,8 @@  virtio_user_pmd_probe(struct rte_vdev_device *vdev)
 		rte_kvargs_free(kvlist);
 	if (path)
 		free(path);
+	if (broker_key)
+		free(broker_key);
 	if (mac_addr)
 		free(mac_addr);
 	if (ifname)
@@ -777,6 +804,7 @@  RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
 	"queue_size=<int> "
 	"queues=<int> "
 	"iface=<string> "
+	"broker_key=<string> "
 	"server=<0|1> "
 	"mrg_rxbuf=<0|1> "
 	"in_order=<0|1> "