From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Np2Fl-0002iQ-Ii for qemu-devel@nongnu.org; Tue, 09 Mar 2010 11:25:53 -0500 Received: from [199.232.76.173] (port=36943 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Np2Fk-0002i3-Fo for qemu-devel@nongnu.org; Tue, 09 Mar 2010 11:25:52 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Np2Fi-0007pk-I5 for qemu-devel@nongnu.org; Tue, 09 Mar 2010 11:25:52 -0500 Received: from mx20.gnu.org ([199.232.41.8]:8454) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Np2Fh-0007pc-Tq for qemu-devel@nongnu.org; Tue, 09 Mar 2010 11:25:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Np2Fg-0005Ga-Io for qemu-devel@nongnu.org; Tue, 09 Mar 2010 11:25:48 -0500 Date: Tue, 9 Mar 2010 18:18:25 +0200 From: "Michael S. Tsirkin" Message-ID: <20100309161825.GA32167@redhat.com> References: <20100309131544.GA15319@redhat.com> <1268147952.14039.70.camel@8530w.home> <20100309153031.GC15457@redhat.com> <1268151093.14039.104.camel@8530w.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1268151093.14039.104.camel@8530w.home> Subject: [Qemu-devel] Re: [PATCH RFC] net: add a flag to disable mac/vlan filtering List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel@nongnu.org, Andreas Plesner Jacobsen On Tue, Mar 09, 2010 at 09:11:33AM -0700, Alex Williamson wrote: > On Tue, 2010-03-09 at 17:30 +0200, Michael S. Tsirkin wrote: > > On Tue, Mar 09, 2010 at 08:19:12AM -0700, Alex Williamson wrote: > > > On Tue, 2010-03-09 at 15:15 +0200, Michael S. Tsirkin wrote: > > > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > > > > index 5c0093e..01b45ed 100644 > > > > --- a/hw/virtio-net.c > > > > +++ b/hw/virtio-net.c > > > > @@ -47,6 +47,7 @@ typedef struct VirtIONet > > > > uint8_t nomulti; > > > > uint8_t nouni; > > > > uint8_t nobcast; > > > > + uint32_t filtering; > > > > struct { > > > > int in_use; > > > > int first_multi; > > > > @@ -475,12 +476,17 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size) > > > > ptr += sizeof(struct virtio_net_hdr); > > > > } > > > > > > > > - if (!memcmp(&ptr[12], vlan, sizeof(vlan))) { > > > > + if ((n->filtering & (0x1 << NICCONF_F_VLAN_FILTERING)) && > > > > + !memcmp(&ptr[12], vlan, sizeof(vlan))) { > > > > int vid = be16_to_cpup((uint16_t *)(ptr + 14)) & 0xfff; > > > > if (!(n->vlans[vid >> 5] & (1U << (vid & 0x1f)))) > > > > return 0; > > > > } > > > > > > > > + if (!(n->filtering & (0x1 << NICCONF_F_MAC_FILTERING))) { > > > > + return 1; > > > > + } > > > > + > > > > > > A filtering flags bitmap is a logical choice here, but I found the > > > overhead to be non-trivial, which is why we have separate variables for > > > the other filtering options. > > > > You suggest more flags for multicast etc? > > I'm suggesting we may get slightly better performance if we use separate > filter_mac and filter_vlan variable flags instead of a single > "filtering" flags bitmap. Why? It's a single operation anyway, and we use less cache. > However, a couple other ideas... Should we > call receive_filter() as a function pointer so we can make filter > specific versions or remove it completely? Some overhead to calling it > as a pointer, but could still be a win. Alternatively we could make > "effective" filter flags which are used by receive_filter(), but > maintained separately from the guest requested flags. For instance: > > virtio_net_handle_rx_mode(...) > { > ... > n->alluni_effective = (n->filtering & (0x1 << NICCONF_F_MAC_FILTERING)) ? n->alluni : 1; > n->allmulti_effective = (n->filtering & (0x1 << NICCONF_F_MAC_FILTERING)) ? n->allmulti : 1; > > These could be recreated on loadvm, so we still wouldn't need to save > them. Question would be whether that creates a sufficiently fast path > through receive_filter(). We'd still need a new flag for vlan filtering > or maybe make the vlan header match bytes settable. > > Alex I agree, merging flags set by guest and by host makes sense, this way we don't need to test both. E.g. all filtering off is equivalent to promisc mode, we test that already. More complex ideas would I guess need to be benchmarked to be shown being worth it. So I'll do the simple thing and we can tweak it further later on. -- MST