[PATCH] vhost: Add VIRTIO_NET_F_RSC_EXT to vhost feature bits

Akihiko Odaki posted 1 patch 3 months, 3 weeks ago
hw/net/vhost_net.c | 2 ++
net/vhost-vdpa.c   | 1 +
2 files changed, 3 insertions(+)
[PATCH] vhost: Add VIRTIO_NET_F_RSC_EXT to vhost feature bits
Posted by Akihiko Odaki 3 months, 3 weeks ago
VIRTIO_NET_F_RSC_EXT is implemented in the rx data path, which vhost
implements, so vhost needs to support the feature if it is ever to be
enabled with vhost. The feature must be disabled otherwise.

Fixes: 2974e916df87 ("virtio-net: support RSC v4/v6 tcp traffic for Windows HCK")
Reported-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/net/vhost_net.c | 2 ++
 net/vhost-vdpa.c   | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index a788e6937e03..dedf9ad7c242 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -50,6 +50,7 @@ static const int kernel_feature_bits[] = {
     VIRTIO_F_RING_RESET,
     VIRTIO_F_IN_ORDER,
     VIRTIO_F_NOTIFICATION_DATA,
+    VIRTIO_NET_F_RSC_EXT,
     VIRTIO_NET_F_HASH_REPORT,
     VHOST_INVALID_FEATURE_BIT
 };
@@ -81,6 +82,7 @@ static const int user_feature_bits[] = {
     VIRTIO_F_RING_RESET,
     VIRTIO_F_IN_ORDER,
     VIRTIO_NET_F_RSS,
+    VIRTIO_NET_F_RSC_EXT,
     VIRTIO_NET_F_HASH_REPORT,
     VIRTIO_NET_F_GUEST_USO4,
     VIRTIO_NET_F_GUEST_USO6,
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 03457ead663a..46b02c50be8c 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -88,6 +88,7 @@ const int vdpa_feature_bits[] = {
     VIRTIO_NET_F_MQ,
     VIRTIO_NET_F_MRG_RXBUF,
     VIRTIO_NET_F_MTU,
+    VIRTIO_NET_F_RSC_EXT,
     VIRTIO_NET_F_RSS,
     VIRTIO_NET_F_STATUS,
     VIRTIO_RING_F_EVENT_IDX,

---
base-commit: 31669121a01a14732f57c49400bc239cf9fd505f
change-id: 20240802-rsc-e90fb452bd7f

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>
Re: [PATCH] vhost: Add VIRTIO_NET_F_RSC_EXT to vhost feature bits
Posted by Jason Wang 3 months, 2 weeks ago
On Fri, Aug 2, 2024 at 1:38 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> VIRTIO_NET_F_RSC_EXT is implemented in the rx data path, which vhost
> implements, so vhost needs to support the feature if it is ever to be
> enabled with vhost. The feature must be disabled otherwise.
>
> Fixes: 2974e916df87 ("virtio-net: support RSC v4/v6 tcp traffic for Windows HCK")
> Reported-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks
Re: [PATCH] vhost: Add VIRTIO_NET_F_RSC_EXT to vhost feature bits
Posted by Yuri Benditovich 3 months, 3 weeks ago
At the moment kernel (vhost) and vhost user backends do not support
this feature (and IMO they do not need to)
In order to support it they need to implement a) coalescing of
segmented TCP packets and b) population of respective fields in
host-to-guest packets (number of coalesced segments and counter of
duplicated acks)
Just to remind - this feature is intended to be used without vhost and
without migration

Thanks
Yuri

On Fri, Aug 2, 2024 at 8:38 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> VIRTIO_NET_F_RSC_EXT is implemented in the rx data path, which vhost
> implements, so vhost needs to support the feature if it is ever to be
> enabled with vhost. The feature must be disabled otherwise.
>
> Fixes: 2974e916df87 ("virtio-net: support RSC v4/v6 tcp traffic for Windows HCK")
> Reported-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  hw/net/vhost_net.c | 2 ++
>  net/vhost-vdpa.c   | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index a788e6937e03..dedf9ad7c242 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -50,6 +50,7 @@ static const int kernel_feature_bits[] = {
>      VIRTIO_F_RING_RESET,
>      VIRTIO_F_IN_ORDER,
>      VIRTIO_F_NOTIFICATION_DATA,
> +    VIRTIO_NET_F_RSC_EXT,
>      VIRTIO_NET_F_HASH_REPORT,
>      VHOST_INVALID_FEATURE_BIT
>  };
> @@ -81,6 +82,7 @@ static const int user_feature_bits[] = {
>      VIRTIO_F_RING_RESET,
>      VIRTIO_F_IN_ORDER,
>      VIRTIO_NET_F_RSS,
> +    VIRTIO_NET_F_RSC_EXT,
>      VIRTIO_NET_F_HASH_REPORT,
>      VIRTIO_NET_F_GUEST_USO4,
>      VIRTIO_NET_F_GUEST_USO6,
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 03457ead663a..46b02c50be8c 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -88,6 +88,7 @@ const int vdpa_feature_bits[] = {
>      VIRTIO_NET_F_MQ,
>      VIRTIO_NET_F_MRG_RXBUF,
>      VIRTIO_NET_F_MTU,
> +    VIRTIO_NET_F_RSC_EXT,
>      VIRTIO_NET_F_RSS,
>      VIRTIO_NET_F_STATUS,
>      VIRTIO_RING_F_EVENT_IDX,
>
> ---
> base-commit: 31669121a01a14732f57c49400bc239cf9fd505f
> change-id: 20240802-rsc-e90fb452bd7f
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>
>
Re: [PATCH] vhost: Add VIRTIO_NET_F_RSC_EXT to vhost feature bits
Posted by Michael S. Tsirkin 3 months, 3 weeks ago
On Fri, Aug 02, 2024 at 10:34:37AM +0300, Yuri Benditovich wrote:
> At the moment kernel (vhost) and vhost user backends do not support
> this feature (and IMO they do not need to)
> In order to support it they need to implement a) coalescing of
> segmented TCP packets and b) population of respective fields in
> host-to-guest packets (number of coalesced segments and counter of
> duplicated acks)
> Just to remind - this feature is intended to be used without vhost and
> without migration
> 
> Thanks
> Yuri

it is off ny default so no big deal ...
if it's incompatiblr with vhost/migration I guess it is a good idea
to make sure users do not enable it like this.