Re: [PATCH v4 6/6] virtio-net: add migration support for RSS and hash report
On Tue, Mar 17, 2020 at 8:33 AM Michael S. Tsirkin wrote:
> 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
> 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
> > > ---
> > > 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:
>
>
IMO, reuse existing (and widely used) is always better than create a new,
even if it is simple to create a new.
But if you find it mandatory, I'll add a new.
Are there some notes to other patches in the batch?
> -->
>
> vmstate: add VMSTATE_VARRAY_UINT16_ALLOC
>
> Signed-off-by: Michael S. Tsirkin
>
> --
>
> 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),\
>
>
Re: [PATCH v4 6/6] virtio-net: add migration support for RSS and hash report
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 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
> > ---
> > 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
--
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),\
Re: [PATCH v4 6/6] virtio-net: add migration support for RSS and hash report
On Tue, Mar 17, 2020 at 1:05 AM Michael S. Tsirkin 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
> > ---
> > 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.
>
> > +VMSTATE_UINT16(default_queue, VirtioNetRssData),
> > +VMSTATE_UINT8_ARRAY(key, VirtioNetRssData,
> > +VIRTIO_NET_RSS_MAX_KEY_SIZE),
> > +VMSTATE_VARRAY_UINT32_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
>
>
Re: [PATCH v4 6/6] virtio-net: add migration support for RSS and hash report
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
> ---
> 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?
> +VMSTATE_UINT16(default_queue, VirtioNetRssData),
> +VMSTATE_UINT8_ARRAY(key, VirtioNetRssData,
> +VIRTIO_NET_RSS_MAX_KEY_SIZE),
> +VMSTATE_VARRAY_UINT32_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
[PATCH v4 6/6] virtio-net: add migration support for RSS and hash report
Save and restore RSS/hash report configuration.
Signed-off-by: Yuri Benditovich
---
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),
+VMSTATE_UINT16(default_queue, VirtioNetRssData),
+VMSTATE_UINT8_ARRAY(key, VirtioNetRssData,
+VIRTIO_NET_RSS_MAX_KEY_SIZE),
+VMSTATE_VARRAY_UINT32_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
