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

[Michael S. Tsirkin]

On Thu, Mar 19, 2020 at 07:19:26PM +0200, Yuri Benditovich wrote:
> 
> 
> On Wed, Mar 18, 2020 at 12:48 PM Dr. David Alan Gilbert <address@hidden>
> wrote:
> 
>     * Yuri Benditovich (address@hidden) wrote:
>     > Save and restore RSS/hash report configuration.
>     >
>     > Signed-off-by: Yuri Benditovich <address@hidden>
>     > ---
>     >  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

works

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

fails gracefully

> 
>     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 / address@hidden / Manchester, UK
> 
>