| Message ID | 20200318091525.27044-7-yuri.benditovich@daynix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | reference implementation of RSS and hash report | expand |
On Wed, Mar 18, 2020 at 11:15:24AM +0200, Yuri Benditovich wrote: > Similar to VMSTATE_VARRAY_UINT32_ALLOC, but the size is > 16-bit field. > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> Hmm this is exactly my patch isn't it? If yes pls fix up attribution (if this is not reposted, then when applying): From: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > include/migration/vmstate.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 30667631bc..baaefb6b9b 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), \ > -- > 2.17.1
* Yuri Benditovich (yuri.benditovich@daynix.com) wrote: > Similar to VMSTATE_VARRAY_UINT32_ALLOC, but the size is > 16-bit field. > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > --- > include/migration/vmstate.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 30667631bc..baaefb6b9b 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), \ > +} > + Once you agree attribution with mst, Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > #define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\ > .name = (stringify(_field)), \ > .version_id = (_version), \ > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Yuri Benditovich <yuri.benditovich@daynix.com> wrote: > Similar to VMSTATE_VARRAY_UINT32_ALLOC, but the size is > 16-bit field. > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Same caveat about attribution to MST. Once told tha, I don't understand why you are using a unit16_t. You define indirections_len as: + uint16_t indirections_len; But its maximum value right now is: +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128 So, are we planning to increase that value in the next future, or we just want to give enough space? Later, Juan.
On Wed, Mar 18, 2020 at 02:02:37PM +0100, Juan Quintela wrote: > Yuri Benditovich <yuri.benditovich@daynix.com> wrote: > > Similar to VMSTATE_VARRAY_UINT32_ALLOC, but the size is > > 16-bit field. > > > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > > Reviewed-by: Juan Quintela <quintela@redhat.com> > > Same caveat about attribution to MST. > > Once told tha, I don't understand why you are using a unit16_t. > You define indirections_len as: > > + uint16_t indirections_len; > > But its maximum value right now is: > > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128 > > So, are we planning to increase that value in the next future, or we > just want to give enough space? > > Later, Juan. The max size according to spec is u16. Using that limits the size and makes it forward compatible.
On Wed, Mar 18, 2020 at 11:42 AM Michael S. Tsirkin <mst@redhat.com> wrote: > On Wed, Mar 18, 2020 at 11:15:24AM +0200, Yuri Benditovich wrote: > > Similar to VMSTATE_VARRAY_UINT32_ALLOC, but the size is > > 16-bit field. > > > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > > Hmm this is exactly my patch isn't it? If yes pls fix up attribution > (if this is not reposted, then when applying): > Of course, it is similar to the one you wrote inline. Unlike one you wrote inline this patch does not fail on checkpatch. But the idea is the same, hard to invent something. Please just let me know what exactly should I do: resubmit or not and whether it is possible to fix it without resubmission. > > From: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > --- > > include/migration/vmstate.h | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 30667631bc..baaefb6b9b 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), \ > > -- > > 2.17.1 > >
On Thu, Mar 19, 2020 at 07:12:20PM +0200, Yuri Benditovich wrote: > > > On Wed, Mar 18, 2020 at 11:42 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Wed, Mar 18, 2020 at 11:15:24AM +0200, Yuri Benditovich wrote: > > Similar to VMSTATE_VARRAY_UINT32_ALLOC, but the size is > > 16-bit field. > > > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > > Hmm this is exactly my patch isn't it? If yes pls fix up attribution > (if this is not reposted, then when applying): > > > Of course, it is similar to the one you wrote inline. > Unlike one you wrote inline this patch does not fail on checkpatch. If you feel you modified it significantly enough, you can write "based on a patch by mst". > But the idea is the same, hard to invent something. > Please just let me know what exactly should I do: resubmit or not and whether > it is possible to fix it without resubmission. > > > > From: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > --- > > include/migration/vmstate.h | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 30667631bc..baaefb6b9b 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), \ > > -- > > 2.17.1 > >
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 30667631bc..baaefb6b9b 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), \
Similar to VMSTATE_VARRAY_UINT32_ALLOC, but the size is 16-bit field. Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> --- include/migration/vmstate.h | 10 ++++++++++ 1 file changed, 10 insertions(+)