Currently vhost-vsock doesn't have any feature bits, so it
don't support parse mergeable rx buffer feature. And the
feature is support in another series of patches named
"VSOCK: support mergeable rx buffer in vhost-vsock".
So we neet to support parse mergeable feature in vhost-vsock
if above patches are merged.
Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
---
hw/virtio/vhost-vsock.c | 9 +++++++--
include/standard-headers/linux/virtio_vsock.h | 3 +++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index aa5af92..5023c05 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -178,8 +178,13 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
uint64_t requested_features,
Error **errp)
{
- /* No feature bits used yet */
- return requested_features;
+ VHostVSock *vsock = VHOST_VSOCK(vdev);
+ uint64_t features;
+
+ virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_MRG_RXBUF);
+ features = requested_features & vsock->vhost_dev.features;
+
+ return features;
}
static void vhost_vsock_handle_output(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/include/standard-headers/linux/virtio_vsock.h b/include/standard-headers/linux/virtio_vsock.h
index be44321..4c583ec 100644
--- a/include/standard-headers/linux/virtio_vsock.h
+++ b/include/standard-headers/linux/virtio_vsock.h
@@ -38,6 +38,9 @@
#include "standard-headers/linux/virtio_ids.h"
#include "standard-headers/linux/virtio_config.h"
+/* Virtio-vsock feature */
+#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */
+
struct virtio_vsock_config {
uint64_t guest_cid;
} QEMU_PACKED;
--
1.8.3.1
On Wed, Dec 12, 2018 at 05:54:49PM +0800, jiangyiwen wrote: > Currently vhost-vsock doesn't have any feature bits, so it > don't support parse mergeable rx buffer feature. And the > feature is support in another series of patches named > "VSOCK: support mergeable rx buffer in vhost-vsock". > > So we neet to support parse mergeable feature in vhost-vsock > if above patches are merged. > > Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> OK but what does it do? this just defines the feature bit ... Also pls copy virtio-dev whenever you change the host/guest interface. Thanks! > --- > hw/virtio/vhost-vsock.c | 9 +++++++-- > include/standard-headers/linux/virtio_vsock.h | 3 +++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c > index aa5af92..5023c05 100644 > --- a/hw/virtio/vhost-vsock.c > +++ b/hw/virtio/vhost-vsock.c > @@ -178,8 +178,13 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev, > uint64_t requested_features, > Error **errp) > { > - /* No feature bits used yet */ > - return requested_features; > + VHostVSock *vsock = VHOST_VSOCK(vdev); > + uint64_t features; > + > + virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_MRG_RXBUF); > + features = requested_features & vsock->vhost_dev.features; > + > + return features; > } > > static void vhost_vsock_handle_output(VirtIODevice *vdev, VirtQueue *vq) > diff --git a/include/standard-headers/linux/virtio_vsock.h b/include/standard-headers/linux/virtio_vsock.h > index be44321..4c583ec 100644 > --- a/include/standard-headers/linux/virtio_vsock.h > +++ b/include/standard-headers/linux/virtio_vsock.h > @@ -38,6 +38,9 @@ > #include "standard-headers/linux/virtio_ids.h" > #include "standard-headers/linux/virtio_config.h" > > +/* Virtio-vsock feature */ > +#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */ > + > struct virtio_vsock_config { > uint64_t guest_cid; > } QEMU_PACKED; > -- > 1.8.3.1 >
On 2018/12/12 21:19, Michael S. Tsirkin wrote: > On Wed, Dec 12, 2018 at 05:54:49PM +0800, jiangyiwen wrote: >> Currently vhost-vsock doesn't have any feature bits, so it >> don't support parse mergeable rx buffer feature. And the >> feature is support in another series of patches named >> "VSOCK: support mergeable rx buffer in vhost-vsock". >> >> So we neet to support parse mergeable feature in vhost-vsock >> if above patches are merged. >> >> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> > > OK but what does it do? this just defines the feature bit ... > Also pls copy virtio-dev whenever you change the host/guest > interface. > > Thanks! > Hi Michael, In my opinion, for vhost device, device feature bits need to be intersected between vhost and qemu, so I add mergeable rx buffer feature into vdev->host_features, and then intersected with vhost_dev features. Later if someone want to add new feature, it can use virtio_add_feature() in vhost_vsock_get_features(). Thanks, Yiwen. >> --- >> hw/virtio/vhost-vsock.c | 9 +++++++-- >> include/standard-headers/linux/virtio_vsock.h | 3 +++ >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c >> index aa5af92..5023c05 100644 >> --- a/hw/virtio/vhost-vsock.c >> +++ b/hw/virtio/vhost-vsock.c >> @@ -178,8 +178,13 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev, >> uint64_t requested_features, >> Error **errp) >> { >> - /* No feature bits used yet */ >> - return requested_features; >> + VHostVSock *vsock = VHOST_VSOCK(vdev); >> + uint64_t features; >> + >> + virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_MRG_RXBUF); >> + features = requested_features & vsock->vhost_dev.features; >> + >> + return features; >> } >> >> static void vhost_vsock_handle_output(VirtIODevice *vdev, VirtQueue *vq) >> diff --git a/include/standard-headers/linux/virtio_vsock.h b/include/standard-headers/linux/virtio_vsock.h >> index be44321..4c583ec 100644 >> --- a/include/standard-headers/linux/virtio_vsock.h >> +++ b/include/standard-headers/linux/virtio_vsock.h >> @@ -38,6 +38,9 @@ >> #include "standard-headers/linux/virtio_ids.h" >> #include "standard-headers/linux/virtio_config.h" >> >> +/* Virtio-vsock feature */ >> +#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */ >> + >> struct virtio_vsock_config { >> uint64_t guest_cid; >> } QEMU_PACKED; >> -- >> 1.8.3.1 >> > > . >
On Thu, Dec 13, 2018 at 11:27:24AM +0800, jiangyiwen wrote: > On 2018/12/12 21:19, Michael S. Tsirkin wrote: > > On Wed, Dec 12, 2018 at 05:54:49PM +0800, jiangyiwen wrote: > >> Currently vhost-vsock doesn't have any feature bits, so it > >> don't support parse mergeable rx buffer feature. And the > >> feature is support in another series of patches named > >> "VSOCK: support mergeable rx buffer in vhost-vsock". > >> > >> So we neet to support parse mergeable feature in vhost-vsock > >> if above patches are merged. > >> > >> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> > > > > OK but what does it do? this just defines the feature bit ... > > Also pls copy virtio-dev whenever you change the host/guest > > interface. > > > > Thanks! > > > > Hi Michael, > > In my opinion, for vhost device, device feature bits need to be > intersected between vhost and qemu, so I add mergeable rx buffer feature > into vdev->host_features, and then intersected with vhost_dev features. > > Later if someone want to add new feature, it can use virtio_add_feature() > in vhost_vsock_get_features(). > > Thanks, > Yiwen. I understand. Sorry. It seems that your patchset wasn't threaded properly, each patch was by its own. Can you pls take a look at fixing that? > >> --- > >> hw/virtio/vhost-vsock.c | 9 +++++++-- > >> include/standard-headers/linux/virtio_vsock.h | 3 +++ > >> 2 files changed, 10 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c > >> index aa5af92..5023c05 100644 > >> --- a/hw/virtio/vhost-vsock.c > >> +++ b/hw/virtio/vhost-vsock.c > >> @@ -178,8 +178,13 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev, > >> uint64_t requested_features, > >> Error **errp) > >> { > >> - /* No feature bits used yet */ > >> - return requested_features; > >> + VHostVSock *vsock = VHOST_VSOCK(vdev); > >> + uint64_t features; > >> + > >> + virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_MRG_RXBUF); > >> + features = requested_features & vsock->vhost_dev.features; > >> + > >> + return features; > >> } > >> > >> static void vhost_vsock_handle_output(VirtIODevice *vdev, VirtQueue *vq) > >> diff --git a/include/standard-headers/linux/virtio_vsock.h b/include/standard-headers/linux/virtio_vsock.h > >> index be44321..4c583ec 100644 > >> --- a/include/standard-headers/linux/virtio_vsock.h > >> +++ b/include/standard-headers/linux/virtio_vsock.h > >> @@ -38,6 +38,9 @@ > >> #include "standard-headers/linux/virtio_ids.h" > >> #include "standard-headers/linux/virtio_config.h" > >> > >> +/* Virtio-vsock feature */ > >> +#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */ > >> + > >> struct virtio_vsock_config { > >> uint64_t guest_cid; > >> } QEMU_PACKED; > >> -- > >> 1.8.3.1 > >> > > > > . > > >
On 2018/12/13 22:54, Michael S. Tsirkin wrote: > On Thu, Dec 13, 2018 at 11:27:24AM +0800, jiangyiwen wrote: >> On 2018/12/12 21:19, Michael S. Tsirkin wrote: >>> On Wed, Dec 12, 2018 at 05:54:49PM +0800, jiangyiwen wrote: >>>> Currently vhost-vsock doesn't have any feature bits, so it >>>> don't support parse mergeable rx buffer feature. And the >>>> feature is support in another series of patches named >>>> "VSOCK: support mergeable rx buffer in vhost-vsock". >>>> >>>> So we neet to support parse mergeable feature in vhost-vsock >>>> if above patches are merged. >>>> >>>> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> >>> >>> OK but what does it do? this just defines the feature bit ... >>> Also pls copy virtio-dev whenever you change the host/guest >>> interface. >>> >>> Thanks! >>> >> >> Hi Michael, >> >> In my opinion, for vhost device, device feature bits need to be >> intersected between vhost and qemu, so I add mergeable rx buffer feature >> into vdev->host_features, and then intersected with vhost_dev features. >> >> Later if someone want to add new feature, it can use virtio_add_feature() >> in vhost_vsock_get_features(). >> >> Thanks, >> Yiwen. > > I understand. Sorry. It seems that your patchset wasn't threaded > properly, each patch was by its own. Can you pls take a look at fixing > that? > Hi Michael, Thanks your suggestions, I want to how to send patch when patch involving multiple communities, like qemu and linux kernel. In this case, how should we send them? Thanks, Yiwen. >>>> --- >>>> hw/virtio/vhost-vsock.c | 9 +++++++-- >>>> include/standard-headers/linux/virtio_vsock.h | 3 +++ >>>> 2 files changed, 10 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c >>>> index aa5af92..5023c05 100644 >>>> --- a/hw/virtio/vhost-vsock.c >>>> +++ b/hw/virtio/vhost-vsock.c >>>> @@ -178,8 +178,13 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev, >>>> uint64_t requested_features, >>>> Error **errp) >>>> { >>>> - /* No feature bits used yet */ >>>> - return requested_features; >>>> + VHostVSock *vsock = VHOST_VSOCK(vdev); >>>> + uint64_t features; >>>> + >>>> + virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_MRG_RXBUF); >>>> + features = requested_features & vsock->vhost_dev.features; >>>> + >>>> + return features; >>>> } >>>> >>>> static void vhost_vsock_handle_output(VirtIODevice *vdev, VirtQueue *vq) >>>> diff --git a/include/standard-headers/linux/virtio_vsock.h b/include/standard-headers/linux/virtio_vsock.h >>>> index be44321..4c583ec 100644 >>>> --- a/include/standard-headers/linux/virtio_vsock.h >>>> +++ b/include/standard-headers/linux/virtio_vsock.h >>>> @@ -38,6 +38,9 @@ >>>> #include "standard-headers/linux/virtio_ids.h" >>>> #include "standard-headers/linux/virtio_config.h" >>>> >>>> +/* Virtio-vsock feature */ >>>> +#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */ >>>> + >>>> struct virtio_vsock_config { >>>> uint64_t guest_cid; >>>> } QEMU_PACKED; >>>> -- >>>> 1.8.3.1 >>>> >>> >>> . >>> >> > > . >
On Fri, Dec 14, 2018 at 05:43:28PM +0800, jiangyiwen wrote: > On 2018/12/13 22:54, Michael S. Tsirkin wrote: > > On Thu, Dec 13, 2018 at 11:27:24AM +0800, jiangyiwen wrote: > >> On 2018/12/12 21:19, Michael S. Tsirkin wrote: > >>> On Wed, Dec 12, 2018 at 05:54:49PM +0800, jiangyiwen wrote: > >>>> Currently vhost-vsock doesn't have any feature bits, so it > >>>> don't support parse mergeable rx buffer feature. And the > >>>> feature is support in another series of patches named > >>>> "VSOCK: support mergeable rx buffer in vhost-vsock". > >>>> > >>>> So we neet to support parse mergeable feature in vhost-vsock > >>>> if above patches are merged. > >>>> > >>>> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> > >>> > >>> OK but what does it do? this just defines the feature bit ... > >>> Also pls copy virtio-dev whenever you change the host/guest > >>> interface. > >>> > >>> Thanks! > >>> > >> > >> Hi Michael, > >> > >> In my opinion, for vhost device, device feature bits need to be > >> intersected between vhost and qemu, so I add mergeable rx buffer feature > >> into vdev->host_features, and then intersected with vhost_dev features. > >> > >> Later if someone want to add new feature, it can use virtio_add_feature() > >> in vhost_vsock_get_features(). > >> > >> Thanks, > >> Yiwen. > > > > I understand. Sorry. It seems that your patchset wasn't threaded > > properly, each patch was by its own. Can you pls take a look at fixing > > that? > > > > Hi Michael, > > Thanks your suggestions, I want to how to send patch when > patch involving multiple communities, like qemu > and linux kernel. In this case, how should we send them? > > Thanks, > Yiwen. Both use the same threading so just copy both. > >>>> --- > >>>> hw/virtio/vhost-vsock.c | 9 +++++++-- > >>>> include/standard-headers/linux/virtio_vsock.h | 3 +++ > >>>> 2 files changed, 10 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c > >>>> index aa5af92..5023c05 100644 > >>>> --- a/hw/virtio/vhost-vsock.c > >>>> +++ b/hw/virtio/vhost-vsock.c > >>>> @@ -178,8 +178,13 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev, > >>>> uint64_t requested_features, > >>>> Error **errp) > >>>> { > >>>> - /* No feature bits used yet */ > >>>> - return requested_features; > >>>> + VHostVSock *vsock = VHOST_VSOCK(vdev); > >>>> + uint64_t features; > >>>> + > >>>> + virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_MRG_RXBUF); > >>>> + features = requested_features & vsock->vhost_dev.features; > >>>> + > >>>> + return features; > >>>> } > >>>> > >>>> static void vhost_vsock_handle_output(VirtIODevice *vdev, VirtQueue *vq) > >>>> diff --git a/include/standard-headers/linux/virtio_vsock.h b/include/standard-headers/linux/virtio_vsock.h > >>>> index be44321..4c583ec 100644 > >>>> --- a/include/standard-headers/linux/virtio_vsock.h > >>>> +++ b/include/standard-headers/linux/virtio_vsock.h > >>>> @@ -38,6 +38,9 @@ > >>>> #include "standard-headers/linux/virtio_ids.h" > >>>> #include "standard-headers/linux/virtio_config.h" > >>>> > >>>> +/* Virtio-vsock feature */ > >>>> +#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */ > >>>> + > >>>> struct virtio_vsock_config { > >>>> uint64_t guest_cid; > >>>> } QEMU_PACKED; > >>>> -- > >>>> 1.8.3.1 > >>>> > >>> > >>> . > >>> > >> > > > > . > > >
On Thu, Dec 13, 2018 at 11:27:24AM +0800, jiangyiwen wrote: > On 2018/12/12 21:19, Michael S. Tsirkin wrote: > > On Wed, Dec 12, 2018 at 05:54:49PM +0800, jiangyiwen wrote: > >> Currently vhost-vsock doesn't have any feature bits, so it > >> don't support parse mergeable rx buffer feature. And the > >> feature is support in another series of patches named > >> "VSOCK: support mergeable rx buffer in vhost-vsock". > >> > >> So we neet to support parse mergeable feature in vhost-vsock > >> if above patches are merged. > >> > >> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> > > > > OK but what does it do? this just defines the feature bit ... > > Also pls copy virtio-dev whenever you change the host/guest > > interface. > > > > Thanks! > > > > Hi Michael, > > In my opinion, for vhost device, device feature bits need to be > intersected between vhost and qemu, so I add mergeable rx buffer feature > into vdev->host_features, and then intersected with vhost_dev features. > > Later if someone want to add new feature, it can use virtio_add_feature() > in vhost_vsock_get_features(). Hi Yiwen, I think Michael's comment is about documenting the behavior of the new feature bit. Each change to a VIRTIO device host<->guest interface needs to be discussed in the VIRTIO community <virtio-dev@lists.oasis-open.org> and described in a patch to the VIRTIO specification (https://github.com/oasis-tcs/virtio-spec/). Stefan
On 2018/12/13 23:34, Stefan Hajnoczi wrote: > On Thu, Dec 13, 2018 at 11:27:24AM +0800, jiangyiwen wrote: >> On 2018/12/12 21:19, Michael S. Tsirkin wrote: >>> On Wed, Dec 12, 2018 at 05:54:49PM +0800, jiangyiwen wrote: >>>> Currently vhost-vsock doesn't have any feature bits, so it >>>> don't support parse mergeable rx buffer feature. And the >>>> feature is support in another series of patches named >>>> "VSOCK: support mergeable rx buffer in vhost-vsock". >>>> >>>> So we neet to support parse mergeable feature in vhost-vsock >>>> if above patches are merged. >>>> >>>> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> >>> >>> OK but what does it do? this just defines the feature bit ... >>> Also pls copy virtio-dev whenever you change the host/guest >>> interface. >>> >>> Thanks! >>> >> >> Hi Michael, >> >> In my opinion, for vhost device, device feature bits need to be >> intersected between vhost and qemu, so I add mergeable rx buffer feature >> into vdev->host_features, and then intersected with vhost_dev features. >> >> Later if someone want to add new feature, it can use virtio_add_feature() >> in vhost_vsock_get_features(). > > Hi Yiwen, > I think Michael's comment is about documenting the behavior of the new > feature bit. > > Each change to a VIRTIO device host<->guest interface needs to be > discussed in the VIRTIO community <virtio-dev@lists.oasis-open.org> and > described in a patch to the VIRTIO specification > (https://github.com/oasis-tcs/virtio-spec/). > > Stefan > Hi Stefan, Ok, I will fix it in the later version, I have not sent a series of patches before, thank you for your help. Thanks again, Yiwen.
© 2016 - 2024 Red Hat, Inc.