Re: [PATCH v5 7/7] virtio-net: add migration support for RSS and hash report

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Yuri Benditovich (yuri.benditovich@daynix.com) wrote:
> On Wed, Mar 18, 2020 at 12:48 PM Dr. David Alan Gilbert <dgilbert@redhat.com>
> wrote:
> 
> > * Yuri Benditovich (yuri.benditovich@daynix.com) wrote:
> > > Save and restore RSS/hash report configuration.
> > >
> > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > > ---
> > >  hw/net/virtio-net.c | 26 ++++++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > >
> > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > index a0614ad4e6..0b058aae9f 100644
> > > --- a/hw/net/virtio-net.c
> > > +++ b/hw/net/virtio-net.c
> > > @@ -2842,6 +2842,13 @@ static int virtio_net_post_load_device(void
> > *opaque, int version_id)
> > >          }
> > >      }
> > >
> > > +    if (n->rss_data.enabled) {
> > > +        trace_virtio_net_rss_enable(n->rss_data.hash_types,
> > > +                                    n->rss_data.indirections_len,
> > > +                                    sizeof(n->rss_data.key));
> > > +    } else {
> > > +        trace_virtio_net_rss_disable();
> > > +    }
> > >      return 0;
> > >  }
> > >
> > > @@ -3019,6 +3026,24 @@ static const VMStateDescription
> > vmstate_virtio_net_has_vnet = {
> > >      },
> > >  };
> > >
> > > +static const VMStateDescription vmstate_rss = {
> > > +    .name      = "vmstate_rss",
> >
> > You need to do something to avoid breaking migration compatibility
> > from/to old QEMU's and from/to QEMU's on hosts without the new virtio
> > features.
> > Probably adding a .needed =   here pointing to a function that
> > checks 'enabled' might do it.
> >
> > Does VMSTATE_STRUCT_TEST(..,..,checker_procedure,...) result the same
> thing?
> 
> Another question about migration support:
> What is expected/required behavior?
> Possible cases:
> old qemu -> new qemu
> new qemu (new feature off) -> old qemu

Just works.

Also be careful about the definition of 'new feature off'; normally we
tie these things to machine types, so that with the old machine type the
guest doesn't even see the feature; it can't turn it on.

> new qemu (new feature on) -> old qemu

Fails; hopefully nicely.

Dave

> 
> 
> > Dave
> >
> >
> > > +    .fields = (VMStateField[]) {
> > > +        VMSTATE_BOOL(enabled, VirtioNetRssData),
> > > +        VMSTATE_BOOL(redirect, VirtioNetRssData),
> > > +        VMSTATE_BOOL(populate_hash, VirtioNetRssData),
> > > +        VMSTATE_UINT32(hash_types, VirtioNetRssData),
> > > +        VMSTATE_UINT16(indirections_len, VirtioNetRssData),
> > > +        VMSTATE_UINT16(default_queue, VirtioNetRssData),
> > > +        VMSTATE_UINT8_ARRAY(key, VirtioNetRssData,
> > > +                            VIRTIO_NET_RSS_MAX_KEY_SIZE),
> > > +        VMSTATE_VARRAY_UINT16_ALLOC(indirections_table,
> > VirtioNetRssData,
> > > +                                    indirections_len, 0,
> > > +                                    vmstate_info_uint16, uint16_t),
> > > +        VMSTATE_END_OF_LIST()
> > > +    },
> > > +};
> > > +
> > >  static const VMStateDescription vmstate_virtio_net_device = {
> > >      .name = "virtio-net-device",
> > >      .version_id = VIRTIO_NET_VM_VERSION,
> > > @@ -3067,6 +3092,7 @@ static const VMStateDescription
> > vmstate_virtio_net_device = {
> > >                           vmstate_virtio_net_tx_waiting),
> > >          VMSTATE_UINT64_TEST(curr_guest_offloads, VirtIONet,
> > >                              has_ctrl_guest_offloads),
> > > +        VMSTATE_STRUCT(rss_data, VirtIONet, 1, vmstate_rss,
> > VirtioNetRssData),
> > >          VMSTATE_END_OF_LIST()
> > >     },
> > >  };
> > > --
> > > 2.17.1
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
> >
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK