diff mbox series

[3/4] virtio-net: added USO support

Message ID 20230719152139.1316570-4-yuri.benditovich@daynix.com
State New
Headers show
Series virtio-net: add USO feature (UDP segmentation offload) | expand

Commit Message

Yuri Benditovich July 19, 2023, 3:21 p.m. UTC 
virtio-net can suggest USO features TX, RX v4 and RX v6,
depending on kernel TUN ability to support them. These
features require explicit enable in command-line.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/net/virtio-net.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Akihiko Odaki July 20, 2023, 12:37 a.m. UTC | #1 
On 2023/07/20 0:21, Yuri Benditovich wrote:
> virtio-net can suggest USO features TX, RX v4 and RX v6,
> depending on kernel TUN ability to support them. These
> features require explicit enable in command-line.

Shouldn't we enable these by default as the other offload features are?

> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>   hw/net/virtio-net.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index d2311e7d6e..e76cad923b 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -796,6 +796,10 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
>           virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
>           virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
>   
> +        virtio_clear_feature(&features, VIRTIO_NET_F_HOST_USO);
> +        virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_USO4);
> +        virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_USO6);
> +
>           virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
>       }
>   
> @@ -864,14 +868,16 @@ static void virtio_net_apply_guest_offloads(VirtIONet *n)
>               !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_USO6)));
>   }
>   
> -static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
> +static uint64_t virtio_net_guest_offloads_by_features(uint64_t features)
>   {
>       static const uint64_t guest_offloads_mask =
>           (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
>           (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
>           (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
>           (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
> -        (1ULL << VIRTIO_NET_F_GUEST_UFO);
> +        (1ULL << VIRTIO_NET_F_GUEST_UFO)  |
> +        (1ULL << VIRTIO_NET_F_GUEST_USO4) |
> +        (1ULL << VIRTIO_NET_F_GUEST_USO6);
>   
>       return guest_offloads_mask & features;
>   }
> @@ -3924,6 +3930,12 @@ static Property virtio_net_properties[] = {
>       DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN),
>       DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
>       DEFINE_PROP_BOOL("failover", VirtIONet, failover, false),
> +    DEFINE_PROP_BIT64("guest_uso4", VirtIONet, host_features,
> +                      VIRTIO_NET_F_GUEST_USO4, false),
> +    DEFINE_PROP_BIT64("guest_uso6", VirtIONet, host_features,
> +                      VIRTIO_NET_F_GUEST_USO6, false),
> +    DEFINE_PROP_BIT64("host_uso", VirtIONet, host_features,
> +                      VIRTIO_NET_F_HOST_USO, false),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>
Yuri Benditovich July 20, 2023, 4:05 p.m. UTC | #2 
On Thu, Jul 20, 2023 at 3:37 AM Akihiko Odaki <akihiko.odaki@daynix.com>
wrote:

> On 2023/07/20 0:21, Yuri Benditovich wrote:
> > virtio-net can suggest USO features TX, RX v4 and RX v6,
> > depending on kernel TUN ability to support them. These
> > features require explicit enable in command-line.
>
> Shouldn't we enable these by default as the other offload features are?
>

My suggestion is to add these features as disabled by default and
reevaluate the
possibility to enable later.
If we enable them by default we'll also need to disable them by default in
previous
generations of machine types.


> >
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >   hw/net/virtio-net.c | 16 ++++++++++++++--
> >   1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index d2311e7d6e..e76cad923b 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -796,6 +796,10 @@ static uint64_t
> virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
> >           virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
> >           virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
> >
> > +        virtio_clear_feature(&features, VIRTIO_NET_F_HOST_USO);
> > +        virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_USO4);
> > +        virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_USO6);
> > +
> >           virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
> >       }
> >
> > @@ -864,14 +868,16 @@ static void
> virtio_net_apply_guest_offloads(VirtIONet *n)
> >               !!(n->curr_guest_offloads & (1ULL <<
> VIRTIO_NET_F_GUEST_USO6)));
> >   }
> >
> > -static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
> > +static uint64_t virtio_net_guest_offloads_by_features(uint64_t features)
> >   {
> >       static const uint64_t guest_offloads_mask =
> >           (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
> >           (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> >           (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
> >           (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
> > -        (1ULL << VIRTIO_NET_F_GUEST_UFO);
> > +        (1ULL << VIRTIO_NET_F_GUEST_UFO)  |
> > +        (1ULL << VIRTIO_NET_F_GUEST_USO4) |
> > +        (1ULL << VIRTIO_NET_F_GUEST_USO6);
> >
> >       return guest_offloads_mask & features;
> >   }
> > @@ -3924,6 +3930,12 @@ static Property virtio_net_properties[] = {
> >       DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed,
> SPEED_UNKNOWN),
> >       DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
> >       DEFINE_PROP_BOOL("failover", VirtIONet, failover, false),
> > +    DEFINE_PROP_BIT64("guest_uso4", VirtIONet, host_features,
> > +                      VIRTIO_NET_F_GUEST_USO4, false),
> > +    DEFINE_PROP_BIT64("guest_uso6", VirtIONet, host_features,
> > +                      VIRTIO_NET_F_GUEST_USO6, false),
> > +    DEFINE_PROP_BIT64("host_uso", VirtIONet, host_features,
> > +                      VIRTIO_NET_F_HOST_USO, false),
> >       DEFINE_PROP_END_OF_LIST(),
> >   };
> >
>
Michael S. Tsirkin July 20, 2023, 5:17 p.m. UTC | #3 
On Thu, Jul 20, 2023 at 07:05:40PM +0300, Yuri Benditovich wrote:
> 
> 
> On Thu, Jul 20, 2023 at 3:37 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
>     On 2023/07/20 0:21, Yuri Benditovich wrote:
>     > virtio-net can suggest USO features TX, RX v4 and RX v6,
>     > depending on kernel TUN ability to support them. These
>     > features require explicit enable in command-line.
> 
>     Shouldn't we enable these by default as the other offload features are?
> 
> 
> My suggestion is to add these features as disabled by default and reevaluate
> the
> possibility to enable later.
> If we enable them by default we'll also need to disable them by default in
> previous
> generations of machine types.

Yea, let's do that, that's how we always did it traditionally.

> 
> 
>     >
>     > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
>     > ---
>     >   hw/net/virtio-net.c | 16 ++++++++++++++--
>     >   1 file changed, 14 insertions(+), 2 deletions(-)
>     >
>     > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>     > index d2311e7d6e..e76cad923b 100644
>     > --- a/hw/net/virtio-net.c
>     > +++ b/hw/net/virtio-net.c
>     > @@ -796,6 +796,10 @@ static uint64_t virtio_net_get_features(VirtIODevice
>     *vdev, uint64_t features,
>     >           virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
>     >           virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
>     >   
>     > +        virtio_clear_feature(&features, VIRTIO_NET_F_HOST_USO);
>     > +        virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_USO4);
>     > +        virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_USO6);
>     > +
>     >           virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
>     >       }
>     >   
>     > @@ -864,14 +868,16 @@ static void virtio_net_apply_guest_offloads
>     (VirtIONet *n)
>     >               !!(n->curr_guest_offloads & (1ULL <<
>     VIRTIO_NET_F_GUEST_USO6)));
>     >   }
>     >   
>     > -static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
>     > +static uint64_t virtio_net_guest_offloads_by_features(uint64_t features)
>     >   {
>     >       static const uint64_t guest_offloads_mask =
>     >           (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
>     >           (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
>     >           (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
>     >           (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
>     > -        (1ULL << VIRTIO_NET_F_GUEST_UFO);
>     > +        (1ULL << VIRTIO_NET_F_GUEST_UFO)  |
>     > +        (1ULL << VIRTIO_NET_F_GUEST_USO4) |
>     > +        (1ULL << VIRTIO_NET_F_GUEST_USO6);
>     >   
>     >       return guest_offloads_mask & features;
>     >   }
>     > @@ -3924,6 +3930,12 @@ static Property virtio_net_properties[] = {
>     >       DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed,
>     SPEED_UNKNOWN),
>     >       DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
>     >       DEFINE_PROP_BOOL("failover", VirtIONet, failover, false),
>     > +    DEFINE_PROP_BIT64("guest_uso4", VirtIONet, host_features,
>     > +                      VIRTIO_NET_F_GUEST_USO4, false),
>     > +    DEFINE_PROP_BIT64("guest_uso6", VirtIONet, host_features,
>     > +                      VIRTIO_NET_F_GUEST_USO6, false),
>     > +    DEFINE_PROP_BIT64("host_uso", VirtIONet, host_features,
>     > +                      VIRTIO_NET_F_HOST_USO, false),
>     >       DEFINE_PROP_END_OF_LIST(),
>     >   };
>     >   
>
diff mbox series

Patch

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d2311e7d6e..e76cad923b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -796,6 +796,10 @@  static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
         virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
         virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
 
+        virtio_clear_feature(&features, VIRTIO_NET_F_HOST_USO);
+        virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_USO4);
+        virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_USO6);
+
         virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
     }
 
