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

[Michael S. Tsirkin]

On Tue, Mar 17, 2020 at 07:48:55AM +0200, Yuri Benditovich wrote:
> 
> 
> On Tue, Mar 17, 2020 at 1:05 AM Michael S. Tsirkin <address@hidden> wrote:
> 
>     On Mon, Mar 16, 2020 at 12:09:33PM +0200, Yuri Benditovich 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..f343762a0f 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",
>     > +    .fields = (VMStateField[]) {
>     > +        VMSTATE_BOOL(enabled, VirtioNetRssData),
>     > +        VMSTATE_BOOL(redirect, VirtioNetRssData),
>     > +        VMSTATE_BOOL(populate_hash, VirtioNetRssData),
>     > +        VMSTATE_UINT32(hash_types, VirtioNetRssData),
>     > +        VMSTATE_UINT32(indirections_len, VirtioNetRssData),
> 
> 
>     Why is this UINT32? Shouldn't it be UINT16?
> 
> 
> It is UINT32 in the _internal_ structure to use VMSTATE_VARRAY_UINT32_ALLOC.
> Otherwise I need to invent additional macro for the same operation with UINT16
> length.
>  

It's not internal - it's exposed as part of the migration stream format.
Adding VMSTATE_VARRAY_UINT16_ALLOC is as easy as:

-->

vmstate: add VMSTATE_VARRAY_UINT16_ALLOC

Signed-off-by: Michael S. Tsirkin <address@hidden>

--

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 30667631bc..b0b89c6fe5 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -432,6 +432,16 @@ extern const VMStateInfo vmstate_info_qlist;
     .offset     = vmstate_offset_pointer(_state, _field, _type),     \
 }
 
+#define VMSTATE_VARRAY_UINT16_ALLOC(_field, _state, _field_num, _version, 
_info, _type) {\
+    .name       = (stringify(_field)),                               \
+    .version_id = (_version),                                        \
+    .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
+    .info       = &(_info),                                          \
+    .size       = sizeof(_type),                                     \
+    .flags      = VMS_VARRAY_UINT16|VMS_POINTER|VMS_ALLOC,           \
+    .offset     = vmstate_offset_pointer(_state, _field, _type),     \
+}
+
 #define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, 
_info, _type) {\
     .name       = (stringify(_field)),                               \
     .version_id = (_version),                                        \