Restore the original code, where VHOST_SET_FEATURES is applied to all vhostfds of the device. Fixes: cc0151b34dee ("net/virtio: add virtio-user features ops") Cc: stable@dpdk.org Cc: Maxime Coquelin <maxime.coquelin@redhat.com> Cc: Chenbo Xia <chenbo.xia@intel.com> Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com> --- drivers/net/virtio/virtio_user/vhost_kernel.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c index ad46f10a9300..d65f89e1fc16 100644 --- a/drivers/net/virtio/virtio_user/vhost_kernel.c +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c @@ -158,6 +158,8 @@ static int vhost_kernel_set_features(struct virtio_user_dev *dev, uint64_t features) { struct vhost_kernel_data *data = dev->backend_data; + uint32_t i; + int ret; /* We don't need memory protection here */ features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM); @@ -166,7 +168,16 @@ vhost_kernel_set_features(struct virtio_user_dev *dev, uint64_t features) features &= ~VHOST_KERNEL_HOST_OFFLOADS_MASK; features &= ~(1ULL << VIRTIO_NET_F_MQ); - return vhost_kernel_ioctl(data->vhostfds[0], VHOST_SET_FEATURES, &features); + for (i = 0; i < dev->max_queue_pairs; ++i) { + if (data->vhostfds[i] < 0) + continue; + + ret = vhost_kernel_ioctl(data->vhostfds[i], VHOST_SET_FEATURES, &features); + if (ret < 0) + return ret; + } + + return 0; } static int -- 2.29.2
On 5/28/21 3:20 PM, Thierry Herbelot wrote:
> Restore the original code, where VHOST_SET_FEATURES is applied to
> all vhostfds of the device.
>
> Fixes: cc0151b34dee ("net/virtio: add virtio-user features ops")
> Cc: stable@dpdk.org
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Cc: Chenbo Xia <chenbo.xia@intel.com>
>
> Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com>
> ---
> drivers/net/virtio/virtio_user/vhost_kernel.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
> index ad46f10a9300..d65f89e1fc16 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
> @@ -158,6 +158,8 @@ static int
> vhost_kernel_set_features(struct virtio_user_dev *dev, uint64_t features)
> {
> struct vhost_kernel_data *data = dev->backend_data;
> + uint32_t i;
> + int ret;
>
> /* We don't need memory protection here */
> features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
> @@ -166,7 +168,16 @@ vhost_kernel_set_features(struct virtio_user_dev *dev, uint64_t features)
> features &= ~VHOST_KERNEL_HOST_OFFLOADS_MASK;
> features &= ~(1ULL << VIRTIO_NET_F_MQ);
>
> - return vhost_kernel_ioctl(data->vhostfds[0], VHOST_SET_FEATURES, &features);
> + for (i = 0; i < dev->max_queue_pairs; ++i) {
> + if (data->vhostfds[i] < 0)
> + continue;
> +
> + ret = vhost_kernel_ioctl(data->vhostfds[i], VHOST_SET_FEATURES, &features);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> }
>
> static int
>
Thanks for fixing it, it should be the last one...
Except GET_FEATURES that was also queried for every queue pair, but I
don't think it makes sense to query it and just drop the value read.
What do you think?
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
Hello Maxime, On 6/1/21 9:51 AM, Maxime Coquelin wrote: > > > On 5/28/21 3:20 PM, Thierry Herbelot wrote: >> Restore the original code, where VHOST_SET_FEATURES is applied to >> all vhostfds of the device. >> >> Fixes: cc0151b34dee ("net/virtio: add virtio-user features ops") >> Cc: stable@dpdk.org >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com> >> Cc: Chenbo Xia <chenbo.xia@intel.com> >> >> Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com> >> --- >> drivers/net/virtio/virtio_user/vhost_kernel.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c >> index ad46f10a9300..d65f89e1fc16 100644 >> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c >> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c >> @@ -158,6 +158,8 @@ static int >> vhost_kernel_set_features(struct virtio_user_dev *dev, uint64_t features) >> { >> struct vhost_kernel_data *data = dev->backend_data; >> + uint32_t i; >> + int ret; >> >> /* We don't need memory protection here */ >> features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM); >> @@ -166,7 +168,16 @@ vhost_kernel_set_features(struct virtio_user_dev *dev, uint64_t features) >> features &= ~VHOST_KERNEL_HOST_OFFLOADS_MASK; >> features &= ~(1ULL << VIRTIO_NET_F_MQ); >> >> - return vhost_kernel_ioctl(data->vhostfds[0], VHOST_SET_FEATURES, &features); >> + for (i = 0; i < dev->max_queue_pairs; ++i) { >> + if (data->vhostfds[i] < 0) >> + continue; >> + >> + ret = vhost_kernel_ioctl(data->vhostfds[i], VHOST_SET_FEATURES, &features); >> + if (ret < 0) >> + return ret; >> + } >> + >> + return 0; >> } >> >> static int >> > > Thanks for fixing it, it should be the last one... > > Except GET_FEATURES that was also queried for every queue pair, but I > don't think it makes sense to query it and just drop the value read. > What do you think? Indeed, GET_FEATURES returns a single value: let's assume the value for the first queue pair is the right one. Thierry > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > Thanks, > Maxime > -- Thierry Herbelot Senior Software Engineer Tel: +33 1 39 30 92 61 http://www.6wind.com/ Follow us: https://www.linkedin.com/company/6wind/ https://twitter.com/6WINDsoftware https://www.youtube.com/user/6windsoftware
On 5/28/21 3:20 PM, Thierry Herbelot wrote:
> Restore the original code, where VHOST_SET_FEATURES is applied to
> all vhostfds of the device.
>
> Fixes: cc0151b34dee ("net/virtio: add virtio-user features ops")
> Cc: stable@dpdk.org
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Cc: Chenbo Xia <chenbo.xia@intel.com>
>
> Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com>
> ---
> drivers/net/virtio/virtio_user/vhost_kernel.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
Applied to dpdk-next-virtio/main.
Thanks,
Maxime