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

[Stefan Hajnoczi]

On Tue, Jun 30, 2015 at 10:29:20AM +0800, Fam Zheng wrote:
> It returns true as long as there is another attached port.

No, other ports may also return false from .can_receive().  So this
function can return false when there are multiple ports.

> 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.

Wait, have you seen:

static
void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge)
{
    nc->receive_disabled = 0;

    if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
        if (net_hub_flush(nc->peer)) {   <--- wake up all ports
            qemu_notify_event();
        }
    }

The net code is designed to propagate across the hub, so I don't think
the bug exists.

Do you have a reproducer?

> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  net/hub.c | 20 --------------------
>  1 file changed, 20 deletions(-)

Deleteing this function disables flow control across the hub.  There is
no way to tell a peer to stop sending.  This is a departure from normal
net client semantics where sending should stop if the peer cannot
receive further packets.

With this patch, packets will be queued if the destination net client is
unable to receive and that queue will keep growing until nq_maxlen is
reached and then packets will be dropped.

So this patch basically says flow control is not needed.

It is needed because:

1. USB network devices would suffer horrible traffic loss without
   queuing due to the nature of the device.  They only have 1 packet rx
   buffer and it needs to be collected by the guest before more packets
   can be received.

2. To save CPU cycles/power by not reading from file descriptors if the
   packet cannot be delivered yet.

pgpQy66UYrh9W.pgp
Description: PGP signature