Message ID | 20200316100933.11499-7-yuri.benditovich@daynix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | reference implementation of RSS and hash report | expand |
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 <yuri.benditovich@daynix.com> > --- > 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
On Tue, Mar 17, 2020 at 1:05 AM Michael S. Tsirkin <mst@redhat.com> 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 <yuri.benditovich@daynix.com> > > --- > > 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 > >
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 <mst@redhat.com> 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 <yuri.benditovich@daynix.com> > > --- > > 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 <mst@redhat.com> -- 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), \
On Tue, Mar 17, 2020 at 8:33 AM Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> > 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 <yuri.benditovich@daynix.com> > > > --- > > > 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 <mst@redhat.com> > > -- > > 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), \ > >
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() }, };
Save and restore RSS/hash report configuration. Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> --- hw/net/virtio-net.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)