qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Zhi Yong Wu <zwu.kernel@gmail.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>,
	Benjamin <mlspirat42@gmail.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [QEMU] net: adapt dump to support the new syntax
Date: Thu, 17 May 2012 15:29:31 +0200	[thread overview]
Message-ID: <4FB4FD3B.7070007@redhat.com> (raw)
In-Reply-To: <CAEH94Lj0ki1pV3x=twzY9pZX-t+LDAeddpq+zV=L+FrGOmg=-g@mail.gmail.com>

Il 17/05/2012 12:05, Zhi Yong Wu ha scritto:
> On Thu, May 17, 2012 at 5:51 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> This case has existed in current upstream code, not only vlan-hub
>>> code. Currently can_send function has been called by backend send
>>> function before deliver/deliver_iov, If we put can_send in queue send
>>> function, your idea will have a big challenge for slirp packet queue.
>>
>> Exactly why?  For SLIRP's receive path, SLIRP doesn't implement
>> can_receive at all so it will never block.  For the send path, when flow
>> control kicks qemu_net_queue_append will copy the packet so it is not a
>> problem for SLIRP's stack-allocated packets.
>
> You know that qemu_send_packet is void type, and has return value, if
> can_send fails, if_encap will not currently get expected return value,
> so this will cause that we have to modify the definition of
> qemu_send_packet to make it return one valid value. a lot of functions
> have called it, so i would not like to modify its definition.

If qemu_can_send_packet returns false of course I would not drop the
packet.  I would append it to the queue (qemu_net_queue_append) for
later processing.

>>> We can implement your idea below later, not in this patchset. What do
>>> you think?
>>
>> Note that my idea above was only means to an end.  If you can remove the
>> TODOs in a convincing manner, that would be fine.
> You mean that we need adopt another handling way? or directly TODO comments?

You need to address the TODOs.  I proposed a way.  It may actually not
be correct, and there may of course be others.  But the TODOs are there,
and have to be solved.

Paolo

  reply	other threads:[~2012-05-17 13:29 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <4F1EF6CE.9060306@gmail.com>
2012-01-24 10:42 ` [Qemu-devel] [QEMU] net: adapt dump to support the new syntax Stefan Hajnoczi
2012-01-24 11:58   ` Markus Armbruster
2012-02-05 16:30     ` Benjamin
2012-02-05 22:18   ` Paolo Bonzini
2012-05-17  5:24     ` Zhi Yong Wu
2012-05-17  5:59     ` Zhi Yong Wu
2012-05-17  9:51       ` Paolo Bonzini
2012-05-17 10:05         ` Zhi Yong Wu
2012-05-17 13:29           ` Paolo Bonzini [this message]
2012-02-17  6:54   ` Zhi Yong Wu
2012-02-17 10:55     ` Stefan Hajnoczi
2012-02-18  4:16       ` Zhi Yong Wu
2012-02-18  8:57       ` Zhi Yong Wu
2012-02-18  9:48         ` Stefan Hajnoczi
2012-02-18 10:48           ` Zhi Yong Wu
2012-02-19 15:33       ` Michael S. Tsirkin

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=4FB4FD3B.7070007@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=mlspirat42@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=zwu.kernel@gmail.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).