From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZC8Cz-0005OX-65 for qemu-devel@nongnu.org; Mon, 06 Jul 2015 11:21:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZC8Ct-0002ZA-FZ for qemu-devel@nongnu.org; Mon, 06 Jul 2015 11:21:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54701) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZC8Ct-0002YZ-8y for qemu-devel@nongnu.org; Mon, 06 Jul 2015 11:21:19 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 7432F8F28F for ; Mon, 6 Jul 2015 15:21:18 +0000 (UTC) Date: Mon, 6 Jul 2015 16:21:16 +0100 From: Stefan Hajnoczi Message-ID: <20150706152116.GA11925@stefanha-thinkpad.redhat.com> References: <1435633611-14023-1-git-send-email-famz@redhat.com> <559254CC.4000106@redhat.com> <20150702124626.GE21214@stefanha-thinkpad.home> <5599F6C9.7070603@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vkogqOf2sHV7VnPd" Content-Disposition: inline In-Reply-To: <5599F6C9.7070603@redhat.com> Subject: Re: [Qemu-devel] [PATCH] virtio-net: Drop net_virtio_info.can_receive List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: Fam Zheng , qemu-devel@nongnu.org, "Michael S. Tsirkin" --vkogqOf2sHV7VnPd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 06, 2015 at 11:32:25AM +0800, Jason Wang wrote: >=20 >=20 > On 07/02/2015 08:46 PM, Stefan Hajnoczi wrote: > > On Tue, Jun 30, 2015 at 04:35:24PM +0800, Jason Wang wrote: > >> On 06/30/2015 11:06 AM, Fam Zheng wrote: > >>> virtio_net_receive still does the check by calling > >>> virtio_net_can_receive, if the device or driver is not ready, the pac= ket > >>> is dropped. > >>> > >>> This is necessary because returning false from can_receive complicates > >>> things: the peer would disable sending until we explicitly flush the > >>> queue. > >>> > >>> Signed-off-by: Fam Zheng > >>> --- > >>> hw/net/virtio-net.c | 1 - > >>> 1 file changed, 1 deletion(-) > >>> > >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >>> index d728233..dbef0d0 100644 > >>> --- a/hw/net/virtio-net.c > >>> +++ b/hw/net/virtio-net.c > >>> @@ -1503,7 +1503,6 @@ static int virtio_net_load_device(VirtIODevice = *vdev, QEMUFile *f, > >>> static NetClientInfo net_virtio_info =3D { > >>> .type =3D NET_CLIENT_OPTIONS_KIND_NIC, > >>> .size =3D sizeof(NICState), > >>> - .can_receive =3D virtio_net_can_receive, > >>> .receive =3D virtio_net_receive, > >>> .link_status_changed =3D virtio_net_set_link_status, > >>> .query_rx_filter =3D virtio_net_query_rxfilter, > >> A side effect of this patch is it will read and then drop packet is > >> guest driver is no ok. > > I think that the semantics of .can_receive() and .receive() return > > values are currently incorrect in many NICs. They have .can_receive() > > functions that return false for conditions where .receive() would > > discard the packet. So what happens is that packets get queued when > > they should actually be discarded. >=20 > Yes, but they are bugs more or less. >=20 > > > > The purpose of the flow control (queuing) mechanism is to tell the > > sender to hold off until the receiver has more rx buffers available. > > It's a short-term thing that doesn't included link down, rx disable, or > > NIC reset states. > > > > Therefore, I think this patch will not introduce a regression. It is > > adjusting the code to stop queuing packets when they should actually be > > dropped. > > > > Thoughts? >=20 > I agree there's no functional issue. But it cause wasting of cpu cycles > (consider guest is being flooded). Sometime it maybe even dangerous. For > tap, we're probably ok since we have 756ae78b but for other backend, we > don't. If the guest uses iptables rules or other mechanisms to drop bogus packets the cost is even higher than discarding them at the QEMU layer. What's more is that if you're using link down as a DoS mitigation strategy then you might as well hot unplug the NIC. Stefan --vkogqOf2sHV7VnPd Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVmpzsAAoJEJykq7OBq3PIp/UH/jHikELDtmd5QYmOxIZylGGl czqD5beCCYvVgrrBPL9KOqO0hISK5YVevnE7RRgq7RUk/o366nUCQiz8xp+Wtfvt nyvSkL/vM5zYB9+NNt6qlmYvhhskKlt1vU7rIQlkYBSNpPXVkgNaQzPDjwU2vo7g 1kSNT7mQ/cVbxqc9sXi3l/58pQ6fynWLB5CIkm1T3qAOTEk0OMJlc7PXPvutYbMK dNOiZ+ixfh55M7gf1HxzqPiSqV37Hk1g6n4oI/Kg7ntSl8ZE0WY2Aho2P3VvtM2X yCo6wUZzWAT+2SRhgi1hcO/k/gujr2QIXdxztahxLiV9ZEXc110DLBdb0m/0waY= =yNxj -----END PGP SIGNATURE----- --vkogqOf2sHV7VnPd--