| Message ID | 20210519162903.1172366-7-eperezma@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vDPA software assisted live migration | expand |
在 2021/5/20 上午12:28, Eugenio Pérez 写道: > So the guest can stop and start net device. It implements the RFC > https://lists.oasis-open.org/archives/virtio-comment/202012/msg00027.html > > To stop (as "pause") the device is required to migrate status and vring > addresses between device and SVQ. > > This is a WIP commit: as with VIRTIO_F_QUEUE_STATE, is introduced in > virtio_config.h before of even proposing for the kernel, with no feature > flag, and, with no checking in the device. It also needs a modified > vp_vdpa driver that supports to set and retrieve status. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > include/standard-headers/linux/virtio_config.h | 2 ++ > hw/net/virtio-net.c | 4 +++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h > index 59fad3eb45..b3f6b1365d 100644 > --- a/include/standard-headers/linux/virtio_config.h > +++ b/include/standard-headers/linux/virtio_config.h > @@ -40,6 +40,8 @@ > #define VIRTIO_CONFIG_S_DRIVER_OK 4 > /* Driver has finished configuring features */ > #define VIRTIO_CONFIG_S_FEATURES_OK 8 > +/* Device is stopped */ > +#define VIRTIO_CONFIG_S_DEVICE_STOPPED 32 > /* Device entered invalid state, driver must reset it */ > #define VIRTIO_CONFIG_S_NEEDS_RESET 0x40 > /* We've given up on this device. */ > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 96a3cc8357..2d3caea289 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -198,7 +198,9 @@ static bool virtio_net_started(VirtIONet *n, uint8_t status) > { > VirtIODevice *vdev = VIRTIO_DEVICE(n); > return (status & VIRTIO_CONFIG_S_DRIVER_OK) && > - (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running; > + (!(n->status & VIRTIO_CONFIG_S_DEVICE_STOPPED)) && > + (n->status & VIRTIO_NET_S_LINK_UP) && > + vdev->vm_running; > } > > static void virtio_net_announce_notify(VirtIONet *net) It looks to me this is only the part of pause. We still need the resume? Thanks
在 2021/5/26 上午9:06, Jason Wang 写道: > > 在 2021/5/20 上午12:28, Eugenio Pérez 写道: >> So the guest can stop and start net device. It implements the RFC >> https://lists.oasis-open.org/archives/virtio-comment/202012/msg00027.html >> >> >> To stop (as "pause") the device is required to migrate status and vring >> addresses between device and SVQ. >> >> This is a WIP commit: as with VIRTIO_F_QUEUE_STATE, is introduced in >> virtio_config.h before of even proposing for the kernel, with no feature >> flag, and, with no checking in the device. It also needs a modified >> vp_vdpa driver that supports to set and retrieve status. >> >> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> >> --- >> include/standard-headers/linux/virtio_config.h | 2 ++ >> hw/net/virtio-net.c | 4 +++- >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/include/standard-headers/linux/virtio_config.h >> b/include/standard-headers/linux/virtio_config.h >> index 59fad3eb45..b3f6b1365d 100644 >> --- a/include/standard-headers/linux/virtio_config.h >> +++ b/include/standard-headers/linux/virtio_config.h >> @@ -40,6 +40,8 @@ >> #define VIRTIO_CONFIG_S_DRIVER_OK 4 >> /* Driver has finished configuring features */ >> #define VIRTIO_CONFIG_S_FEATURES_OK 8 >> +/* Device is stopped */ >> +#define VIRTIO_CONFIG_S_DEVICE_STOPPED 32 >> /* Device entered invalid state, driver must reset it */ >> #define VIRTIO_CONFIG_S_NEEDS_RESET 0x40 >> /* We've given up on this device. */ >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 96a3cc8357..2d3caea289 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -198,7 +198,9 @@ static bool virtio_net_started(VirtIONet *n, >> uint8_t status) >> { >> VirtIODevice *vdev = VIRTIO_DEVICE(n); >> return (status & VIRTIO_CONFIG_S_DRIVER_OK) && >> - (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running; >> + (!(n->status & VIRTIO_CONFIG_S_DEVICE_STOPPED)) && >> + (n->status & VIRTIO_NET_S_LINK_UP) && >> + vdev->vm_running; >> } >> static void virtio_net_announce_notify(VirtIONet *net) > > > It looks to me this is only the part of pause. And even for pause, I don't see anything that prevents rx/tx from being executed? (E.g virtio_net_handle_tx_bh or virtio_net_handle_rx). Thanks > We still need the resume? > > Thanks > >
On Wed, May 26, 2021 at 3:10 AM Jason Wang <jasowang@redhat.com> wrote: > > > 在 2021/5/26 上午9:06, Jason Wang 写道: > > > > 在 2021/5/20 上午12:28, Eugenio Pérez 写道: > >> So the guest can stop and start net device. It implements the RFC > >> https://lists.oasis-open.org/archives/virtio-comment/202012/msg00027.html > >> > >> > >> To stop (as "pause") the device is required to migrate status and vring > >> addresses between device and SVQ. > >> > >> This is a WIP commit: as with VIRTIO_F_QUEUE_STATE, is introduced in > >> virtio_config.h before of even proposing for the kernel, with no feature > >> flag, and, with no checking in the device. It also needs a modified > >> vp_vdpa driver that supports to set and retrieve status. > >> > >> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > >> --- > >> include/standard-headers/linux/virtio_config.h | 2 ++ > >> hw/net/virtio-net.c | 4 +++- > >> 2 files changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/include/standard-headers/linux/virtio_config.h > >> b/include/standard-headers/linux/virtio_config.h > >> index 59fad3eb45..b3f6b1365d 100644 > >> --- a/include/standard-headers/linux/virtio_config.h > >> +++ b/include/standard-headers/linux/virtio_config.h > >> @@ -40,6 +40,8 @@ > >> #define VIRTIO_CONFIG_S_DRIVER_OK 4 > >> /* Driver has finished configuring features */ > >> #define VIRTIO_CONFIG_S_FEATURES_OK 8 > >> +/* Device is stopped */ > >> +#define VIRTIO_CONFIG_S_DEVICE_STOPPED 32 > >> /* Device entered invalid state, driver must reset it */ > >> #define VIRTIO_CONFIG_S_NEEDS_RESET 0x40 > >> /* We've given up on this device. */ > >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >> index 96a3cc8357..2d3caea289 100644 > >> --- a/hw/net/virtio-net.c > >> +++ b/hw/net/virtio-net.c > >> @@ -198,7 +198,9 @@ static bool virtio_net_started(VirtIONet *n, > >> uint8_t status) > >> { > >> VirtIODevice *vdev = VIRTIO_DEVICE(n); > >> return (status & VIRTIO_CONFIG_S_DRIVER_OK) && > >> - (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running; > >> + (!(n->status & VIRTIO_CONFIG_S_DEVICE_STOPPED)) && > >> + (n->status & VIRTIO_NET_S_LINK_UP) && > >> + vdev->vm_running; > >> } > >> static void virtio_net_announce_notify(VirtIONet *net) > > > > > > It looks to me this is only the part of pause. > For SVQ we need to switch vring addresses, and a full reset of the device is required for that. Actually, the pause is just used to recover If you prefer this could be sent as a separate series where the full pause/resume cycle is implemented, and then SVQ uses the pause part. However there are no use for the resume part at the moment. > > And even for pause, I don't see anything that prevents rx/tx from being > executed? (E.g virtio_net_handle_tx_bh or virtio_net_handle_rx). > virtio_net_started is called from virtio_net_set_status. If _S_DEVICE_STOPPED is true, the former return false, and variable queue_started is false in the latter: queue_started = virtio_net_started(n, queue_status) && !n->vhost_started; After that, it should work like a regular device reset or link down if I'm not wrong, and the last part of virtio_net_set_status should delete timer or cancel bh. > Thanks > > > > We still need the resume? > > > > Thanks > > > > >
在 2021/6/1 下午3:13, Eugenio Perez Martin 写道: > On Wed, May 26, 2021 at 3:10 AM Jason Wang <jasowang@redhat.com> wrote: >> >> 在 2021/5/26 上午9:06, Jason Wang 写道: >>> 在 2021/5/20 上午12:28, Eugenio Pérez 写道: >>>> So the guest can stop and start net device. It implements the RFC >>>> https://lists.oasis-open.org/archives/virtio-comment/202012/msg00027.html >>>> >>>> >>>> To stop (as "pause") the device is required to migrate status and vring >>>> addresses between device and SVQ. >>>> >>>> This is a WIP commit: as with VIRTIO_F_QUEUE_STATE, is introduced in >>>> virtio_config.h before of even proposing for the kernel, with no feature >>>> flag, and, with no checking in the device. It also needs a modified >>>> vp_vdpa driver that supports to set and retrieve status. >>>> >>>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> >>>> --- >>>> include/standard-headers/linux/virtio_config.h | 2 ++ >>>> hw/net/virtio-net.c | 4 +++- >>>> 2 files changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/standard-headers/linux/virtio_config.h >>>> b/include/standard-headers/linux/virtio_config.h >>>> index 59fad3eb45..b3f6b1365d 100644 >>>> --- a/include/standard-headers/linux/virtio_config.h >>>> +++ b/include/standard-headers/linux/virtio_config.h >>>> @@ -40,6 +40,8 @@ >>>> #define VIRTIO_CONFIG_S_DRIVER_OK 4 >>>> /* Driver has finished configuring features */ >>>> #define VIRTIO_CONFIG_S_FEATURES_OK 8 >>>> +/* Device is stopped */ >>>> +#define VIRTIO_CONFIG_S_DEVICE_STOPPED 32 >>>> /* Device entered invalid state, driver must reset it */ >>>> #define VIRTIO_CONFIG_S_NEEDS_RESET 0x40 >>>> /* We've given up on this device. */ >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>> index 96a3cc8357..2d3caea289 100644 >>>> --- a/hw/net/virtio-net.c >>>> +++ b/hw/net/virtio-net.c >>>> @@ -198,7 +198,9 @@ static bool virtio_net_started(VirtIONet *n, >>>> uint8_t status) >>>> { >>>> VirtIODevice *vdev = VIRTIO_DEVICE(n); >>>> return (status & VIRTIO_CONFIG_S_DRIVER_OK) && >>>> - (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running; >>>> + (!(n->status & VIRTIO_CONFIG_S_DEVICE_STOPPED)) && >>>> + (n->status & VIRTIO_NET_S_LINK_UP) && >>>> + vdev->vm_running; >>>> } >>>> static void virtio_net_announce_notify(VirtIONet *net) >>> >>> It looks to me this is only the part of pause. > For SVQ we need to switch vring addresses, and a full reset of the > device is required for that. Actually, the pause is just used to > recover > > If you prefer this could be sent as a separate series where the full > pause/resume cycle is implemented, and then SVQ uses the pause part. > However there are no use for the resume part at the moment. That would be fine if you can send it in another series. > >> And even for pause, I don't see anything that prevents rx/tx from being >> executed? (E.g virtio_net_handle_tx_bh or virtio_net_handle_rx). >> > virtio_net_started is called from virtio_net_set_status. If > _S_DEVICE_STOPPED is true, the former return false, and variable > queue_started is false in the latter: > queue_started = > virtio_net_started(n, queue_status) && !n->vhost_started; > > After that, it should work like a regular device reset or link down if > I'm not wrong, and the last part of virtio_net_set_status should > delete timer or cancel bh. You are right. Thanks > >> Thanks >> >> >>> We still need the resume? >>> >>> Thanks >>> >>>
diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h index 59fad3eb45..b3f6b1365d 100644 --- a/include/standard-headers/linux/virtio_config.h +++ b/include/standard-headers/linux/virtio_config.h @@ -40,6 +40,8 @@ #define VIRTIO_CONFIG_S_DRIVER_OK 4 /* Driver has finished configuring features */ #define VIRTIO_CONFIG_S_FEATURES_OK 8 +/* Device is stopped */ +#define VIRTIO_CONFIG_S_DEVICE_STOPPED 32 /* Device entered invalid state, driver must reset it */ #define VIRTIO_CONFIG_S_NEEDS_RESET 0x40 /* We've given up on this device. */ diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 96a3cc8357..2d3caea289 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -198,7 +198,9 @@ static bool virtio_net_started(VirtIONet *n, uint8_t status) { VirtIODevice *vdev = VIRTIO_DEVICE(n); return (status & VIRTIO_CONFIG_S_DRIVER_OK) && - (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running; + (!(n->status & VIRTIO_CONFIG_S_DEVICE_STOPPED)) && + (n->status & VIRTIO_NET_S_LINK_UP) && + vdev->vm_running; } static void virtio_net_announce_notify(VirtIONet *net)
So the guest can stop and start net device. It implements the RFC https://lists.oasis-open.org/archives/virtio-comment/202012/msg00027.html To stop (as "pause") the device is required to migrate status and vring addresses between device and SVQ. This is a WIP commit: as with VIRTIO_F_QUEUE_STATE, is introduced in virtio_config.h before of even proposing for the kernel, with no feature flag, and, with no checking in the device. It also needs a modified vp_vdpa driver that supports to set and retrieve status. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- include/standard-headers/linux/virtio_config.h | 2 ++ hw/net/virtio-net.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)