From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54138) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAekT-0003NL-Ba for qemu-devel@nongnu.org; Thu, 02 Jul 2015 09:41:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZAekP-0004Aq-59 for qemu-devel@nongnu.org; Thu, 02 Jul 2015 09:41:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50800) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAekO-0004Ad-Ur for qemu-devel@nongnu.org; Thu, 02 Jul 2015 09:41:49 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 93A092DC3DE for ; Thu, 2 Jul 2015 13:41:48 +0000 (UTC) Date: Thu, 2 Jul 2015 13:27:56 +0100 From: Stefan Hajnoczi Message-ID: <20150702122756.GC21214@stefanha-thinkpad.home> References: <1435631360-4978-1-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="rQ2U398070+RC21q" Content-Disposition: inline In-Reply-To: <1435631360-4978-1-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [PATCH for-2.4] net-hub: Drop can_receive List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Jason Wang , qemu-devel@nongnu.org --rQ2U398070+RC21q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > 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 =3D 0; if (nc->peer && nc->peer->info->type =3D=3D NET_CLIENT_OPTIONS_KIND_HUB= PORT) { 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 > --- > 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. --rQ2U398070+RC21q Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVlS5MAAoJEJykq7OBq3PIHKwIAIH35n+DW2qNzWWc17o3+h/Z J8qSAOM8jw/qRAcw7EIQBlwyH/BEWdOmwfop7zMC4oc2uCXc8xNyeZOH4pVkIirx VAeZTNNmq2L7U/PlzoCenAmJd6sHK3QqlHAHCw5RO7Dirj66H0z1ieoKCRrrixEg 7KsiIldkwV+IsUKiWnJE/geT/z8dChuRoX+LjN06GHIt3LR9ppFrpWj/GpcE6rbG 9FwpEaFY4Ps2LXrVrwD3dWUT6FAsEFU5JoVGRYh52gafktabG176NiYYdmZOUUZo ZcmHALUBlTbyAkAY2I+hzlXHRS6jVuo2usIR3st4lFpwmKKfb+qDH+SItpRzqfQ= =3LVY -----END PGP SIGNATURE----- --rQ2U398070+RC21q--