From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark McLoughlin Subject: Re: [PATCH 1/1] net: fix vnet_hdr bustage with slirp Date: Fri, 07 Aug 2009 13:19:19 +0100 Message-ID: <1249647559.3260.17.camel@blaa> References: <1249634851-24005-1-git-send-email-markmc@redhat.com> <200908071351.02086.arnd@arndb.de> Reply-To: Mark McLoughlin Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: avi@redhat.com, kvm@vger.kernel.org, Jens Osterkamp To: Arnd Bergmann Return-path: Received: from mx2.redhat.com ([66.187.237.31]:37849 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753185AbZHGMUE (ORCPT ); Fri, 7 Aug 2009 08:20:04 -0400 In-Reply-To: <200908071351.02086.arnd@arndb.de> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, 2009-08-07 at 13:51 +0200, Arnd Bergmann wrote: > On Friday 07 August 2009, Mark McLoughlin wrote: > > slirp has started using VLANClientState::opaque and this has caused the > > kvm specific tap_has_vnet_hdr() hack to break because we blindly use > > this opaque pointer even if it is not a tap client. > > > > Add yet another hack to check that we're actually getting called with a > > tap client. > > > > [Needed on stable-0.11 too] > > > > Signed-off-by: Mark McLoughlin > > Jens and I discovered the same bug before, but then we forgot about > sending a fix (sorry). Your patch should work fine as a workaround, > but I wonder if it is the right solution. > > The abstraction of struct VLANClientState is otherwise done through > function pointers taking the VLANClientState pointer as their > first argument. IMHO a cleaner abstraction would be to do the same > for tap_has_vnet_hdr(), like the patch below, and similar for > other functions passing 'opaque' pointers. Indeed, but using vc->vlan->first_client is a great big hole in the abstraction as it is. The vnet_hdr code in qemu-kvm.git is a hack which we plan to (eventually) replace by allowing a nic to be paired directly with a backend. Your patch is fine, but I'd suggest since both are a hack we stick with mine since it'll reduce merge conflicts. Both hacks will go away eventually, anyway. Thanks, Mark.