Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/85559/?format=api
{ "id": 85559, "url": "http://patches.dpdk.org/api/patches/85559/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/20201220211405.313012-36-maxime.coquelin@redhat.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<20201220211405.313012-36-maxime.coquelin@redhat.com>", "list_archive_url": "https://inbox.dpdk.org/dev/20201220211405.313012-36-maxime.coquelin@redhat.com", "date": "2020-12-20T21:14:00", "name": "[35/40] net/virtio: make server mode blocking", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "b3b308b3281f95d9b174dbdae3e58e73635a5d00", "submitter": { "id": 512, "url": "http://patches.dpdk.org/api/people/512/?format=api", "name": "Maxime Coquelin", "email": "maxime.coquelin@redhat.com" }, "delegate": { "id": 2642, "url": "http://patches.dpdk.org/api/users/2642/?format=api", "username": "mcoquelin", "first_name": "Maxime", "last_name": "Coquelin", "email": "maxime.coquelin@redhat.com" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/20201220211405.313012-36-maxime.coquelin@redhat.com/mbox/", "series": [ { "id": 14390, "url": "http://patches.dpdk.org/api/series/14390/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=14390", "date": "2020-12-20T21:13:25", "name": "net/virtio: Virtio PMD rework", "version": 1, "mbox": "http://patches.dpdk.org/series/14390/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/85559/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/85559/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from dpdk.org (dpdk.org [92.243.14.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 7879EA09FD;\n\tSun, 20 Dec 2020 22:25:18 +0100 (CET)", "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 07E00CEB8;\n\tSun, 20 Dec 2020 22:16:44 +0100 (CET)", "from us-smtp-delivery-124.mimecast.com\n (us-smtp-delivery-124.mimecast.com [63.128.21.124])\n by dpdk.org (Postfix) with ESMTP id EBEB2CED6\n for <dev@dpdk.org>; Sun, 20 Dec 2020 22:16:42 +0100 (CET)", "from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com\n [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id\n us-mta-502-axQcj3FuNHWceo-jOHEx4g-1; Sun, 20 Dec 2020 16:16:39 -0500", "from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com\n [10.5.11.12])\n (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n (No client certificate requested)\n by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AE459805EE3;\n Sun, 20 Dec 2020 21:16:38 +0000 (UTC)", "from max-t490s.redhat.com (unknown [10.36.110.43])\n by smtp.corp.redhat.com (Postfix) with ESMTP id 3976E60C43;\n Sun, 20 Dec 2020 21:16:31 +0000 (UTC)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1608499001;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n content-transfer-encoding:content-transfer-encoding:\n in-reply-to:in-reply-to:references:references;\n bh=v9N4clUcbeV3+fDfwtPuD5EgJ+d4jB3IwnovhFhWS3Q=;\n b=jH+4x5C/xKmhxM0SBNQVGa5107eTEIpVwQ6rxs7AGebF1a3ZhelLxv5fd5H0k71o1m8zDI\n t5B8NoZMELZlEBU7INlCrXR1ALsQRitEA+mkvbp1i7BheB+i6Yq3Fmcgu2j1AO4xuF3U3f\n sVVZMmy4VOI20LKSGGoAV11de/V1m2w=", "X-MC-Unique": "axQcj3FuNHWceo-jOHEx4g-1", "From": "Maxime Coquelin <maxime.coquelin@redhat.com>", "To": "dev@dpdk.org, chenbo.xia@intel.com, olivier.matz@6wind.com,\n amorenoz@redhat.com, david.marchand@redhat.com", "Cc": "Maxime Coquelin <maxime.coquelin@redhat.com>", "Date": "Sun, 20 Dec 2020 22:14:00 +0100", "Message-Id": "<20201220211405.313012-36-maxime.coquelin@redhat.com>", "In-Reply-To": "<20201220211405.313012-1-maxime.coquelin@redhat.com>", "References": "<20201220211405.313012-1-maxime.coquelin@redhat.com>", "MIME-Version": "1.0", "X-Scanned-By": "MIMEDefang 2.79 on 10.5.11.12", "Authentication-Results": "relay.mimecast.com;\n auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com", "X-Mimecast-Spam-Score": "0", "X-Mimecast-Originator": "redhat.com", "Content-Transfer-Encoding": "8bit", "Content-Type": "text/plain; charset=\"US-ASCII\"", "Subject": "[dpdk-dev] [PATCH 35/40] net/virtio: make server mode blocking", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "This patch makes the Vhost-user backend server mode\nblocking at init, waiting for the client connection.\n\nThe goal is to make the driver more reliable, as without\nwaiting for client connection, the Virtio driver has to\nassume the Vhost-user backend will support all the\nfeatures it has advertized, which could lead to undefined\nbehaviour.\n\nFor example, without this patch, if the user enables packed\nring Virtio feature but the backend does not support it,\nthe ring initialized by the driver will not be compatible\nwith the backend.\n\nSigned-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>\n---\n drivers/net/virtio/virtio_user/vhost_user.c | 9 +-\n .../net/virtio/virtio_user/virtio_user_dev.c | 118 +++++++-----------\n drivers/net/virtio/virtio_user_ethdev.c | 5 -\n 3 files changed, 54 insertions(+), 78 deletions(-)", "diff": "diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c\nindex a57106a468..94a33326ae 100644\n--- a/drivers/net/virtio/virtio_user/vhost_user.c\n+++ b/drivers/net/virtio/virtio_user/vhost_user.c\n@@ -677,6 +677,14 @@ virtio_user_start_server(struct virtio_user_dev *dev, struct sockaddr_un *un)\n \tif (ret < 0)\n \t\treturn -1;\n \n+\tPMD_DRV_LOG(NOTICE, \"(%s) waiting for client connection...\", dev->path);\n+\tdev->vhostfd = accept(fd, NULL, NULL);\n+\tif (dev->vhostfd < 0) {\n+\t\tPMD_DRV_LOG(ERR, \"Failed to accept initial client connection (%s)\",\n+\t\t\t\tstrerror(errno));\n+\t\treturn -1;\n+\t}\n+\n \tflag = fcntl(fd, F_GETFL);\n \tif (fcntl(fd, F_SETFL, flag | O_NONBLOCK) < 0) {\n \t\tPMD_DRV_LOG(ERR, \"fcntl failed, %s\", strerror(errno));\n@@ -721,7 +729,6 @@ vhost_user_setup(struct virtio_user_dev *dev)\n \t\t\tclose(fd);\n \t\t\treturn -1;\n \t\t}\n-\t\tdev->vhostfd = -1;\n \t} else {\n \t\tif (connect(fd, (struct sockaddr *)&un, sizeof(un)) < 0) {\n \t\t\tPMD_DRV_LOG(ERR, \"connect error, %s\", strerror(errno));\ndiff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c\nindex b92b7f7aae..19d59d401e 100644\n--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c\n+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c\n@@ -144,10 +144,6 @@ virtio_user_dev_set_features(struct virtio_user_dev *dev)\n \n \tpthread_mutex_lock(&dev->mutex);\n \n-\tif (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&\n-\t\t\tdev->vhostfd < 0)\n-\t\tgoto error;\n-\n \t/* Step 0: tell vhost to create queues */\n \tif (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0)\n \t\tgoto error;\n@@ -190,11 +186,6 @@ virtio_user_start_device(struct virtio_user_dev *dev)\n \trte_mcfg_mem_read_lock();\n \tpthread_mutex_lock(&dev->mutex);\n \n-\t/* Vhost-user client not connected yet, will start later */\n-\tif (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&\n-\t\t\tdev->vhostfd < 0)\n-\t\tgoto out;\n-\n \t/* Step 2: share memory regions */\n \tret = dev->ops->set_memory_table(dev);\n \tif (ret < 0)\n@@ -213,7 +204,7 @@ virtio_user_start_device(struct virtio_user_dev *dev)\n \t\tgoto error;\n \n \tdev->started = true;\n-out:\n+\n \tpthread_mutex_unlock(&dev->mutex);\n \trte_mcfg_mem_read_unlock();\n \n@@ -421,36 +412,36 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)\n \t\t\tPMD_DRV_LOG(ERR, \"Server mode only supports vhost-user!\");\n \t\t\treturn -1;\n \t\t}\n+\t}\n+\n+\tif (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {\n \t\tdev->ops = &virtio_ops_user;\n-\t} else {\n-\t\tif (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {\n-\t\t\tdev->ops = &virtio_ops_user;\n-\t\t} else if (dev->backend_type ==\n-\t\t\t\t\tVIRTIO_USER_BACKEND_VHOST_KERNEL) {\n-\t\t\tdev->ops = &virtio_ops_kernel;\n-\n-\t\t\tdev->vhostfds = malloc(dev->max_queue_pairs *\n-\t\t\t\t\t sizeof(int));\n-\t\t\tdev->tapfds = malloc(dev->max_queue_pairs *\n-\t\t\t\t\t sizeof(int));\n-\t\t\tif (!dev->vhostfds || !dev->tapfds) {\n-\t\t\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to allocate FDs\", dev->path);\n-\t\t\t\treturn -1;\n-\t\t\t}\n-\n-\t\t\tfor (q = 0; q < dev->max_queue_pairs; ++q) {\n-\t\t\t\tdev->vhostfds[q] = -1;\n-\t\t\t\tdev->tapfds[q] = -1;\n-\t\t\t}\n-\t\t} else if (dev->backend_type ==\n-\t\t\t\tVIRTIO_USER_BACKEND_VHOST_VDPA) {\n-\t\t\tdev->ops = &virtio_ops_vdpa;\n-\t\t} else {\n-\t\t\tPMD_DRV_LOG(ERR, \"(%s) Unknown backend type\", dev->path);\n+\t} else if (dev->backend_type ==\n+\t\t\tVIRTIO_USER_BACKEND_VHOST_KERNEL) {\n+\t\tdev->ops = &virtio_ops_kernel;\n+\n+\t\tdev->vhostfds = malloc(dev->max_queue_pairs *\n+\t\t\t\tsizeof(int));\n+\t\tdev->tapfds = malloc(dev->max_queue_pairs *\n+\t\t\t\tsizeof(int));\n+\t\tif (!dev->vhostfds || !dev->tapfds) {\n+\t\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to allocate FDs\", dev->path);\n \t\t\treturn -1;\n \t\t}\n+\n+\t\tfor (q = 0; q < dev->max_queue_pairs; ++q) {\n+\t\t\tdev->vhostfds[q] = -1;\n+\t\t\tdev->tapfds[q] = -1;\n+\t\t}\n+\t} else if (dev->backend_type ==\n+\t\t\tVIRTIO_USER_BACKEND_VHOST_VDPA) {\n+\t\tdev->ops = &virtio_ops_vdpa;\n+\t} else {\n+\t\tPMD_DRV_LOG(ERR, \"(%s) Unknown backend type\", dev->path);\n+\t\treturn -1;\n \t}\n \n+\n \tif (dev->ops->setup(dev) < 0) {\n \t\tPMD_INIT_LOG(ERR, \"(%s) Failed to setup backend\\n\", dev->path);\n \t\treturn -1;\n@@ -533,52 +524,35 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,\n \t\tdev->unsupported_features |=\n \t\t\t(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);\n \n-\tif (!dev->is_server) {\n-\t\tif (dev->ops->set_owner(dev) < 0) {\n-\t\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to set backend owner\", dev->path);\n-\t\t\treturn -1;\n-\t\t}\n+\tif (dev->ops->set_owner(dev) < 0) {\n+\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to set backend owner\", dev->path);\n+\t\treturn -1;\n+\t}\n+\n+\tif (dev->ops->get_features(dev, &dev->device_features) < 0) {\n+\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to get backend features\", dev->path);\n+\t\treturn -1;\n+\t}\n \n-\t\tif (dev->ops->get_features(dev, &dev->device_features) < 0) {\n-\t\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to get backend features\", dev->path);\n+\tif (dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) {\n+\t\tif (dev->ops->get_protocol_features(dev, &protocol_features)) {\n+\t\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to get backend protocol features\",\n+\t\t\t\t\tdev->path);\n \t\t\treturn -1;\n \t\t}\n \n-\t\tif (dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) {\n-\t\t\tif (dev->ops->get_protocol_features(dev, &protocol_features)) {\n-\t\t\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to get backend protocol features\",\n-\t\t\t\t\t\tdev->path);\n-\t\t\t\treturn -1;\n-\t\t\t}\n-\n-\t\t\tdev->protocol_features &= protocol_features;\n-\n-\t\t\tif (dev->ops->set_protocol_features(dev, dev->protocol_features)) {\n-\t\t\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to set backend protocol features\",\n-\t\t\t\t\t\tdev->path);\n-\t\t\t\treturn -1;\n-\t\t\t}\n+\t\tdev->protocol_features &= protocol_features;\n \n-\t\t\tif (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))\n-\t\t\t\tdev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);\n+\t\tif (dev->ops->set_protocol_features(dev, dev->protocol_features)) {\n+\t\t\tPMD_INIT_LOG(ERR, \"(%s) Failed to set backend protocol features\",\n+\t\t\t\t\tdev->path);\n+\t\t\treturn -1;\n \t\t}\n-\t} else {\n-\t\t/* We just pretend vhost-user can support all these features.\n-\t\t * Note that this could be problematic that if some feature is\n-\t\t * negotiated but not supported by the vhost-user which comes\n-\t\t * later.\n-\t\t */\n-\t\tdev->device_features = VIRTIO_USER_SUPPORTED_FEATURES;\n \n-\t\t/* We cannot assume VHOST_USER_PROTOCOL_F_STATUS is supported\n-\t\t * until it's negotiated\n-\t\t */\n-\t\tdev->protocol_features &=\n-\t\t\t~(1ULL << VHOST_USER_PROTOCOL_F_STATUS);\n+\t\tif (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))\n+\t\t\tdev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);\n \t}\n \n-\n-\n \tif (!mrg_rxbuf)\n \t\tdev->unsupported_features |= (1ull << VIRTIO_NET_F_MRG_RXBUF);\n \ndiff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c\nindex c63d010e16..c4cfd3daed 100644\n--- a/drivers/net/virtio/virtio_user_ethdev.c\n+++ b/drivers/net/virtio/virtio_user_ethdev.c\n@@ -174,11 +174,6 @@ virtio_user_delayed_handler(void *param)\n \t\tif (dev->vhostfd >= 0) {\n \t\t\tclose(dev->vhostfd);\n \t\t\tdev->vhostfd = -1;\n-\t\t\t/* Until the featuers are negotiated again, don't assume\n-\t\t\t * the backend supports VHOST_USER_PROTOCOL_F_STATUS\n-\t\t\t */\n-\t\t\tdev->protocol_features &=\n-\t\t\t\t~(1ULL << VHOST_USER_PROTOCOL_F_STATUS);\n \t\t}\n \t\teth_dev->intr_handle->fd = dev->listenfd;\n \t\trte_intr_callback_register(eth_dev->intr_handle,\n", "prefixes": [ "35/40" ] }