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

[Thomas Huth]

On Tue, 30 Jun 2015 10:29:20 +0800
Fam Zheng <address@hidden> 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(-)
> 
> diff --git a/net/hub.c b/net/hub.c
> index 3047f12..65a8e09 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -87,25 +87,6 @@ static NetHub *net_hub_new(int id)
>      return hub;
>  }
>  
> -static int net_hub_port_can_receive(NetClientState *nc)
> -{
> -    NetHubPort *port;
> -    NetHubPort *src_port = DO_UPCAST(NetHubPort, nc, nc);
> -    NetHub *hub = src_port->hub;
> -
> -    QLIST_FOREACH(port, &hub->ports, next) {
> -        if (port == src_port) {
> -            continue;
> -        }
> -
> -        if (qemu_can_send_packet(&port->nc)) {
> -            return 1;
> -        }
> -    }
> -
> -    return 0;
> -}
> -
>  static ssize_t net_hub_port_receive(NetClientState *nc,
>                                      const uint8_t *buf, size_t len)
>  {
> @@ -132,7 +113,6 @@ static void net_hub_port_cleanup(NetClientState *nc)
>  static NetClientInfo net_hub_port_info = {
>      .type = NET_CLIENT_OPTIONS_KIND_HUBPORT,
>      .size = sizeof(NetHubPort),
> -    .can_receive = net_hub_port_can_receive,
>      .receive = net_hub_port_receive,
>      .receive_iov = net_hub_port_receive_iov,
>      .cleanup = net_hub_port_cleanup,

Sounds reasonable.

Reviewed-by: Thomas Huth <address@hidden>