From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36928) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RG6eQ-0005eZ-Dg for qemu-devel@nongnu.org; Tue, 18 Oct 2011 06:12:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RG6eO-0001Ig-Ap for qemu-devel@nongnu.org; Tue, 18 Oct 2011 06:12:02 -0400 Received: from e28smtp02.in.ibm.com ([122.248.162.2]:39774) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RG6eM-0001I0-Kk for qemu-devel@nongnu.org; Tue, 18 Oct 2011 06:12:00 -0400 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by e28smtp02.in.ibm.com (8.14.4/8.13.1) with ESMTP id p9IABq7e010798 for ; Tue, 18 Oct 2011 15:41:52 +0530 Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p9IAAZ6F2773142 for ; Tue, 18 Oct 2011 15:40:35 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p9IAAZr1015143 for ; Tue, 18 Oct 2011 21:10:35 +1100 Message-ID: <4E9D5066.7070305@vnet.linux.ibm.com> Date: Tue, 18 Oct 2011 18:09:42 +0800 From: Mark Wu MIME-Version: 1.0 References: <20111017025557.4572.2621.stgit@dhcp-8-146.nay.redhat.com> In-Reply-To: <20111017025557.4572.2621.stgit@dhcp-8-146.nay.redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [V2 PATCH] rtl8139: check the buffer availiability List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: blauwirbel@gmail.com, aliguori@us.ibm.com, mst@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com Hi Jason, Could you please elaborate what problem you try to resolve by this patch? And do you think we need notify I/O thread re-polling tap fd when receive descriptor becomes available? Otherwise, tap read polling will be disabled until the I/O handlers are updated by other reasons. Am I right? On 10/17/2011 10:55 AM, Jason Wang wrote: > Reduce spurious packet drops on RX ring empty when in c+ mode by verifying that > we have at least 1 buffer ahead of the time. > > Change from v1: > Fix style comments from Stefan. > > Signed-off-by: Jason Wang > --- > hw/rtl8139.c | 44 ++++++++++++++++++++++++++++++-------------- > 1 files changed, 30 insertions(+), 14 deletions(-) > > diff --git a/hw/rtl8139.c b/hw/rtl8139.c > index 3753950..bcbc5e3 100644 > --- a/hw/rtl8139.c > +++ b/hw/rtl8139.c > @@ -84,6 +84,19 @@ > #define VLAN_TCI_LEN 2 > #define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN) > > +/* w0 ownership flag */ > +#define CP_RX_OWN (1<<31) > +/* w0 end of ring flag */ > +#define CP_RX_EOR (1<<30) > +/* w0 bits 0...12 : buffer size */ > +#define CP_RX_BUFFER_SIZE_MASK ((1<<13) - 1) > +/* w1 tag available flag */ > +#define CP_RX_TAVA (1<<16) > +/* w1 bits 0...15 : VLAN tag */ > +#define CP_RX_VLAN_TAG_MASK ((1<<16) - 1) > +/* w2 low 32bit of Rx buffer ptr */ > +/* w3 high 32bit of Rx buffer ptr */ > + > #if defined (DEBUG_RTL8139) > # define DPRINTF(fmt, ...) \ > do { fprintf(stderr, "RTL8139: " fmt, ## __VA_ARGS__); } while (0) > @@ -805,6 +818,22 @@ static inline target_phys_addr_t rtl8139_addr64(uint32_t low, uint32_t high) > #endif > } > > +/* Verify that we have at least one available rx buffer */ > +static int rtl8139_cp_has_rxbuf(RTL8139State *s) > +{ > + uint32_t val, rxdw0; > + target_phys_addr_t cplus_rx_ring_desc = rtl8139_addr64(s->RxRingAddrLO, > + s->RxRingAddrHI); > + cplus_rx_ring_desc += 16 * s->currCPlusRxDesc; > + cpu_physical_memory_read(cplus_rx_ring_desc,&val, 4); > + rxdw0 = le32_to_cpu(val); > + if (rxdw0& CP_RX_OWN) { > + return 1; > + } else { > + return 0; > + } > +} > + > static int rtl8139_can_receive(VLANClientState *nc) > { > RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque; > @@ -819,7 +848,7 @@ static int rtl8139_can_receive(VLANClientState *nc) > if (rtl8139_cp_receiver_enabled(s)) { > /* ??? Flow control not implemented in c+ mode. > This is a hack to work around slirp deficiencies anyway. */ > - return 1; > + return rtl8139_cp_has_rxbuf(s); > } else { > avail = MOD2(s->RxBufferSize + s->RxBufPtr - s->RxBufAddr, > s->RxBufferSize); > @@ -965,19 +994,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_ > > /* begin C+ receiver mode */ > > -/* w0 ownership flag */ > -#define CP_RX_OWN (1<<31) > -/* w0 end of ring flag */ > -#define CP_RX_EOR (1<<30) > -/* w0 bits 0...12 : buffer size */ > -#define CP_RX_BUFFER_SIZE_MASK ((1<<13) - 1) > -/* w1 tag available flag */ > -#define CP_RX_TAVA (1<<16) > -/* w1 bits 0...15 : VLAN tag */ > -#define CP_RX_VLAN_TAG_MASK ((1<<16) - 1) > -/* w2 low 32bit of Rx buffer ptr */ > -/* w3 high 32bit of Rx buffer ptr */ > - > int descriptor = s->currCPlusRxDesc; > target_phys_addr_t cplus_rx_ring_desc; > > >