netdev
[Top] [All Lists]

Re: [PATCH] r8169 NAPI addition

To: Jon D Mason <jonmason@xxxxxxxxxx>
Subject: Re: [PATCH] r8169 NAPI addition
From: Francois Romieu <romieu@xxxxxxxxxxxxx>
Date: Fri, 23 Apr 2004 01:18:34 +0200
Cc: jgarzik@xxxxxxxxx, netdev@xxxxxxxxxxx
In-reply-to: <OF877F55DF.2DDFA7D4-ON87256E7E.005842ED-86256E7E.005967E0@us.ibm.com>; from jonmason@us.ibm.com on Thu, Apr 22, 2004 at 10:20:58AM -0600
References: <20040420234524.A1374@electric-eye.fr.zoreil.com> <OF877F55DF.2DDFA7D4-ON87256E7E.005842ED-86256E7E.005967E0@us.ibm.com>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.2.5.1i
Jon D Mason <jonmason@xxxxxxxxxx> :
[...]
--- r8169.c     2004-04-18 22:51:33.000000000 -0500
+++ r8169.c     2004-04-22 08:08:57.526577048 -0500
[...]
@@ -1692,6 +1737,31 @@ static struct net_device_stats *rtl8169_
        return &tp->stats;
 }
 
+#ifdef CONFIG_R8169_NAPI
+static int rtl8169_poll(struct net_device *dev, int *budget)
+{
+       u32 status, work_done = 0, work_to_do = min(*budget, dev->quota);
+       struct rtl8169_private *tp = dev->priv;
+       void *ioaddr = tp->mmio_addr;
+       
+       work_done = rtl8169_rx_interrupt(dev, tp, ioaddr);
+       
+       *budget -= work_done;
+       dev->quota -= work_done;
+       
+       if ((work_done < work_to_do) || !netif_running(dev)) {

Packets are received here.
Tx interrupt is issued.
--> Interrupts status register is completely cleared by the irq handler <--
There are pending Rx packets.
r8169_poll() is not activated by the irq handler.

-> Pending Rx packets may wait long.

+               netif_rx_complete(dev);
+               status = RTL_R16(IntrStatus);
+               if (!status) {
+                       RTL_W16(IntrMask, rtl8169_intr_mask);
+                       status = RTL_R16(IntrMask);
+               }       

The previous lines imply that if, according to IntrStatus, there is pending
Rx work but no pending Tx work, the Rx interrupts are kept disabled.

If I am not mistaken (1:55 AM here), either 1) you do not clear the RX
related bits of IntrStatus in r8169_interrupt() but you clear it in
rtl8169_poll() and you add local_irq_{en/dis}able() to rtl8169_poll()
(ala 8139cp.c) or 2) you modify r8169_interrupt() so that it can signal
to rtl8169_poll() that it cleared itself the Rx bits of IntrStatus whereas
rtl8169_poll() was running (ala epic100-netdev).

If you hesitate, go for 1). 2) works but I have not benchmarketed it yet :o)

If you have doubts regarding your Tx part, feel free to send any code.

--
Ueimor

<Prev in Thread] Current Thread [Next in Thread>