From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59882) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QXCfa-0004dt-6S for qemu-devel@nongnu.org; Thu, 16 Jun 2011 09:31:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QXCfT-00052F-0r for qemu-devel@nongnu.org; Thu, 16 Jun 2011 09:31:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55962) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QXCfR-00051L-Az for qemu-devel@nongnu.org; Thu, 16 Jun 2011 09:31:29 -0400 Message-ID: <4DF9DD77.5040401@redhat.com> Date: Thu, 16 Jun 2011 12:39:51 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <4DF9BD9C.1060307@cn.fujitsu.com> In-Reply-To: <4DF9BD9C.1060307@cn.fujitsu.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] fix the return value of rtl8139_can_receive() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang Cc: qemu-devel , Aurelien Jarno , "Michael S. Tsirkin" Am 16.06.2011 10:23, schrieb Wen Congyang: > If rtl8139_can_receive() returns 1, it means that the nic can receive packet, > otherwise, it means the nic can not receive packet. > > If !s->clock_enabled or !rtl8139_receiver_enabled(s), it means that the nic > can not receive packet. So the return value should be 0, not 1. > > Signed-off-by: Wen Congyang > > --- > hw/rtl8139.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/rtl8139.c b/hw/rtl8139.c > index 2f8db58..9084678 100644 > --- a/hw/rtl8139.c > +++ b/hw/rtl8139.c > @@ -810,9 +810,9 @@ static int rtl8139_can_receive(VLANClientState *nc) > > /* Receive (drop) packets if card is disabled. */ > if (!s->clock_enabled) > - return 1; > + return 0; > if (!rtl8139_receiver_enabled(s)) > - return 1; > + return 0; > > if (rtl8139_cp_receiver_enabled(s)) { > /* ??? Flow control not implemented in c+ mode. NACK. The old behaviour is clearly intentional. IIRC, can_receive() returning 0 means that the packet is kept in a queue and qemu tries to deliver it later. For a disabled receiver, what I would expect is that it should just drop the packets. This is what this code does by returning 1 in can_receive() and then return -1 without processing the packet in receive(). That said, e1000 has a check for (s->mac_reg[RCTL] & E1000_RCTL_EN) in can_receive. Should it be changed or is there a reason behind it? If there is, we may as well change rtl8139, but it definitely needs a better justification. Kevin