From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id DD924A0561; Wed, 17 Mar 2021 21:26:12 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 920F7140F73; Wed, 17 Mar 2021 21:25:58 +0100 (CET) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by mails.dpdk.org (Postfix) with ESMTP id 05634140F67 for ; Wed, 17 Mar 2021 21:25:57 +0100 (CET) X-Originating-IP: 78.45.89.65 Received: from im-t490s.redhat.com (ip-78-45-89-65.net.upcbroadband.cz [78.45.89.65]) (Authenticated sender: i.maximets@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 90E3960002; Wed, 17 Mar 2021 20:25:55 +0000 (UTC) From: Ilya Maximets To: Maxime Coquelin Cc: Chenbo Xia , dev@dpdk.org, Adrian Moreno , Stefan Hajnoczi , Julia Suvorova , Ilya Maximets Date: Wed, 17 Mar 2021 21:25:30 +0100 Message-Id: <20210317202530.4145673-5-i.maximets@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210317202530.4145673-1-i.maximets@ovn.org> References: <20210317202530.4145673-1-i.maximets@ovn.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [RFC 4/4] net/virtio: add support for SocketPair Broker X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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=,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 --- 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 --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 #include +#ifdef VIRTIO_SOCKETPAIR_BROKER +#include +#endif + + #include #include #include @@ -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= " "queues= " "iface= " + "broker_key= " "server=<0|1> " "mrg_rxbuf=<0|1> " "in_order=<0|1> " -- 2.26.2