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

["Michael S. Tsirkin" <mst@redhat.com>]

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(),
>     >   };
>     >   
>