| Message ID | 20200309083438.2389-5-yuri.benditovich@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | reference implementation of RSS | expand |
On 2020/3/9 下午4:34, Yuri Benditovich wrote: > Block migration for reference implementation of > RSS feature in QEMU. When we add support for RSS > on backend side, we'll implement migration of > current RSS settings. > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > --- > hw/net/virtio-net.c | 18 ++++++++++++++++++ > include/hw/virtio/virtio-net.h | 1 + > 2 files changed, 19 insertions(+) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index abc41fdb16..943d1931a2 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -37,6 +37,7 @@ > #include "qapi/qapi-events-migration.h" > #include "hw/virtio/virtio-access.h" > #include "migration/misc.h" > +#include "migration/blocker.h" > #include "standard-headers/linux/ethtool.h" > #include "sysemu/sysemu.h" > #include "trace.h" > @@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev) > n->announce_timer.round = 0; > n->status &= ~VIRTIO_NET_S_ANNOUNCE; > > + if (n->migration_blocker) { > + migrate_del_blocker(n->migration_blocker); > + error_free(n->migration_blocker); > + n->migration_blocker = NULL; > + } > + > /* Flush any MAC and VLAN filter table state */ > n->mac_table.in_use = 0; > n->mac_table.first_multi = 0; > @@ -1003,6 +1010,17 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > vhost_net_ack_features(get_vhost_net(nc->peer), features); > } > > + if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) { > + if (!n->migration_blocker) { > + error_setg(&n->migration_blocker, "virtio-net: RSS negotiated"); > + migrate_add_blocker(n->migration_blocker, &err); > + if (err) { > + error_report_err(err); > + err = NULL; > + } > + } > + } It looks to me we should add the blocker unconditionally based on virtio_host_has_feature() instead of depending the negotiated feature here. Thanks > + > if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { > memset(n->vlans, 0, MAX_VLAN >> 3); > } else { > diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h > index 45670dd054..fba768ba9b 100644 > --- a/include/hw/virtio/virtio-net.h > +++ b/include/hw/virtio/virtio-net.h > @@ -180,6 +180,7 @@ struct VirtIONet { > virtio_net_conf net_conf; > NICConf nic_conf; > DeviceState *qdev; > + Error *migration_blocker; > int multiqueue; > uint16_t max_queues; > uint16_t curr_queues;
On Tue, Mar 10, 2020 at 11:12:05AM +0800, Jason Wang wrote: > > On 2020/3/9 下午4:34, Yuri Benditovich wrote: > > Block migration for reference implementation of > > RSS feature in QEMU. When we add support for RSS > > on backend side, we'll implement migration of > > current RSS settings. > > > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > > --- > > hw/net/virtio-net.c | 18 ++++++++++++++++++ > > include/hw/virtio/virtio-net.h | 1 + > > 2 files changed, 19 insertions(+) > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > index abc41fdb16..943d1931a2 100644 > > --- a/hw/net/virtio-net.c > > +++ b/hw/net/virtio-net.c > > @@ -37,6 +37,7 @@ > > #include "qapi/qapi-events-migration.h" > > #include "hw/virtio/virtio-access.h" > > #include "migration/misc.h" > > +#include "migration/blocker.h" > > #include "standard-headers/linux/ethtool.h" > > #include "sysemu/sysemu.h" > > #include "trace.h" > > @@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev) > > n->announce_timer.round = 0; > > n->status &= ~VIRTIO_NET_S_ANNOUNCE; > > + if (n->migration_blocker) { > > + migrate_del_blocker(n->migration_blocker); > > + error_free(n->migration_blocker); > > + n->migration_blocker = NULL; > > + } > > + > > /* Flush any MAC and VLAN filter table state */ > > n->mac_table.in_use = 0; > > n->mac_table.first_multi = 0; > > @@ -1003,6 +1010,17 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > > vhost_net_ack_features(get_vhost_net(nc->peer), features); > > } > > + if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) { > > + if (!n->migration_blocker) { > > + error_setg(&n->migration_blocker, "virtio-net: RSS negotiated"); > > + migrate_add_blocker(n->migration_blocker, &err); > > + if (err) { > > + error_report_err(err); > > + err = NULL; > > + } > > + } > > + } > > > It looks to me we should add the blocker unconditionally based on > virtio_host_has_feature() instead of depending the negotiated feature here. > > Thanks I agree, this is a stopgap measure anyway. If this is merged in its current form, there should also be a big TODO here that we must fix this ASAP, and maybe a warning produced. > > > + > > if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { > > memset(n->vlans, 0, MAX_VLAN >> 3); > > } else { > > diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h > > index 45670dd054..fba768ba9b 100644 > > --- a/include/hw/virtio/virtio-net.h > > +++ b/include/hw/virtio/virtio-net.h > > @@ -180,6 +180,7 @@ struct VirtIONet { > > virtio_net_conf net_conf; > > NICConf nic_conf; > > DeviceState *qdev; > > + Error *migration_blocker; > > int multiqueue; > > uint16_t max_queues; > > uint16_t curr_queues;
IMO, this does not depend on features of vhost as soon as we're not able to provide parameters to it. On Tue, Mar 10, 2020 at 8:17 AM Michael S. Tsirkin <mst@redhat.com> wrote: > On Tue, Mar 10, 2020 at 11:12:05AM +0800, Jason Wang wrote: > > > > On 2020/3/9 下午4:34, Yuri Benditovich wrote: > > > Block migration for reference implementation of > > > RSS feature in QEMU. When we add support for RSS > > > on backend side, we'll implement migration of > > > current RSS settings. > > > > > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > > > --- > > > hw/net/virtio-net.c | 18 ++++++++++++++++++ > > > include/hw/virtio/virtio-net.h | 1 + > > > 2 files changed, 19 insertions(+) > > > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > > index abc41fdb16..943d1931a2 100644 > > > --- a/hw/net/virtio-net.c > > > +++ b/hw/net/virtio-net.c > > > @@ -37,6 +37,7 @@ > > > #include "qapi/qapi-events-migration.h" > > > #include "hw/virtio/virtio-access.h" > > > #include "migration/misc.h" > > > +#include "migration/blocker.h" > > > #include "standard-headers/linux/ethtool.h" > > > #include "sysemu/sysemu.h" > > > #include "trace.h" > > > @@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev) > > > n->announce_timer.round = 0; > > > n->status &= ~VIRTIO_NET_S_ANNOUNCE; > > > + if (n->migration_blocker) { > > > + migrate_del_blocker(n->migration_blocker); > > > + error_free(n->migration_blocker); > > > + n->migration_blocker = NULL; > > > + } > > > + > > > /* Flush any MAC and VLAN filter table state */ > > > n->mac_table.in_use = 0; > > > n->mac_table.first_multi = 0; > > > @@ -1003,6 +1010,17 @@ static void > virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > > > vhost_net_ack_features(get_vhost_net(nc->peer), features); > > > } > > > + if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) { > > > + if (!n->migration_blocker) { > > > + error_setg(&n->migration_blocker, "virtio-net: RSS > negotiated"); > > > + migrate_add_blocker(n->migration_blocker, &err); > > > + if (err) { > > > + error_report_err(err); > > > + err = NULL; > > > + } > > > + } > > > + } > > > > > > It looks to me we should add the blocker unconditionally based on > > virtio_host_has_feature() instead of depending the negotiated feature > here. > > > > Thanks > > I agree, this is a stopgap measure anyway. If this is merged in its > current form, there should also be a big TODO here that we must fix this > ASAP, and maybe a warning produced. > > > > > > > + > > > if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { > > > memset(n->vlans, 0, MAX_VLAN >> 3); > > > } else { > > > diff --git a/include/hw/virtio/virtio-net.h > b/include/hw/virtio/virtio-net.h > > > index 45670dd054..fba768ba9b 100644 > > > --- a/include/hw/virtio/virtio-net.h > > > +++ b/include/hw/virtio/virtio-net.h > > > @@ -180,6 +180,7 @@ struct VirtIONet { > > > virtio_net_conf net_conf; > > > NICConf nic_conf; > > > DeviceState *qdev; > > > + Error *migration_blocker; > > > int multiqueue; > > > uint16_t max_queues; > > > uint16_t curr_queues; > >
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index abc41fdb16..943d1931a2 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -37,6 +37,7 @@ #include "qapi/qapi-events-migration.h" #include "hw/virtio/virtio-access.h" #include "migration/misc.h" +#include "migration/blocker.h" #include "standard-headers/linux/ethtool.h" #include "sysemu/sysemu.h" #include "trace.h" @@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev) n->announce_timer.round = 0; n->status &= ~VIRTIO_NET_S_ANNOUNCE; + if (n->migration_blocker) { + migrate_del_blocker(n->migration_blocker); + error_free(n->migration_blocker); + n->migration_blocker = NULL; + } + /* Flush any MAC and VLAN filter table state */ n->mac_table.in_use = 0; n->mac_table.first_multi = 0; @@ -1003,6 +1010,17 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) vhost_net_ack_features(get_vhost_net(nc->peer), features); } + if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) { + if (!n->migration_blocker) { + error_setg(&n->migration_blocker, "virtio-net: RSS negotiated"); + migrate_add_blocker(n->migration_blocker, &err); + if (err) { + error_report_err(err); + err = NULL; + } + } + } + if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { memset(n->vlans, 0, MAX_VLAN >> 3); } else { diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 45670dd054..fba768ba9b 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -180,6 +180,7 @@ struct VirtIONet { virtio_net_conf net_conf; NICConf nic_conf; DeviceState *qdev; + Error *migration_blocker; int multiqueue; uint16_t max_queues; uint16_t curr_queues;
Block migration for reference implementation of RSS feature in QEMU. When we add support for RSS on backend side, we'll implement migration of current RSS settings. Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> --- hw/net/virtio-net.c | 18 ++++++++++++++++++ include/hw/virtio/virtio-net.h | 1 + 2 files changed, 19 insertions(+)