@@ -864,14 +868,16 @@  static void virtio_net_apply_guest_offloads(VirtIONet *n)
             !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_USO6)));
 }
 
-static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
+static uint64_t virtio_net_guest_offloads_by_features(uint64_t features)
 {
     static const uint64_t guest_offloads_mask =
         (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
         (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
         (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
         (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
-        (1ULL << VIRTIO_NET_F_GUEST_UFO);
+        (1ULL << VIRTIO_NET_F_GUEST_UFO)  |
+        (1ULL << VIRTIO_NET_F_GUEST_USO4) |
+        (1ULL << VIRTIO_NET_F_GUEST_USO6);
 
     return guest_offloads_mask & features;
 }
@@ -3924,6 +3930,12 @@  static Property virtio_net_properties[] = {
     DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN),
     DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
     DEFINE_PROP_BOOL("failover", VirtIONet, failover, false),
+    DEFINE_PROP_BIT64("guest_uso4", VirtIONet, host_features,
+                      VIRTIO_NET_F_GUEST_USO4, false),
+    DEFINE_PROP_BIT64("guest_uso6", VirtIONet, host_features,
+                      VIRTIO_NET_F_GUEST_USO6, false),
+    DEFINE_PROP_BIT64("host_uso", VirtIONet, host_features,
+                      VIRTIO_NET_F_HOST_USO, false),
     DEFINE_PROP_END_OF_LIST(),
 };