* Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Roland Dreier @ 2007-10-09 21:40 UTC (permalink / raw)
To: David Miller
Cc: krkumar2, gaagaan, general, hadi, herbert, jagana, jeff, johnpol,
kaber, mcarlson, mchan, netdev, peter.p.waskiewicz.jr,
randy.dunlap, rick.jones2, Robert.Olsson, shemminger, sri, tgraf,
xma
In-Reply-To: <20071009.135145.95506679.davem@davemloft.net>
> Before you add new entries to your list, how is that ibm driver NAPI
> conversion coming along? :-)
I still haven't done much. OK, I will try to get my board booting
again this week.
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: jamal @ 2007-10-09 21:56 UTC (permalink / raw)
To: David Miller
Cc: johnpol, Robert.Olsson, herbert, gaagaan, jeff, rdreier,
peter.p.waskiewicz.jr, mcarlson, andi, general, netdev, tgraf,
randy.dunlap, sri, shemminger, kaber, mchan, jagana
In-Reply-To: <20071009.142235.74385364.davem@davemloft.net>
On Tue, 2007-09-10 at 14:22 -0700, David Miller wrote:
> Even if the device has "huge queues" I still think we need a software
> queue for when the hardware one backs up.
It should be fine to just "pretend" the qdisc exists despite it sitting
in the driver and not have s/ware queues at all to avoid all the
challenges that qdiscs bring;
if the h/ware queues are full because of link pressure etc, you drop. We
drop today when the s/ware queues are full. The driver txmit lock takes
place of the qdisc queue lock etc. I am assuming there is still need for
that locking. The filter/classification scheme still works as is and
select classes which map to rings. tc still works as is etc.
cheers,
jamal
^ permalink raw reply
* [PATCH] natsemi: Use NATSEMI_TIMER_FREQ consistently
From: Mark Brown @ 2007-10-09 21:57 UTC (permalink / raw)
To: jgarzik, thockin; +Cc: netdev, linux-kernel, Mark Brown
In-Reply-To: <1191967059-7143-1-git-send-email-broonie@sirena.org.uk>
The natsemi driver has a define NATSEMI_TIMER_FREQ which looks like it
controls the normal frequency of the chip poll timer but in fact only
takes effect for the first run of the timer. Adjust the value of the
define to match that used by the timer and use the define consistently.
Signed-off-by: Mark Brown <broonie@sirena.org.uk>
---
drivers/net/natsemi.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
index 0b33a58..1f88604 100644
--- a/drivers/net/natsemi.c
+++ b/drivers/net/natsemi.c
@@ -108,7 +108,7 @@ static int full_duplex[MAX_UNITS];
#define TX_TIMEOUT (2*HZ)
#define NATSEMI_HW_TIMEOUT 400
-#define NATSEMI_TIMER_FREQ 3*HZ
+#define NATSEMI_TIMER_FREQ 5*HZ
#define NATSEMI_PG0_NREGS 64
#define NATSEMI_RFDR_NREGS 8
#define NATSEMI_PG1_NREGS 4
@@ -1797,7 +1797,7 @@ static void netdev_timer(unsigned long data)
struct net_device *dev = (struct net_device *)data;
struct netdev_private *np = netdev_priv(dev);
void __iomem * ioaddr = ns_ioaddr(dev);
- int next_tick = 5*HZ;
+ int next_tick = NATSEMI_TIMER_FREQ;
if (netif_msg_timer(np)) {
/* DO NOT read the IntrStatus register,
--
1.5.3.4
^ permalink raw reply related
* [ofa-general] [PATCHES] TX batching rev2.5
From: jamal @ 2007-10-09 22:07 UTC (permalink / raw)
To: David Miller
Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
general, mchan, tgraf, jeff, sri, shemminger, kaber
Please provide feedback on the code and/or architecture.
They are now updated to work with the latest rebased net-2.6.24
from a few hours ago.
I am on travel mode so wont have time to do more testing for the next
few days - i do consider this to be stable at this point based on
what i have been testing (famous last words).
Patch 1: Introduces batching interface
Patch 2: Core uses batching interface
Patch 3: get rid of dev->gso_skb
What has changed since i posted last:
-------------------------------------
1) There was some cruft leftover from prep_frame feature that i forgot
to remove last time - it is now gone.
2) In the shower this AM, i realized that it is plausible that a
batch of packets sent to the driver may all be dropped because
they are badly formatted. Current drivers all return NETDEV_TX_OK
in all such cases. This will lead for us to call dev->hard_end_xmit()
to be invoked unnecessarily. I already had a NETDEV_TX_DROPPED within
batching drivers, so i made that global and make the batching drivers
return it if they drop packets. The core calls dev->hard_end_xmit()
when at least one packet makes it through.
Things i am gonna say that nobody will see (wink)
-------------------------------------------------
Dave please let me know if this meets your desires to allow devices
which are SG and able to compute CSUM benefit just in case i
misunderstood.
Herbert, if you can look at at least patch 3 i will appreaciate it
(since it kills dev->gso_skb that you introduced).
UPCOMING PATCHES
---------------
As before:
More patches to follow later if i get some feedback - i didnt want to
overload people by dumping too many patches. Most of these patches
mentioned below are ready to go; some need some re-testing and others
need a little porting from an earlier kernel:
- tg3 driver
- tun driver
- pktgen
- netiron driver
- e1000e driver (non-LLTX)
- ethtool interface
- There is at least one other driver promised to me
Theres also a driver-howto that i will post soon today.
PERFORMANCE TESTING
--------------------
System under test hardware is still a 2xdual core opteron with a
couple of tg3s.
A test tool generates udp traffic of different sizes for upto 60
seconds per run or a total of 30M packets. I have 4 threads each
running on a specific CPU which keep all the CPUs as busy as they can
sending packets targetted at a directly connected box's udp discard
port.
All 4 CPUs target a single tg3 to send. The receiving box has a tc rule
which counts and drops all incoming udp packets to discard port - this
allows me to make sure that the receiver is not the bottleneck in the
testing. Packet sizes sent are {8B, 32B, 64B, 128B, 256B, 512B, 1024B}.
Each packet size run is repeated 10 times to ensure that there are no
transients. The average of all 10 runs is then computed and collected.
I do plan also to run forwarding and TCP tests in the future when the
dust settles.
cheers,
jamal
^ permalink raw reply
* [ofa-general] [PATCH 1/3] [NET_BATCH] Introduce batching interface Rev2.5
From: jamal @ 2007-10-09 22:10 UTC (permalink / raw)
To: David Miller
Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
general, mchan, tgraf, jeff, sri, shemminger, kaber
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
This patch introduces the netdevice interface for batching.
cheers,
jamal
[-- Attachment #2: 01-introduce-batching-interface.patch --]
[-- Type: text/x-patch, Size: 8834 bytes --]
[NET_BATCH] Introduce batching interface
This patch introduces the netdevice interface for batching.
BACKGROUND
---------
A driver dev->hard_start_xmit() has 4 typical parts:
a) packet formating (example vlan, mss, descriptor counting etc)
b) chip specific formatting
c) enqueueing the packet on a DMA ring
d) IO operations to complete packet transmit, tell DMA engine to chew on,
tx completion interupts, set last tx time, etc
[For code cleanliness/readability sake, regardless of this work,
one should break the dev->hard_start_xmit() into those 4 functions
anyways].
INTRODUCING API
---------------
With the api introduced in this patch, a driver which has all
4 parts and needing to support batching is advised to split its
dev->hard_start_xmit() in the following manner:
1)Remove #d from dev->hard_start_xmit() and put it in
dev->hard_end_xmit() method.
2)#b and #c can stay in ->hard_start_xmit() (or whichever way you want
to do this)
3) #a is deffered to future work to reduce confusion (since it holds
on its own).
Note: There are drivers which may need not support any of the two
approaches (example the tun driver i patched) so the methods are
optional.
xmit_win variable is set by the driver to tell the core how much space
it has to take on new skbs. It is introduced to ensure that when we pass
the driver a list of packets it will swallow all of them - which is
useful because we dont requeue to the qdisc (and avoids burning
unnecessary cpu cycles or introducing any strange re-ordering). The driver
tells us when it invokes netif_wake_queue how much space it has for
descriptors by setting this variable.
Refer to the driver howto for more details.
THEORY OF OPERATION
-------------------
1. Core dequeues from qdiscs upto dev->xmit_win packets. Fragmented
and GSO packets are accounted for as well.
2. Core grabs TX_LOCK
3. Core loop for all skbs:
invokes driver dev->hard_start_xmit()
4. Core invokes driver dev->hard_end_xmit()
ACKNOWLEDGEMENT AND SOME HISTORY
--------------------------------
There's a lot of history and reasoning of "why batching" in a document
i am writting which i may submit as a patch.
Thomas Graf (who doesnt know this probably) gave me the impetus to
start looking at this back in 2004 when he invited me to the linux
conference he was organizing. Parts of what i presented in SUCON in
2004 talk about batching. Herbert Xu forced me to take a second look around
2.6.18 - refer to my netconf 2006 presentation. Krishna Kumar provided
me with more motivation in May 2007 when he posted on netdev and engaged
me.
Sridhar Samudrala, Krishna Kumar, Matt Carlson, Michael Chan,
Jeremy Ethridge, Evgeniy Polyakov, Sivakumar Subramani, David Miller,
and Patrick McHardy, Jeff Garzik and Bill Fink have contributed in one or
more of {bug fixes, enhancements, testing, lively discussion}. The
Broadcom and neterion folks have been outstanding in their help.
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
---
commit 98d39e2222a7922fa2719a80eecd02cae359f3d7
tree 63822bf3040ea41846399c589c912c2be654f008
parent 7b4cd20628fe5c4e145c383fcd8d954d38f7be61
author Jamal Hadi Salim <hadi@cyberus.ca> Tue, 09 Oct 2007 11:06:28 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Tue, 09 Oct 2007 11:06:28 -0400
include/linux/netdevice.h | 9 +++++-
net/core/dev.c | 67 ++++++++++++++++++++++++++++++++++++++++++---
net/sched/sch_generic.c | 4 +--
3 files changed, 73 insertions(+), 7 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 91cd3f3..b0e71c9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -86,6 +86,7 @@ struct wireless_dev;
/* Driver transmit return codes */
#define NETDEV_TX_OK 0 /* driver took care of packet */
#define NETDEV_TX_BUSY 1 /* driver tx path was busy*/
+#define NETDEV_TX_DROPPED 2 /* driver tx path dropped packet*/
#define NETDEV_TX_LOCKED -1 /* driver tx lock was already taken */
/*
@@ -467,6 +468,7 @@ struct net_device
#define NETIF_F_NETNS_LOCAL 8192 /* Does not change network namespaces */
#define NETIF_F_MULTI_QUEUE 16384 /* Has multiple TX/RX queues */
#define NETIF_F_LRO 32768 /* large receive offload */
+#define NETIF_F_BTX 65536 /* Capable of batch tx */
/* Segmentation offload features */
#define NETIF_F_GSO_SHIFT 16
@@ -595,6 +597,9 @@ struct net_device
void *priv; /* pointer to private data */
int (*hard_start_xmit) (struct sk_buff *skb,
struct net_device *dev);
+ void (*hard_end_xmit) (struct net_device *dev);
+ int xmit_win;
+
/* These may be needed for future network-power-down code. */
unsigned long trans_start; /* Time (in jiffies) of last Tx */
@@ -609,6 +614,7 @@ struct net_device
/* delayed register/unregister */
struct list_head todo_list;
+ struct sk_buff_head blist;
/* device index hash chain */
struct hlist_node index_hlist;
@@ -1043,7 +1049,8 @@ extern int dev_set_mtu(struct net_device *, int);
extern int dev_set_mac_address(struct net_device *,
struct sockaddr *);
extern int dev_hard_start_xmit(struct sk_buff *skb,
- struct net_device *dev);
+ struct net_device *dev, int *cnt);
+extern int dev_batch_xmit(struct net_device *dev);
extern int netdev_budget;
diff --git a/net/core/dev.c b/net/core/dev.c
index e7e728a..eaa43d6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1517,8 +1517,10 @@ static int dev_gso_segment(struct sk_buff *skb)
return 0;
}
-int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
+int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev, int *tcnt)
{
+ int rc = NETDEV_TX_OK;
+
if (likely(!skb->next)) {
if (!list_empty(&ptype_all))
dev_queue_xmit_nit(skb, dev);
@@ -1530,7 +1532,15 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
goto gso;
}
- return dev->hard_start_xmit(skb, dev);
+ rc = dev->hard_start_xmit(skb, dev);
+ if (unlikely(rc)) {
+ /*Life goes on if driver dropped packet */
+ if (unlikely(rc == NETDEV_TX_DROPPED))
+ rc = NETDEV_TX_OK;
+ } else {
+ *tcnt += 1;
+ }
+ return rc;
}
gso:
@@ -1542,10 +1552,17 @@ gso:
nskb->next = NULL;
rc = dev->hard_start_xmit(nskb, dev);
if (unlikely(rc)) {
+ if (unlikely(rc == NETDEV_TX_DROPPED)) {
+ rc = NETDEV_TX_OK;
+ continue;
+ }
+
nskb->next = skb->next;
skb->next = nskb;
return rc;
- }
+ } else
+ *tcnt += 1;
+
if (unlikely((netif_queue_stopped(dev) ||
netif_subqueue_stopped(dev, skb->queue_mapping)) &&
skb->next))
@@ -1559,6 +1576,45 @@ out_kfree_skb:
return 0;
}
+int dev_batch_xmit(struct net_device *dev)
+{
+ struct sk_buff_head *skbs = &dev->blist;
+ int rc = NETDEV_TX_OK;
+ int tqd = 0, xcnt;
+ struct sk_buff *skb;
+ int orig_w = dev->xmit_win;
+ int orig_pkts = skb_queue_len(skbs);
+
+ while ((skb = __skb_dequeue(skbs)) != NULL) {
+ xcnt = 0;
+ rc = dev_hard_start_xmit(skb, dev, &xcnt);
+ tqd += xcnt;
+ if (unlikely(rc))
+ break;
+ }
+
+ /* Batching driver is likely buggy and lied to us on how much
+ * space it had. Damn you driver ..
+ */
+ if (unlikely(skb_queue_len(skbs)) && (dev->features & NETIF_F_BTX)) {
+ printk(KERN_WARNING "Likely bug %s %s (%d) "
+ "left %d/%d window now %d, orig %d\n",
+ dev->name, rc?"busy":"locked",
+ netif_queue_stopped(dev),
+ skb_queue_len(skbs),
+ orig_pkts,
+ dev->xmit_win,
+ orig_w);
+ rc = NETDEV_TX_BUSY;
+ }
+
+ if (tqd)
+ if (dev->hard_end_xmit)
+ dev->hard_end_xmit(dev);
+
+ return rc;
+}
+
/**
* dev_queue_xmit - transmit a buffer
* @skb: buffer to transmit
@@ -1687,8 +1743,9 @@ gso:
if (!netif_queue_stopped(dev) &&
!netif_subqueue_stopped(dev, skb->queue_mapping)) {
+ int c = 0;
rc = 0;
- if (!dev_hard_start_xmit(skb, dev)) {
+ if (!dev_hard_start_xmit(skb, dev, &c)) {
HARD_TX_UNLOCK(dev);
goto out;
}
@@ -3581,6 +3638,8 @@ int register_netdevice(struct net_device *dev)
}
}
+ dev->xmit_win = 1;
+ skb_queue_head_init(&dev->blist);
ret = netdev_register_kobject(dev);
if (ret)
goto err_uninit;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 95ae119..424c08b 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -134,7 +134,7 @@ static inline int qdisc_restart(struct net_device *dev)
{
struct Qdisc *q = dev->qdisc;
struct sk_buff *skb;
- int ret;
+ int ret, xcnt = 0;
/* Dequeue packet */
if (unlikely((skb = dev_dequeue_skb(dev, q)) == NULL))
@@ -145,7 +145,7 @@ static inline int qdisc_restart(struct net_device *dev)
spin_unlock(&dev->queue_lock);
HARD_TX_LOCK(dev, smp_processor_id());
- ret = dev_hard_start_xmit(skb, dev);
+ ret = dev_hard_start_xmit(skb, dev, &xcnt);
HARD_TX_UNLOCK(dev);
spin_lock(&dev->queue_lock);
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related
* [ofa-general] [PATCH 2/3][NET_BATCH] Rev2.5 net core use batching
From: jamal @ 2007-10-09 22:12 UTC (permalink / raw)
To: David Miller
Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
general, mchan, tgraf, jeff, sri, shemminger, kaber
[-- Attachment #1: Type: text/plain, Size: 70 bytes --]
This patch adds the usage of batching within the core.
cheers,
jamal
[-- Attachment #2: 02-net-core-use-batching.patch --]
[-- Type: text/x-patch, Size: 4508 bytes --]
[NET_BATCH] net core use batching
This patch adds the usage of batching within the core.
Performance results demonstrating improvement are provided separately.
I have #if-0ed some of the old functions so the patch is more readable.
A future patch will remove all if-0ed content.
Patrick McHardy eyeballed a bug that will cause re-ordering in case
of a requeue.
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
---
commit c73d8ee8cce61a98f55fbfb2cafe813a7eca472c
tree 8b9155fe15baa4c2e7adb69585c7aa275a6bc896
parent 98d39e2222a7922fa2719a80eecd02cae359f3d7
author Jamal Hadi Salim <hadi@cyberus.ca> Tue, 09 Oct 2007 11:13:30 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Tue, 09 Oct 2007 11:13:30 -0400
net/sched/sch_generic.c | 104 ++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 94 insertions(+), 10 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 424c08b..d98c680 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -56,6 +56,7 @@ static inline int qdisc_qlen(struct Qdisc *q)
return q->q.qlen;
}
+#if 0
static inline int dev_requeue_skb(struct sk_buff *skb, struct net_device *dev,
struct Qdisc *q)
{
@@ -110,6 +111,85 @@ static inline int handle_dev_cpu_collision(struct sk_buff *skb,
return ret;
}
+#endif
+
+static inline int handle_dev_cpu_collision(struct net_device *dev)
+{
+ if (unlikely(dev->xmit_lock_owner == smp_processor_id())) {
+ if (net_ratelimit())
+ printk(KERN_WARNING
+ "Dead loop on netdevice %s, fix it urgently!\n",
+ dev->name);
+ return 1;
+ }
+ __get_cpu_var(netdev_rx_stat).cpu_collision++;
+ return 0;
+}
+
+static inline int
+dev_requeue_skbs(struct sk_buff_head *skbs, struct net_device *dev,
+ struct Qdisc *q)
+{
+
+ struct sk_buff *skb;
+
+ while ((skb = __skb_dequeue_tail(skbs)) != NULL)
+ q->ops->requeue(skb, q);
+
+ netif_schedule(dev);
+ return 0;
+}
+
+static inline int
+xmit_islocked(struct sk_buff_head *skbs, struct net_device *dev,
+ struct Qdisc *q)
+{
+ int ret = handle_dev_cpu_collision(dev);
+
+ if (ret) {
+ if (!skb_queue_empty(skbs))
+ skb_queue_purge(skbs);
+ return qdisc_qlen(q);
+ }
+
+ return dev_requeue_skbs(skbs, dev, q);
+}
+
+static int xmit_count_skbs(struct sk_buff *skb)
+{
+ int count = 0;
+ for (; skb; skb = skb->next) {
+ count += skb_shinfo(skb)->nr_frags;
+ count += 1;
+ }
+ return count;
+}
+
+static int xmit_get_pkts(struct net_device *dev,
+ struct Qdisc *q,
+ struct sk_buff_head *pktlist)
+{
+ struct sk_buff *skb;
+ int count = dev->xmit_win;
+
+ if (count && dev->gso_skb) {
+ skb = dev->gso_skb;
+ dev->gso_skb = NULL;
+ count -= xmit_count_skbs(skb);
+ __skb_queue_tail(pktlist, skb);
+ }
+
+ while (count > 0) {
+ skb = q->dequeue(q);
+ if (!skb)
+ break;
+
+ count -= xmit_count_skbs(skb);
+ __skb_queue_tail(pktlist, skb);
+ }
+
+ return skb_queue_len(pktlist);
+}
/*
* NOTE: Called under dev->queue_lock with locally disabled BH.
@@ -133,19 +213,20 @@ static inline int handle_dev_cpu_collision(struct sk_buff *skb,
static inline int qdisc_restart(struct net_device *dev)
{
struct Qdisc *q = dev->qdisc;
- struct sk_buff *skb;
- int ret, xcnt = 0;
+ int ret = 0;
- /* Dequeue packet */
- if (unlikely((skb = dev_dequeue_skb(dev, q)) == NULL))
- return 0;
+ /* Dequeue packets */
+ ret = xmit_get_pkts(dev, q, &dev->blist);
+ if (!ret)
+ return 0;
- /* And release queue */
+ /* We got em packets */
spin_unlock(&dev->queue_lock);
+ /* bye packets ....*/
HARD_TX_LOCK(dev, smp_processor_id());
- ret = dev_hard_start_xmit(skb, dev, &xcnt);
+ ret = dev_batch_xmit(dev);
HARD_TX_UNLOCK(dev);
spin_lock(&dev->queue_lock);
@@ -158,8 +239,8 @@ static inline int qdisc_restart(struct net_device *dev)
break;
case NETDEV_TX_LOCKED:
- /* Driver try lock failed */
- ret = handle_dev_cpu_collision(skb, dev, q);
+ /* Driver lock failed */
+ ret = xmit_islocked(&dev->blist, dev, q);
break;
default:
@@ -168,7 +249,7 @@ static inline int qdisc_restart(struct net_device *dev)
printk(KERN_WARNING "BUG %s code %d qlen %d\n",
dev->name, ret, q->q.qlen);
- ret = dev_requeue_skb(skb, dev, q);
+ ret = dev_requeue_skbs(&dev->blist, dev, q);
break;
}
@@ -564,6 +645,9 @@ void dev_deactivate(struct net_device *dev)
skb = dev->gso_skb;
dev->gso_skb = NULL;
+ if (!skb_queue_empty(&dev->blist))
+ skb_queue_purge(&dev->blist);
+ dev->xmit_win = 1;
spin_unlock_bh(&dev->queue_lock);
kfree_skb(skb);
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related
* [ofa-general] [PATCH 3/3][NET_BATCH] Rev2.5 kill dev->gso_skb
From: jamal @ 2007-10-09 22:13 UTC (permalink / raw)
To: David Miller
Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
general, mchan, tgraf, jeff, sri, shemminger, kaber
[-- Attachment #1: Type: text/plain, Size: 97 bytes --]
This patch removes dev->gso_skb as it is no longer necessary with
batching code.
cheers,
jamal
[-- Attachment #2: 03-kill-dev-gso-skb.patch --]
[-- Type: text/x-patch, Size: 2277 bytes --]
[NET_BATCH] kill dev->gso_skb
The batching code does what gso used to batch at the drivers.
There is no more need for gso_skb. If for whatever reason the
requeueing is a bad idea we are going to leave packets in dev->blist
(and still not need dev->gso_skb)
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
---
commit fac8a4147548f314d4edb74634e78e5b06e0e135
tree 72114acb327bc7e3eb219275df6b3aab7459795c
parent c73d8ee8cce61a98f55fbfb2cafe813a7eca472c
author Jamal Hadi Salim <hadi@cyberus.ca> Tue, 09 Oct 2007 11:22:43 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Tue, 09 Oct 2007 11:22:43 -0400
include/linux/netdevice.h | 3 ---
net/sched/sch_generic.c | 12 ------------
2 files changed, 0 insertions(+), 15 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b0e71c9..7592a56 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -578,9 +578,6 @@ struct net_device
struct list_head qdisc_list;
unsigned long tx_queue_len; /* Max frames per queue allowed */
- /* Partially transmitted GSO packet. */
- struct sk_buff *gso_skb;
-
/* ingress path synchronizer */
spinlock_t ingress_lock;
struct Qdisc *qdisc_ingress;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index d98c680..36b6972 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -172,13 +172,6 @@ static int xmit_get_pkts(struct net_device *dev,
struct sk_buff *skb;
int count = dev->xmit_win;
- if (count && dev->gso_skb) {
- skb = dev->gso_skb;
- dev->gso_skb = NULL;
- count -= xmit_count_skbs(skb);
- __skb_queue_tail(pktlist, skb);
- }
-
while (count > 0) {
skb = q->dequeue(q);
if (!skb)
@@ -635,7 +628,6 @@ void dev_activate(struct net_device *dev)
void dev_deactivate(struct net_device *dev)
{
struct Qdisc *qdisc;
- struct sk_buff *skb;
spin_lock_bh(&dev->queue_lock);
qdisc = dev->qdisc;
@@ -643,15 +635,11 @@ void dev_deactivate(struct net_device *dev)
qdisc_reset(qdisc);
- skb = dev->gso_skb;
- dev->gso_skb = NULL;
if (!skb_queue_empty(&dev->blist))
skb_queue_purge(&dev->blist);
dev->xmit_win = 1;
spin_unlock_bh(&dev->queue_lock);
- kfree_skb(skb);
-
dev_watchdog_down(dev);
/* Wait for outstanding dev_queue_xmit calls. */
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related
* Re: [PATCH][E1000E] some cleanups
From: jamal @ 2007-10-09 22:18 UTC (permalink / raw)
To: Kok, Auke; +Cc: netdev
In-Reply-To: <470BA602.7060400@intel.com>
On Tue, 2007-09-10 at 09:02 -0700, Kok, Auke wrote:
>
> if we're going to remove LLTX from e1000 I prefer to do that at a much later time.
> Let's focus on e1000e instead - while it is still moving ;)
I think you may be in luck ;-> I just saw a patch posted by jgarzik
which touched both e1000/e LLTX. I am on travel mode starting tommorow,
I will try to extract it on my laptop while on the road but cant test
it.
cheers,
jamal
^ permalink raw reply
* Re: [PATCHES] TX batching rev2.5
From: jamal @ 2007-10-09 22:20 UTC (permalink / raw)
To: David Miller
Cc: krkumar2, johnpol, herbert, kaber, shemminger, jagana,
Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
peter.p.waskiewicz.jr, mcarlson, jeff, mchan, general, kumarkr,
tgraf, randy.dunlap, sri
In-Reply-To: <1191967639.5324.25.camel@localhost>
On Tue, 2007-09-10 at 18:07 -0400, jamal wrote:
> Please provide feedback on the code and/or architecture.
> They are now updated to work with the latest rebased net-2.6.24
> from a few hours ago.
I should have added i have tested this with just the batching changes
and it is within the performance realm of the changes from yesterday. If
anyone wants exact numbers, i can send them.
cheers,
jamal
^ permalink raw reply
* [DOC][NET_BATCH]Rev2.5 Driver Howto
From: jamal @ 2007-10-09 22:29 UTC (permalink / raw)
To: David Miller
Cc: krkumar2, johnpol, herbert, kaber, shemminger, jagana,
Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
peter.p.waskiewicz.jr, mcarlson, jeff, mchan, general, kumarkr,
tgraf, randy.dunlap, sri
[-- Attachment #1: Type: text/plain, Size: 62 bytes --]
I updated this doc to match the latest patch.
cheers,
jamal
[-- Attachment #2: batch-driver-howto.txt --]
[-- Type: text/plain, Size: 8550 bytes --]
Here's the beginning of a howto for driver authors.
The intended audience for this howto is people already
familiar with netdevices.
1.0 Netdevice Prerequisites
------------------------------
For hardware-based netdevices, you must have at least hardware that
is capable of doing DMA with many descriptors; i.e., having hardware
with a queue length of 3 (as in some fscked ethernet hardware) is
not very useful in this case.
2.0 What is new in the driver API
-----------------------------------
There is 1 new method and one new variable introduced that the
driver author needs to be aware of. These are:
1) dev->hard_end_xmit()
2) dev->xmit_win
2.1 Using Core driver changes
-----------------------------
To provide context, let's look at a typical driver abstraction
for dev->hard_start_xmit(). It has 4 parts:
a) packet formatting (example: vlan, mss, descriptor counting, etc.)
b) chip-specific formatting
c) enqueueing the packet on a DMA ring
d) IO operations to complete packet transmit, tell DMA engine to chew
on, tx completion interrupts, etc.
[For code cleanliness/readability sake, regardless of this work,
one should break the dev->hard_start_xmit() into those 4 functional
blocks anyways].
A driver which has all 4 parts and needing to support batching is
advised to split its dev->hard_start_xmit() in the following manner:
1) use its dev->hard_end_xmit() method to achieve #d
2) use dev->xmit_win to tell the core how much space you have.
#b and #c can stay in ->hard_start_xmit() (or whichever way you
want to do this)
Section 3. shows more details on the suggested usage.
2.1.1 Theory of operation
--------------------------
1. Core dequeues from qdiscs upto dev->xmit_win packets. Fragmented
and GSO packets are accounted for as well.
2. Core grabs device's TX_LOCK
3. Core loop for all skbs:
->invokes driver dev->hard_start_xmit()
4. Core invokes driver dev->hard_end_xmit() if packets xmitted
2.1.1.1 The slippery LLTX
-------------------------
Since these type of drivers are being phased out and they require extra
code they will not be supported anymore. So as oct07 the code that
supports them has been removed.
2.1.1.2 xmit_win
----------------
dev->xmit_win variable is set by the driver to tell us how
much space it has in its rings/queues. This detail is then
used to figure out how many packets are retrieved from the qdisc
queues (in order to send to the driver).
dev->xmit_win is introduced to ensure that when we pass the driver
a list of packets it will swallow all of them -- which is useful
because we don't requeue to the qdisc (and avoids burning unnecessary
CPU cycles or introducing any strange re-ordering).
Essentially the driver signals us how much space it has for descriptors
by setting this variable.
2.1.1.2.1 Setting xmit_win
--------------------------
This variable should be set during xmit path shutdown(netif_stop),
wakeup(netif_wake) and ->hard_end_xmit(). In the case of the first
one the value is set to 1 and in the other two it is set to whatever
the driver deems to be available space on the ring.
3.0 Driver Essentials
---------------------
The typical driver tx state machine is:
----
-1-> +Core sends packets
+--> Driver puts packet onto hardware queue
+ if hardware queue is full, netif_stop_queue(dev)
+
-2-> +core stops sending because of netif_stop_queue(dev)
..
.. time passes ...
..
-3-> +---> driver has transmitted packets, opens up tx path by
invoking netif_wake_queue(dev)
-1-> +Cycle repeats and core sends more packets (step 1).
----
3.1 Driver prerequisite
--------------------------
This is _a very important_ requirement in making batching useful.
The prerequisite for batching changes is that the driver should
provide a low threshold to open up the tx path.
Drivers such as tg3 and e1000 already do this.
Before you invoke netif_wake_queue(dev) you check if there is a
threshold of space reached to insert new packets.
Here's an example of how I added it to tun driver. Observe the
setting of dev->xmit_win.
---
+#define NETDEV_LTT 4 /* the low threshold to open up the tx path */
..
..
u32 t = skb_queue_len(&tun->readq);
if (netif_queue_stopped(tun->dev) && t < NETDEV_LTT) {
tun->dev->xmit_win = tun->dev->tx_queue_len;
netif_wake_queue(tun->dev);
}
---
Heres how the batching e1000 driver does it:
--
if (unlikely(cleaned && netif_carrier_ok(netdev) &&
E1000_DESC_UNUSED(tx_ring) >= TX_WAKE_THRESHOLD)) {
if (netif_queue_stopped(netdev)) {
int rspace = E1000_DESC_UNUSED(tx_ring) - (MAX_SKB_FRAGS + 2);
netdev->xmit_win = rspace;
netif_wake_queue(netdev);
}
---
in tg3 code (with no batching changes) looks like:
-----
if (netif_queue_stopped(tp->dev) &&
(tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp)))
netif_wake_queue(tp->dev);
---
3.2 Driver Setup
-----------------
*) On initialization (before netdev registration)
1) set NETIF_F_BTX in dev->features
i.e., dev->features |= NETIF_F_BTX
This makes the core do proper initialization.
2) set dev->xmit_win to something reasonable like
maybe half the tx DMA ring size etc.
3) create proper pointer to the ->hard_end_xmit() method.
3.3 Annotation on the different methods
----------------------------------------
This section shows examples and offers suggestions on how the different
methods and variable could be used.
3.3.1 dev->hard_start_xmit()
----------------------------
Here's an example of tx routine that is similar to the one I added
to the current tun driver. bxmit suffix is kept so that you can turn
off batching if needed via an ethtool interface
and call already existing interface.
----
static int xxx_net_bxmit(struct net_device *dev)
{
....
....
enqueue onto hardware ring
if (hardware ring full) {
netif_stop_queue(dev);
dev->xmit_win = 1;
}
.......
..
.
}
------
All return codes like NETDEV_TX_OK etc. still apply.
In addition a new code NETDEV_TX_DROPPED should be returned
if the packet is dropped. This helps the core layer to account
for transmitted packets and invoke dev->hard_end_xmit() at the
end of batch when one or more packets are transmitted..
3.3.2 The tx complete, dev->hard_end_xmit()
-------------------------------------------------
In this method, if there are any IO operations that apply to a
set of packets such as kicking DMA, setting of interrupt thresholds etc.,
leave them to the end and apply them once if you have successfully enqueued.
This provides a mechanism for saving a lot of CPU cycles since IO
is cycle expensive.
Here is a simplified tg3 dev->hard_end_xmit():
----
void tg3_complete_xmit(struct net_device *dev)
{
/* Packets are ready, update Tx producer idx local and on card. */
tw32_tx_mbox((MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW), entry);
if (unlikely(tg3_tx_avail(tp) <= (MAX_SKB_FRAGS + 1))) {
netif_stop_queue(dev);
dev->xmit_win = 1;
if (tg3_tx_avail(tp) >= TG3_TX_WAKEUP_THRESH(tp)) {
tg3_set_win(tp);
netif_wake_queue(dev);
}
} else {
tg3_set_win(tp);
}
mmiowb();
dev->trans_start = jiffies;
}
-------
3.3.3 setting the dev->xmit_win
---------------------------------
As mentioned earlier this variable provides hints on how much
data to send from the core to the driver. Here are the obvious ways:
a) on doing a netif_stop, set it to 1. By default all drivers have
this value set to 1 to emulate old behavior where a driver only
receives one packet at a time.
b) on netif_wake_queue set it to the max available space. You have
to be careful if your hardware does scatter-gather since the core
will pass you scatter-gatherable skbs and so you want to at least
leave enough space for the maximum allowed. Look at the tg3 and
e1000 to see how this is implemented.
The variable is important because it avoids the core sending
any more than what the driver can handle, therefore avoiding
any need to muck with packet scheduling mechanisms.
Appendix 1: History
-------------------
June 11/2007: Initial revision
June 11/2007: Fixed typo on e1000 netif_wake description ..
Aug 08/2007: Added info on VLAN and the skb->cb[] danger ..
Sep 24/2007: Revised and cleaned up
Sep 25/2007: Cleanups from Randy Dunlap
Oct 08/2007: Removed references to LLTX and packet formatting
Oct 09/2007: Added reference to NETDEV_TX_DROPPED
^ permalink raw reply
* [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Roland Dreier @ 2007-10-09 22:44 UTC (permalink / raw)
To: David Miller
Cc: jagana, herbert, gaagaan, Robert.Olsson, mcarlson,
peter.p.waskiewicz.jr, hadi, randy.dunlap, jeff, general, mchan,
tgraf, netdev, johnpol, shemminger, kaber, sri
In-Reply-To: <20071009.135145.95506679.davem@davemloft.net>
> Before you add new entries to your list, how is that ibm driver NAPI
> conversion coming along? :-)
OK, thanks for the kick in the pants, I have a couple of patches for
net-2.6.24 coming (including an unrelated trivial warning fix for
IPoIB).
- R.
^ permalink raw reply
* [PATCH] natsemi: Use round_jiffies() for slow timers
From: Mark Brown @ 2007-10-09 21:57 UTC (permalink / raw)
To: jgarzik, thockin; +Cc: netdev, linux-kernel, Mark Brown
Unless we have failed to fill the RX ring the timer used by the natsemi
driver is not particularly urgent and can use round_jiffies() to allow
grouping with other timers.
Signed-off-by: Mark Brown <broonie@sirena.org.uk>
---
drivers/net/natsemi.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
index b47a12d..0b33a58 100644
--- a/drivers/net/natsemi.c
+++ b/drivers/net/natsemi.c
@@ -1575,7 +1575,7 @@ static int netdev_open(struct net_device *dev)
/* Set the timer to check for link beat. */
init_timer(&np->timer);
- np->timer.expires = jiffies + NATSEMI_TIMER_FREQ;
+ np->timer.expires = round_jiffies(jiffies + NATSEMI_TIMER_FREQ);
np->timer.data = (unsigned long)dev;
np->timer.function = &netdev_timer; /* timer handler */
add_timer(&np->timer);
@@ -1855,7 +1855,11 @@ static void netdev_timer(unsigned long data)
next_tick = 1;
}
}
- mod_timer(&np->timer, jiffies + next_tick);
+
+ if (next_tick > 1)
+ mod_timer(&np->timer, round_jiffies(jiffies + next_tick));
+ else
+ mod_timer(&np->timer, jiffies + next_tick);
}
static void dump_ring(struct net_device *dev)
@@ -3330,7 +3334,7 @@ static int natsemi_resume (struct pci_dev *pdev)
spin_unlock_irq(&np->lock);
enable_irq(dev->irq);
- mod_timer(&np->timer, jiffies + 1*HZ);
+ mod_timer(&np->timer, round_jiffies(jiffies + 1*HZ));
}
netif_device_attach(dev);
netif_poll_enable(dev);
--
1.5.3.4
^ permalink raw reply related
* [ofa-general] [PATCH 1/4] IPoIB: Fix unused variable warning
From: Roland Dreier @ 2007-10-09 22:46 UTC (permalink / raw)
To: David Miller; +Cc: netdev, general, jeff
In-Reply-To: <adahckzhprc.fsf@cisco.com>
The conversion to use netdevice internal stats left an unused variable
in ipoib_neigh_free(), since there's no longer any reason to get
netdev_priv() in order to increment dropped packets. Delete the
unused priv variable.
Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 6b1b4b2..855c9de 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -854,7 +854,6 @@ struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour *neighbour)
void ipoib_neigh_free(struct net_device *dev, struct ipoib_neigh *neigh)
{
- struct ipoib_dev_priv *priv = netdev_priv(dev);
struct sk_buff *skb;
*to_ipoib_neigh(neigh->neighbour) = NULL;
while ((skb = __skb_dequeue(&neigh->queue))) {
^ permalink raw reply related
* [ofa-general] [PATCH 2/4] ibm_emac: Convert to use napi_struct independent of struct net_device
From: Roland Dreier @ 2007-10-09 22:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jeff, general
In-Reply-To: <adad4vnhpoq.fsf_-_@cisco.com>
Commit da3dedd9 ("[NET]: Make NAPI polling independent of struct
net_device objects.") changed the interface to NAPI polling. Fix up
the ibm_emac driver so that it works with this new interface. This is
actually a nice cleanup because ibm_emac is one of the drivers that
wants to have multiple NAPI structures for a single net_device.
Tested with the internal MAC of a PowerPC 440SPe SoC with an AMCC
'Yucca' evaluation board.
Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
drivers/net/ibm_emac/ibm_emac_mal.c | 48 ++++++++++++----------------------
drivers/net/ibm_emac/ibm_emac_mal.h | 2 +-
include/linux/netdevice.h | 10 +++++++
3 files changed, 28 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ibm_emac/ibm_emac_mal.c b/drivers/net/ibm_emac/ibm_emac_mal.c
index cabd984..cc3ddc9 100644
--- a/drivers/net/ibm_emac/ibm_emac_mal.c
+++ b/drivers/net/ibm_emac/ibm_emac_mal.c
@@ -207,10 +207,10 @@ static irqreturn_t mal_serr(int irq, void *dev_instance)
static inline void mal_schedule_poll(struct ibm_ocp_mal *mal)
{
- if (likely(netif_rx_schedule_prep(&mal->poll_dev))) {
+ if (likely(napi_schedule_prep(&mal->napi))) {
MAL_DBG2("%d: schedule_poll" NL, mal->def->index);
mal_disable_eob_irq(mal);
- __netif_rx_schedule(&mal->poll_dev);
+ __napi_schedule(&mal->napi);
} else
MAL_DBG2("%d: already in poll" NL, mal->def->index);
}
@@ -273,11 +273,11 @@ static irqreturn_t mal_rxde(int irq, void *dev_instance)
return IRQ_HANDLED;
}
-static int mal_poll(struct net_device *ndev, int *budget)
+static int mal_poll(struct napi_struct *napi, int budget)
{
- struct ibm_ocp_mal *mal = ndev->priv;
+ struct ibm_ocp_mal *mal = container_of(napi, struct ibm_ocp_mal, napi);
struct list_head *l;
- int rx_work_limit = min(ndev->quota, *budget), received = 0, done;
+ int received = 0;
MAL_DBG2("%d: poll(%d) %d ->" NL, mal->def->index, *budget,
rx_work_limit);
@@ -295,38 +295,34 @@ static int mal_poll(struct net_device *ndev, int *budget)
list_for_each(l, &mal->poll_list) {
struct mal_commac *mc =
list_entry(l, struct mal_commac, poll_list);
- int n = mc->ops->poll_rx(mc->dev, rx_work_limit);
+ int n = mc->ops->poll_rx(mc->dev, budget);
if (n) {
received += n;
- rx_work_limit -= n;
- if (rx_work_limit <= 0) {
- done = 0;
+ budget -= n;
+ if (budget <= 0)
goto more_work; // XXX What if this is the last one ?
- }
}
}
/* We need to disable IRQs to protect from RXDE IRQ here */
local_irq_disable();
- __netif_rx_complete(ndev);
+ __napi_complete(napi);
mal_enable_eob_irq(mal);
local_irq_enable();
- done = 1;
-
/* Check for "rotting" packet(s) */
list_for_each(l, &mal->poll_list) {
struct mal_commac *mc =
list_entry(l, struct mal_commac, poll_list);
if (unlikely(mc->ops->peek_rx(mc->dev) || mc->rx_stopped)) {
MAL_DBG2("%d: rotting packet" NL, mal->def->index);
- if (netif_rx_reschedule(ndev, received))
+ if (napi_reschedule(napi))
mal_disable_eob_irq(mal);
else
MAL_DBG2("%d: already in poll list" NL,
mal->def->index);
- if (rx_work_limit > 0)
+ if (budget > 0)
goto again;
else
goto more_work;
@@ -335,12 +331,8 @@ static int mal_poll(struct net_device *ndev, int *budget)
}
more_work:
- ndev->quota -= received;
- *budget -= received;
-
- MAL_DBG2("%d: poll() %d <- %d" NL, mal->def->index, *budget,
- done ? 0 : 1);
- return done ? 0 : 1;
+ MAL_DBG2("%d: poll() %d <- %d" NL, mal->def->index, budget, received);
+ return received;
}
static void mal_reset(struct ibm_ocp_mal *mal)
@@ -425,11 +417,8 @@ static int __init mal_probe(struct ocp_device *ocpdev)
mal->def = ocpdev->def;
INIT_LIST_HEAD(&mal->poll_list);
- set_bit(__LINK_STATE_START, &mal->poll_dev.state);
- mal->poll_dev.weight = CONFIG_IBM_EMAC_POLL_WEIGHT;
- mal->poll_dev.poll = mal_poll;
- mal->poll_dev.priv = mal;
- atomic_set(&mal->poll_dev.refcnt, 1);
+ mal->napi.weight = CONFIG_IBM_EMAC_POLL_WEIGHT;
+ mal->napi.poll = mal_poll;
INIT_LIST_HEAD(&mal->list);
@@ -520,11 +509,8 @@ static void __exit mal_remove(struct ocp_device *ocpdev)
MAL_DBG("%d: remove" NL, mal->def->index);
- /* Syncronize with scheduled polling,
- stolen from net/core/dev.c:dev_close()
- */
- clear_bit(__LINK_STATE_START, &mal->poll_dev.state);
- netif_poll_disable(&mal->poll_dev);
+ /* Synchronize with scheduled polling */
+ napi_disable(&mal->napi);
if (!list_empty(&mal->list)) {
/* This is *very* bad */
diff --git a/drivers/net/ibm_emac/ibm_emac_mal.h b/drivers/net/ibm_emac/ibm_emac_mal.h
index 64bc338..8f54d62 100644
--- a/drivers/net/ibm_emac/ibm_emac_mal.h
+++ b/drivers/net/ibm_emac/ibm_emac_mal.h
@@ -195,7 +195,7 @@ struct ibm_ocp_mal {
dcr_host_t dcrhost;
struct list_head poll_list;
- struct net_device poll_dev;
+ struct napi_struct napi;
struct list_head list;
u32 tx_chan_mask;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 91cd3f3..4848c7a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -349,6 +349,16 @@ static inline void napi_schedule(struct napi_struct *n)
__napi_schedule(n);
}
+/* Try to reschedule poll. Called by dev->poll() after napi_complete(). */
+static inline int napi_reschedule(struct napi_struct *napi)
+{
+ if (napi_schedule_prep(napi)) {
+ __napi_schedule(napi);
+ return 1;
+ }
+ return 0;
+}
+
/**
* napi_complete - NAPI processing complete
* @n: napi context
^ permalink raw reply related
* [PATCH 3/4] ibm_new_emac: Nuke SET_MODULE_OWNER() use
From: Roland Dreier @ 2007-10-09 22:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, general, jeff
In-Reply-To: <adad4vnhpoq.fsf_-_@cisco.com>
Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
drivers/net/ibm_newemac/core.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c
index ce127b9..8ea5009 100644
--- a/drivers/net/ibm_newemac/core.c
+++ b/drivers/net/ibm_newemac/core.c
@@ -2549,7 +2549,6 @@ static int __devinit emac_probe(struct of_device *ofdev,
dev->ndev = ndev;
dev->ofdev = ofdev;
dev->blist = blist;
- SET_MODULE_OWNER(ndev);
SET_NETDEV_DEV(ndev, &ofdev->dev);
/* Initialize some embedded data structures */
^ permalink raw reply related
* [PATCH 4/4] ibm_emac: Convert to use napi_struct independent of struct net_device
From: Roland Dreier @ 2007-10-09 22:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, general, jeff
In-Reply-To: <adad4vnhpoq.fsf_-_@cisco.com>
Commit da3dedd9 ("[NET]: Make NAPI polling independent of struct
net_device objects.") changed the interface to NAPI polling. Fix up
the ibm_newemac driver so that it works with this new interface. This
is actually a nice cleanup because ibm_newemac is one of the drivers
that wants to have multiple NAPI structures for a single net_device.
Compile-tested only as I don't have a system that uses the ibm_newemac
driver. This conversion the conversion for the ibm_emac driver that
was tested on real PowerPC 440SPe hardware.
Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
drivers/net/ibm_newemac/mal.c | 55 ++++++++++++++--------------------------
drivers/net/ibm_newemac/mal.h | 2 +-
2 files changed, 20 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ibm_newemac/mal.c b/drivers/net/ibm_newemac/mal.c
index c4335b7..5885411 100644
--- a/drivers/net/ibm_newemac/mal.c
+++ b/drivers/net/ibm_newemac/mal.c
@@ -235,10 +235,10 @@ static irqreturn_t mal_serr(int irq, void *dev_instance)
static inline void mal_schedule_poll(struct mal_instance *mal)
{
- if (likely(netif_rx_schedule_prep(&mal->poll_dev))) {
+ if (likely(napi_schedule_prep(&mal->napi))) {
MAL_DBG2(mal, "schedule_poll" NL);
mal_disable_eob_irq(mal);
- __netif_rx_schedule(&mal->poll_dev);
+ __napi_schedule(&mal->napi);
} else
MAL_DBG2(mal, "already in poll" NL);
}
@@ -318,8 +318,7 @@ void mal_poll_disable(struct mal_instance *mal, struct mal_commac *commac)
msleep(1);
/* Synchronize with the MAL NAPI poller. */
- while (test_bit(__LINK_STATE_RX_SCHED, &mal->poll_dev.state))
- msleep(1);
+ napi_disable(&mal->napi);
}
void mal_poll_enable(struct mal_instance *mal, struct mal_commac *commac)
@@ -330,11 +329,11 @@ void mal_poll_enable(struct mal_instance *mal, struct mal_commac *commac)
// XXX might want to kick a poll now...
}
-static int mal_poll(struct net_device *ndev, int *budget)
+static int mal_poll(struct napi_struct *napi, int budget)
{
- struct mal_instance *mal = netdev_priv(ndev);
+ struct mal_instance *mal = container_of(napi, struct mal_instance, napi);
struct list_head *l;
- int rx_work_limit = min(ndev->quota, *budget), received = 0, done;
+ int received = 0;
unsigned long flags;
MAL_DBG2(mal, "poll(%d) %d ->" NL, *budget,
@@ -358,26 +357,21 @@ static int mal_poll(struct net_device *ndev, int *budget)
int n;
if (unlikely(test_bit(MAL_COMMAC_POLL_DISABLED, &mc->flags)))
continue;
- n = mc->ops->poll_rx(mc->dev, rx_work_limit);
+ n = mc->ops->poll_rx(mc->dev, budget);
if (n) {
received += n;
- rx_work_limit -= n;
- if (rx_work_limit <= 0) {
- done = 0;
- // XXX What if this is the last one ?
- goto more_work;
- }
+ budget -= n;
+ if (budget <= 0)
+ goto more_work; // XXX What if this is the last one ?
}
}
/* We need to disable IRQs to protect from RXDE IRQ here */
spin_lock_irqsave(&mal->lock, flags);
- __netif_rx_complete(ndev);
+ __napi_complete(napi);
mal_enable_eob_irq(mal);
spin_unlock_irqrestore(&mal->lock, flags);
- done = 1;
-
/* Check for "rotting" packet(s) */
list_for_each(l, &mal->poll_list) {
struct mal_commac *mc =
@@ -387,12 +381,12 @@ static int mal_poll(struct net_device *ndev, int *budget)
if (unlikely(mc->ops->peek_rx(mc->dev) ||
test_bit(MAL_COMMAC_RX_STOPPED, &mc->flags))) {
MAL_DBG2(mal, "rotting packet" NL);
- if (netif_rx_reschedule(ndev, received))
+ if (napi_reschedule(napi))
mal_disable_eob_irq(mal);
else
MAL_DBG2(mal, "already in poll list" NL);
- if (rx_work_limit > 0)
+ if (budget > 0)
goto again;
else
goto more_work;
@@ -401,13 +395,8 @@ static int mal_poll(struct net_device *ndev, int *budget)
}
more_work:
- ndev->quota -= received;
- *budget -= received;
-
- MAL_DBG2(mal, "poll() %d <- %d" NL, *budget,
- done ? 0 : 1);
-
- return done ? 0 : 1;
+ MAL_DBG2(mal, "poll() %d <- %d" NL, budget, received);
+ return received;
}
static void mal_reset(struct mal_instance *mal)
@@ -538,11 +527,8 @@ static int __devinit mal_probe(struct of_device *ofdev,
}
INIT_LIST_HEAD(&mal->poll_list);
- set_bit(__LINK_STATE_START, &mal->poll_dev.state);
- mal->poll_dev.weight = CONFIG_IBM_NEW_EMAC_POLL_WEIGHT;
- mal->poll_dev.poll = mal_poll;
- mal->poll_dev.priv = mal;
- atomic_set(&mal->poll_dev.refcnt, 1);
+ mal->napi.weight = CONFIG_IBM_NEW_EMAC_POLL_WEIGHT;
+ mal->napi.poll = mal_poll;
INIT_LIST_HEAD(&mal->list);
spin_lock_init(&mal->lock);
@@ -653,11 +639,8 @@ static int __devexit mal_remove(struct of_device *ofdev)
MAL_DBG(mal, "remove" NL);
- /* Syncronize with scheduled polling,
- stolen from net/core/dev.c:dev_close()
- */
- clear_bit(__LINK_STATE_START, &mal->poll_dev.state);
- netif_poll_disable(&mal->poll_dev);
+ /* Synchronize with scheduled polling */
+ napi_disable(&mal->napi);
if (!list_empty(&mal->list)) {
/* This is *very* bad */
diff --git a/drivers/net/ibm_newemac/mal.h b/drivers/net/ibm_newemac/mal.h
index 57b69dc..cb1a16d 100644
--- a/drivers/net/ibm_newemac/mal.h
+++ b/drivers/net/ibm_newemac/mal.h
@@ -197,7 +197,7 @@ struct mal_instance {
int serr_irq; /* MAL System Error IRQ */
struct list_head poll_list;
- struct net_device poll_dev;
+ struct napi_struct napi;
struct list_head list;
u32 tx_chan_mask;
^ permalink raw reply related
* Re: [ofa-general] [PATCH 4/4] ibm_emac: Convert to use napi_struct independent of struct net_device
From: Roland Dreier @ 2007-10-09 22:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, general, jeff
In-Reply-To: <adazlyrgazr.fsf_-_@cisco.com>
Sorry... wrong subject here; it should have been "ibm_newemac: ..."
- R.
^ permalink raw reply
* Re: [PATCH 1/4] IPoIB: Fix unused variable warning
From: David Miller @ 2007-10-09 23:17 UTC (permalink / raw)
To: rdreier; +Cc: jeff, general, netdev
In-Reply-To: <adad4vnhpoq.fsf_-_@cisco.com>
From: Roland Dreier <rdreier@cisco.com>
Date: Tue, 09 Oct 2007 15:46:13 -0700
> The conversion to use netdevice internal stats left an unused variable
> in ipoib_neigh_free(), since there's no longer any reason to get
> netdev_priv() in order to increment dropped packets. Delete the
> unused priv variable.
>
> Signed-off-by: Roland Dreier <rolandd@cisco.com>
Jeff, do you want to merge in Roland's 4 patches to your tree then do
a sync with me so I can pull it all in from you?
Alternative I can merge in Roland's work directly if that's easier
for you.
Just let me know.
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: David Miller @ 2007-10-10 0:04 UTC (permalink / raw)
To: hadi
Cc: johnpol, Robert.Olsson, herbert, gaagaan, jeff, rdreier,
peter.p.waskiewicz.jr, mcarlson, andi, general, netdev, tgraf,
randy.dunlap, sri, shemminger, kaber, mchan, jagana
In-Reply-To: <1191967006.5324.14.camel@localhost>
From: jamal <hadi@cyberus.ca>
Date: Tue, 09 Oct 2007 17:56:46 -0400
> if the h/ware queues are full because of link pressure etc, you drop. We
> drop today when the s/ware queues are full. The driver txmit lock takes
> place of the qdisc queue lock etc. I am assuming there is still need for
> that locking. The filter/classification scheme still works as is and
> select classes which map to rings. tc still works as is etc.
I understand your suggestion.
We have to keep in mind, however, that the sw queue right now is 1000
packets. I heavily discourage any driver author to try and use any
single TX queue of that size. Which means that just dropping on back
pressure might not work so well.
Or it might be perfect and signal TCP to backoff, who knows! :-)
While working out this issue in my mind, it occured to me that we
can put the sw queue into the driver as well.
The idea is that the network stack, as in the pure hw queue scheme,
unconditionally always submits new packets to the driver. Therefore
even if the hw TX queue is full, the driver can still queue to an
internal sw queue with some limit (say 1000 for ethernet, as is used
now).
When the hw TX queue gains space, the driver self-batches packets
from the sw queue to the hw queue.
It sort of obviates the need for mid-level queue batching in the
generic networking. Compared to letting the driver self-batch,
the mid-level batching approach is pure overhead.
We seem to be sort of all mentioning similar ideas. For example, you
can get the above kind of scheme today by using a mid-level queue
length of zero, and I believe this idea was mentioned by Stephen
Hemminger earlier.
I may experiment with this in the NIU driver.
^ permalink raw reply
* Re: [PATCH 1/4] IPoIB: Fix unused variable warning
From: Jeff Garzik @ 2007-10-10 0:32 UTC (permalink / raw)
To: David Miller; +Cc: rdreier, general, netdev
In-Reply-To: <20071009.161750.02298910.davem@davemloft.net>
David Miller wrote:
> From: Roland Dreier <rdreier@cisco.com>
> Date: Tue, 09 Oct 2007 15:46:13 -0700
>
>> The conversion to use netdevice internal stats left an unused variable
>> in ipoib_neigh_free(), since there's no longer any reason to get
>> netdev_priv() in order to increment dropped packets. Delete the
>> unused priv variable.
>>
>> Signed-off-by: Roland Dreier <rolandd@cisco.com>
>
> Jeff, do you want to merge in Roland's 4 patches to your tree then do
> a sync with me so I can pull it all in from you?
Grabbing them now...
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Andi Kleen @ 2007-10-10 0:37 UTC (permalink / raw)
To: David Miller
Cc: johnpol, Robert.Olsson, herbert, jeff, netdev, rdreier,
peter.p.waskiewicz.jr, hadi, mcarlson, gaagaan, andi, general,
jagana, tgraf, randy.dunlap, shemminger, kaber, mchan, sri
In-Reply-To: <20071009.170435.43504422.davem@davemloft.net>
On Tue, Oct 09, 2007 at 05:04:35PM -0700, David Miller wrote:
> We have to keep in mind, however, that the sw queue right now is 1000
> packets. I heavily discourage any driver author to try and use any
> single TX queue of that size.
Why would you discourage them?
If 1000 is ok for a software queue why would it not be ok
for a hardware queue?
> Which means that just dropping on back
> pressure might not work so well.
>
> Or it might be perfect and signal TCP to backoff, who knows! :-)
1000 packets is a lot. I don't have hard data, but gut feeling
is less would also do.
And if the hw queues are not enough a better scheme might be to
just manage this in the sockets in sendmsg. e.g. provide a wait queue that
drivers can wake up and let them block on more queue.
> The idea is that the network stack, as in the pure hw queue scheme,
> unconditionally always submits new packets to the driver. Therefore
> even if the hw TX queue is full, the driver can still queue to an
> internal sw queue with some limit (say 1000 for ethernet, as is used
> now).
>
>
> When the hw TX queue gains space, the driver self-batches packets
> from the sw queue to the hw queue.
I don't really see the advantage over the qdisc in that scheme.
It's certainly not simpler and probably more code and would likely
also not require less locks (e.g. a currently lockless driver
would need a new lock for its sw queue). Also it is unclear to me
it would be really any faster.
-Andi
^ permalink raw reply
* Please pull 'upstream-davem' branch of wireless-2.6
From: John W. Linville @ 2007-10-10 0:21 UTC (permalink / raw)
To: davem; +Cc: jeff, linux-wireless, netdev
[-- Attachment #1: Type: text/plain, Size: 4219 bytes --]
Jeff & Dave,
A last round of patches, hopefully in time for 2.6.24.
Thanks!
John
---
The following changes since commit 7b4cd20628fe5c4e145c383fcd8d954d38f7be61:
Benjamin Thery (1):
[IPv6]: use container_of() macro in fib6_clean_node()
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git upstream-davem
Dan Williams (2):
ipw2200: batch non-user-requested scan result notifications
libertas: problems setting wpa keys
Daniel Drake (1):
zd1211rw: Add ID for TalkTalk SNU5630NS/05
Geert Uytterhoeven (1):
libertas link error due to gcc `smartness'
Holger Schurig (6):
libertas: fix u8 constant
libertas: fix a debug statement
libertas: fix "warning: Using plain integer as NULL pointer" sparse warnings
libertas: clean up scan debug messages
libertas: remove one superfluous include
libertas: let get nick return what set nick has set
Ivo van Doorn (16):
rt2x00: Remove duplicate code in MAC & BSSID handling
rt2x00: Remove radio check from rt2x00lib_toggle_rx
rt2x00: Store "STARTED" state during suspend
rt2x00: Move rt2x00dev flags into enumeration
rt2x00: Don't use changed_flags inside configure_packet_filter
rt2x00: Fix rfkill handling
rt2x00: Move TSF sync values into rt2x00config
rt2x00: get_duration expects values in 100kbs
rt2x00: Cut lines down to 80 characters
rt2x00: Add get_tx_data_len callback function
rt2x00: Pass dev_state to rt2x00lib_toggle_rx
rt2x00: Small optimizations
rt2x00: Reorganize configuration handler
rt2x00: Clean disabling of rt73usb_get_tsf
rt2x00: Allways memset memory obtained from skb_push()
rt2x00: Release 2.0.10
John W. Linville (1):
ieee80211_if_set_type: make check for master dev more explicit
Marcin Juszkiewicz (1):
Add Linksys card to HostAP driver
Martti Huttunen (1):
p54usb: Add device ID for Linksys WUSB54AG
Mattias Nissler (1):
mac80211: Defer setting of RX_FLAG_DECRYPTED.
Michael Wu (1):
rtl8187: Add device ID for HP wireless print kit usb dongle
Stefan Lippers-Hollmann (1):
hostap_cs: Add device ID for Telekom T-Sinus 111card
drivers/net/wireless/hostap/hostap_cs.c | 5 +
drivers/net/wireless/ipw2200.c | 56 ++++++--
drivers/net/wireless/ipw2200.h | 3 +
drivers/net/wireless/libertas/assoc.c | 35 ++++-
drivers/net/wireless/libertas/cmd.c | 4 +-
drivers/net/wireless/libertas/cmdresp.c | 2 +-
drivers/net/wireless/libertas/debugfs.c | 5 +-
drivers/net/wireless/libertas/host.h | 4 +-
drivers/net/wireless/libertas/if_usb.c | 1 -
drivers/net/wireless/libertas/join.c | 2 -
drivers/net/wireless/libertas/scan.c | 223 +++++++++++++++++++---------
drivers/net/wireless/libertas/wext.c | 34 +----
drivers/net/wireless/p54usb.c | 1 +
drivers/net/wireless/rt2x00/rt2400pci.c | 186 +++++++++--------------
drivers/net/wireless/rt2x00/rt2500pci.c | 201 ++++++++++---------------
drivers/net/wireless/rt2x00/rt2500usb.c | 200 +++++++++++--------------
drivers/net/wireless/rt2x00/rt2x00.h | 93 +++++++++---
drivers/net/wireless/rt2x00/rt2x00config.c | 113 +++++++++++++-
drivers/net/wireless/rt2x00/rt2x00dev.c | 52 +++++---
drivers/net/wireless/rt2x00/rt2x00lib.h | 14 +--
drivers/net/wireless/rt2x00/rt2x00mac.c | 38 +++++-
drivers/net/wireless/rt2x00/rt2x00reg.h | 9 +
drivers/net/wireless/rt2x00/rt2x00rfkill.c | 4 +-
drivers/net/wireless/rt2x00/rt2x00usb.c | 29 ++--
drivers/net/wireless/rt2x00/rt61pci.c | 166 +++++++--------------
drivers/net/wireless/rt2x00/rt73usb.c | 195 ++++++++++---------------
drivers/net/wireless/rtl8187_dev.c | 2 +
drivers/net/wireless/zd1211rw/zd_usb.c | 1 +
net/mac80211/ieee80211_iface.c | 2 +-
net/mac80211/rx.c | 20 ++-
30 files changed, 913 insertions(+), 787 deletions(-)
Omnibus patch attached as 'upstream-davem.patch.bz2'.
--
John W. Linville
linville@tuxdriver.com
[-- Attachment #2: upstream-davem.patch.bz2 --]
[-- Type: application/x-bzip2, Size: 21603 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] e1000: Simple optimizations in e1000_xmit_frame
From: Jeff Garzik @ 2007-10-10 0:38 UTC (permalink / raw)
To: Auke Kok; +Cc: netdev, krkumar2
In-Reply-To: <20071005211516.13167.80891.stgit@localhost.localdomain>
Auke Kok wrote:
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> Some simple optimizations in e1000_xmit_frame.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>
> drivers/net/e1000/e1000_main.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
applied 1-2
^ permalink raw reply
* Re: [PATCH net-2.6.24] network drivers: sparse warning fixes
From: Jeff Garzik @ 2007-10-10 0:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20071005171947.340f60af@freepuppy.rosehill>
Stephen Hemminger wrote:
> Fix some of the easy warnings in network device drivers.
>
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
>
> ---
> drivers/net/acenic.c | 37 +++++++++++++++++++------------------
> drivers/net/atl1/atl1_main.c | 5 +++--
> drivers/net/dl2k.c | 8 ++++----
> drivers/net/e100.c | 2 +-
> drivers/net/hamachi.c | 3 +--
> drivers/net/natsemi.c | 6 +++---
> drivers/net/tehuti.c | 4 +---
> 7 files changed, 32 insertions(+), 33 deletions(-)
applied
^ permalink raw reply
* Re: [PATCH] ehea: use kernel event queue
From: Jeff Garzik @ 2007-10-10 0:39 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: netdev, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, Thomas Klein, Stefan Roscher,
Anton Blanchard
In-Reply-To: <200710081601.34067.ossthema@de.ibm.com>
Jan-Bernd Themann wrote:
> eHEA recovery and DLPAR functions are called seldomly. The eHEA workqueues
> are replaced by the kernel event queue.
>
> Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com>
>
> ---
> The patch has been built against upstream git
>
> drivers/net/ehea/ehea.h | 3 +--
> drivers/net/ehea/ehea_main.c | 28 ++++++++--------------------
> drivers/net/ehea/ehea_qmr.c | 3 +--
> 3 files changed, 10 insertions(+), 24 deletions(-)
applied
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox