From: Tomasz Kulasek <tomaszx.kulasek@intel.com>
To: yliu@fridaylinux.org
Cc: dev@dpdk.org, Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Subject: [dpdk-dev] [PATCH] vhost: maintain separate virtio features field
Date: Mon, 5 Mar 2018 16:49:09 +0100 [thread overview]
Message-ID: <20180305154909.216-1-tomaszx.kulasek@intel.com> (raw)
There are two separate abstraction layers:
* vsocket - which represents a unix domain socket
* virtio_net - which represents a vsocket connection
There can be many connections on the same socket. vsocket provides an
API to enable/disable particular virtio features on the fly, but it's
the virtio_net that uses these features.
virtio_net used to rely on vsocket->features during feature negotiation,
breaking the layer encapsulation (and yet causing a deadlock - two locks
were being locked in a separate order). Now each virtio_net device has
it's own copy of vsocket features, created at the time of virtio_net
creation.
vsocket->features have to be still present, as features can be
enabled/disabled while no virtio_net device has been created yet.
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
lib/librte_vhost/socket.c | 2 +-
lib/librte_vhost/vhost.c | 9 +++++----
lib/librte_vhost/vhost.h | 8 +++++---
lib/librte_vhost/vhost_user.c | 33 +++++++++++++++++----------------
4 files changed, 28 insertions(+), 24 deletions(-)
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 83befdced..260e38dbe 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -188,7 +188,7 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
return;
}
- vid = vhost_new_device();
+ vid = vhost_new_device(vsocket->features);
if (vid == -1) {
goto err;
}
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index a407067e2..a307a19ed 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -256,7 +256,7 @@ reset_device(struct virtio_net *dev)
{
uint32_t i;
- dev->features = 0;
+ dev->negotiated_features = 0;
dev->protocol_features = 0;
dev->flags &= VIRTIO_DEV_BUILTIN_VIRTIO_NET;
@@ -269,7 +269,7 @@ reset_device(struct virtio_net *dev)
* there is a new virtio device being attached).
*/
int
-vhost_new_device(void)
+vhost_new_device(uint64_t features)
{
struct virtio_net *dev;
int i;
@@ -296,6 +296,7 @@ vhost_new_device(void)
dev->vid = i;
dev->flags = VIRTIO_DEV_BUILTIN_VIRTIO_NET;
dev->slave_req_fd = -1;
+ dev->features = features;
return i;
}
@@ -376,7 +377,7 @@ rte_vhost_get_mtu(int vid, uint16_t *mtu)
if (!(dev->flags & VIRTIO_DEV_READY))
return -EAGAIN;
- if (!(dev->features & (1ULL << VIRTIO_NET_F_MTU)))
+ if (!(dev->negotiated_features & (1ULL << VIRTIO_NET_F_MTU)))
return -ENOTSUP;
*mtu = dev->mtu;
@@ -458,7 +459,7 @@ rte_vhost_get_negotiated_features(int vid, uint64_t *features)
if (!dev)
return -1;
- *features = dev->features;
+ *features = dev->negotiated_features;
return 0;
}
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index d947bc9e3..efbc89857 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -217,6 +217,7 @@ struct virtio_net {
/* Frontend (QEMU) memory and memory region information */
struct rte_vhost_memory *mem;
uint64_t features;
+ uint64_t negotiated_features;
uint64_t protocol_features;
int vid;
uint32_t flags;
@@ -266,8 +267,9 @@ vhost_log_write(struct virtio_net *dev, uint64_t addr, uint64_t len)
{
uint64_t page;
- if (likely(((dev->features & (1ULL << VHOST_F_LOG_ALL)) == 0) ||
- !dev->log_base || !len))
+ if (likely(((dev->negotiated_features &
+ (1ULL << VHOST_F_LOG_ALL)) == 0) || !dev->log_base ||
+ !len))
return;
if (unlikely(dev->log_size <= ((addr + len - 1) / VHOST_LOG_PAGE / 8)))
@@ -347,7 +349,7 @@ gpa_to_hpa(struct virtio_net *dev, uint64_t gpa, uint64_t size)
struct virtio_net *get_device(int vid);
-int vhost_new_device(void);
+int vhost_new_device(uint64_t features);
void cleanup_device(struct virtio_net *dev, int destroy);
void reset_device(struct virtio_net *dev);
void vhost_destroy_device(int);
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 65ee33919..818fc4263 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -132,10 +132,7 @@ vhost_user_reset_owner(struct virtio_net *dev)
static uint64_t
vhost_user_get_features(struct virtio_net *dev)
{
- uint64_t features = 0;
-
- rte_vhost_driver_get_features(dev->ifname, &features);
- return features;
+ return dev->features;
}
/*
@@ -146,7 +143,7 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
{
uint64_t vhost_features = 0;
- rte_vhost_driver_get_features(dev->ifname, &vhost_features);
+ vhost_features = vhost_user_get_features(dev);
if (features & ~vhost_features) {
RTE_LOG(ERR, VHOST_CONFIG,
"(%d) received invalid negotiated features.\n",
@@ -155,7 +152,7 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
}
if (dev->flags & VIRTIO_DEV_RUNNING) {
- if (dev->features == features)
+ if (dev->negotiated_features == features)
return 0;
/*
@@ -163,7 +160,8 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
* in running state. The exception being VHOST_F_LOG_ALL, which
* is enabled when the live-migration starts.
*/
- if ((dev->features ^ features) & ~(1ULL << VHOST_F_LOG_ALL)) {
+ if ((dev->negotiated_features ^ features) &
+ ~(1ULL << VHOST_F_LOG_ALL)) {
RTE_LOG(ERR, VHOST_CONFIG,
"(%d) features changed while device is running.\n",
dev->vid);
@@ -174,9 +172,9 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
dev->notify_ops->features_changed(dev->vid, features);
}
- dev->features = features;
- if (dev->features &
- ((1 << VIRTIO_NET_F_MRG_RXBUF) | (1ULL << VIRTIO_F_VERSION_1))) {
+ dev->negotiated_features = features;
+ if (dev->negotiated_features & ((1 << VIRTIO_NET_F_MRG_RXBUF) |
+ (1ULL << VIRTIO_F_VERSION_1))) {
dev->vhost_hlen = sizeof(struct virtio_net_hdr_mrg_rxbuf);
} else {
dev->vhost_hlen = sizeof(struct virtio_net_hdr);
@@ -184,11 +182,13 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
LOG_DEBUG(VHOST_CONFIG,
"(%d) mergeable RX buffers %s, virtio 1 %s\n",
dev->vid,
- (dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ? "on" : "off",
- (dev->features & (1ULL << VIRTIO_F_VERSION_1)) ? "on" : "off");
+ (dev->negotiated_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ?
+ "on" : "off",
+ (dev->negotiated_features & (1ULL << VIRTIO_F_VERSION_1)) ?
+ "on" : "off");
if ((dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET) &&
- !(dev->features & (1ULL << VIRTIO_NET_F_MQ))) {
+ !(dev->negotiated_features & (1ULL << VIRTIO_NET_F_MQ))) {
/*
* Remove all but first queue pair if MQ hasn't been
* negotiated. This is safe because the device is not
@@ -395,7 +395,7 @@ static uint64_t
ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
uint64_t ra, uint64_t size)
{
- if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) {
+ if (dev->negotiated_features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) {
uint64_t vva;
vva = vhost_user_iotlb_cache_find(vq, ra,
@@ -498,7 +498,7 @@ vhost_user_set_vring_addr(struct virtio_net **pdev, VhostUserMsg *msg)
vring_invalidate(dev, vq);
- if (vq->enabled && (dev->features &
+ if (vq->enabled && (dev->negotiated_features &
(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
dev = translate_ring_addresses(dev, msg->payload.state.index);
if (!dev)
@@ -840,7 +840,8 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg)
* the ring starts already enabled. Otherwise, it is enabled via
* the SET_VRING_ENABLE message.
*/
- if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
+ if (!(dev->negotiated_features &
+ (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
vq->enabled = 1;
if (vq->kickfd >= 0)
--
2.14.1
next reply other threads:[~2018-03-05 15:49 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-05 15:49 Tomasz Kulasek [this message]
2018-03-28 8:53 ` Maxime Coquelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180305154909.216-1-tomaszx.kulasek@intel.com \
--to=tomaszx.kulasek@intel.com \
--cc=dariuszx.stojaczyk@intel.com \
--cc=dev@dpdk.org \
--cc=yliu@fridaylinux.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).