From: Jan Kiszka <jan.kiszka@siemens.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] net/tap.c: Possibly a way to stall tap input
Date: Fri, 02 Aug 2013 18:49:11 +0200 [thread overview]
Message-ID: <51FBE307.40201@siemens.com> (raw)
In-Reply-To: <51FBA9FE.9050505@siemens.com>
On 2013-08-02 14:45, Jan Kiszka wrote:
> On 2013-08-02 13:46, Stefan Hajnoczi wrote:
>> On Thu, Aug 01, 2013 at 07:15:54PM +0200, Jan Kiszka wrote:
>>> I was digging into the involved code and found something fishy:
>>>
>>> net/tap.c:
>>> static void tap_send(void *opaque)
>>> {
>>> ...
>>> size = qemu_send_packet_async(&s->nc, buf, size,
>>> tap_send_completed);
>>> if (size == 0) {
>>> tap_read_poll(s, false);
>>> }
>>>
>>> So, if tap_send is registered for the mainloop polling (ie. can_receive
>>> returned true before starting to poll) but qemu_send_packet_async
>>> returns 0 now as qemu_can_send_packet/can_receive happens to report
>>> false in the meantime, we will disable read polling. If also write
>>> polling is off, the fd will be completely removed from the iohandler
>>> list. But even if write polling remains on, I wonder what should bring
>>> read polling back?
>>
>> This behavior seems fine to me. Once the peer (pcnet) is able to
>> receive again it must flush the queue, this will re-enable
>> tap_read_poll().
>>
>> Can you explain a bit more why this would be a problem?
>
> The problem is that I don't see at all what will call tap_read_poll(s,
> 1), neither in theory nor in reality.
>
> As long as the real test case is out of reach, I tried to emulate the
> faulty behaviour by letting tap_can_send always return 1. Result:
> reception stalls during boot as even qemu_flush_queued_packets cannot
> get it running again once tap_read_poll(s, 0) was called.
OK, this is the bug: When a NIC becomes ready to send or receive again,
the issued qemu_flush_queued_packets will only flush queued packets that
are supposed to leave the NIC, none that may have been queued at the
output of the corresponding backend. For the case of hub-based setups,
we need to propagate this flush via the hub to all attached peers. This
flush will trigger the send callback of tap, and that will re-enable
receive polling.
So this is actually a generic bug that should theoretically affect any
user space NIC, with or without a hub in the middle. I'll cook up a fix,
play with it on Monday and share the outcome.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
next prev parent reply other threads:[~2013-08-02 19:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-01 17:15 [Qemu-devel] net/tap.c: Possibly a way to stall tap input Jan Kiszka
2013-08-01 17:24 ` Jan Kiszka
2013-08-02 7:33 ` Stefan Hajnoczi
2013-08-02 7:45 ` Jan Kiszka
2013-08-02 11:46 ` Stefan Hajnoczi
2013-08-02 12:45 ` Jan Kiszka
2013-08-02 16:49 ` Jan Kiszka [this message]
2013-08-02 19:41 ` Jan Kiszka
2013-08-05 11:38 ` Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51FBE307.40201@siemens.com \
--to=jan.kiszka@siemens.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).