diff mbox series

[v4,6/6] virtio-net: add migration support for RSS and hash report

Message ID 20200316100933.11499-7-yuri.benditovich@daynix.com
State New
Headers show
Series reference implementation of RSS and hash report | expand

Commit Message

Yuri Benditovich March 16, 2020, 10:09 a.m. UTC 
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(+)

Comments

Michael S. Tsirkin March 16, 2020, 11:05 p.m. UTC | #1 
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
Yuri Benditovich March 17, 2020, 5:48 a.m. UTC | #2 
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
>
>
Michael S. Tsirkin March 17, 2020, 6:33 a.m. UTC | #3 
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),                                        \
Yuri Benditovich March 17, 2020, 8:09 a.m. UTC | #4 
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 mbox series

Patch

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()
    },
 };