* [PATCH] virtio-net: prevent offloads reset on migration @ 2019-10-01 12:18 Mikhail Sennikovsky 2019-10-01 12:18 ` Mikhail Sennikovsky 0 siblings, 1 reply; 7+ messages in thread From: Mikhail Sennikovsky @ 2019-10-01 12:18 UTC (permalink / raw) To: qemu-devel; +Cc: Mikhail Sennikovsky Hi all, I've recently faced an issue that the offloads settings of my Windows VM's tap devices reported with "ethtool -k" get changed after the VM migration. After a quick look it appears that the offloads always get resent to the complete set of features the guest has reported. Here I'm sending a small patch which fixes this with all the detail in the commit description. Perhaps a cleaner approach would've been to actually restore the curr_guest_offloads after the virtio guest device features. This approach is much less invasive though, and more intended to illustrate the problem and potentially initiate a further discussion on the best way of fixing it. Thanks & Regards, Mikhail Mikhail Sennikovsky (1): virtio-net: prevent offloads reset on migration hw/display/virtio-gpu-base.c | 3 ++- hw/net/virtio-net.c | 5 +++-- hw/virtio/virtio.c | 10 +++++----- include/hw/virtio/virtio.h | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] virtio-net: prevent offloads reset on migration 2019-10-01 12:18 [PATCH] virtio-net: prevent offloads reset on migration Mikhail Sennikovsky @ 2019-10-01 12:18 ` Mikhail Sennikovsky 2019-10-01 16:14 ` no-reply 2019-10-02 9:55 ` Dr. David Alan Gilbert 0 siblings, 2 replies; 7+ messages in thread From: Mikhail Sennikovsky @ 2019-10-01 12:18 UTC (permalink / raw) To: qemu-devel; +Cc: Mikhail Sennikovsky Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET command are not preserved on VM migration. Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES) get enabled. What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads are getting set correctly: #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474 #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 #2 virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334 #3 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:168 #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197 #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 However later on the features are getting restored, and offloads get reset to everything supported by features: #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474 #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 #2 virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773 #3 virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052 #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220 #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 This patch fixes this by adding an extra argument to the set_features callback, specifying whether the offloads are to be reset, and setting it to false for the migration case. Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com> --- hw/display/virtio-gpu-base.c | 3 ++- hw/net/virtio-net.c | 5 +++-- hw/virtio/virtio.c | 10 +++++----- include/hw/virtio/virtio.h | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c index 55e0799..04d8a23 100644 --- a/hw/display/virtio-gpu-base.c +++ b/hw/display/virtio-gpu-base.c @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, } static void -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features) +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features, + bool reset_offloads) { static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL); VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev); diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index b9e1cd7..5d108e5 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) return virtio_net_guest_offloads_by_features(vdev->guest_features); } -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features, + bool reset_offloads) { VirtIONet *n = VIRTIO_NET(vdev); int i; @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) && virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6); - if (n->has_vnet_hdr) { + if (reset_offloads && n->has_vnet_hdr) { n->curr_guest_offloads = virtio_net_guest_offloads_by_features(features); virtio_net_apply_guest_offloads(n); diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index a94ea18..b89f7b0 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2042,14 +2042,14 @@ const VMStateInfo virtio_vmstate_info = { .put = virtio_device_put, }; -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val) +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads) { VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); bool bad = (val & ~(vdev->host_features)) != 0; val &= vdev->host_features; if (k->set_features) { - k->set_features(vdev, val); + k->set_features(vdev, val, reset_offloads); } vdev->guest_features = val; return bad ? -1 : 0; @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { return -EINVAL; } - ret = virtio_set_features_nocheck(vdev, val); + ret = virtio_set_features_nocheck(vdev, val, true); if (!ret) { if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */ @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) * host_features. */ uint64_t features64 = vdev->guest_features; - if (virtio_set_features_nocheck(vdev, features64) < 0) { + if (virtio_set_features_nocheck(vdev, features64, false) < 0) { error_report("Features 0x%" PRIx64 " unsupported. " "Allowed features: 0x%" PRIx64, features64, vdev->host_features); return -1; } } else { - if (virtio_set_features_nocheck(vdev, features) < 0) { + if (virtio_set_features_nocheck(vdev, features, false) < 0) { error_report("Features 0x%x unsupported. " "Allowed features: 0x%" PRIx64, features, vdev->host_features); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index b189788..fd8cac5 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass { uint64_t requested_features, Error **errp); uint64_t (*bad_features)(VirtIODevice *vdev); - void (*set_features)(VirtIODevice *vdev, uint64_t val); + void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads); int (*validate_features)(VirtIODevice *vdev); void (*get_config)(VirtIODevice *vdev, uint8_t *config); void (*set_config)(VirtIODevice *vdev, const uint8_t *config); -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] virtio-net: prevent offloads reset on migration 2019-10-01 12:18 ` Mikhail Sennikovsky @ 2019-10-01 16:14 ` no-reply 2019-10-02 9:55 ` Dr. David Alan Gilbert 1 sibling, 0 replies; 7+ messages in thread From: no-reply @ 2019-10-01 16:14 UTC (permalink / raw) To: mikhail.sennikovskii; +Cc: mikhail.sennikovskii, qemu-devel Patchew URL: https://patchew.org/QEMU/1569932308-30478-2-git-send-email-mikhail.sennikovskii@cloud.ionos.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 1569932308-30478-2-git-send-email-mikhail.sennikovskii@cloud.ionos.com Subject: [PATCH] virtio-net: prevent offloads reset on migration === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 9d2774e virtio-net: prevent offloads reset on migration === OUTPUT BEGIN === ERROR: line over 90 characters #97: FILE: hw/virtio/virtio.c:2049: +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads) total: 1 errors, 0 warnings, 74 lines checked Commit 9d2774ea7eb1 (virtio-net: prevent offloads reset on migration) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/1569932308-30478-2-git-send-email-mikhail.sennikovskii@cloud.ionos.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtio-net: prevent offloads reset on migration 2019-10-01 12:18 ` Mikhail Sennikovsky 2019-10-01 16:14 ` no-reply @ 2019-10-02 9:55 ` Dr. David Alan Gilbert 2019-10-07 10:31 ` Mikhail Sennikovsky 2019-10-10 5:33 ` Jason Wang 1 sibling, 2 replies; 7+ messages in thread From: Dr. David Alan Gilbert @ 2019-10-02 9:55 UTC (permalink / raw) To: Mikhail Sennikovsky, stefanha, jasowang, mst; +Cc: qemu-devel Copying in Stefan, Jason and Michael who know the virtio details Dave * Mikhail Sennikovsky (mikhail.sennikovskii@cloud.ionos.com) wrote: > Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET > command are not preserved on VM migration. > Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES) > get enabled. > What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads > are getting set correctly: > > #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474 > #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 > #2 virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334 > #3 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11) > at migration/vmstate.c:168 > #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197 > #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 > #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 > #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 > #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 > #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 > #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 > #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 > > However later on the features are getting restored, and offloads get reset to > everything supported by features: > > #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474 > #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 > #2 virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773 > #3 virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052 > #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220 > #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 > #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 > #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 > #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 > #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 > #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 > #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 > > This patch fixes this by adding an extra argument to the set_features callback, > specifying whether the offloads are to be reset, and setting it to false > for the migration case. > > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com> > --- > hw/display/virtio-gpu-base.c | 3 ++- > hw/net/virtio-net.c | 5 +++-- > hw/virtio/virtio.c | 10 +++++----- > include/hw/virtio/virtio.h | 2 +- > 4 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c > index 55e0799..04d8a23 100644 > --- a/hw/display/virtio-gpu-base.c > +++ b/hw/display/virtio-gpu-base.c > @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, > } > > static void > -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features) > +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features, > + bool reset_offloads) > { > static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL); > VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev); > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index b9e1cd7..5d108e5 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) > return virtio_net_guest_offloads_by_features(vdev->guest_features); > } > > -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features, > + bool reset_offloads) > { > VirtIONet *n = VIRTIO_NET(vdev); > int i; > @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) && > virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6); > > - if (n->has_vnet_hdr) { > + if (reset_offloads && n->has_vnet_hdr) { > n->curr_guest_offloads = > virtio_net_guest_offloads_by_features(features); > virtio_net_apply_guest_offloads(n); > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index a94ea18..b89f7b0 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -2042,14 +2042,14 @@ const VMStateInfo virtio_vmstate_info = { > .put = virtio_device_put, > }; > > -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val) > +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads) > { > VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); > bool bad = (val & ~(vdev->host_features)) != 0; > > val &= vdev->host_features; > if (k->set_features) { > - k->set_features(vdev, val); > + k->set_features(vdev, val, reset_offloads); > } > vdev->guest_features = val; > return bad ? -1 : 0; > @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) > if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { > return -EINVAL; > } > - ret = virtio_set_features_nocheck(vdev, val); > + ret = virtio_set_features_nocheck(vdev, val, true); > if (!ret) { > if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { > /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */ > @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) > * host_features. > */ > uint64_t features64 = vdev->guest_features; > - if (virtio_set_features_nocheck(vdev, features64) < 0) { > + if (virtio_set_features_nocheck(vdev, features64, false) < 0) { > error_report("Features 0x%" PRIx64 " unsupported. " > "Allowed features: 0x%" PRIx64, > features64, vdev->host_features); > return -1; > } > } else { > - if (virtio_set_features_nocheck(vdev, features) < 0) { > + if (virtio_set_features_nocheck(vdev, features, false) < 0) { > error_report("Features 0x%x unsupported. " > "Allowed features: 0x%" PRIx64, > features, vdev->host_features); > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index b189788..fd8cac5 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass { > uint64_t requested_features, > Error **errp); > uint64_t (*bad_features)(VirtIODevice *vdev); > - void (*set_features)(VirtIODevice *vdev, uint64_t val); > + void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads); > int (*validate_features)(VirtIODevice *vdev); > void (*get_config)(VirtIODevice *vdev, uint8_t *config); > void (*set_config)(VirtIODevice *vdev, const uint8_t *config); > -- > 2.7.4 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtio-net: prevent offloads reset on migration 2019-10-02 9:55 ` Dr. David Alan Gilbert @ 2019-10-07 10:31 ` Mikhail Sennikovsky 2019-10-10 5:33 ` Jason Wang 1 sibling, 0 replies; 7+ messages in thread From: Mikhail Sennikovsky @ 2019-10-07 10:31 UTC (permalink / raw) To: Dr. David Alan Gilbert; +Cc: jasowang, qemu-devel, stefanha, mst Guys, any update on this? Note that I have also sent a second version of the patch here, which has the coding style issue fixed. Thanks, Mikhail Am Mi., 2. Okt. 2019 um 11:55 Uhr schrieb Dr. David Alan Gilbert <dgilbert@redhat.com>: > > Copying in Stefan, Jason and Michael who know the virtio details > > Dave > > * Mikhail Sennikovsky (mikhail.sennikovskii@cloud.ionos.com) wrote: > > Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET > > command are not preserved on VM migration. > > Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES) > > get enabled. > > What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads > > are getting set correctly: > > > > #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474 > > #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 > > #2 virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334 > > #3 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11) > > at migration/vmstate.c:168 > > #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197 > > #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 > > #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 > > #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 > > #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 > > #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 > > #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 > > #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 > > > > However later on the features are getting restored, and offloads get reset to > > everything supported by features: > > > > #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474 > > #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 > > #2 virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773 > > #3 virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052 > > #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220 > > #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 > > #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 > > #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 > > #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 > > #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 > > #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 > > #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 > > > > This patch fixes this by adding an extra argument to the set_features callback, > > specifying whether the offloads are to be reset, and setting it to false > > for the migration case. > > > > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com> > > --- > > hw/display/virtio-gpu-base.c | 3 ++- > > hw/net/virtio-net.c | 5 +++-- > > hw/virtio/virtio.c | 10 +++++----- > > include/hw/virtio/virtio.h | 2 +- > > 4 files changed, 11 insertions(+), 9 deletions(-) > > > > diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c > > index 55e0799..04d8a23 100644 > > --- a/hw/display/virtio-gpu-base.c > > +++ b/hw/display/virtio-gpu-base.c > > @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, > > } > > > > static void > > -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features) > > +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features, > > + bool reset_offloads) > > { > > static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL); > > VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev); > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > index b9e1cd7..5d108e5 100644 > > --- a/hw/net/virtio-net.c > > +++ b/hw/net/virtio-net.c > > @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) > > return virtio_net_guest_offloads_by_features(vdev->guest_features); > > } > > > > -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > > +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features, > > + bool reset_offloads) > > { > > VirtIONet *n = VIRTIO_NET(vdev); > > int i; > > @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > > n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) && > > virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6); > > > > - if (n->has_vnet_hdr) { > > + if (reset_offloads && n->has_vnet_hdr) { > > n->curr_guest_offloads = > > virtio_net_guest_offloads_by_features(features); > > virtio_net_apply_guest_offloads(n); > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > index a94ea18..b89f7b0 100644 > > --- a/hw/virtio/virtio.c > > +++ b/hw/virtio/virtio.c > > @@ -2042,14 +2042,14 @@ const VMStateInfo virtio_vmstate_info = { > > .put = virtio_device_put, > > }; > > > > -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val) > > +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads) > > { > > VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); > > bool bad = (val & ~(vdev->host_features)) != 0; > > > > val &= vdev->host_features; > > if (k->set_features) { > > - k->set_features(vdev, val); > > + k->set_features(vdev, val, reset_offloads); > > } > > vdev->guest_features = val; > > return bad ? -1 : 0; > > @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) > > if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { > > return -EINVAL; > > } > > - ret = virtio_set_features_nocheck(vdev, val); > > + ret = virtio_set_features_nocheck(vdev, val, true); > > if (!ret) { > > if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { > > /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */ > > @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) > > * host_features. > > */ > > uint64_t features64 = vdev->guest_features; > > - if (virtio_set_features_nocheck(vdev, features64) < 0) { > > + if (virtio_set_features_nocheck(vdev, features64, false) < 0) { > > error_report("Features 0x%" PRIx64 " unsupported. " > > "Allowed features: 0x%" PRIx64, > > features64, vdev->host_features); > > return -1; > > } > > } else { > > - if (virtio_set_features_nocheck(vdev, features) < 0) { > > + if (virtio_set_features_nocheck(vdev, features, false) < 0) { > > error_report("Features 0x%x unsupported. " > > "Allowed features: 0x%" PRIx64, > > features, vdev->host_features); > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > > index b189788..fd8cac5 100644 > > --- a/include/hw/virtio/virtio.h > > +++ b/include/hw/virtio/virtio.h > > @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass { > > uint64_t requested_features, > > Error **errp); > > uint64_t (*bad_features)(VirtIODevice *vdev); > > - void (*set_features)(VirtIODevice *vdev, uint64_t val); > > + void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads); > > int (*validate_features)(VirtIODevice *vdev); > > void (*get_config)(VirtIODevice *vdev, uint8_t *config); > > void (*set_config)(VirtIODevice *vdev, const uint8_t *config); > > -- > > 2.7.4 > > > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtio-net: prevent offloads reset on migration 2019-10-02 9:55 ` Dr. David Alan Gilbert 2019-10-07 10:31 ` Mikhail Sennikovsky @ 2019-10-10 5:33 ` Jason Wang 2019-10-10 12:15 ` Mikhail Sennikovsky 1 sibling, 1 reply; 7+ messages in thread From: Jason Wang @ 2019-10-10 5:33 UTC (permalink / raw) To: Dr. David Alan Gilbert, Mikhail Sennikovsky, stefanha, mst; +Cc: qemu-devel On 2019/10/2 下午5:55, Dr. David Alan Gilbert wrote: > Copying in Stefan, Jason and Michael who know the virtio details > > Dave > > * Mikhail Sennikovsky (mikhail.sennikovskii@cloud.ionos.com) wrote: >> Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET >> command are not preserved on VM migration. >> Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES) >> get enabled. >> What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads >> are getting set correctly: >> >> #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474 >> #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 >> #2 virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334 >> #3 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11) >> at migration/vmstate.c:168 >> #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197 >> #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 >> #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 >> #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 >> #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 >> #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 >> #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 >> #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 >> >> However later on the features are getting restored, and offloads get reset to >> everything supported by features: >> >> #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474 >> #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 >> #2 virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773 >> #3 virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052 >> #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220 >> #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 >> #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 >> #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 >> #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 >> #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 >> #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 >> #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 >> >> This patch fixes this by adding an extra argument to the set_features callback, >> specifying whether the offloads are to be reset, and setting it to false >> for the migration case. >> >> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com> >> --- >> hw/display/virtio-gpu-base.c | 3 ++- >> hw/net/virtio-net.c | 5 +++-- >> hw/virtio/virtio.c | 10 +++++----- >> include/hw/virtio/virtio.h | 2 +- >> 4 files changed, 11 insertions(+), 9 deletions(-) >> >> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c >> index 55e0799..04d8a23 100644 >> --- a/hw/display/virtio-gpu-base.c >> +++ b/hw/display/virtio-gpu-base.c >> @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, >> } >> >> static void >> -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features) >> +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features, >> + bool reset_offloads) It's not good for e.g gpu to know anything about net. How about checking runstate and do not call apply_guest_offload() in virtio_net_set_features() when in the state of migration? Thanks >> { >> static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL); >> VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev); >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index b9e1cd7..5d108e5 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) >> return virtio_net_guest_offloads_by_features(vdev->guest_features); >> } >> >> -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) >> +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features, >> + bool reset_offloads) >> { >> VirtIONet *n = VIRTIO_NET(vdev); >> int i; >> @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) >> n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) && >> virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6); >> >> - if (n->has_vnet_hdr) { >> + if (reset_offloads && n->has_vnet_hdr) { >> n->curr_guest_offloads = >> virtio_net_guest_offloads_by_features(features); >> virtio_net_apply_guest_offloads(n); >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index a94ea18..b89f7b0 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -2042,14 +2042,14 @@ const VMStateInfo virtio_vmstate_info = { >> .put = virtio_device_put, >> }; >> >> -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val) >> +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads) >> { >> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> bool bad = (val & ~(vdev->host_features)) != 0; >> >> val &= vdev->host_features; >> if (k->set_features) { >> - k->set_features(vdev, val); >> + k->set_features(vdev, val, reset_offloads); >> } >> vdev->guest_features = val; >> return bad ? -1 : 0; >> @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) >> if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { >> return -EINVAL; >> } >> - ret = virtio_set_features_nocheck(vdev, val); >> + ret = virtio_set_features_nocheck(vdev, val, true); >> if (!ret) { >> if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { >> /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */ >> @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) >> * host_features. >> */ >> uint64_t features64 = vdev->guest_features; >> - if (virtio_set_features_nocheck(vdev, features64) < 0) { >> + if (virtio_set_features_nocheck(vdev, features64, false) < 0) { >> error_report("Features 0x%" PRIx64 " unsupported. " >> "Allowed features: 0x%" PRIx64, >> features64, vdev->host_features); >> return -1; >> } >> } else { >> - if (virtio_set_features_nocheck(vdev, features) < 0) { >> + if (virtio_set_features_nocheck(vdev, features, false) < 0) { >> error_report("Features 0x%x unsupported. " >> "Allowed features: 0x%" PRIx64, >> features, vdev->host_features); >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h >> index b189788..fd8cac5 100644 >> --- a/include/hw/virtio/virtio.h >> +++ b/include/hw/virtio/virtio.h >> @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass { >> uint64_t requested_features, >> Error **errp); >> uint64_t (*bad_features)(VirtIODevice *vdev); >> - void (*set_features)(VirtIODevice *vdev, uint64_t val); >> + void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads); >> int (*validate_features)(VirtIODevice *vdev); >> void (*get_config)(VirtIODevice *vdev, uint8_t *config); >> void (*set_config)(VirtIODevice *vdev, const uint8_t *config); >> -- >> 2.7.4 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtio-net: prevent offloads reset on migration 2019-10-10 5:33 ` Jason Wang @ 2019-10-10 12:15 ` Mikhail Sennikovsky 0 siblings, 0 replies; 7+ messages in thread From: Mikhail Sennikovsky @ 2019-10-10 12:15 UTC (permalink / raw) To: Jason Wang; +Cc: qemu-devel, Dr. David Alan Gilbert, stefanha, mst Hi Jason, Thank you for your reply. I've submitted the third version of the patch which does the runstate check as you propose. Thanks, Mikhail Am Do., 10. Okt. 2019 um 07:33 Uhr schrieb Jason Wang <jasowang@redhat.com>: > > > On 2019/10/2 下午5:55, Dr. David Alan Gilbert wrote: > > Copying in Stefan, Jason and Michael who know the virtio details > > > > Dave > > > > * Mikhail Sennikovsky (mikhail.sennikovskii@cloud.ionos.com) wrote: > >> Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET > >> command are not preserved on VM migration. > >> Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES) > >> get enabled. > >> What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads > >> are getting set correctly: > >> > >> #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474 > >> #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 > >> #2 virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334 > >> #3 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11) > >> at migration/vmstate.c:168 > >> #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197 > >> #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 > >> #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 > >> #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 > >> #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 > >> #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 > >> #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 > >> #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 > >> > >> However later on the features are getting restored, and offloads get reset to > >> everything supported by features: > >> > >> #0 qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474 > >> #1 virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720 > >> #2 virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773 > >> #3 virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052 > >> #4 virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220 > >> #5 virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036 > >> #6 vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143 > >> #7 vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829 > >> #8 qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211 > >> #9 qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395 > >> #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467 > >> #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449 > >> > >> This patch fixes this by adding an extra argument to the set_features callback, > >> specifying whether the offloads are to be reset, and setting it to false > >> for the migration case. > >> > >> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com> > >> --- > >> hw/display/virtio-gpu-base.c | 3 ++- > >> hw/net/virtio-net.c | 5 +++-- > >> hw/virtio/virtio.c | 10 +++++----- > >> include/hw/virtio/virtio.h | 2 +- > >> 4 files changed, 11 insertions(+), 9 deletions(-) > >> > >> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c > >> index 55e0799..04d8a23 100644 > >> --- a/hw/display/virtio-gpu-base.c > >> +++ b/hw/display/virtio-gpu-base.c > >> @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, > >> } > >> > >> static void > >> -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features) > >> +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features, > >> + bool reset_offloads) > > > It's not good for e.g gpu to know anything about net. > > How about checking runstate and do not call apply_guest_offload() in > virtio_net_set_features() when in the state of migration? > > Thanks > > > >> { > >> static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL); > >> VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev); > >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >> index b9e1cd7..5d108e5 100644 > >> --- a/hw/net/virtio-net.c > >> +++ b/hw/net/virtio-net.c > >> @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) > >> return virtio_net_guest_offloads_by_features(vdev->guest_features); > >> } > >> > >> -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > >> +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features, > >> + bool reset_offloads) > >> { > >> VirtIONet *n = VIRTIO_NET(vdev); > >> int i; > >> @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > >> n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) && > >> virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6); > >> > >> - if (n->has_vnet_hdr) { > >> + if (reset_offloads && n->has_vnet_hdr) { > >> n->curr_guest_offloads = > >> virtio_net_guest_offloads_by_features(features); > >> virtio_net_apply_guest_offloads(n); > >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >> index a94ea18..b89f7b0 100644 > >> --- a/hw/virtio/virtio.c > >> +++ b/hw/virtio/virtio.c > >> @@ -2042,14 +2042,14 @@ const VMStateInfo virtio_vmstate_info = { > >> .put = virtio_device_put, > >> }; > >> > >> -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val) > >> +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads) > >> { > >> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); > >> bool bad = (val & ~(vdev->host_features)) != 0; > >> > >> val &= vdev->host_features; > >> if (k->set_features) { > >> - k->set_features(vdev, val); > >> + k->set_features(vdev, val, reset_offloads); > >> } > >> vdev->guest_features = val; > >> return bad ? -1 : 0; > >> @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) > >> if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { > >> return -EINVAL; > >> } > >> - ret = virtio_set_features_nocheck(vdev, val); > >> + ret = virtio_set_features_nocheck(vdev, val, true); > >> if (!ret) { > >> if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { > >> /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */ > >> @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) > >> * host_features. > >> */ > >> uint64_t features64 = vdev->guest_features; > >> - if (virtio_set_features_nocheck(vdev, features64) < 0) { > >> + if (virtio_set_features_nocheck(vdev, features64, false) < 0) { > >> error_report("Features 0x%" PRIx64 " unsupported. " > >> "Allowed features: 0x%" PRIx64, > >> features64, vdev->host_features); > >> return -1; > >> } > >> } else { > >> - if (virtio_set_features_nocheck(vdev, features) < 0) { > >> + if (virtio_set_features_nocheck(vdev, features, false) < 0) { > >> error_report("Features 0x%x unsupported. " > >> "Allowed features: 0x%" PRIx64, > >> features, vdev->host_features); > >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > >> index b189788..fd8cac5 100644 > >> --- a/include/hw/virtio/virtio.h > >> +++ b/include/hw/virtio/virtio.h > >> @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass { > >> uint64_t requested_features, > >> Error **errp); > >> uint64_t (*bad_features)(VirtIODevice *vdev); > >> - void (*set_features)(VirtIODevice *vdev, uint64_t val); > >> + void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads); > >> int (*validate_features)(VirtIODevice *vdev); > >> void (*get_config)(VirtIODevice *vdev, uint8_t *config); > >> void (*set_config)(VirtIODevice *vdev, const uint8_t *config); > >> -- > >> 2.7.4 > >> > >> > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-10 12:33 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-01 12:18 [PATCH] virtio-net: prevent offloads reset on migration Mikhail Sennikovsky 2019-10-01 12:18 ` Mikhail Sennikovsky 2019-10-01 16:14 ` no-reply 2019-10-02 9:55 ` Dr. David Alan Gilbert 2019-10-07 10:31 ` Mikhail Sennikovsky 2019-10-10 5:33 ` Jason Wang 2019-10-10 12:15 ` Mikhail Sennikovsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).