Patchwork rtl8139: check the buffer availiability

login
register
mail settings
Submitter Jason Wang
Date Oct. 14, 2011, 10:49 a.m.
Message ID <20111014104927.22252.96179.stgit@dhcp-8-146.nay.redhat.com>
Download mbox | patch
Permalink /patch/119767/
State New
Headers show

Comments

Jason Wang - Oct. 14, 2011, 10:49 a.m.
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.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/rtl8139.c |   43 +++++++++++++++++++++++++++++--------------
 1 files changed, 29 insertions(+), 14 deletions(-)
Stefan Weil - Oct. 14, 2011, 4:27 p.m.
See my comments below.

Am 14.10.2011 12:49, schrieb Jason Wang:
> 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.
>
> Signed-off-by: Jason Wang<jasowang@redhat.com>
> ---
>   hw/rtl8139.c |   43 +++++++++++++++++++++++++++++--------------
>   1 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index 3753950..c654d5d 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,21 @@ 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, (uint8_t *)&val, 4);

The type cast is not needed, please remove it.

> +    rxdw0 = le32_to_cpu(val);
> +    if (rxdw0&  CP_RX_OWN)
> +        return 1;
> +    else
> +        return 0;

The QEMU coding conventions require braces here.
Check your patch with scripts/checkpatch.pl.

Regards,
Stefan W.

Patch

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 3753950..c654d5d 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,21 @@  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, (uint8_t *)&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 +847,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 +993,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;