Re: [PATCH v3 5/6] virtio-net: add migration support for RSS and hast report

[Michael S. Tsirkin]

On Wed, Mar 11, 2020 at 04:00:44PM +0200, Yuri Benditovich wrote:
> 
> 
> On Wed, Mar 11, 2020 at 3:48 PM Michael S. Tsirkin <address@hidden> wrote:
> 
>     On Wed, Mar 11, 2020 at 02:35:17PM +0200, Yuri Benditovich wrote:
>     > Save and restore RSS/hash report configuration.
>     >
>     > Signed-off-by: Yuri Benditovich <address@hidden>
>     > ---
>     >  hw/net/virtio-net.c | 9 +++++++++
>     >  1 file changed, 9 insertions(+)
>     >
>     > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>     > index 7b6a929e8c..c8d97d45cd 100644
>     > --- a/hw/net/virtio-net.c
>     > +++ b/hw/net/virtio-net.c
>     > @@ -2869,6 +2869,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;
>     >  }
>     > 
>     > @@ -3094,6 +3101,8 @@ static const VMStateDescription
>     vmstate_virtio_net_device = {
>     >                           vmstate_virtio_net_tx_waiting),
>     >          VMSTATE_UINT64_TEST(curr_guest_offloads, VirtIONet,
>     >                              has_ctrl_guest_offloads),
>     > +        VMSTATE_UINT8_ARRAY(rss_data_migration, VirtIONet,
>     > +                            sizeof(VirtioNetRssData)),
>     >          VMSTATE_END_OF_LIST()
>     >     },
> 
> 
>     I think we should migrate the length too. Avoid arbitrary limits.
> 
> 
> The length of what?

Of the tables.
> The structure is fixed-length and the intention is just to
> keep/restore it.
> The length of indirection table and the table itself are part of the 
> structure.


And that's a problem, because
1. we are wasting memory for a rarely used feature
2. if we want to make the table bigger, we'll need to break
   migration compatibility

Just allocate these dynamically as needed, and migrate length.


> 
>     Yes this means we should allocate the indirection arrays on the fly.
>     But that's probably a good idea anyway.
> 
>     >  };
>     > --
>     > 2.17.1
> 
>