From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52192) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vgh1O-00077q-UT for qemu-devel@nongnu.org; Wed, 13 Nov 2013 15:26:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vgh1M-000224-Mk for qemu-devel@nongnu.org; Wed, 13 Nov 2013 15:26:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:15862) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vgh1M-00021o-Ci for qemu-devel@nongnu.org; Wed, 13 Nov 2013 15:26:40 -0500 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 (8.14.4/8.14.4) with ESMTP id rADKQdNx001749 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 13 Nov 2013 15:26:39 -0500 Message-ID: <5283E07C.6040701@redhat.com> Date: Wed, 13 Nov 2013 15:26:36 -0500 From: Vlad Yasevich MIME-Version: 1.0 References: <1383650238-16015-1-git-send-email-akong@redhat.com> <1383939747-10158-1-git-send-email-vyasevic@redhat.com> <20131110121151.GA5908@redhat.com> <5283A6FE.60205@redhat.com> <20131113200014.GA8005@redhat.com> In-Reply-To: <20131113200014.GA8005@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] e1000/rtl8139: update HMP NIC when every bit is written Reply-To: vyasevic@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: jasowang@redhat.com, akong@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On 11/13/2013 03:00 PM, Michael S. Tsirkin wrote: > On Wed, Nov 13, 2013 at 11:21:18AM -0500, Vlad Yasevich wrote: >> On 11/10/2013 07:11 AM, Michael S. Tsirkin wrote: >>> On Fri, Nov 08, 2013 at 02:42:27PM -0500, Vlad Yasevich wrote: >>>> What about this approach? This only updates the monitory when all the >>>> bits have been written to. >>>> >>>> -vlad >>> >>> >>> Thanks! >>> Some comments below. >>> >>>> -- >8 -- >>>> Subject: [PATCH] e1000/rtl8139: update HMP NIC when every bit is written >>>> >>>> We currently just update the HMP NIC info when the last bit of macaddr >>>> is written. This assumes that guest driver will write all the macaddr >>> >from bit 0 to bit 5 when it changes the macaddr, this is the current >>>> behavior of linux driver (e1000/rtl8139cp), but we can't do this >>>> assumption. >>> >>> I would rather say "it seems better not to make this assumption". >>> This does look somewhat safer than what Amos proposed. >>> >>>> >>>> The macaddr that is used for rx-filter will be updated when every bit >>>> is changed. This patch updates the e1000/rtl8139 nic to update HMP NIC >>>> info when every bit has been changed. It will be same as virtio-net. >>>> >>>> Signed-off-by: Vlad Yasevich >>>> --- >>>> hw/net/e1000.c | 17 ++++++++++++++++- >>>> hw/net/rtl8139.c | 14 +++++++++----- >>>> 2 files changed, 25 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c >>>> index 8387443..a5967ed 100644 >>>> --- a/hw/net/e1000.c >>>> +++ b/hw/net/e1000.c >>>> @@ -149,6 +149,10 @@ typedef struct E1000State_st { >>>> #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT) >>>> #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT) >>>> uint32_t compat_flags; >>>> + uint32_t mac_changed; >>> >>> Hmm why uint32_t? uint8_t is enough here isn't? >>> >>> This new state has to be migrated then, and >>> we need to fallback to old behaviour if migrating to/from >>> an old version (see compat_flags for one way to >>> detect this compatibility mode). >>> >> >> Hi Michael >> >> I started looking at migrating this thing and I now starting to question >> the whole approach. >> >> The only reason to migrate this is if we can migrate between writes to >> the mac address registers. > > Absolutely. For some reason below you only discuss cross version > migration but it needs to be migrated for same version migration too. > >> We can fairly easily solve the issue of >> migrating from net to old versions. > > I'm not sure it's easier, but it needs to happen anymore. > >> The more interesting question >> is migrating from old to new versions. >> >> If someone is migrating from an older version (without this feature) >> to a newer version and doing so between writes, the bitmap state will >> have no idea that a partial write has already happened. The completing >> write will just set one of the bits and notifications that we are >> looking for do not happen. >> >> -vlad > > Yep, that's a problem too. > > > For 1.8 just send the bitmap across. Right. That's simple enough. > > For cross version migration I would say we should just detect -M 1.7 > and older and just emulate old behaviour, disregard the bitmap > completely. > Don't do special tricks just for migration. > The compat flags allow for simple migration to 1.7. However, it's the migration from 1.7 to 1.8 that's the issue. There is a version number on the E1000 state and I am thinking of maybe bumping that so that we can catch older state that doesn't support mac sate change. Thoughts? -vlad