* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
@ 2015-07-10 9:24 Fam Zheng
2015-07-13 5:21 ` Jason Wang
0 siblings, 1 reply; 18+ messages in thread
From: Fam Zheng @ 2015-07-10 9:24 UTC (permalink / raw)
To: Jason Wang; +Cc: jcmvbkbc, qemu-devel, Stefan Hajnoczi, mst
On Wed, 07/08 17:40, Jason Wang wrote:
>
>
> On 07/07/2015 05:03 PM, Fam Zheng wrote:
> > On Tue, 07/07 15:44, Jason Wang wrote:
> >>
> >> On 07/07/2015 09:21 AM, Fam Zheng wrote:
> >>> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> >>> net queues need to be explicitly flushed after qemu_can_send_packet()
> >>> returns false, because the netdev side will disable the polling of fd.
> >>>
> >>> This fixes the case of "cont" after "stop" (or migration).
> >>>
> >>> Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>
> >>> ---
> >>>
> >>> v2: Unify with VM stop handler. (Stefan)
> >>> ---
> >>> net/net.c | 19 ++++++++++++-------
> >>> 1 file changed, 12 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/net/net.c b/net/net.c
> >>> index 6ff7fec..28a5597 100644
> >>> --- a/net/net.c
> >>> +++ b/net/net.c
> >>> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> >>> static void net_vm_change_state_handler(void *opaque, int running,
> >>> RunState state)
> >>> {
> >>> - /* Complete all queued packets, to guarantee we don't modify
> >>> - * state later when VM is not running.
> >>> - */
> >>> - if (!running) {
> >>> - NetClientState *nc;
> >>> - NetClientState *tmp;
> >>> + NetClientState *nc;
> >>> + NetClientState *tmp;
> >>>
> >>> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> >>> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> >>> + if (running) {
> >>> + /* Flush queued packets and wake up backends. */
> >>> + if (nc->peer && qemu_can_send_packet(nc)) {
> >>> + qemu_flush_queued_packets(nc->peer);
> >>> + }
> >>> + } else {
> >>> + /* Complete all queued packets, to guarantee we don't modify
> >>> + * state later when VM is not running.
> >>> + */
> >>> qemu_flush_or_purge_queued_packets(nc, true);
> >>> }
> >> Looks like qemu_can_send_packet() checks both nc->peer and runstate. So
> >> probably, we can simplify this to:
> >>
> >> if (qemu_can_send_packet(nc))
> >> qemu_flush_queued_packets(nc->peer);
> >> else
> >> qemu_flush_or_purge_queued_packets(nc, true);
> >>
> >>> }
> > qemu_can_send_packet returns 1 if !nc->peer, so this doesn't work.
> >
> > Fam
>
> Yes, I was wrong.
>
> Btw, instead of depending on vm handler (which seems racy with other
> state change handler). Can we do this in places like vm_start() and
> vm_stop(). Like we drain and flush block queue during vm stop.
>
Because that's a bit hacky.
It won't be racy if we replace vdev->vm_running with runstate_is_running() in
virtio-net, and/or don't check it in virtio_net_can_receive(). Is that OK?
Fam
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-10 9:24 [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes Fam Zheng
@ 2015-07-13 5:21 ` Jason Wang
0 siblings, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-07-13 5:21 UTC (permalink / raw)
To: Fam Zheng; +Cc: jcmvbkbc, qemu-devel, Stefan Hajnoczi, mst
On 07/10/2015 05:24 PM, Fam Zheng wrote:
> On Wed, 07/08 17:40, Jason Wang wrote:
>>
>> On 07/07/2015 05:03 PM, Fam Zheng wrote:
>>> On Tue, 07/07 15:44, Jason Wang wrote:
>>>> On 07/07/2015 09:21 AM, Fam Zheng wrote:
>>>>> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
>>>>> net queues need to be explicitly flushed after qemu_can_send_packet()
>>>>> returns false, because the netdev side will disable the polling of fd.
>>>>>
>>>>> This fixes the case of "cont" after "stop" (or migration).
>>>>>
>>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>>>
>>>>> ---
>>>>>
>>>>> v2: Unify with VM stop handler. (Stefan)
>>>>> ---
>>>>> net/net.c | 19 ++++++++++++-------
>>>>> 1 file changed, 12 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/net/net.c b/net/net.c
>>>>> index 6ff7fec..28a5597 100644
>>>>> --- a/net/net.c
>>>>> +++ b/net/net.c
>>>>> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
>>>>> static void net_vm_change_state_handler(void *opaque, int running,
>>>>> RunState state)
>>>>> {
>>>>> - /* Complete all queued packets, to guarantee we don't modify
>>>>> - * state later when VM is not running.
>>>>> - */
>>>>> - if (!running) {
>>>>> - NetClientState *nc;
>>>>> - NetClientState *tmp;
>>>>> + NetClientState *nc;
>>>>> + NetClientState *tmp;
>>>>>
>>>>> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
>>>>> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
>>>>> + if (running) {
>>>>> + /* Flush queued packets and wake up backends. */
>>>>> + if (nc->peer && qemu_can_send_packet(nc)) {
>>>>> + qemu_flush_queued_packets(nc->peer);
>>>>> + }
>>>>> + } else {
>>>>> + /* Complete all queued packets, to guarantee we don't modify
>>>>> + * state later when VM is not running.
>>>>> + */
>>>>> qemu_flush_or_purge_queued_packets(nc, true);
>>>>> }
>>>> Looks like qemu_can_send_packet() checks both nc->peer and runstate. So
>>>> probably, we can simplify this to:
>>>>
>>>> if (qemu_can_send_packet(nc))
>>>> qemu_flush_queued_packets(nc->peer);
>>>> else
>>>> qemu_flush_or_purge_queued_packets(nc, true);
>>>>
>>>>> }
>>> qemu_can_send_packet returns 1 if !nc->peer, so this doesn't work.
>>>
>>> Fam
>> Yes, I was wrong.
>>
>> Btw, instead of depending on vm handler (which seems racy with other
>> state change handler). Can we do this in places like vm_start() and
>> vm_stop(). Like we drain and flush block queue during vm stop.
>>
> Because that's a bit hacky.
>
> It won't be racy if we replace vdev->vm_running with runstate_is_running() in
> virtio-net, and/or don't check it in virtio_net_can_receive(). Is that OK?
Ok but not necessary. I believe this patch will be used with the patch
that drops virtio_net_can_receive(). So at any order two vmstate
handlers were called, the sent_cb will be called and poll will be
enabled again. I'm ok with the patch. So
Reviewed-by: Jason Wang <jasowang@redhat.com>
Thanks
>
> Fam
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-14 12:42 ` Michael S. Tsirkin
@ 2015-07-15 2:50 ` Fam Zheng
0 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 2:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jcmvbkbc, Jason Wang, qemu-devel, Stefan Hajnoczi
On Tue, 07/14 15:42, Michael S. Tsirkin wrote:
> On Tue, Jul 14, 2015 at 01:20:03PM +0100, Stefan Hajnoczi wrote:
> > On Fri, Jul 10, 2015 at 05:03:22PM +0800, Fam Zheng wrote:
> > > On Tue, 07/07 12:19, Michael S. Tsirkin wrote:
> > > > On Tue, Jul 07, 2015 at 05:09:09PM +0800, Fam Zheng wrote:
> > > > > On Tue, 07/07 11:13, Michael S. Tsirkin wrote:
> > > > > > On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> > > > > > > Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > > > > > > net queues need to be explicitly flushed after qemu_can_send_packet()
> > > > > > > returns false, because the netdev side will disable the polling of fd.
> > > > > > >
> > > > > > > This fixes the case of "cont" after "stop" (or migration).
> > > > > > >
> > > > > > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > > > >
> > > > > > Note virtio has its own handler which must be used to
> > > > > > flush packets - this one might run too early or too late.
> > > > >
> > > > > Which handler do you mean? I don't think virtio-net handles resume now. (If it
> > > > > does, we probably should drop it together with this change, since it's needed
> > > > > by as all NICs.)
> > > > >
> > > > > Fam
> > > >
> > > > virtio_vmstate_change
> > > >
> > > > It's all far from trivial. I suspect these whack-a-mole approach
> > > > spreading purge here and there will only create more bugs.
> > > >
> > > > Why would we ever need to process network packets when
> > > > VM is not running? I don't see any point to it.
> > > > How about we simply stop the job processing network on
> > > > vm stop and restart on vm start?
> > >
> > > I suppose it is too much for 2.4. I think this approach, adding
> > > qemu_flush_queued_packets(), is consistent with its existing usage (when a
> > > device is becoming active from inactive), like in e1000_write_config.
> > >
> > > How about applying this and let's work on "stopping tap when VM not running"
> > > for 2.5?
> >
> > Jason has gone happy on this and the virtio-net .can_receive() patch.
> >
> > Michael: Any further comments? Are you okay with this patch too?
FWIW, I am sending another virtio-net patch to flush at .set_status.
>
> I think it doesn't help virtio - am I wrong?
>
Right, virtio doesn't need this because it gets notified with '.set_status()'.
But this patch does help:
http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg01396.html
(as well as all other NICs).
Fam
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-14 12:20 ` Stefan Hajnoczi
@ 2015-07-14 12:42 ` Michael S. Tsirkin
2015-07-15 2:50 ` Fam Zheng
0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-07-14 12:42 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: jcmvbkbc, Jason Wang, Fam Zheng, qemu-devel
On Tue, Jul 14, 2015 at 01:20:03PM +0100, Stefan Hajnoczi wrote:
> On Fri, Jul 10, 2015 at 05:03:22PM +0800, Fam Zheng wrote:
> > On Tue, 07/07 12:19, Michael S. Tsirkin wrote:
> > > On Tue, Jul 07, 2015 at 05:09:09PM +0800, Fam Zheng wrote:
> > > > On Tue, 07/07 11:13, Michael S. Tsirkin wrote:
> > > > > On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> > > > > > Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > > > > > net queues need to be explicitly flushed after qemu_can_send_packet()
> > > > > > returns false, because the netdev side will disable the polling of fd.
> > > > > >
> > > > > > This fixes the case of "cont" after "stop" (or migration).
> > > > > >
> > > > > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > > >
> > > > > Note virtio has its own handler which must be used to
> > > > > flush packets - this one might run too early or too late.
> > > >
> > > > Which handler do you mean? I don't think virtio-net handles resume now. (If it
> > > > does, we probably should drop it together with this change, since it's needed
> > > > by as all NICs.)
> > > >
> > > > Fam
> > >
> > > virtio_vmstate_change
> > >
> > > It's all far from trivial. I suspect these whack-a-mole approach
> > > spreading purge here and there will only create more bugs.
> > >
> > > Why would we ever need to process network packets when
> > > VM is not running? I don't see any point to it.
> > > How about we simply stop the job processing network on
> > > vm stop and restart on vm start?
> >
> > I suppose it is too much for 2.4. I think this approach, adding
> > qemu_flush_queued_packets(), is consistent with its existing usage (when a
> > device is becoming active from inactive), like in e1000_write_config.
> >
> > How about applying this and let's work on "stopping tap when VM not running"
> > for 2.5?
>
> Jason has gone happy on this and the virtio-net .can_receive() patch.
>
> Michael: Any further comments? Are you okay with this patch too?
I think it doesn't help virtio - am I wrong?
--
MST
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-10 9:03 ` Fam Zheng
@ 2015-07-14 12:20 ` Stefan Hajnoczi
2015-07-14 12:42 ` Michael S. Tsirkin
0 siblings, 1 reply; 18+ messages in thread
From: Stefan Hajnoczi @ 2015-07-14 12:20 UTC (permalink / raw)
To: Fam Zheng; +Cc: jcmvbkbc, Jason Wang, qemu-devel, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]
On Fri, Jul 10, 2015 at 05:03:22PM +0800, Fam Zheng wrote:
> On Tue, 07/07 12:19, Michael S. Tsirkin wrote:
> > On Tue, Jul 07, 2015 at 05:09:09PM +0800, Fam Zheng wrote:
> > > On Tue, 07/07 11:13, Michael S. Tsirkin wrote:
> > > > On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> > > > > Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > > > > net queues need to be explicitly flushed after qemu_can_send_packet()
> > > > > returns false, because the netdev side will disable the polling of fd.
> > > > >
> > > > > This fixes the case of "cont" after "stop" (or migration).
> > > > >
> > > > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > >
> > > > Note virtio has its own handler which must be used to
> > > > flush packets - this one might run too early or too late.
> > >
> > > Which handler do you mean? I don't think virtio-net handles resume now. (If it
> > > does, we probably should drop it together with this change, since it's needed
> > > by as all NICs.)
> > >
> > > Fam
> >
> > virtio_vmstate_change
> >
> > It's all far from trivial. I suspect these whack-a-mole approach
> > spreading purge here and there will only create more bugs.
> >
> > Why would we ever need to process network packets when
> > VM is not running? I don't see any point to it.
> > How about we simply stop the job processing network on
> > vm stop and restart on vm start?
>
> I suppose it is too much for 2.4. I think this approach, adding
> qemu_flush_queued_packets(), is consistent with its existing usage (when a
> device is becoming active from inactive), like in e1000_write_config.
>
> How about applying this and let's work on "stopping tap when VM not running"
> for 2.5?
Jason has gone happy on this and the virtio-net .can_receive() patch.
Michael: Any further comments? Are you okay with this patch too?
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 9:19 ` Michael S. Tsirkin
@ 2015-07-10 9:03 ` Fam Zheng
2015-07-14 12:20 ` Stefan Hajnoczi
0 siblings, 1 reply; 18+ messages in thread
From: Fam Zheng @ 2015-07-10 9:03 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jcmvbkbc, Jason Wang, qemu-devel, Stefan Hajnoczi
On Tue, 07/07 12:19, Michael S. Tsirkin wrote:
> On Tue, Jul 07, 2015 at 05:09:09PM +0800, Fam Zheng wrote:
> > On Tue, 07/07 11:13, Michael S. Tsirkin wrote:
> > > On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> > > > Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > > > net queues need to be explicitly flushed after qemu_can_send_packet()
> > > > returns false, because the netdev side will disable the polling of fd.
> > > >
> > > > This fixes the case of "cont" after "stop" (or migration).
> > > >
> > > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > >
> > > Note virtio has its own handler which must be used to
> > > flush packets - this one might run too early or too late.
> >
> > Which handler do you mean? I don't think virtio-net handles resume now. (If it
> > does, we probably should drop it together with this change, since it's needed
> > by as all NICs.)
> >
> > Fam
>
> virtio_vmstate_change
>
> It's all far from trivial. I suspect these whack-a-mole approach
> spreading purge here and there will only create more bugs.
>
> Why would we ever need to process network packets when
> VM is not running? I don't see any point to it.
> How about we simply stop the job processing network on
> vm stop and restart on vm start?
I suppose it is too much for 2.4. I think this approach, adding
qemu_flush_queued_packets(), is consistent with its existing usage (when a
device is becoming active from inactive), like in e1000_write_config.
How about applying this and let's work on "stopping tap when VM not running"
for 2.5?
Fam
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 9:10 ` Michael S. Tsirkin
2015-07-07 10:02 ` Fam Zheng
@ 2015-07-09 9:25 ` Stefan Hajnoczi
1 sibling, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2015-07-09 9:25 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jcmvbkbc, Jason Wang, Fam Zheng, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]
On Tue, Jul 07, 2015 at 12:10:15PM +0300, Michael S. Tsirkin wrote:
> On Tue, Jul 07, 2015 at 04:58:29PM +0800, Jason Wang wrote:
> >
> >
> > On 07/07/2015 04:13 PM, Michael S. Tsirkin wrote:
> > > On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> > >> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > >> net queues need to be explicitly flushed after qemu_can_send_packet()
> > >> returns false, because the netdev side will disable the polling of fd.
> > >>
> > >> This fixes the case of "cont" after "stop" (or migration).
> > >>
> > >> Signed-off-by: Fam Zheng <famz@redhat.com>
> > > Note virtio has its own handler which must be used to
> > > flush packets - this one might run too early or too late.
> >
> > If runs too realy (DRIVER_OK is not set), then: packet will be dropped
> > (if this patch is used with "drop virtio_net_can_receive()", or the
> > queue will be purged since qemu_can_send_packet() returns false. If too
> > late, at least tap read poll will be enabled. So still looks ok?
>
> It's still not helpful for virtio. So which cards are fixed?
>
> I just find the comment
> This fixes the case of "cont" after "stop" (or migration)
> too vague.
> Can you please include the info about what's broken in the commit log?
Max Fillipov reported that it's needed to fix the gdb stub:
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg01015.html
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 9:03 ` Fam Zheng
@ 2015-07-08 9:40 ` Jason Wang
0 siblings, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-07-08 9:40 UTC (permalink / raw)
To: Fam Zheng; +Cc: jcmvbkbc, qemu-devel, Stefan Hajnoczi, mst
On 07/07/2015 05:03 PM, Fam Zheng wrote:
> On Tue, 07/07 15:44, Jason Wang wrote:
>>
>> On 07/07/2015 09:21 AM, Fam Zheng wrote:
>>> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
>>> net queues need to be explicitly flushed after qemu_can_send_packet()
>>> returns false, because the netdev side will disable the polling of fd.
>>>
>>> This fixes the case of "cont" after "stop" (or migration).
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>
>>> ---
>>>
>>> v2: Unify with VM stop handler. (Stefan)
>>> ---
>>> net/net.c | 19 ++++++++++++-------
>>> 1 file changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 6ff7fec..28a5597 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
>>> static void net_vm_change_state_handler(void *opaque, int running,
>>> RunState state)
>>> {
>>> - /* Complete all queued packets, to guarantee we don't modify
>>> - * state later when VM is not running.
>>> - */
>>> - if (!running) {
>>> - NetClientState *nc;
>>> - NetClientState *tmp;
>>> + NetClientState *nc;
>>> + NetClientState *tmp;
>>>
>>> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
>>> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
>>> + if (running) {
>>> + /* Flush queued packets and wake up backends. */
>>> + if (nc->peer && qemu_can_send_packet(nc)) {
>>> + qemu_flush_queued_packets(nc->peer);
>>> + }
>>> + } else {
>>> + /* Complete all queued packets, to guarantee we don't modify
>>> + * state later when VM is not running.
>>> + */
>>> qemu_flush_or_purge_queued_packets(nc, true);
>>> }
>> Looks like qemu_can_send_packet() checks both nc->peer and runstate. So
>> probably, we can simplify this to:
>>
>> if (qemu_can_send_packet(nc))
>> qemu_flush_queued_packets(nc->peer);
>> else
>> qemu_flush_or_purge_queued_packets(nc, true);
>>
>>> }
> qemu_can_send_packet returns 1 if !nc->peer, so this doesn't work.
>
> Fam
Yes, I was wrong.
Btw, instead of depending on vm handler (which seems racy with other
state change handler). Can we do this in places like vm_start() and
vm_stop(). Like we drain and flush block queue during vm stop.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 9:10 ` Michael S. Tsirkin
@ 2015-07-07 10:02 ` Fam Zheng
2015-07-09 9:25 ` Stefan Hajnoczi
1 sibling, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-07 10:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jcmvbkbc, Jason Wang, qemu-devel, Stefan Hajnoczi
On Tue, 07/07 12:10, Michael S. Tsirkin wrote:
> On Tue, Jul 07, 2015 at 04:58:29PM +0800, Jason Wang wrote:
> >
> >
> > On 07/07/2015 04:13 PM, Michael S. Tsirkin wrote:
> > > On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> > >> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > >> net queues need to be explicitly flushed after qemu_can_send_packet()
> > >> returns false, because the netdev side will disable the polling of fd.
> > >>
> > >> This fixes the case of "cont" after "stop" (or migration).
> > >>
> > >> Signed-off-by: Fam Zheng <famz@redhat.com>
> > > Note virtio has its own handler which must be used to
> > > flush packets - this one might run too early or too late.
> >
> > If runs too realy (DRIVER_OK is not set), then: packet will be dropped
> > (if this patch is used with "drop virtio_net_can_receive()", or the
> > queue will be purged since qemu_can_send_packet() returns false. If too
> > late, at least tap read poll will be enabled. So still looks ok?
>
> It's still not helpful for virtio. So which cards are fixed?
>
> I just find the comment
> This fixes the case of "cont" after "stop" (or migration)
> too vague.
> Can you please include the info about what's broken in the commit log?
It does fix virtio-net, as well as most other cards. HMP "stop" then "cont"
breaks net, tested with host pinging guest while doing it.
Fam
>
> > >
> > >> ---
> > >>
> > >> v2: Unify with VM stop handler. (Stefan)
> > >> ---
> > >> net/net.c | 19 ++++++++++++-------
> > >> 1 file changed, 12 insertions(+), 7 deletions(-)
> > >>
> > >> diff --git a/net/net.c b/net/net.c
> > >> index 6ff7fec..28a5597 100644
> > >> --- a/net/net.c
> > >> +++ b/net/net.c
> > >> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> > >> static void net_vm_change_state_handler(void *opaque, int running,
> > >> RunState state)
> > >> {
> > >> - /* Complete all queued packets, to guarantee we don't modify
> > >> - * state later when VM is not running.
> > >> - */
> > >> - if (!running) {
> > >> - NetClientState *nc;
> > >> - NetClientState *tmp;
> > >> + NetClientState *nc;
> > >> + NetClientState *tmp;
> > >>
> > >> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> > >> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> > >> + if (running) {
> > >> + /* Flush queued packets and wake up backends. */
> > >> + if (nc->peer && qemu_can_send_packet(nc)) {
> > >> + qemu_flush_queued_packets(nc->peer);
> > >> + }
> > >> + } else {
> > >> + /* Complete all queued packets, to guarantee we don't modify
> > >> + * state later when VM is not running.
> > >> + */
> > >> qemu_flush_or_purge_queued_packets(nc, true);
> > >> }
> > >> }
> > >> --
> > >> 2.4.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 9:09 ` Fam Zheng
@ 2015-07-07 9:19 ` Michael S. Tsirkin
2015-07-10 9:03 ` Fam Zheng
0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-07-07 9:19 UTC (permalink / raw)
To: Fam Zheng; +Cc: jcmvbkbc, Jason Wang, qemu-devel, Stefan Hajnoczi
On Tue, Jul 07, 2015 at 05:09:09PM +0800, Fam Zheng wrote:
> On Tue, 07/07 11:13, Michael S. Tsirkin wrote:
> > On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> > > Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > > net queues need to be explicitly flushed after qemu_can_send_packet()
> > > returns false, because the netdev side will disable the polling of fd.
> > >
> > > This fixes the case of "cont" after "stop" (or migration).
> > >
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> >
> > Note virtio has its own handler which must be used to
> > flush packets - this one might run too early or too late.
>
> Which handler do you mean? I don't think virtio-net handles resume now. (If it
> does, we probably should drop it together with this change, since it's needed
> by as all NICs.)
>
> Fam
virtio_vmstate_change
It's all far from trivial. I suspect these whack-a-mole approach
spreading purge here and there will only create more bugs.
Why would we ever need to process network packets when
VM is not running? I don't see any point to it.
How about we simply stop the job processing network on
vm stop and restart on vm start?
> >
> > > ---
> > >
> > > v2: Unify with VM stop handler. (Stefan)
> > > ---
> > > net/net.c | 19 ++++++++++++-------
> > > 1 file changed, 12 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/net/net.c b/net/net.c
> > > index 6ff7fec..28a5597 100644
> > > --- a/net/net.c
> > > +++ b/net/net.c
> > > @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> > > static void net_vm_change_state_handler(void *opaque, int running,
> > > RunState state)
> > > {
> > > - /* Complete all queued packets, to guarantee we don't modify
> > > - * state later when VM is not running.
> > > - */
> > > - if (!running) {
> > > - NetClientState *nc;
> > > - NetClientState *tmp;
> > > + NetClientState *nc;
> > > + NetClientState *tmp;
> > >
> > > - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> > > + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> > > + if (running) {
> > > + /* Flush queued packets and wake up backends. */
> > > + if (nc->peer && qemu_can_send_packet(nc)) {
> > > + qemu_flush_queued_packets(nc->peer);
> > > + }
> > > + } else {
> > > + /* Complete all queued packets, to guarantee we don't modify
> > > + * state later when VM is not running.
> > > + */
> > > qemu_flush_or_purge_queued_packets(nc, true);
> > > }
> > > }
> > > --
> > > 2.4.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 8:58 ` Jason Wang
@ 2015-07-07 9:10 ` Michael S. Tsirkin
2015-07-07 10:02 ` Fam Zheng
2015-07-09 9:25 ` Stefan Hajnoczi
0 siblings, 2 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-07-07 9:10 UTC (permalink / raw)
To: Jason Wang; +Cc: jcmvbkbc, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Tue, Jul 07, 2015 at 04:58:29PM +0800, Jason Wang wrote:
>
>
> On 07/07/2015 04:13 PM, Michael S. Tsirkin wrote:
> > On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> >> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> >> net queues need to be explicitly flushed after qemu_can_send_packet()
> >> returns false, because the netdev side will disable the polling of fd.
> >>
> >> This fixes the case of "cont" after "stop" (or migration).
> >>
> >> Signed-off-by: Fam Zheng <famz@redhat.com>
> > Note virtio has its own handler which must be used to
> > flush packets - this one might run too early or too late.
>
> If runs too realy (DRIVER_OK is not set), then: packet will be dropped
> (if this patch is used with "drop virtio_net_can_receive()", or the
> queue will be purged since qemu_can_send_packet() returns false. If too
> late, at least tap read poll will be enabled. So still looks ok?
It's still not helpful for virtio. So which cards are fixed?
I just find the comment
This fixes the case of "cont" after "stop" (or migration)
too vague.
Can you please include the info about what's broken in the commit log?
> >
> >> ---
> >>
> >> v2: Unify with VM stop handler. (Stefan)
> >> ---
> >> net/net.c | 19 ++++++++++++-------
> >> 1 file changed, 12 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/net/net.c b/net/net.c
> >> index 6ff7fec..28a5597 100644
> >> --- a/net/net.c
> >> +++ b/net/net.c
> >> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> >> static void net_vm_change_state_handler(void *opaque, int running,
> >> RunState state)
> >> {
> >> - /* Complete all queued packets, to guarantee we don't modify
> >> - * state later when VM is not running.
> >> - */
> >> - if (!running) {
> >> - NetClientState *nc;
> >> - NetClientState *tmp;
> >> + NetClientState *nc;
> >> + NetClientState *tmp;
> >>
> >> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> >> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> >> + if (running) {
> >> + /* Flush queued packets and wake up backends. */
> >> + if (nc->peer && qemu_can_send_packet(nc)) {
> >> + qemu_flush_queued_packets(nc->peer);
> >> + }
> >> + } else {
> >> + /* Complete all queued packets, to guarantee we don't modify
> >> + * state later when VM is not running.
> >> + */
> >> qemu_flush_or_purge_queued_packets(nc, true);
> >> }
> >> }
> >> --
> >> 2.4.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 8:13 ` Michael S. Tsirkin
2015-07-07 8:58 ` Jason Wang
@ 2015-07-07 9:09 ` Fam Zheng
2015-07-07 9:19 ` Michael S. Tsirkin
1 sibling, 1 reply; 18+ messages in thread
From: Fam Zheng @ 2015-07-07 9:09 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jcmvbkbc, Jason Wang, qemu-devel, Stefan Hajnoczi
On Tue, 07/07 11:13, Michael S. Tsirkin wrote:
> On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> > Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > net queues need to be explicitly flushed after qemu_can_send_packet()
> > returns false, because the netdev side will disable the polling of fd.
> >
> > This fixes the case of "cont" after "stop" (or migration).
> >
> > Signed-off-by: Fam Zheng <famz@redhat.com>
>
> Note virtio has its own handler which must be used to
> flush packets - this one might run too early or too late.
Which handler do you mean? I don't think virtio-net handles resume now. (If it
does, we probably should drop it together with this change, since it's needed
by as all NICs.)
Fam
>
> > ---
> >
> > v2: Unify with VM stop handler. (Stefan)
> > ---
> > net/net.c | 19 ++++++++++++-------
> > 1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 6ff7fec..28a5597 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> > static void net_vm_change_state_handler(void *opaque, int running,
> > RunState state)
> > {
> > - /* Complete all queued packets, to guarantee we don't modify
> > - * state later when VM is not running.
> > - */
> > - if (!running) {
> > - NetClientState *nc;
> > - NetClientState *tmp;
> > + NetClientState *nc;
> > + NetClientState *tmp;
> >
> > - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> > + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> > + if (running) {
> > + /* Flush queued packets and wake up backends. */
> > + if (nc->peer && qemu_can_send_packet(nc)) {
> > + qemu_flush_queued_packets(nc->peer);
> > + }
> > + } else {
> > + /* Complete all queued packets, to guarantee we don't modify
> > + * state later when VM is not running.
> > + */
> > qemu_flush_or_purge_queued_packets(nc, true);
> > }
> > }
> > --
> > 2.4.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 7:44 ` Jason Wang
@ 2015-07-07 9:03 ` Fam Zheng
2015-07-08 9:40 ` Jason Wang
0 siblings, 1 reply; 18+ messages in thread
From: Fam Zheng @ 2015-07-07 9:03 UTC (permalink / raw)
To: Jason Wang; +Cc: jcmvbkbc, qemu-devel, Stefan Hajnoczi, mst
On Tue, 07/07 15:44, Jason Wang wrote:
>
>
> On 07/07/2015 09:21 AM, Fam Zheng wrote:
> > Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> > net queues need to be explicitly flushed after qemu_can_send_packet()
> > returns false, because the netdev side will disable the polling of fd.
> >
> > This fixes the case of "cont" after "stop" (or migration).
> >
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> >
> > ---
> >
> > v2: Unify with VM stop handler. (Stefan)
> > ---
> > net/net.c | 19 ++++++++++++-------
> > 1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 6ff7fec..28a5597 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> > static void net_vm_change_state_handler(void *opaque, int running,
> > RunState state)
> > {
> > - /* Complete all queued packets, to guarantee we don't modify
> > - * state later when VM is not running.
> > - */
> > - if (!running) {
> > - NetClientState *nc;
> > - NetClientState *tmp;
> > + NetClientState *nc;
> > + NetClientState *tmp;
> >
> > - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> > + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> > + if (running) {
> > + /* Flush queued packets and wake up backends. */
> > + if (nc->peer && qemu_can_send_packet(nc)) {
> > + qemu_flush_queued_packets(nc->peer);
> > + }
> > + } else {
> > + /* Complete all queued packets, to guarantee we don't modify
> > + * state later when VM is not running.
> > + */
> > qemu_flush_or_purge_queued_packets(nc, true);
> > }
>
> Looks like qemu_can_send_packet() checks both nc->peer and runstate. So
> probably, we can simplify this to:
>
> if (qemu_can_send_packet(nc))
> qemu_flush_queued_packets(nc->peer);
> else
> qemu_flush_or_purge_queued_packets(nc, true);
>
> > }
>
qemu_can_send_packet returns 1 if !nc->peer, so this doesn't work.
Fam
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 8:13 ` Michael S. Tsirkin
@ 2015-07-07 8:58 ` Jason Wang
2015-07-07 9:10 ` Michael S. Tsirkin
2015-07-07 9:09 ` Fam Zheng
1 sibling, 1 reply; 18+ messages in thread
From: Jason Wang @ 2015-07-07 8:58 UTC (permalink / raw)
To: Michael S. Tsirkin, Fam Zheng; +Cc: jcmvbkbc, qemu-devel, Stefan Hajnoczi
On 07/07/2015 04:13 PM, Michael S. Tsirkin wrote:
> On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
>> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
>> net queues need to be explicitly flushed after qemu_can_send_packet()
>> returns false, because the netdev side will disable the polling of fd.
>>
>> This fixes the case of "cont" after "stop" (or migration).
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
> Note virtio has its own handler which must be used to
> flush packets - this one might run too early or too late.
If runs too realy (DRIVER_OK is not set), then: packet will be dropped
(if this patch is used with "drop virtio_net_can_receive()", or the
queue will be purged since qemu_can_send_packet() returns false. If too
late, at least tap read poll will be enabled. So still looks ok?
>
>> ---
>>
>> v2: Unify with VM stop handler. (Stefan)
>> ---
>> net/net.c | 19 ++++++++++++-------
>> 1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/net.c b/net/net.c
>> index 6ff7fec..28a5597 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
>> static void net_vm_change_state_handler(void *opaque, int running,
>> RunState state)
>> {
>> - /* Complete all queued packets, to guarantee we don't modify
>> - * state later when VM is not running.
>> - */
>> - if (!running) {
>> - NetClientState *nc;
>> - NetClientState *tmp;
>> + NetClientState *nc;
>> + NetClientState *tmp;
>>
>> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
>> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
>> + if (running) {
>> + /* Flush queued packets and wake up backends. */
>> + if (nc->peer && qemu_can_send_packet(nc)) {
>> + qemu_flush_queued_packets(nc->peer);
>> + }
>> + } else {
>> + /* Complete all queued packets, to guarantee we don't modify
>> + * state later when VM is not running.
>> + */
>> qemu_flush_or_purge_queued_packets(nc, true);
>> }
>> }
>> --
>> 2.4.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 1:21 Fam Zheng
2015-07-07 7:44 ` Jason Wang
2015-07-07 8:13 ` Michael S. Tsirkin
@ 2015-07-07 8:51 ` Stefan Hajnoczi
2 siblings, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2015-07-07 8:51 UTC (permalink / raw)
To: Fam Zheng; +Cc: jcmvbkbc, Jason Wang, qemu-devel, mst
[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]
On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> net queues need to be explicitly flushed after qemu_can_send_packet()
> returns false, because the netdev side will disable the polling of fd.
>
> This fixes the case of "cont" after "stop" (or migration).
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
>
> v2: Unify with VM stop handler. (Stefan)
Thanks! I'm happy with this but I'll wait for you to respond to Jason's
comment.
> ---
> net/net.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/net/net.c b/net/net.c
> index 6ff7fec..28a5597 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> static void net_vm_change_state_handler(void *opaque, int running,
> RunState state)
> {
> - /* Complete all queued packets, to guarantee we don't modify
> - * state later when VM is not running.
> - */
> - if (!running) {
> - NetClientState *nc;
> - NetClientState *tmp;
> + NetClientState *nc;
> + NetClientState *tmp;
>
> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> + if (running) {
> + /* Flush queued packets and wake up backends. */
> + if (nc->peer && qemu_can_send_packet(nc)) {
> + qemu_flush_queued_packets(nc->peer);
> + }
> + } else {
> + /* Complete all queued packets, to guarantee we don't modify
> + * state later when VM is not running.
> + */
> qemu_flush_or_purge_queued_packets(nc, true);
> }
> }
> --
> 2.4.3
>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 1:21 Fam Zheng
2015-07-07 7:44 ` Jason Wang
@ 2015-07-07 8:13 ` Michael S. Tsirkin
2015-07-07 8:58 ` Jason Wang
2015-07-07 9:09 ` Fam Zheng
2015-07-07 8:51 ` Stefan Hajnoczi
2 siblings, 2 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-07-07 8:13 UTC (permalink / raw)
To: Fam Zheng; +Cc: jcmvbkbc, Jason Wang, qemu-devel, Stefan Hajnoczi
On Tue, Jul 07, 2015 at 09:21:07AM +0800, Fam Zheng wrote:
> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> net queues need to be explicitly flushed after qemu_can_send_packet()
> returns false, because the netdev side will disable the polling of fd.
>
> This fixes the case of "cont" after "stop" (or migration).
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
Note virtio has its own handler which must be used to
flush packets - this one might run too early or too late.
> ---
>
> v2: Unify with VM stop handler. (Stefan)
> ---
> net/net.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/net/net.c b/net/net.c
> index 6ff7fec..28a5597 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> static void net_vm_change_state_handler(void *opaque, int running,
> RunState state)
> {
> - /* Complete all queued packets, to guarantee we don't modify
> - * state later when VM is not running.
> - */
> - if (!running) {
> - NetClientState *nc;
> - NetClientState *tmp;
> + NetClientState *nc;
> + NetClientState *tmp;
>
> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> + if (running) {
> + /* Flush queued packets and wake up backends. */
> + if (nc->peer && qemu_can_send_packet(nc)) {
> + qemu_flush_queued_packets(nc->peer);
> + }
> + } else {
> + /* Complete all queued packets, to guarantee we don't modify
> + * state later when VM is not running.
> + */
> qemu_flush_or_purge_queued_packets(nc, true);
> }
> }
> --
> 2.4.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
2015-07-07 1:21 Fam Zheng
@ 2015-07-07 7:44 ` Jason Wang
2015-07-07 9:03 ` Fam Zheng
2015-07-07 8:13 ` Michael S. Tsirkin
2015-07-07 8:51 ` Stefan Hajnoczi
2 siblings, 1 reply; 18+ messages in thread
From: Jason Wang @ 2015-07-07 7:44 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: jcmvbkbc, Stefan Hajnoczi, mst
On 07/07/2015 09:21 AM, Fam Zheng wrote:
> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
> net queues need to be explicitly flushed after qemu_can_send_packet()
> returns false, because the netdev side will disable the polling of fd.
>
> This fixes the case of "cont" after "stop" (or migration).
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
>
> v2: Unify with VM stop handler. (Stefan)
> ---
> net/net.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/net/net.c b/net/net.c
> index 6ff7fec..28a5597 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
> static void net_vm_change_state_handler(void *opaque, int running,
> RunState state)
> {
> - /* Complete all queued packets, to guarantee we don't modify
> - * state later when VM is not running.
> - */
> - if (!running) {
> - NetClientState *nc;
> - NetClientState *tmp;
> + NetClientState *nc;
> + NetClientState *tmp;
>
> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> + if (running) {
> + /* Flush queued packets and wake up backends. */
> + if (nc->peer && qemu_can_send_packet(nc)) {
> + qemu_flush_queued_packets(nc->peer);
> + }
> + } else {
> + /* Complete all queued packets, to guarantee we don't modify
> + * state later when VM is not running.
> + */
> qemu_flush_or_purge_queued_packets(nc, true);
> }
Looks like qemu_can_send_packet() checks both nc->peer and runstate. So
probably, we can simplify this to:
if (qemu_can_send_packet(nc))
qemu_flush_queued_packets(nc->peer);
else
qemu_flush_or_purge_queued_packets(nc, true);
> }
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes
@ 2015-07-07 1:21 Fam Zheng
2015-07-07 7:44 ` Jason Wang
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-07 1:21 UTC (permalink / raw)
To: qemu-devel; +Cc: jcmvbkbc, Jason Wang, Stefan Hajnoczi, mst
Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
net queues need to be explicitly flushed after qemu_can_send_packet()
returns false, because the netdev side will disable the polling of fd.
This fixes the case of "cont" after "stop" (or migration).
Signed-off-by: Fam Zheng <famz@redhat.com>
---
v2: Unify with VM stop handler. (Stefan)
---
net/net.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/net/net.c b/net/net.c
index 6ff7fec..28a5597 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, Error **errp)
static void net_vm_change_state_handler(void *opaque, int running,
RunState state)
{
- /* Complete all queued packets, to guarantee we don't modify
- * state later when VM is not running.
- */
- if (!running) {
- NetClientState *nc;
- NetClientState *tmp;
+ NetClientState *nc;
+ NetClientState *tmp;
- QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
+ QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
+ if (running) {
+ /* Flush queued packets and wake up backends. */
+ if (nc->peer && qemu_can_send_packet(nc)) {
+ qemu_flush_queued_packets(nc->peer);
+ }
+ } else {
+ /* Complete all queued packets, to guarantee we don't modify
+ * state later when VM is not running.
+ */
qemu_flush_or_purge_queued_packets(nc, true);
}
}
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-07-15 2:50 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-10 9:24 [Qemu-devel] [PATCH v2] net: Flush queued packets when guest resumes Fam Zheng
2015-07-13 5:21 ` Jason Wang
-- strict thread matches above, loose matches on Subject: below --
2015-07-07 1:21 Fam Zheng
2015-07-07 7:44 ` Jason Wang
2015-07-07 9:03 ` Fam Zheng
2015-07-08 9:40 ` Jason Wang
2015-07-07 8:13 ` Michael S. Tsirkin
2015-07-07 8:58 ` Jason Wang
2015-07-07 9:10 ` Michael S. Tsirkin
2015-07-07 10:02 ` Fam Zheng
2015-07-09 9:25 ` Stefan Hajnoczi
2015-07-07 9:09 ` Fam Zheng
2015-07-07 9:19 ` Michael S. Tsirkin
2015-07-10 9:03 ` Fam Zheng
2015-07-14 12:20 ` Stefan Hajnoczi
2015-07-14 12:42 ` Michael S. Tsirkin
2015-07-15 2:50 ` Fam Zheng
2015-07-07 8:51 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.