On Wed, Mar 11, 2020 at 10:21 PM Michael S. Tsirkin <
address@hidden> 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 <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.
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.
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
>
> Yes this means we should allocate the indirection arrays on the fly.
> But that's probably a good idea anyway.
>
> > };
> > --
> > 2.17.1
>
>