* [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set
@ 2015-07-14 9:41 Fam Zheng
2015-07-14 10:09 ` Michael S. Tsirkin
0 siblings, 1 reply; 6+ messages in thread
From: Fam Zheng @ 2015-07-14 9:41 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, stefanha, Michael S. Tsirkin
This patch fixes network hang after "stop" then "cont", while network
packets keep arriving.
Tested both manually (tap, host pinging guest) and with Jason's qtest
series (plus his "[PATCH 2.4] socket: pass correct size in
net_socket_send()" fix).
As virtio_net_set_status is called when guest driver is setting status
byte and when vm state is changing, it is a good opportunity to flush
queued packets.
This is necessary because during vm stop the backend (e.g. tap) would
stop rx processing after .can_receive returns false, until the queue is
explicitly flushed or purged.
The other interesting condition in .can_receive, virtio_queue_ready(),
is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
is invalid queue index which doesn't need flushing.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/net/virtio-net.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d728233..7c178c6 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -162,8 +162,13 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
virtio_net_vhost_status(n, status);
for (i = 0; i < n->max_queues; i++) {
+ NetClientState *ncs = qemu_get_subqueue(n->nic, i);
q = &n->vqs[i];
+ if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
+ qemu_flush_queued_packets(ncs);
+ }
+
if ((!n->multiqueue && i != 0) || i >= n->curr_queues) {
queue_status = 0;
} else {
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set
2015-07-14 9:41 [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set Fam Zheng
@ 2015-07-14 10:09 ` Michael S. Tsirkin
2015-07-14 10:17 ` Fam Zheng
0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2015-07-14 10:09 UTC (permalink / raw)
To: Fam Zheng; +Cc: jasowang, qemu-devel, stefanha
On Tue, Jul 14, 2015 at 05:41:04PM +0800, Fam Zheng wrote:
> This patch fixes network hang after "stop" then "cont", while network
> packets keep arriving.
>
> Tested both manually (tap, host pinging guest) and with Jason's qtest
> series (plus his "[PATCH 2.4] socket: pass correct size in
> net_socket_send()" fix).
>
> As virtio_net_set_status is called when guest driver is setting status
> byte and when vm state is changing, it is a good opportunity to flush
> queued packets.
>
> This is necessary because during vm stop the backend (e.g. tap) would
> stop rx processing after .can_receive returns false, until the queue is
> explicitly flushed or purged.
>
> The other interesting condition in .can_receive, virtio_queue_ready(),
> is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
> is invalid queue index which doesn't need flushing.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> hw/net/virtio-net.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index d728233..7c178c6 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -162,8 +162,13 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> virtio_net_vhost_status(n, status);
>
> for (i = 0; i < n->max_queues; i++) {
> + NetClientState *ncs = qemu_get_subqueue(n->nic, i);
> q = &n->vqs[i];
>
> + if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> + qemu_flush_queued_packets(ncs);
> + }
> +
> if ((!n->multiqueue && i != 0) || i >= n->curr_queues) {
> queue_status = 0;
> } else {
I think this should be limited to
virtio_net_started(n, queue_status) && !n->vhost_started
> --
> 2.4.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set
2015-07-14 10:09 ` Michael S. Tsirkin
@ 2015-07-14 10:17 ` Fam Zheng
0 siblings, 0 replies; 6+ messages in thread
From: Fam Zheng @ 2015-07-14 10:17 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jasowang, qemu-devel, stefanha
On Tue, 07/14 13:09, Michael S. Tsirkin wrote:
> On Tue, Jul 14, 2015 at 05:41:04PM +0800, Fam Zheng wrote:
> > This patch fixes network hang after "stop" then "cont", while network
> > packets keep arriving.
> >
> > Tested both manually (tap, host pinging guest) and with Jason's qtest
> > series (plus his "[PATCH 2.4] socket: pass correct size in
> > net_socket_send()" fix).
> >
> > As virtio_net_set_status is called when guest driver is setting status
> > byte and when vm state is changing, it is a good opportunity to flush
> > queued packets.
> >
> > This is necessary because during vm stop the backend (e.g. tap) would
> > stop rx processing after .can_receive returns false, until the queue is
> > explicitly flushed or purged.
> >
> > The other interesting condition in .can_receive, virtio_queue_ready(),
> > is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
> > is invalid queue index which doesn't need flushing.
> >
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> > hw/net/virtio-net.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index d728233..7c178c6 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -162,8 +162,13 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> > virtio_net_vhost_status(n, status);
> >
> > for (i = 0; i < n->max_queues; i++) {
> > + NetClientState *ncs = qemu_get_subqueue(n->nic, i);
> > q = &n->vqs[i];
> >
> > + if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> > + qemu_flush_queued_packets(ncs);
> > + }
> > +
> > if ((!n->multiqueue && i != 0) || i >= n->curr_queues) {
> > queue_status = 0;
> > } else {
>
> I think this should be limited to
> virtio_net_started(n, queue_status) && !n->vhost_started
Yes, that looks better.
Fam
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set
@ 2015-07-15 3:02 Fam Zheng
2015-07-15 6:52 ` Wen Congyang
2015-07-15 7:24 ` Jason Wang
0 siblings, 2 replies; 6+ messages in thread
From: Fam Zheng @ 2015-07-15 3:02 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, stefanha, Michael S. Tsirkin
This patch fixes network hang after "stop" then "cont", while network
packets keep arriving.
Tested both manually (tap, host pinging guest) and with Jason's qtest
series (plus his "[PATCH 2.4] socket: pass correct size in
net_socket_send()" fix).
As virtio_net_set_status is called when guest driver is setting status
byte and when vm state is changing, it is a good opportunity to flush
queued packets.
This is necessary because during vm stop the backend (e.g. tap) would
stop rx processing after .can_receive returns false, until the queue is
explicitly flushed or purged.
The other interesting condition in .can_receive, virtio_queue_ready(),
is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
is invalid queue index which doesn't need flushing.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
v2: Limit to "virtio_net_started(n, queue_status) &&
!n->vhost_started".[MST]
---
hw/net/virtio-net.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d728233..24c7be1 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -162,6 +162,8 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
virtio_net_vhost_status(n, status);
for (i = 0; i < n->max_queues; i++) {
+ NetClientState *ncs = qemu_get_subqueue(n->nic, i);
+ bool queue_started;
q = &n->vqs[i];
if ((!n->multiqueue && i != 0) || i >= n->curr_queues) {
@@ -169,12 +171,18 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
} else {
queue_status = status;
}
+ queue_started =
+ virtio_net_started(n, queue_status) && !n->vhost_started;
+
+ if (queue_started) {
+ qemu_flush_queued_packets(ncs);
+ }
if (!q->tx_waiting) {
continue;
}
- if (virtio_net_started(n, queue_status) && !n->vhost_started) {
+ if (queue_started) {
if (q->tx_timer) {
timer_mod(q->tx_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout);
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set
2015-07-15 3:02 Fam Zheng
@ 2015-07-15 6:52 ` Wen Congyang
2015-07-15 7:24 ` Jason Wang
1 sibling, 0 replies; 6+ messages in thread
From: Wen Congyang @ 2015-07-15 6:52 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: jasowang, stefanha, Michael S. Tsirkin
On 07/15/2015 11:02 AM, Fam Zheng wrote:
> This patch fixes network hang after "stop" then "cont", while network
> packets keep arriving.
I think it also fixes network hand when the guest boots, while network packets
keep arriving.
Thanks
Wen Congyang
>
> Tested both manually (tap, host pinging guest) and with Jason's qtest
> series (plus his "[PATCH 2.4] socket: pass correct size in
> net_socket_send()" fix).
>
> As virtio_net_set_status is called when guest driver is setting status
> byte and when vm state is changing, it is a good opportunity to flush
> queued packets.
>
> This is necessary because during vm stop the backend (e.g. tap) would
> stop rx processing after .can_receive returns false, until the queue is
> explicitly flushed or purged.
>
> The other interesting condition in .can_receive, virtio_queue_ready(),
> is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
> is invalid queue index which doesn't need flushing.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
>
> v2: Limit to "virtio_net_started(n, queue_status) &&
> !n->vhost_started".[MST]
> ---
> hw/net/virtio-net.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index d728233..24c7be1 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -162,6 +162,8 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> virtio_net_vhost_status(n, status);
>
> for (i = 0; i < n->max_queues; i++) {
> + NetClientState *ncs = qemu_get_subqueue(n->nic, i);
> + bool queue_started;
> q = &n->vqs[i];
>
> if ((!n->multiqueue && i != 0) || i >= n->curr_queues) {
> @@ -169,12 +171,18 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> } else {
> queue_status = status;
> }
> + queue_started =
> + virtio_net_started(n, queue_status) && !n->vhost_started;
> +
> + if (queue_started) {
> + qemu_flush_queued_packets(ncs);
> + }
>
> if (!q->tx_waiting) {
> continue;
> }
>
> - if (virtio_net_started(n, queue_status) && !n->vhost_started) {
> + if (queue_started) {
> if (q->tx_timer) {
> timer_mod(q->tx_timer,
> qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set
2015-07-15 3:02 Fam Zheng
2015-07-15 6:52 ` Wen Congyang
@ 2015-07-15 7:24 ` Jason Wang
1 sibling, 0 replies; 6+ messages in thread
From: Jason Wang @ 2015-07-15 7:24 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: stefanha, Michael S. Tsirkin
On 07/15/2015 11:02 AM, Fam Zheng wrote:
> This patch fixes network hang after "stop" then "cont", while network
> packets keep arriving.
>
> Tested both manually (tap, host pinging guest) and with Jason's qtest
> series (plus his "[PATCH 2.4] socket: pass correct size in
> net_socket_send()" fix).
>
> As virtio_net_set_status is called when guest driver is setting status
> byte and when vm state is changing, it is a good opportunity to flush
> queued packets.
>
> This is necessary because during vm stop the backend (e.g. tap) would
> stop rx processing after .can_receive returns false, until the queue is
> explicitly flushed or purged.
>
> The other interesting condition in .can_receive, virtio_queue_ready(),
> is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
> is invalid queue index which doesn't need flushing.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
Reviewed-by: Jason Wang <jasowang@redhat.com>
btw, there's another condition in can_receive() which is suspicious:
...
if (nc->queue_index >= n->curr_queues) {
return 0;
}
...
This requires queue to be flushed when the number of queues was
increased. But it looks unnecessary since both guest and vhost does not
care about this. So I think we could safely just remove this condition.
Maybe a patch on top.
Thanks
>
> v2: Limit to "virtio_net_started(n, queue_status) &&
> !n->vhost_started".[MST]
> ---
> hw/net/virtio-net.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index d728233..24c7be1 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -162,6 +162,8 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> virtio_net_vhost_status(n, status);
>
> for (i = 0; i < n->max_queues; i++) {
> + NetClientState *ncs = qemu_get_subqueue(n->nic, i);
> + bool queue_started;
> q = &n->vqs[i];
>
> if ((!n->multiqueue && i != 0) || i >= n->curr_queues) {
> @@ -169,12 +171,18 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> } else {
> queue_status = status;
> }
> + queue_started =
> + virtio_net_started(n, queue_status) && !n->vhost_started;
> +
> + if (queue_started) {
> + qemu_flush_queued_packets(ncs);
> + }
>
> if (!q->tx_waiting) {
> continue;
> }
>
> - if (virtio_net_started(n, queue_status) && !n->vhost_started) {
> + if (queue_started) {
> if (q->tx_timer) {
> timer_mod(q->tx_timer,
> qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-15 7:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-14 9:41 [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set Fam Zheng
2015-07-14 10:09 ` Michael S. Tsirkin
2015-07-14 10:17 ` Fam Zheng
-- strict thread matches above, loose matches on Subject: below --
2015-07-15 3:02 Fam Zheng
2015-07-15 6:52 ` Wen Congyang
2015-07-15 7:24 ` Jason Wang
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).