On Thu, Mar 12, 2020 at 09:37:06AM +0200, Yuri Benditovich wrote:
>
>
> On Wed, Mar 11, 2020 at 10:21 PM Michael S. Tsirkin <m...@redhat.com> wrote:
>
> 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 <m...@redhat.com>
> 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 <yuri.benditov...@daynix.com>
> > > ---
> > > 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.
>
>
> Unfortunately, this does not make things much better.
> The maximum table size is 128, i.e we have persistent allocation of 256 bytes.
> 1. Addition of the code to make the allocation dynamic and migrate it will eat
> most of this.
But that's shared between all VMs. Table is per VM.
> 2. If we decide to change the maximum size if future, we anyway create
> incompatibility. The driver asks what is maximum indirection table size at the
> initialization time and the OS provides a table according to this. If we
> migrate between two different implementations we find ourselves with queue
> mask
> that is not compatible with maximum size. I'd rather add the comment "do not
> change these numbers".
> 3. Size of key for Toeplitz is always 40
>
> Please confirm you want to make it dynamic anyway
>
Let's just make the code future-proof for when we want to change it.
>
>
>
>
>
> >
> > Yes this means we should allocate the indirection arrays on the fly.
> > But that's probably a good idea anyway.
> >
> > > };
> > > --
> > > 2.17.1
> >
> >
>
>
