* [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message
@ 2015-08-19 9:51 Tetsuya Mukawa
2015-08-19 9:51 ` [dpdk-dev] [PATCH 2/3] vhost: Fix RESET_OWNER handling not to close callfd Tetsuya Mukawa
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tetsuya Mukawa @ 2015-08-19 9:51 UTC (permalink / raw)
To: dev
When vhost-user frontend sends GET_VRING_BASE, last used index of vring
should be returned. In DPDK vhost library, 'last_used_idx' represents it.
But the value can be over max index value. To return correct value to
vhost frontend, it's needed to be masked.
Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
lib/librte_vhost/virtio-net.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index b520ec5..144f301 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -661,7 +661,8 @@ get_vring_base(struct vhost_device_ctx ctx, uint32_t index,
state->index = index;
/* State->index refers to the queue index. The txq is 1, rxq is 0. */
- state->num = dev->virtqueue[state->index]->last_used_idx;
+ state->num = dev->virtqueue[state->index]->last_used_idx
+ & (dev->virtqueue[state->index]->size - 1);
return 0;
}
--
2.1.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [dpdk-dev] [PATCH 2/3] vhost: Fix RESET_OWNER handling not to close callfd 2015-08-19 9:51 [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message Tetsuya Mukawa @ 2015-08-19 9:51 ` Tetsuya Mukawa 2015-10-08 6:19 ` Yuanhan Liu 2015-08-19 9:51 ` [dpdk-dev] [PATCH 3/3] vhost: Fix RESET_OWNER handling not to free virtqueue Tetsuya Mukawa 2015-09-25 13:12 ` [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message Thomas Monjalon 2 siblings, 1 reply; 6+ messages in thread From: Tetsuya Mukawa @ 2015-08-19 9:51 UTC (permalink / raw) To: dev When RESET_OWNER message is issued, vhost backend shouldn't close 'callfd', because it will be valid while vhost-user connection is established. It should be closed when connection is closed. Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> --- lib/librte_vhost/virtio-net.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c index 144f301..7794e8b 100644 --- a/lib/librte_vhost/virtio-net.c +++ b/lib/librte_vhost/virtio-net.c @@ -189,13 +189,9 @@ cleanup_device(struct virtio_net *dev) free(dev->mem); } - /* Close any event notifiers opened by device. */ - if ((int)dev->virtqueue[VIRTIO_RXQ]->callfd >= 0) - close((int)dev->virtqueue[VIRTIO_RXQ]->callfd); + /* Close kickfd event notifiers opened by device. */ if ((int)dev->virtqueue[VIRTIO_RXQ]->kickfd >= 0) close((int)dev->virtqueue[VIRTIO_RXQ]->kickfd); - if ((int)dev->virtqueue[VIRTIO_TXQ]->callfd >= 0) - close((int)dev->virtqueue[VIRTIO_TXQ]->callfd); if ((int)dev->virtqueue[VIRTIO_TXQ]->kickfd >= 0) close((int)dev->virtqueue[VIRTIO_TXQ]->kickfd); } @@ -206,6 +202,12 @@ cleanup_device(struct virtio_net *dev) static void free_device(struct virtio_net_config_ll *ll_dev) { + /* close callfd when connection is closed */ + if ((int)ll_dev->dev.virtqueue[VIRTIO_RXQ]->callfd >= 0) + close((int)ll_dev->dev.virtqueue[VIRTIO_RXQ]->callfd); + if ((int)ll_dev->dev.virtqueue[VIRTIO_TXQ]->callfd >= 0) + close((int)ll_dev->dev.virtqueue[VIRTIO_TXQ]->callfd); + /* Free any malloc'd memory */ rte_free(ll_dev->dev.virtqueue[VIRTIO_RXQ]); rte_free(ll_dev->dev.virtqueue[VIRTIO_TXQ]); @@ -241,6 +243,21 @@ rm_config_ll_entry(struct virtio_net_config_ll *ll_dev, } } + +/* + * Rset variables in device structure. + */ +static void +reset_device(struct virtio_net *dev) +{ + dev->virtqueue[VIRTIO_RXQ]->kickfd = (eventfd_t)-1; + dev->virtqueue[VIRTIO_TXQ]->kickfd = (eventfd_t)-1; + + /* Backends are set to -1 indicating an inactive device. */ + dev->virtqueue[VIRTIO_RXQ]->backend = VIRTIO_DEV_STOPPED; + dev->virtqueue[VIRTIO_TXQ]->backend = VIRTIO_DEV_STOPPED; +} + /* * Initialise all variables in device structure. */ @@ -261,14 +278,10 @@ init_device(struct virtio_net *dev) memset(dev->virtqueue[VIRTIO_RXQ], 0, sizeof(struct vhost_virtqueue)); memset(dev->virtqueue[VIRTIO_TXQ], 0, sizeof(struct vhost_virtqueue)); - dev->virtqueue[VIRTIO_RXQ]->kickfd = (eventfd_t)-1; dev->virtqueue[VIRTIO_RXQ]->callfd = (eventfd_t)-1; - dev->virtqueue[VIRTIO_TXQ]->kickfd = (eventfd_t)-1; dev->virtqueue[VIRTIO_TXQ]->callfd = (eventfd_t)-1; - /* Backends are set to -1 indicating an inactive device. */ - dev->virtqueue[VIRTIO_RXQ]->backend = VIRTIO_DEV_STOPPED; - dev->virtqueue[VIRTIO_TXQ]->backend = VIRTIO_DEV_STOPPED; + reset_device(dev); } /* @@ -403,7 +416,7 @@ reset_owner(struct vhost_device_ctx ctx) ll_dev = get_config_ll_entry(ctx); cleanup_device(&ll_dev->dev); - init_device(&ll_dev->dev); + reset_device(&ll_dev->dev); return 0; } -- 2.1.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 2/3] vhost: Fix RESET_OWNER handling not to close callfd 2015-08-19 9:51 ` [dpdk-dev] [PATCH 2/3] vhost: Fix RESET_OWNER handling not to close callfd Tetsuya Mukawa @ 2015-10-08 6:19 ` Yuanhan Liu 0 siblings, 0 replies; 6+ messages in thread From: Yuanhan Liu @ 2015-10-08 6:19 UTC (permalink / raw) To: Tetsuya Mukawa; +Cc: dev On Wed, Aug 19, 2015 at 06:51:08PM +0900, Tetsuya Mukawa wrote: > When RESET_OWNER message is issued, vhost backend shouldn't close > 'callfd', because it will be valid while vhost-user connection > is established. It should be closed when connection is closed. Doesn't it mean the end of connection when RESET_OWNER is received? If you check my latest patch set for enabling vhost mq, you will find free_device() will be invoked immediately once RESET_OWNER signal is received. http://dpdk.org/ml/archives/dev/2015-September/023752.html So, I doubt this patch is necessary. BTW, does these 3 patches fix a real issue, or just some potential issue in your mind? I saw that you mentioned your vhost pmd won't work without those 3 patches, but I never saw you mentioned what issue you fixed exactly. And it'd be good if you can post them in commit log. And, if we encounter same issue, will my above patch fix yours? --yliu > > Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> > --- > lib/librte_vhost/virtio-net.c | 35 ++++++++++++++++++++++++----------- > 1 file changed, 24 insertions(+), 11 deletions(-) > > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c > index 144f301..7794e8b 100644 > --- a/lib/librte_vhost/virtio-net.c > +++ b/lib/librte_vhost/virtio-net.c > @@ -189,13 +189,9 @@ cleanup_device(struct virtio_net *dev) > free(dev->mem); > } > > - /* Close any event notifiers opened by device. */ > - if ((int)dev->virtqueue[VIRTIO_RXQ]->callfd >= 0) > - close((int)dev->virtqueue[VIRTIO_RXQ]->callfd); > + /* Close kickfd event notifiers opened by device. */ > if ((int)dev->virtqueue[VIRTIO_RXQ]->kickfd >= 0) > close((int)dev->virtqueue[VIRTIO_RXQ]->kickfd); > - if ((int)dev->virtqueue[VIRTIO_TXQ]->callfd >= 0) > - close((int)dev->virtqueue[VIRTIO_TXQ]->callfd); > if ((int)dev->virtqueue[VIRTIO_TXQ]->kickfd >= 0) > close((int)dev->virtqueue[VIRTIO_TXQ]->kickfd); > } > @@ -206,6 +202,12 @@ cleanup_device(struct virtio_net *dev) > static void > free_device(struct virtio_net_config_ll *ll_dev) > { > + /* close callfd when connection is closed */ > + if ((int)ll_dev->dev.virtqueue[VIRTIO_RXQ]->callfd >= 0) > + close((int)ll_dev->dev.virtqueue[VIRTIO_RXQ]->callfd); > + if ((int)ll_dev->dev.virtqueue[VIRTIO_TXQ]->callfd >= 0) > + close((int)ll_dev->dev.virtqueue[VIRTIO_TXQ]->callfd); > + > /* Free any malloc'd memory */ > rte_free(ll_dev->dev.virtqueue[VIRTIO_RXQ]); > rte_free(ll_dev->dev.virtqueue[VIRTIO_TXQ]); > @@ -241,6 +243,21 @@ rm_config_ll_entry(struct virtio_net_config_ll *ll_dev, > } > } > > + > +/* > + * Rset variables in device structure. > + */ > +static void > +reset_device(struct virtio_net *dev) > +{ > + dev->virtqueue[VIRTIO_RXQ]->kickfd = (eventfd_t)-1; > + dev->virtqueue[VIRTIO_TXQ]->kickfd = (eventfd_t)-1; > + > + /* Backends are set to -1 indicating an inactive device. */ > + dev->virtqueue[VIRTIO_RXQ]->backend = VIRTIO_DEV_STOPPED; > + dev->virtqueue[VIRTIO_TXQ]->backend = VIRTIO_DEV_STOPPED; > +} > + > /* > * Initialise all variables in device structure. > */ > @@ -261,14 +278,10 @@ init_device(struct virtio_net *dev) > memset(dev->virtqueue[VIRTIO_RXQ], 0, sizeof(struct vhost_virtqueue)); > memset(dev->virtqueue[VIRTIO_TXQ], 0, sizeof(struct vhost_virtqueue)); > > - dev->virtqueue[VIRTIO_RXQ]->kickfd = (eventfd_t)-1; > dev->virtqueue[VIRTIO_RXQ]->callfd = (eventfd_t)-1; > - dev->virtqueue[VIRTIO_TXQ]->kickfd = (eventfd_t)-1; > dev->virtqueue[VIRTIO_TXQ]->callfd = (eventfd_t)-1; > > - /* Backends are set to -1 indicating an inactive device. */ > - dev->virtqueue[VIRTIO_RXQ]->backend = VIRTIO_DEV_STOPPED; > - dev->virtqueue[VIRTIO_TXQ]->backend = VIRTIO_DEV_STOPPED; > + reset_device(dev); > } > > /* > @@ -403,7 +416,7 @@ reset_owner(struct vhost_device_ctx ctx) > ll_dev = get_config_ll_entry(ctx); > > cleanup_device(&ll_dev->dev); > - init_device(&ll_dev->dev); > + reset_device(&ll_dev->dev); > > return 0; > } > -- > 2.1.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH 3/3] vhost: Fix RESET_OWNER handling not to free virtqueue 2015-08-19 9:51 [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message Tetsuya Mukawa 2015-08-19 9:51 ` [dpdk-dev] [PATCH 2/3] vhost: Fix RESET_OWNER handling not to close callfd Tetsuya Mukawa @ 2015-08-19 9:51 ` Tetsuya Mukawa 2015-09-25 13:12 ` [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message Thomas Monjalon 2 siblings, 0 replies; 6+ messages in thread From: Tetsuya Mukawa @ 2015-08-19 9:51 UTC (permalink / raw) To: dev When RESET_OWNER message is issued, vhost backend should not clear and free virtqueue memories. This is because vhost backend should handle GET_VRING_BASE message followed by the RESET_OWNER, so still vhost backend need to access virtqueue memories. These memories should be freed when SET_MEM_TABLE message is issued, and vhost library has already had such a functionality. Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> --- lib/librte_vhost/virtio-net.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c index 7794e8b..9475d00 100644 --- a/lib/librte_vhost/virtio-net.c +++ b/lib/librte_vhost/virtio-net.c @@ -182,13 +182,6 @@ add_config_ll_entry(struct virtio_net_config_ll *new_ll_dev) static void cleanup_device(struct virtio_net *dev) { - /* Unmap QEMU memory file if mapped. */ - if (dev->mem) { - munmap((void *)(uintptr_t)dev->mem->mapped_address, - (size_t)dev->mem->mapped_size); - free(dev->mem); - } - /* Close kickfd event notifiers opened by device. */ if ((int)dev->virtqueue[VIRTIO_RXQ]->kickfd >= 0) close((int)dev->virtqueue[VIRTIO_RXQ]->kickfd); -- 2.1.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message 2015-08-19 9:51 [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message Tetsuya Mukawa 2015-08-19 9:51 ` [dpdk-dev] [PATCH 2/3] vhost: Fix RESET_OWNER handling not to close callfd Tetsuya Mukawa 2015-08-19 9:51 ` [dpdk-dev] [PATCH 3/3] vhost: Fix RESET_OWNER handling not to free virtqueue Tetsuya Mukawa @ 2015-09-25 13:12 ` Thomas Monjalon 2015-09-28 6:38 ` Xie, Huawei 2 siblings, 1 reply; 6+ messages in thread From: Thomas Monjalon @ 2015-09-25 13:12 UTC (permalink / raw) To: Huawei Xie, Changchun Ouyang; +Cc: dev Huawei, Changchun, Please could you review this series? Thanks ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message 2015-09-25 13:12 ` [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message Thomas Monjalon @ 2015-09-28 6:38 ` Xie, Huawei 0 siblings, 0 replies; 6+ messages in thread From: Xie, Huawei @ 2015-09-28 6:38 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev On 9/25/2015 9:15 PM, Thomas Monjalon wrote: > Huawei, Changchun, > Please could you review this series? > Thanks > > Sorry, recently quite busy. Would check this and Pavel's patch after Chinese Holiday(Oct.1-Oct.7). Btw, Changchun left Intel and could be no longer reached by this mail address. I think he will show up with another address soon. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-08 6:16 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-08-19 9:51 [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message Tetsuya Mukawa 2015-08-19 9:51 ` [dpdk-dev] [PATCH 2/3] vhost: Fix RESET_OWNER handling not to close callfd Tetsuya Mukawa 2015-10-08 6:19 ` Yuanhan Liu 2015-08-19 9:51 ` [dpdk-dev] [PATCH 3/3] vhost: Fix RESET_OWNER handling not to free virtqueue Tetsuya Mukawa 2015-09-25 13:12 ` [dpdk-dev] [PATCH 1/3] vhost: Fix return value of GET_VRING_BASE message Thomas Monjalon 2015-09-28 6:38 ` Xie, Huawei
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).