On Wed, May 31, 2023 at 10:23 AM Hawkins Jiawei <yin31...@gmail.com> wrote:
>
> On 2023/5/30 0:19, Eugenio Perez Martin wrote:
> > On Mon, May 29, 2023 at 3:18 PM Hawkins Jiawei <yin31...@gmail.com> wrote:
> >>
> >> This patch introduces vhost_vdpa_net_load_offloads() to
> >> restore offloads state at device's startup.
> >>
> >> Signed-off-by: Hawkins Jiawei <yin31...@gmail.com>
> >> ---
> >> net/vhost-vdpa.c | 26 ++++++++++++++++++++++++++
> >> 1 file changed, 26 insertions(+)
> >>
> >> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> >> index 37cdc84562..682c749b19 100644
> >> --- a/net/vhost-vdpa.c
> >> +++ b/net/vhost-vdpa.c
> >> @@ -680,6 +680,28 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
> >> return *s->status != VIRTIO_NET_OK;
> >> }
> >>
> >> +static int vhost_vdpa_net_load_offloads(VhostVDPAState *s,
> >> + const VirtIONet *n)
> >> +{
> >> + uint64_t features, offloads;
> >> + ssize_t dev_written;
> >> +
> >> + features = n->parent_obj.guest_features;
> >> + if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_GUEST_OFFLOADS))) {
> >> + return 0;
> >> + }
> >> +
> >
> > Maybe we can avoid sending this CVQ command if the guest already uses
> > the default values?
>
> Hi Eugenio,
>
> Thanks for the review. However, I'm curious why we don't need to send
> this CVQ command if the guest is using the default values. Is it because
> the device automatically applies these default offloads, when the
> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature is negotiated and QEMU doesn't
> send the CVQ command?
>
Exactly. You can check that either by the standard text or (sometimes
easier) qemu virtio or virtio-net device code.
The standard text says that:
"Upon feature negotiation corresponding offload gets enabled to
preserve backward compatibility."
And you can check in the qemu code by
hw/net/virtio-net:virtio_net_set_features(vdev, features), this chunk
of code:
n->curr_guest_offloads = virtio_net_guest_offloads_by_features(features);
virtio_net_apply_guest_offloads(n);
Thanks!
> Thanks!
>
>
> >
> > By default all features are enabled if I'm not wrong. I think the best
> > way is to expose virtio_net_supported_guest_offloads or
> > virtio_net_guest_offloads_by_features and then check if
> > n->curr_guest_offloads is the same.
> >
> > We should do the same with vhost_vdpa_net_load_mq, but that is out of
> > the scope of this series.
> >
> > Thanks!
> >
> >> + offloads = cpu_to_le64(n->curr_guest_offloads);
> >> + dev_written = vhost_vdpa_net_load_cmd(s,
> >> VIRTIO_NET_CTRL_GUEST_OFFLOADS,
> >> +
> >> VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET,
> >> + &offloads, sizeof(offloads));
> >> + if (unlikely(dev_written < 0)) {
> >> + return dev_written;
> >> + }
> >> +
> >> + return *s->status != VIRTIO_NET_OK;
> >> +}
> >> +
> >> static int vhost_vdpa_net_load(NetClientState *nc)
> >> {
> >> VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> >> @@ -702,6 +724,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
> >> if (unlikely(r)) {
> >> return r;
> >> }
> >> + r = vhost_vdpa_net_load_offloads(s, n);
> >> + if (unlikely(r)) {
> >> + return r;
> >> + }
> >>
> >> return 0;
> >> }
> >> --
> >> 2.25.1
> >>
> >
>
