Re: [PATCH v2 4/6] virtio-net: expose virtio_net_supported_guest_offloads()

[Eugenio Perez Martin]

On Thu, Jun 1, 2023 at 3:59 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> On 2023/6/1 21:43, Eugenio Perez Martin wrote:
> > On Thu, Jun 1, 2023 at 11:05 AM Hawkins Jiawei <yin31149@gmail.com> wrote:
> >>
> >> On Thu, 1 Jun 2023 at 16:48, Hawkins Jiawei <yin31149@gmail.com> wrote:
> >>>
> >>> To support restoring offloads state in vdpa, need to expose
> >>> the function virtio_net_supported_guest_offloads(), then vdpa
> >>> uses this function to get the guest supported offloads.
> >>
> >> Here it should be changed to "then QEMU uses this function
> >> to get the guest supported offloads.". I will correct the commit
> >> message in the v3 patch.
> >>
> >
> > Maybe "to get the device supported offloads allow qemu to know the
> > defaults, so it can skip the control message sending if they match
> > with the driver's configuration"?
> >
> > We can also add "This will be the default at guest's startup, these
> > values can mismatch only at live migration".
>
> Hi Eugenio,
>
> I sent the v3 patch before seeing this email:(.
>

Moving the discussion to v3 then :).

Thanks!

> I refactor the commit message to
> "To support restoring offloads state in vdpa, need to expose
> the function virtio_net_supported_guest_offloads().
> QEMU uses this function to get the guest supported offloads
> and no needs to send the corresponding CVQ command if guest
> enables all supported features." in v3 patch.
>
> Do you think there is still some refactoring needed here?
>
> Thanks!
>
>
> >
> > What do you think?
> >
> > Thanks!
> >
> >> Thanks!
> >>
> >>>
> >>> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
> >>> ---
> >>>   hw/net/virtio-net.c            | 2 +-
> >>>   include/hw/virtio/virtio-net.h | 1 +
> >>>   2 files changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>> index 7b27dad6c4..7e8897a8bc 100644
> >>> --- a/hw/net/virtio-net.c
> >>> +++ b/hw/net/virtio-net.c
> >>> @@ -874,7 +874,7 @@ static uint64_t 
> >>> virtio_net_guest_offloads_by_features(uint32_t features)
> >>>       return guest_offloads_mask & features;
> >>>   }
> >>>
> >>> -static inline uint64_t virtio_net_supported_guest_offloads(const 
> >>> VirtIONet *n)
> >>> +uint64_t virtio_net_supported_guest_offloads(const VirtIONet *n)
> >>>   {
> >>>       VirtIODevice *vdev = VIRTIO_DEVICE(n);
> >>>       return virtio_net_guest_offloads_by_features(vdev->guest_features);
> >>> diff --git a/include/hw/virtio/virtio-net.h 
> >>> b/include/hw/virtio/virtio-net.h
> >>> index ef234ffe7e..5f5dcb4572 100644
> >>> --- a/include/hw/virtio/virtio-net.h
> >>> +++ b/include/hw/virtio/virtio-net.h
> >>> @@ -227,5 +227,6 @@ size_t virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
> >>>                                     unsigned out_num);
> >>>   void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
> >>>                                      const char *type);
> >>> +uint64_t virtio_net_supported_guest_offloads(const VirtIONet *n);
> >>>
> >>>   #endif
> >>> --
> >>> 2.25.1
> >>>
> >>
> >
>