[dpdk-dev] [RFC 0/2] vhost: packed ring support completion

[dpdk-dev] [RFC 0/2] vhost: packed ring support completion

[Maxime Coquelin]

Initial series ading packed ring layout support to the
vhost library was missing the save and restore of the
wrap counter on vring base requests used for migration.

First patch saves/restores the wrap counter value into/
from bit 31 of the vring state num field.

Patch 2 advertise support for the packed ring.

Maxime Coquelin (2):
  vhost: add packed ring support to vring base requests
  vhost: advertize packed ring layout support

 lib/librte_vhost/vhost.h      |  3 ++-
 lib/librte_vhost/vhost_user.c | 16 ++++++++++++----
 2 files changed, 14 insertions(+), 5 deletions(-)

-- 
2.17.1

[dpdk-dev] [RFC 1/2] vhost: add packed ring support to vring base requests

[Maxime Coquelin]

For consistency with Vhost kernel backend, save the wrap
counter value into bit 31 of num's vring state field.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index a2d4c9ffc..31cfdf163 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -648,10 +648,15 @@ static int
 vhost_user_set_vring_base(struct virtio_net *dev,
 			  VhostUserMsg *msg)
 {
-	dev->virtqueue[msg->payload.state.index]->last_used_idx  =
-			msg->payload.state.num;
-	dev->virtqueue[msg->payload.state.index]->last_avail_idx =
-			msg->payload.state.num;
+	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
+	if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
+		vq->avail_wrap_counter = msg->payload.state.num >> 31;
+		vq->used_wrap_counter = vq->avail_wrap_counter;
+		msg->payload.state.num &= ~(1 << 31);
+	}
+
+	vq->last_used_idx = msg->payload.state.num;
+	vq->last_avail_idx = msg->payload.state.num;
 
 	return 0;
 }
@@ -1083,6 +1088,9 @@ vhost_user_get_vring_base(struct virtio_net *dev,
 	/* Here we are safe to get the last avail index */
 	msg->payload.state.num = vq->last_avail_idx;
 
+	if (dev->features & (1ULL << VIRTIO_F_RING_PACKED))
+		msg->payload.state.num |= vq->avail_wrap_counter << 31;
+
 	RTE_LOG(INFO, VHOST_CONFIG,
 		"vring base idx:%d file:%d\n", msg->payload.state.index,
 		msg->payload.state.num);
-- 
2.17.1

[dpdk-dev] [RFC 2/2] vhost: advertize packed ring layout support

[Maxime Coquelin]

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 760a09c0d..9b0ebb754 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -275,7 +275,8 @@ struct vring_packed_desc_event {
 				(1ULL << VIRTIO_RING_F_EVENT_IDX) | \
 				(1ULL << VIRTIO_NET_F_MTU)  | \
 				(1ULL << VIRTIO_F_IN_ORDER) | \
-				(1ULL << VIRTIO_F_IOMMU_PLATFORM))
+				(1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
+				(1ULL << VIRTIO_F_RING_PACKED))
 
 
 struct guest_page {
-- 
2.17.1

Re: [dpdk-dev] [RFC 1/2] vhost: add packed ring support to vring base requests

[Jens Freimann]

On Wed, Aug 08, 2018 at 05:23:22PM +0200, Maxime Coquelin wrote:
>For consistency with Vhost kernel backend, save the wrap
>counter value into bit 31 of num's vring state field.
>
>Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>---
> lib/librte_vhost/vhost_user.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
>diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>index a2d4c9ffc..31cfdf163 100644
>--- a/lib/librte_vhost/vhost_user.c
>+++ b/lib/librte_vhost/vhost_user.c
>@@ -648,10 +648,15 @@ static int
> vhost_user_set_vring_base(struct virtio_net *dev,
> 			  VhostUserMsg *msg)
> {
>-	dev->virtqueue[msg->payload.state.index]->last_used_idx  =
>-			msg->payload.state.num;
>-	dev->virtqueue[msg->payload.state.index]->last_avail_idx =
>-			msg->payload.state.num;
>+	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
>+	if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
>+		vq->avail_wrap_counter = msg->payload.state.num >> 31;
>+		vq->used_wrap_counter = vq->avail_wrap_counter;

Maybe I don't understand when this is called, but the wrap counters
could have different values, no?

Apart from that:

Reviewed-by: Jens Freimann <jfreimann@redhat.com> 

regards,
Jens 

Re: [dpdk-dev] [RFC 1/2] vhost: add packed ring support to vring base requests

[Maxime Coquelin]

Hi Jens,

On 08/10/2018 09:09 AM, Jens Freimann wrote:
> On Wed, Aug 08, 2018 at 05:23:22PM +0200, Maxime Coquelin wrote:
>> For consistency with Vhost kernel backend, save the wrap
>> counter value into bit 31 of num's vring state field.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> lib/librte_vhost/vhost_user.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/librte_vhost/vhost_user.c 
>> b/lib/librte_vhost/vhost_user.c
>> index a2d4c9ffc..31cfdf163 100644
>> --- a/lib/librte_vhost/vhost_user.c
>> +++ b/lib/librte_vhost/vhost_user.c
>> @@ -648,10 +648,15 @@ static int
>> vhost_user_set_vring_base(struct virtio_net *dev,
>>               VhostUserMsg *msg)
>> {
>> -    dev->virtqueue[msg->payload.state.index]->last_used_idx  =
>> -            msg->payload.state.num;
>> -    dev->virtqueue[msg->payload.state.index]->last_avail_idx =
>> -            msg->payload.state.num;
>> +    struct vhost_virtqueue *vq = 
>> dev->virtqueue[msg->payload.state.index];
>> +    if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
>> +        vq->avail_wrap_counter = msg->payload.state.num >> 31;
>> +        vq->used_wrap_counter = vq->avail_wrap_counter;
> 
> Maybe I don't understand when this is called, but the wrap counters
> could have different values, no?

No, because we set the used index to the saved avail index value, so
wrap counters must be the same.

> Apart from that:
> 
> Reviewed-by: Jens Freimann <jfreimann@redhat.com>
> regards,
> Jens