From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58838) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWc30-0006rx-V5 for qemu-devel@nongnu.org; Mon, 31 Aug 2015 23:15:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWc2w-0001nr-0i for qemu-devel@nongnu.org; Mon, 31 Aug 2015 23:15:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59406) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWc2v-0001mz-R8 for qemu-devel@nongnu.org; Mon, 31 Aug 2015 23:15:41 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id D44C38C1B1 for ; Tue, 1 Sep 2015 03:15:40 +0000 (UTC) Message-ID: <55E51858.40607@redhat.com> Date: Tue, 01 Sep 2015 11:15:36 +0800 From: Jason Wang MIME-Version: 1.0 References: <1440770817-20555-1-git-send-email-vyasevic@redhat.com> <1440770817-20555-3-git-send-email-vyasevic@redhat.com> <55E4256F.9010302@redhat.com> <55E4638E.3090404@redhat.com> In-Reply-To: <55E4638E.3090404@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/2] rtl8139: correctly track full receive buffer in standard mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: vyasevic@redhat.com, qemu-devel@nongnu.org Cc: stefanha@redhat.com On 08/31/2015 10:24 PM, Vlad Yasevich wrote: > On 08/31/2015 05:59 AM, Jason Wang wrote: >> >> On 08/28/2015 10:06 PM, Vladislav Yasevich wrote: >>> In standard operation mode, when the receive ring buffer >>> is full, the buffer actually appears empty to the driver since >>> the RxBufAddr (the location we wirte new data to) and RxBufPtr >>> (the location guest would stat reading from) are the same. >>> As a result, the call to rtl8139_RxBufferEmpty ends up >>> returning true indicating that the receive buffer is empty. >>> This would result in the next packet overwriting the recevie buffer >>> again and stalling receive operations. >>> >>> This patch tracks the number of unread bytes in the rxbuffer >>> using an unused C+ register. On every read and write, the >>> number is adjsted and the special case of a full buffer is also >>> trapped. >>> >>> The C+ register trick is used to simplify migration and not require >>> a new machine type. This register is not used in regular mode >>> and C+ mode doesn't have the same issue. >>> >>> Signed-off-by: Vladislav Yasevich >>> --- >>> hw/net/rtl8139.c | 45 +++++++++++++++++++++++++++++++++++++++++---- >>> 1 file changed, 41 insertions(+), 4 deletions(-) >> I'm not sure this can happen. For example, looks like the following >> check in rtl8139_do_receive(): >> >> if (avail != 0 && size + 8 >= avail) >> { >> >> can guarantee there's no overwriting? >> > The problem is the calculation of avail. When the buffer is full, > avail will be the the size of the receive buffer. So the test > above will be false because the driver thinks there is actually > enough room. > > With his patch, 'avail' will be calculated to 0. > > -vlad > If believe the condition size + 8 >= avail can guarantee that the buffer won't be full (if we allow size + 8 == avail, buffer will be full)? So avail == 0 means the buffer is empty. Or is there anything I miss?