Re: [Qemu-devel] [PATCH for-2.4] net-hub: Drop can_receive

[Stefan Hajnoczi]

On Fri, Jul 03, 2015 at 08:42:15AM +0800, Fam Zheng wrote:
> On Thu, 07/02 14:30, Stefan Hajnoczi wrote:
> > On Tue, Jun 30, 2015 at 10:29:20AM +0800, Fam Zheng wrote:
> > > It returns true as long as there is another attached port. This is not
> > > strictly necessary because even if there is only one port (the sender),
> > > net_hub_port_receive could succeed with a NOP. So always deliver the
> > > packets, instead of queuing them.
> > > 
> > > This fixes the possible hanging issue after net layer changed how
> > > can_read is handled. That is, if net_hub_port_can_receive returned
> > > false, the peer would disable the queue until it's explicitly flushed
> > > (for example, a call to qemu_flush_queued_packets() in net_hub_add_port,
> > > where net_hub_port_can_receive() would become true). This patch avoids
> > > that complication.
> > > 
> > > Signed-off-by: Fam Zheng <address@hidden>
> > > ---
> > >  net/hub.c | 20 --------------------
> > >  1 file changed, 20 deletions(-)
> > 
> > Hmm...I misread the hub code:
> > 
> > net_hub_port_can_receive() returns true if *any* port can receive.
> > 
> > net_hub_receive_iov() always accepts packets.  It never discards or
> > queues.
> > 
> > So in order to move to the semantics you want, let's drop
> > net_hub_port_can_receive() *and* change net_hub_receive_iov():
> > 
> >   static ssize_t net_hub_receive(NetHub *hub, NetHubPort *source_port,
> >                                  const uint8_t *buf, size_t len)
> >   {
> >       NetHubPort *port;
> > 
> >       QLIST_FOREACH(port, &hub->ports, next) {
> >           if (port == source_port) {
> >               continue;
> >           }
> > 
> >           /* No need for a callback because net_hub_flush() is called
> >            * when the peer flushes the queue anyway.
> >            *
> >            * Note that packets are duplicated if there are multiple
> >            * ports and some of them accepted a packet before a later
> >            * port queued it.  Live with it, the network tolerates
> >            * duplicates.
> >            */
> >           if (qemu_send_packet(&port->nc, buf, len) == 0) {
> >               return 0;
> 
> I'm not sure if it is a good idea to return 0 from either net_hub_port_receive
> or net_hub_receive. Because that way if one out of ten ports is down, other
> ones will no longer get more packets from tap.

You are right, in order to carry over existing semantics it needs to
return 0 when all ports returned 0.

That way .can_receive has been eliminated but behavior is identical
except for duplicate packets, which can happen on the network anyway.

pgp55tUHxxkTg.pgp
Description: PGP signature