Netdev List
 help / color / mirror / Atom feed
* [PATCH 00/01] pktgen: Lindent run.
From: Luiz Fernando Capitulino @ 2006-01-23 15:44 UTC (permalink / raw)
  To: davem; +Cc: lkml, netdev, robert.olsson


 This patch is not in-lined because it's 120K bytes long, you can found it at:

http://www.cpu.eti.br/patches/pktgen_lindent_1.patch

-- 
Luiz Fernando N. Capitulino

^ permalink raw reply

* [PATCH 00/02] pktgen: Ports thread list to Kernel list implementation.
From: Luiz Fernando Capitulino @ 2006-01-23 15:45 UTC (permalink / raw)
  To: davem; +Cc: lkml, netdev, robert.olsson


  Ports the thread list to use the Linux Kernel linked list implementation.
The final result is a simpler and smaller code.

 Note that I'm adding a new member in the struct pktgen_thread called
'removed'. The reason is that I didn't find a better wait condition to be
used in the place of the replaced one. Suggestions are very welcome.

Signed-off-by: Luiz Capitulino <lcapitulino@mandriva.com.br>

 net/core/pktgen.c |   96 +++++++++++++++++++++++-------------------------------
 1 file changed, 41 insertions(+), 55 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index c6be6cd..ee26c69 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -125,6 +125,7 @@
 #include <linux/capability.h>
 #include <linux/delay.h>
 #include <linux/timer.h>
+#include <linux/list.h>
 #include <linux/init.h>
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
@@ -327,7 +328,8 @@ struct pktgen_hdr {
 struct pktgen_thread {
 	spinlock_t if_lock;
 	struct pktgen_dev *if_list;	/* All device here */
-	struct pktgen_thread *next;
+	struct list_head th_list;
+	int removed;
 	char name[32];
 	char result[512];
 	u32 max_before_softirq;	/* We'll call do_softirq to prevent starvation. */
@@ -489,7 +491,7 @@ static int pg_clone_skb_d;
 static int debug;
 
 static DECLARE_MUTEX(pktgen_sem);
-static struct pktgen_thread *pktgen_threads = NULL;
+static LIST_HEAD(pktgen_threads);
 
 static struct notifier_block pktgen_notifier_block = {
 	.notifier_call = pktgen_device_event,
@@ -1521,9 +1523,7 @@ static struct pktgen_dev *__pktgen_NN_th
 	struct pktgen_thread *t;
 	struct pktgen_dev *pkt_dev = NULL;
 
-	t = pktgen_threads;
-
-	while (t) {
+	list_for_each_entry(t, &pktgen_threads, th_list) {
 		pkt_dev = pktgen_find_dev(t, ifname);
 		if (pkt_dev) {
 			if (remove) {
@@ -1533,7 +1533,6 @@ static struct pktgen_dev *__pktgen_NN_th
 			}
 			break;
 		}
-		t = t->next;
 	}
 	return pkt_dev;
 }
@@ -2421,15 +2420,15 @@ static void pktgen_run(struct pktgen_thr
 
 static void pktgen_stop_all_threads_ifs(void)
 {
-	struct pktgen_thread *t = pktgen_threads;
+	struct pktgen_thread *t;
 
 	PG_DEBUG(printk("pktgen: entering pktgen_stop_all_threads.\n"));
 
 	thread_lock();
-	while (t) {
+
+	list_for_each_entry(t, &pktgen_threads, th_list)
 		pktgen_stop(t);
-		t = t->next;
-	}
+
 	thread_unlock();
 }
 
@@ -2469,40 +2468,36 @@ static int pktgen_wait_thread_run(struct
 
 static int pktgen_wait_all_threads_run(void)
 {
-	struct pktgen_thread *t = pktgen_threads;
+	struct pktgen_thread *t;
 	int sig = 1;
 
-	while (t) {
+	thread_lock();
+
+	list_for_each_entry(t, &pktgen_threads, th_list) {
 		sig = pktgen_wait_thread_run(t);
 		if (sig == 0)
 			break;
-		thread_lock();
-		t = t->next;
-		thread_unlock();
 	}
-	if (sig == 0) {
-		thread_lock();
-		while (t) {
+
+	if (sig == 0)
+		list_for_each_entry(t, &pktgen_threads, th_list)
 			t->control |= (T_STOP);
-			t = t->next;
-		}
-		thread_unlock();
-	}
+
+	thread_unlock();
 	return sig;
 }
 
 static void pktgen_run_all_threads(void)
 {
-	struct pktgen_thread *t = pktgen_threads;
+	struct pktgen_thread *t;
 
 	PG_DEBUG(printk("pktgen: entering pktgen_run_all_threads.\n"));
 
 	thread_lock();
 
-	while (t) {
+	list_for_each_entry(t, &pktgen_threads, th_list)
 		t->control |= (T_RUN);
-		t = t->next;
-	}
+
 	thread_unlock();
 
 	schedule_timeout_interruptible(msecs_to_jiffies(125));	/* Propagate thread->control  */
@@ -2622,24 +2617,12 @@ static void pktgen_rem_thread(struct pkt
 {
 	/* Remove from the thread list */
 
-	struct pktgen_thread *tmp = pktgen_threads;
-
 	remove_proc_entry(t->name, pg_proc_dir);
 
 	thread_lock();
 
-	if (tmp == t)
-		pktgen_threads = tmp->next;
-	else {
-		while (tmp) {
-			if (tmp->next == t) {
-				tmp->next = t->next;
-				t->next = NULL;
-				break;
-			}
-			tmp = tmp->next;
-		}
-	}
+	list_del(&t->th_list);
+
 	thread_unlock();
 }
 
@@ -2886,6 +2869,8 @@ static void pktgen_thread_worker(struct 
 
 	PG_DEBUG(printk("pktgen: %s removing thread.\n", t->name));
 	pktgen_rem_thread(t);
+
+	t->removed = 1;
 }
 
 static struct pktgen_dev *pktgen_find_dev(struct pktgen_thread *t,
@@ -2997,19 +2982,18 @@ static int pktgen_add_device(struct pktg
 
 static struct pktgen_thread *__init pktgen_find_thread(const char *name)
 {
-	struct pktgen_thread *t = NULL;
+	struct pktgen_thread *t;
 
 	thread_lock();
 
-	t = pktgen_threads;
-	while (t) {
-		if (strcmp(t->name, name) == 0)
-			break;
+	list_for_each_entry(t, &pktgen_threads, th_list)
+		if (strcmp(t->name, name) == 0) {
+			thread_unlock();
+			return t;
+		}
 
-		t = t->next;
-	}
 	thread_unlock();
-	return t;
+	return NULL;
 }
 
 static int __init pktgen_create_thread(const char *name, int cpu)
@@ -3048,8 +3032,9 @@ static int __init pktgen_create_thread(c
 	pe->proc_fops = &pktgen_thread_fops;
 	pe->data = t;
 
-	t->next = pktgen_threads;
-	pktgen_threads = t;
+	list_add_tail(&t->th_list, &pktgen_threads);
+
+	t->removed = 0;
 
 	if (kernel_thread((void *)pktgen_thread_worker, (void *)t,
 			  CLONE_FS | CLONE_FILES | CLONE_SIGHAND) < 0)
@@ -3150,17 +3135,18 @@ static int __init pg_init(void)
 
 static void __exit pg_cleanup(void)
 {
+	struct pktgen_thread *t;
+	struct list_head *q, *n;
 	wait_queue_head_t queue;
 	init_waitqueue_head(&queue);
 
 	/* Stop all interfaces & threads */
 
-	while (pktgen_threads) {
-		struct pktgen_thread *t = pktgen_threads;
-		pktgen_threads->control |= (T_TERMINATE);
+	list_for_each_safe(q, n, &pktgen_threads) {
+		t = list_entry(q, struct pktgen_thread, th_list);
+		t->control |= (T_TERMINATE);
 
-		wait_event_interruptible_timeout(queue, (t != pktgen_threads),
-						 HZ);
+		wait_event_interruptible_timeout(queue, (t->removed == 1), HZ);
 	}
 
 	/* Un-register us from receiving netdevice events */


-- 
Luiz Fernando N. Capitulino

^ permalink raw reply related

* [PATCH 00/03] pktgen: Fix kernel_thread() fail leak.
From: Luiz Fernando Capitulino @ 2006-01-23 15:46 UTC (permalink / raw)
  To: davem; +Cc: lkml, netdev, robert.olsson


 Leak fix: free all the alocated resources if kernel_thread() call fails.

Signed-off-by: Luiz Capitulino <lcapitulino@mandriva.com.br>

 net/core/pktgen.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index ee26c69..ccffcbb 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2998,6 +2998,7 @@ static struct pktgen_thread *__init pktg
 
 static int __init pktgen_create_thread(const char *name, int cpu)
 {
+	int err;
 	struct pktgen_thread *t = NULL;
 	struct proc_dir_entry *pe;
 
@@ -3036,9 +3037,15 @@ static int __init pktgen_create_thread(c
 
 	t->removed = 0;
 
-	if (kernel_thread((void *)pktgen_thread_worker, (void *)t,
-			  CLONE_FS | CLONE_FILES | CLONE_SIGHAND) < 0)
+	err = kernel_thread((void *)pktgen_thread_worker, (void *)t,
+			  CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
+	if (err < 0) {
 		printk("pktgen: kernel_thread() failed for cpu %d\n", t->cpu);
+		remove_proc_entry(t->name, pg_proc_dir);
+		list_del(&t->th_list);
+		kfree(t);
+		return err;
+	}
 
 	return 0;
 }


-- 
Luiz Fernando N. Capitulino

^ permalink raw reply related

* [PATCH 00/04] pktgen: Fix Initialization fail leak.
From: Luiz Fernando Capitulino @ 2006-01-23 15:47 UTC (permalink / raw)
  To: davem; +Cc: lkml, netdev, robert.olsson


 Even if pktgen's thread initialization fails for all CPUs, the module
will be successfully loaded.

 This patch changes that behaivor, by returning error on module load time,
and also freeing all the resources allocated. It also prints a warning if a
thread initialization has failed.

Signed-off-by: Luiz Capitulino <lcapitulino@mandriva.com.br>

 net/core/pktgen.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index ccffcbb..83d875b 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3132,11 +3132,24 @@ static int __init pg_init(void)
 	register_netdevice_notifier(&pktgen_notifier_block);
 
 	for_each_online_cpu(cpu) {
+		int err;
 		char buf[30];
 
 		sprintf(buf, "kpktgend_%i", cpu);
-		pktgen_create_thread(buf, cpu);
+		err = pktgen_create_thread(buf, cpu);
+		if (err)
+			printk("pktgen: WARNING: Cannot create thread for cpu %d (%d)\n",
+					cpu, err);
 	}
+
+	if (list_empty(&pktgen_threads)) {
+		printk("pktgen: ERROR: Initialization failed for all threads\n");
+		unregister_netdevice_notifier(&pktgen_notifier_block);
+		remove_proc_entry(PGCTRL, pg_proc_dir);
+		proc_net_remove(PG_PROC_DIR);
+		return -ENODEV;
+	}
+
 	return 0;
 }

-- 
Luiz Fernando N. Capitulino

^ permalink raw reply related

* proper way to deal with a sparse warning (fwd)
From: Kumar Gala @ 2006-01-23 16:21 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel

I wasn't getting much of a response on lkml to this, so maybe netdev will 
be more helpful :)

thanks

- kumar

---------- Forwarded message ----------
Date: Thu, 19 Jan 2006 00:19:47 -0600
From: Kumar Gala <galak@kernel.crashing.org>
To: LKML List <linux-kernel@vger.kernel.org>
Subject: proper way to deal with a sparse warning

I'm getting the following sparse warning:

drivers/net/gianfar_mii.c:165:16: warning: incorrect type in  
assignment (different address spaces)
drivers/net/gianfar_mii.c:165:16:    expected void *priv
drivers/net/gianfar_mii.c:165:16:    got struct gfar_mii [noderef] * 
[assigned] regs<asn:2>

This is line 165 of gianfar_mii.c:

         new_bus->priv = regs;

new_bus->priv is of type void *.  regs is of type struct gfar_mii  
__iomem *.

Is it acceptable to do the following:

	new_bus->priv = (void __force *)regs;

- kumar


^ permalink raw reply

* [PATCH 00/00] pktgen: refinements and small fixes.
From: Robert Olsson @ 2006-01-23 17:00 UTC (permalink / raw)
  To: Luiz Fernando Capitulino; +Cc: davem, lkml, netdev, robert.olsson
In-Reply-To: <20060123134140.b04ad994.lcapitulino@mandriva.com.br>


Luiz Fernando Capitulino writes:

 >  [PATCH 00/02] pktgen: Ports thread list to Kernel list implementation.
 >  [PATCH 00/03] pktgen: Fix kernel_thread() fail leak.
 >  [PATCH 00/04] pktgen: Fix Initialization fail leak. 
 > 

 >  But I'm sending these patches first, just to know if I'm doing something
 > wrong.

 Thanks!

 Looks interesting. Yes of course better to use kernel list functions. I'll 
 patch my lab version and run through some tests. 
 
 Yes keep on hacking...

 Cheers.
					--ro

^ permalink raw reply

* Re: [BUG] sky2 broken for Yukon PCI-E Gigabit Ethernet Controller 11ab:4362 (rev 19)
From: Stephen Hemminger @ 2006-01-23 18:15 UTC (permalink / raw)
  To: Knut Petersen; +Cc: linux-kernel, netdev
In-Reply-To: <43D1C99E.2000506@t-online.de>

O
> 
> It seems that the SuSE Firewall locked something ....
> 
> I started with kernel 2.6.15-git7, tried 2.6.15.1 and 2.6.16-rc1*.
> At the moment I do use a kernel 2.6.15-git7 patched with an updated sky2 
> (v.013).
> I could not find a single working sky2 configuration.
> 

Are you using the full kernel.org kernel, or are you putting sky2 driver into
the SUSE kernel? There are a number of bug fixes related to hardware checksumming
that are in the kernel.org kernel (2.6.15 or later).  There was one in ICMP.
These fixes relate to places in the code where a protocol decides to trim a
packet by removing bytes. I am not familiar with the SuSE Firewall. Is it just
standard netfilter modules or additional code?

-- 
Stephen Hemminger <shemminger@osdl.org>
OSDL http://developer.osdl.org/~shemminger

^ permalink raw reply

* Re: [softmac-dev] [PATCH] ieee80211_rx_any: filter out packets, call ieee80211_rx or ieee80211_rx_mgt
From: Stefan Rompf @ 2006-01-23 19:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Denis Vlasenko, John W. Linville, jbenc, netdev, softmac-dev,
	linux-kernel, bcm43xx-dev
In-Reply-To: <1138026752.3957.98.camel@localhost>

Am Montag 23 Januar 2006 15:32 schrieb Johannes Berg:

> Shouldn't you BSS-filter management packets too?

no, management packets are used to populate f.e. scanning results.

Stefan

^ permalink raw reply

* Re: [BUG] sky2 broken for Yukon PCI-E Gigabit Ethernet Controller 11ab:4362 (rev 19)
From: Knut Petersen @ 2006-01-23 19:20 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: linux-kernel, netdev
In-Reply-To: <20060123101538.14d21bf4@dxpl.pdx.osdl.net>

Stephen Hemminger wrote:

>O
>  
>
>>It seems that the SuSE Firewall locked something ....
>>
>>I started with kernel 2.6.15-git7, tried 2.6.15.1 and 2.6.16-rc1*.
>>At the moment I do use a kernel 2.6.15-git7 patched with an updated sky2 
>>(v.013).
>>I could not find a single working sky2 configuration.
>>
>>    
>>
>
>Are you using the full kernel.org kernel, or are you putting sky2 driver into
>the SUSE kernel?
>
No SuSE kernels here. I started with kernel.org kernel 2.6.15-git7 and
had those problems. Then I tried 2.6.16-rc3-git3, the most recent kernel
at the time of my original writing. As I had some other problems with
that kernel I had a look at the git tree of Linus and applied all sky2 
related
patches to the otherwise unchanged 2.6.15-git7. 2.6.15.1 also was no
solution as sky2 seems to be not available there.

> There are a number of bug fixes related to hardware checksumming
>that are in the kernel.org kernel (2.6.15 or later).  There was one in ICMP.
>These fixes relate to places in the code where a protocol decides to trim a
>packet by removing bytes. I am not familiar with the SuSE Firewall. Is it just
>standard netfilter modules or additional code?
>  
>
I have to admit that I don´t know enough about the network layers
of the kernel, so here is a list of the network modules loaded (sky2
compiled into the kernel). SuSEFirewall is just standard netfiltering
using those modules.

ipt_MASQUERADE          3968  1
pppoe                  15360  2
pppox                   4616  1 pppoe
af_packet              23240  2
ppp_generic            30740  6 pppoe,pppox
slhc                    7040  1 ppp_generic
ipt_TOS                 2816  28
ipt_TCPMSS              4800  2
ipt_LOG                 7232  77
ipt_limit               2880  77
ipt_pkttype             1984  2
ipt_state               2240  45
ip6t_REJECT             5824  3
ipt_REJECT              5952  3
iptable_mangle          3200  1
iptable_nat             8836  1
iptable_filter          3264  1
ip6table_mangle         2752  0
ip_nat_ftp              3776  0
ip_nat                 18284  3 ipt_MASQUERADE,iptable_nat,ip_nat_ftp
ip_conntrack_ftp        8240  1 ip_nat_ftp
ip_conntrack           51020  6 
ipt_MASQUERADE,ipt_state,iptable_nat,ip_nat_ftp,ip_nat,ip_conntrack_ftp
ip_tables              24088  11 
ipt_MASQUERADE,ipt_TOS,ipt_TCPMSS,ipt_LOG,ipt_limit,ipt_pkttype,ipt_state,ipt_REJECT,iptable_mangle,iptable_nat,iptable_filter
ip6table_filter         3136  1
ip6_tables             25624  3 ip6t_REJECT,ip6table_mangle,ip6table_filter
ipv6                  271712  14 ip6t_REJECT

As said before, rtl8139 does work perfectly well, the same is true
for an identical system with a Via Rhine adapter.


cu,
 Knut

^ permalink raw reply

* RE: My vote against eepro* removal
From: Jesse Brandeburg @ 2006-01-23 20:23 UTC (permalink / raw)
  To: kus Kusche Klaus
  Cc: Lee Revell, Evgeniy Polyakov, Adrian Bunk, linux-kernel,
	Ronciak, John, Brandeburg, Jesse, netdev, Steven Rostedt
In-Reply-To: <AAD6DA242BC63C488511C611BD51F367323329@MAILIT.keba.co.at>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3497 bytes --]

On Mon, 23 Jan 2006, kus Kusche Klaus wrote:
> From: John Ronciak
> > Can we try a couple of things? 1) just comment out all the check for
> > link code in the e100 driver and give that a try and 2) just comment
> > out the update stats call and see if that works.  These seem to be the
> > differences and we need to know which one is causing the problem.
> 
> First of all, I am still unable to get any traces of this in the
> latency tracer. Moreover, as I told before, removing parts of the
> watchdog usually made my eth0 nonfunctional (which is bad - this
> is an embedded system with ssh access).
> 
> Hence, I explicitely instrumented the watchdog function with tsc.
> Output of the timings is done by a background thread, so the
> timings should not increase the runtime of the watchdog.
> 
> Here are my results:
> 
> If the watchdog doesn't get interrupted, preempted, or whatever,
> it spends 340 us in its body:
> * 303 us in the mii code
> *  36 us in the following code up to e100_adjust_adaptive_ifs
> *   1 us in the remaining code (I think my chip doesn't need any
> of those chip-specific fixups)
> 
> The 303 us in the mii code are divided in the following way:
> * 101 us in mii_ethtool_gset
> * 135 us in the whole if
> *  67 us in mii_check_link
> 
> This is with the udelay(2) instead of udelay(20) hack applied.
> With udelay(20), the mii times are 128 + 170 + 85 us,
> i.e. 383 us instead of 303 us, or >= 420 us for the whole watchdog.
> 
> As the RTC runs with 8192 Hz during my tests, the watchdog is hit
> by 2-3 interrupts, which adds another 75 - 110 us to its total
> execution time, i.e. the time it blocks other rtprio 1 threads.

Thank you very much for that detailed analysis!  okay, so calls to mii.c 
take too long, but those depend on mmio_read in e100 to do the work, so 
this patch attempts to minimize the latency.

This patch is against linus-2.6.git, I compile and ssh/ping tested it. 
Would you be willing to send your instrumentation patches?  I could then 
test any fixes easier.

e100: attempt a shorter delay for mdio reads

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Simply reorder our write/read sequence for mdio reads to minimize latency
as well as delay a shorter interval for each loop.
---

  drivers/net/e100.c |   12 +++++++-----
  1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -891,23 +891,25 @@ static u16 mdio_ctrl(struct nic *nic, u3
  	 * procedure it should be done under lock.
  	 */
  	spin_lock_irqsave(&nic->mdio_lock, flags);
-	for (i = 100; i; --i) {
+	for (i = 1000; i; --i) {
  		if (readl(&nic->csr->mdi_ctrl) & mdi_ready)
  			break;
-		udelay(20);
+		udelay(2);
  	}
  	if (unlikely(!i)) {
-		printk("e100.mdio_ctrl(%s) won't go Ready\n",
+		DPRINTK(PROBE, ERR, "e100.mdio_ctrl(%s) won't go Ready\n",
  			nic->netdev->name );
  		spin_unlock_irqrestore(&nic->mdio_lock, flags);
  		return 0;		/* No way to indicate timeout error */
  	}
  	writel((reg << 16) | (addr << 21) | dir | data, &nic->csr->mdi_ctrl);

-	for (i = 0; i < 100; i++) {
-		udelay(20);
+	/* to avoid latency, read to flush the write, then delay, and only
+	 * delay 2us per loop, manual says read should complete in < 64us */
+	for (i = 0; i < 1000; i++) {
  		if ((data_out = readl(&nic->csr->mdi_ctrl)) & mdi_ready)
  			break;
+		udelay(2);
  	}
  	spin_unlock_irqrestore(&nic->mdio_lock, flags);
  	DPRINTK(HW, DEBUG,

^ permalink raw reply

* [PATCH] net: Move destructor from neigh->ops to neigh_params
From: Roland Dreier @ 2006-01-23 21:27 UTC (permalink / raw)
  To: davem; +Cc: netdev, openib-general

This is a resend of a patch written by Michael S. Tsirkin
<mst@mellanox.co.il>.  I'd like to get an ACK or NAK of it from Dave
and other networking people, so that we can either merge it upstream
or try a different approach.  There definitely is a problem with
neighbour destructors that IP-over-IB is running into.

It would be good to know what the design was behind putting the
destructor method in neigh->ops in the first place.

Dave, if you want to merge this directly, that's fine.  Or I'm fine
with merging this through the IB tree if you'd prefer (if you want me
to do that, let me know if you think it's 2.6.16 material).

Thanks,
  Roland



struct neigh_ops currently has a destructor field, which no in-kernel
drivers outside of infiniband use.  The infiniband/ulp/ipoib in-tree
driver stashes some info in the neighbour structure (the results of
the second-stage lookup from ARP results to real link-level path), and
it uses neigh->ops->destructor to get a callback so it can clean up
this extra info when a neighbour is freed.  We've run into problems
with this: since the destructor is in an ops field that is shared
between neighbours that may belong to different net devices, there's
no way to set/clear it safely.

The following patch moves this field to neigh_parms where it can be
safely set, together with its twin neigh_setup.  Two additional
patches in the patch series update ipoib to use this new interface.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 6fa9ae1..b0666d6 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -68,6 +68,7 @@ struct neigh_parms
 	struct net_device *dev;
 	struct neigh_parms *next;
 	int	(*neigh_setup)(struct neighbour *);
+	void	(*neigh_destructor)(struct neighbour *);
 	struct neigh_table *tbl;
 
 	void	*sysctl_table;
@@ -145,7 +146,6 @@ struct neighbour
 struct neigh_ops
 {
 	int			family;
-	void			(*destructor)(struct neighbour *);
 	void			(*solicit)(struct neighbour *, struct sk_buff*);
 	void			(*error_report)(struct neighbour *, struct sk_buff*);
 	int			(*output)(struct sk_buff*);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index e68700f..3489e23 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -586,8 +586,8 @@ void neigh_destroy(struct neighbour *nei
 			kfree(hh);
 	}
 
-	if (neigh->ops && neigh->ops->destructor)
-		(neigh->ops->destructor)(neigh);
+	if (neigh->parms->neigh_destructor)
+		(neigh->parms->neigh_destructor)(neigh);
 
 	skb_queue_purge(&neigh->arp_queue);
 
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index fd3f5c8..9588124 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -247,7 +247,6 @@ static void path_free(struct net_device 
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
 		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		neigh->neighbour->ops->destructor = NULL;
 		kfree(neigh);
 	}
 
@@ -530,7 +529,6 @@ static void neigh_add_path(struct sk_buf
 err:
 	*to_ipoib_neigh(skb->dst->neighbour) = NULL;
 	list_del(&neigh->list);
-	neigh->neighbour->ops->destructor = NULL;
 	kfree(neigh);
 
 	++priv->stats.tx_dropped;
@@ -769,21 +767,9 @@ static void ipoib_neigh_destructor(struc
 		ipoib_put_ah(ah);
 }
 
-static int ipoib_neigh_setup(struct neighbour *neigh)
-{
-	/*
-	 * Is this kosher?  I can't find anybody in the kernel that
-	 * sets neigh->destructor, so we should be able to set it here
-	 * without trouble.
-	 */
-	neigh->ops->destructor = ipoib_neigh_destructor;
-
-	return 0;
-}
-
 static int ipoib_neigh_setup_dev(struct net_device *dev, struct neigh_parms *parms)
 {
-	parms->neigh_setup = ipoib_neigh_setup;
+	parms->neigh_destructor = ipoib_neigh_destructor;
 
 	return 0;
 }

^ permalink raw reply related

* Re: [PATCH] net: Move destructor from neigh->ops to neigh_params
From: David S. Miller @ 2006-01-23 21:54 UTC (permalink / raw)
  To: rdreier; +Cc: netdev, openib-general
In-Reply-To: <adazmlmmy7v.fsf@cisco.com>

From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 23 Jan 2006 13:27:32 -0800

> I'd like to get an ACK or NAK of it from Dave

Dave is in New Zealand at linux.conf.au, don't expect him to
be too active for at least a week...

^ permalink raw reply

* Re: Fw: [Bugme-new] [Bug 5936] New: Openswan tunnels + netfilter problem
From: Herbert Xu @ 2006-01-24  7:25 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: akpm, netdev, netfilter-devel, webmaster
In-Reply-To: <43D3F186.6030206@trash.net>

Patrick McHardy <kaber@trash.net> wrote:
> Andrew Morton wrote:
>> 
>> http://bugzilla.kernel.org/show_bug.cgi?id=5936
> 
> Please post your iptables rules and the full list of loaded modules.

The problem is caused by SNAT on a dst that already has an xfrm set.
When ip_route_me_harder processes the dst it will cause the dst to
lose its xfrm since it has IPSKB_XFRM_TRANSFORMED set.

Since xfrm4_output_finish does not expect dst's to lose their xfrm's
after POST_ROUTING, it crashes.

Obviously we could add a check in xfrm4_output_finish to prevent this
crash, however, I think we need to consider this a bit more since it
breaks a fairly common setup where people just stick a rule into the
NAT table that says

iptables -t nat -I POSTROUTING -i eth1 -j MASQUERADE

where eth1 is the outbound interface.  If this rule catches any IPsec
VPN traffic then it'll SNAT them even though the intention is obviously
to let them through without SNAT.

Perhaps it's best to have SNAT not touch packets with dst->xfrm set.
Unfortunately that leads to problems as well (albeit rarer) since you
may have catch-all IPsec policies that every packet matches, but you
want certain packets to be SNATed so that they match more specific
policies.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* RE: My vote against eepro* removal
From: kus Kusche Klaus @ 2006-01-24  7:38 UTC (permalink / raw)
  To: Jesse Brandeburg
  Cc: Lee Revell, Evgeniy Polyakov, Adrian Bunk, linux-kernel,
	Ronciak, John, netdev, Steven Rostedt

From: Jesse Brandeburg
> On Mon, 23 Jan 2006, kus Kusche Klaus wrote:
> > Here are my results:
> > 
> > If the watchdog doesn't get interrupted, preempted, or whatever,
> > it spends 340 us in its body:
> > * 303 us in the mii code
> > *  36 us in the following code up to e100_adjust_adaptive_ifs
> > *   1 us in the remaining code (I think my chip doesn't need any
> > of those chip-specific fixups)
> > 
> > The 303 us in the mii code are divided in the following way:
> > * 101 us in mii_ethtool_gset
> > * 135 us in the whole if
> > *  67 us in mii_check_link
> > 
> > This is with the udelay(2) instead of udelay(20) hack applied.
> > With udelay(20), the mii times are 128 + 170 + 85 us,
> > i.e. 383 us instead of 303 us, or >= 420 us for the whole watchdog.
> 
> Thank you very much for that detailed analysis!  okay, so 
> calls to mii.c 
> take too long, but those depend on mmio_read in e100 to do 
> the work, so 
> this patch attempts to minimize the latency.
> 
> This patch is against linus-2.6.git, I compile and ssh/ping 
> tested it. 
> Would you be willing to send your instrumentation patches?  I 
> could then 
> test any fixes easier.

No deep magic behind my instrumentation:

A few global variables and some rdtscl in the watchdog:

unsigned long my_tsc_1 = 0;
unsigned long my_tsc_2 = 0;
unsigned long my_tsc_3 = 0;
unsigned long my_tsc_4 = 0;
EXPORT_SYMBOL(my_tsc_1);
EXPORT_SYMBOL(my_tsc_2);
EXPORT_SYMBOL(my_tsc_3);
EXPORT_SYMBOL(my_tsc_4);

static void e100_watchdog(unsigned long data)
{
	struct nic *nic = (struct nic *)data;
	struct ethtool_cmd cmd;

	DPRINTK(TIMER, DEBUG, "right now = %ld\n", jiffies);

	/* mii library handles link maintenance tasks */
	rdtscl(my_tsc_1);

	mii_ethtool_gset(&nic->mii, &cmd);

	rdtscl(my_tsc_2);
	if(mii_link_ok(&nic->mii) && !netif_carrier_ok(nic->netdev)) {
		DPRINTK(LINK, INFO, "link up, %sMbps, %s-duplex\n",
			cmd.speed == SPEED_100 ? "100" : "10",
			cmd.duplex == DUPLEX_FULL ? "full" : "half");
	} else if(!mii_link_ok(&nic->mii) && netif_carrier_ok(nic->netdev)) {
		DPRINTK(LINK, INFO, "link down\n");
	}

	rdtscl(my_tsc_3);
	mii_check_link(&nic->mii);
	rdtscl(my_tsc_4);

	/* Software generated interrupt to recover from (rare) Rx
	* allocation failure.
...

And a small module which prints the timings periodically 
when loaded:

/* Example module, built after LDD book release 3 */

#include <linux/init.h>
#include <linux/module.h>
#include <linux/version.h>
#include <linux/errno.h>
#include <linux/timer.h>

MODULE_LICENSE("GPL");

/* Output interval, in jiffies */
#define INTERVAL 2111
/* Output scaling: TSC ==> microseconds */
#define SCALE(x) ((x)/500)

extern unsigned long my_tsc_1;
extern unsigned long my_tsc_2;
extern unsigned long my_tsc_3;
extern unsigned long my_tsc_4;

static struct timer_list my_timer;

static void timer_func(unsigned long dummy)
{
  printk(KERN_NOTICE "my_timer: diff = %lu / %lu / %lu\n",
         SCALE(my_tsc_2 - my_tsc_1),
         SCALE(my_tsc_3 - my_tsc_2),
         SCALE(my_tsc_4 - my_tsc_3));

  my_timer.expires += INTERVAL;
  add_timer(&my_timer);
}

static int __init mymod_init(void)
{
  init_timer(&my_timer);
  my_timer.function = timer_func;
  my_timer.expires = jiffies + INTERVAL;
  add_timer(&my_timer);
  
  printk(KERN_NOTICE "Started mymod...\n");
  return 0;
}

static void __exit mymod_exit(void)
{
  del_timer_sync(&my_timer);
  printk(KERN_NOTICE "Finished mymod...\n");
}

module_init(mymod_init);
module_exit(mymod_exit);


> 
> e100: attempt a shorter delay for mdio reads
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> Simply reorder our write/read sequence for mdio reads to 
> minimize latency
> as well as delay a shorter interval for each loop.
>
> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> --- a/drivers/net/e100.c
> +++ b/drivers/net/e100.c
> @@ -891,23 +891,25 @@ static u16 mdio_ctrl(struct nic *nic, u3
>   	 * procedure it should be done under lock.
>   	 */
>   	spin_lock_irqsave(&nic->mdio_lock, flags);
> -	for (i = 100; i; --i) {
> +	for (i = 1000; i; --i) {
>   		if (readl(&nic->csr->mdi_ctrl) & mdi_ready)
>   			break;
> -		udelay(20);
> +		udelay(2);
>   	}
>   	if (unlikely(!i)) {
> -		printk("e100.mdio_ctrl(%s) won't go Ready\n",
> +		DPRINTK(PROBE, ERR, "e100.mdio_ctrl(%s) won't 
> go Ready\n",
>   			nic->netdev->name );
>   		spin_unlock_irqrestore(&nic->mdio_lock, flags);
>   		return 0;		/* No way to indicate 
> timeout error */
>   	}

The piece of code above is not yet present 
in my version of e100.
(I'm still at 2.6.15, there is no -rt patch for 2.6.16 yet)

>   	writel((reg << 16) | (addr << 21) | dir | data, 
> &nic->csr->mdi_ctrl);
> 
> -	for (i = 0; i < 100; i++) {
> -		udelay(20);
> +	/* to avoid latency, read to flush the write, then 
> delay, and only
> +	 * delay 2us per loop, manual says read should complete 
> in < 64us */
> +	for (i = 0; i < 1000; i++) {
>   		if ((data_out = readl(&nic->csr->mdi_ctrl)) & mdi_ready)
>   			break;
> +		udelay(2);
>   	}

Exchanging the if and the udelay made things slightly worse:
It runs with 103 / 136 / 68 instead of 101 / 135 / 67 us.


-- 
Klaus Kusche                 (Software Development - Control Systems)
KEBA AG             Gewerbepark Urfahr, A-4041 Linz, Austria (Europe)
Tel: +43 / 732 / 7090-3120                 Fax: +43 / 732 / 7090-6301
E-Mail: kus@keba.com                                WWW: www.keba.com
 

^ permalink raw reply

* Re: [Bcm43xx-dev] Re: [softmac-dev] [PATCH] ieee80211_rx_any: filter out packets, call ieee80211_rx or ieee80211_rx_mgt
From: Denis Vlasenko @ 2006-01-24  8:06 UTC (permalink / raw)
  To: bcm43xx-dev
  Cc: Johannes Berg, John W. Linville, jbenc, netdev, softmac-dev,
	linux-kernel
In-Reply-To: <1138026752.3957.98.camel@localhost>

On Monday 23 January 2006 16:32, Johannes Berg wrote:
> On Sun, 2006-01-22 at 14:04 +0200, Denis Vlasenko wrote:
> > +       hdr = (struct ieee80211_hdr_4addr *)skb->data;: 
> > +       fc = le16_to_cpu(hdr->frame_ctl);: 
> > +: 
> > +       switch (fc & IEEE80211_FCTL_FTYPE) {: 
> > +       case IEEE80211_FTYPE_MGMT: 
> > +               ieee80211_rx_mgt(ieee, hdr, stats);: 
> > +               return 0;: 
> 
> Shouldn't you BSS-filter management packets too?
> 
> > +       is_packet_for_us = 0;: 
> > +       switch (ieee->iw_mode) {: 
> > +       case IW_MODE_ADHOC: 
> > +               /* promisc: get all */
> > +               if (ieee->dev->flags & IFF_PROMISC): 
> > +                       is_packet_for_us = 1;
> 
> And I still think BSS-filtering is correct even in the promisc case. Any
> other opinions why either way is right or not? [I think we should filter
> because upper layers won't know the packet wasn't for us if it was
> broadcast in another BSSID]

In wired networks promisc literally means "receive all packets", right?

But for wireless, maybe we should filter them out, or else running tcpdump
on the iface will force us to listen to ARP packets from unrelated networks.
That would be rather surprising and disrupting.
--
vda

^ permalink raw reply

* Re: [BUG] sky2 broken for Yukon PCI-E Gigabit Ethernet Controller 11ab:4362 (rev 19)
From: Knut Petersen @ 2006-01-24  9:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, linux-kernel
In-Reply-To: <20060123112751.2e3f1b15@dxpl.pdx.osdl.net>

Stephen Hemminger schrieb:

>Could you try turning off rx checksumming (with ethtool).
>	ethtool -K eth0 rx off
>
>There probably still are (generic) bugs in the netfilter code for CHECKSUM_HW
>socket buffers.
>
>  
>
"ethtool -K eth0 rx off" does cure my problem with sky2.

Anybody is invited to send patches as the problem is 100% reproducible here.

cu,
 Knut

^ permalink raw reply

* [PATCH] sky2: fix ethtool ops
From: Carl-Daniel Hailfinger @ 2006-01-24 12:49 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Linux Kernel Mailing List, netdev

This fixes setting rx_coalesce_usecs_irq via ethtool in sky2.
The write was directed to the wrong register.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

--- linux/drivers/net/sky2.c	2006-01-23 23:41:35.000000000 +0100
+++ linux/drivers/net/sky2.c	2006-01-24 12:52:11.000000000 +0100
@@ -2843,7 +2843,7 @@
 	if (ecmd->rx_coalesce_usecs_irq == 0)
 		sky2_write8(hw, STAT_ISR_TIMER_CTRL, TIM_STOP);
 	else {
-		sky2_write32(hw, STAT_TX_TIMER_INI,
+		sky2_write32(hw, STAT_ISR_TIMER_INI,
 			     sky2_us2clk(hw, ecmd->rx_coalesce_usecs_irq));
 		sky2_write8(hw, STAT_ISR_TIMER_CTRL, TIM_START);
 	}

^ permalink raw reply

* [PATCH] sky2: fix hang on Yukon-EC (0xb6) rev 1
From: Carl-Daniel Hailfinger @ 2006-01-24 13:19 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Linux Kernel Mailing List, netdev

This patch for sky2 fixes a hang on Yukon-EC (0xb6) rev 1
where suddenly no more interrupts were delivered.

I don't know the real cause of the hang due to lack of docs,
but the patch has been running stable for a few hours
whereas the unmodified driver will hang after less than
2 minutes.

Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

--- linux-2.6.15/drivers/net/sky2.c	2006-01-23 23:41:35.000000000 +0100
+++ linux-2.6.15/drivers/net/sky2.c	2006-01-24 14:12:12.000000000 +0100
@@ -1913,8 +1913,26 @@
 	}
 
 exit_loop:
+	/* Is this really a good idea?
+	 * We clear all IRQs although there may be pending work due to
+	 * - packets arrived since start of this function
+	 * - the (++work_done >= to_do) abort
+	 */
 	sky2_write32(hw, STAT_CTRL, SC_STAT_CLR_IRQ);
 
+	/* Pending resolution of the comment above, at least kick the
+	 * STAT_LEV_TIMER_CTRL timer.
+	 * This fixes my hangs on Yukon-EC (0xb6) rev 1.
+	 * The if clause is there to start the timer only if it has been
+	 * configured correctly and not been disabled via ethtool.
+	 * Maybe it would be sufficient to only restart the timer if
+	 * there is pending work. Without docs, that is hard to say.
+	 */
+	if (sky2_read8(hw, STAT_LEV_TIMER_CTRL) == TIM_START) {
+		sky2_write8(hw, STAT_LEV_TIMER_CTRL, TIM_STOP);
+		sky2_write8(hw, STAT_LEV_TIMER_CTRL, TIM_START);
+	}
+
 	sky2_tx_check(hw, 0, tx_done[0]);
 	sky2_tx_check(hw, 1, tx_done[1]);
 

^ permalink raw reply

* Re: [BUG] sky2 broken for Yukon PCI-E Gigabit Ethernet Controller 11ab:4362 (rev 19)
From: Stephen Hemminger @ 2006-01-24 17:54 UTC (permalink / raw)
  To: Knut Petersen; +Cc: netdev, linux-kernel
In-Reply-To: <43D5F6DD.70702@t-online.de>

On Tue, 24 Jan 2006 10:43:57 +0100
Knut Petersen <Knut_Petersen@t-online.de> wrote:

> Stephen Hemminger schrieb:
> 
> >Could you try turning off rx checksumming (with ethtool).
> >	ethtool -K eth0 rx off
> >
> >There probably still are (generic) bugs in the netfilter code for CHECKSUM_HW
> >socket buffers.
> >
> >  
> >
> "ethtool -K eth0 rx off" does cure my problem with sky2.
> 
> Anybody is invited to send patches as the problem is 100% reproducible here.
>

Does it always show up on icmp only?

What are the iptables rules (iptables -L)

-- 
Stephen Hemminger <shemminger@osdl.org>
OSDL http://developer.osdl.org/~shemminger

^ permalink raw reply

* Re: [BUG] sky2 broken for Yukon PCI-E Gigabit Ethernet Controller 11ab:4362 (rev 19)
From: Herbert Xu @ 2006-01-24 20:32 UTC (permalink / raw)
  To: Knut Petersen; +Cc: shemminger, netdev, linux-kernel
In-Reply-To: <43D5F6DD.70702@t-online.de>

Knut Petersen <Knut_Petersen@t-online.de> wrote:
>
> "ethtool -K eth0 rx off" does cure my problem with sky2.
> 
> Anybody is invited to send patches as the problem is 100% reproducible here.

Does the problem go away if you disable conntrack by unloading its module?

Please try to capture the offending ICMP packet with tcpdump and show us
what it looks like.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* e100 oops on resume
From: Stefan Seyfried @ 2006-01-24 22:59 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: netdev

Hi,
since 2.6.16rc1-git3, e100 dies on resume (regardless if from disk, ram or
runtime powermanagement). Unfortunately i only have a bad photo of
the oops right now, it is available from
https://bugzilla.novell.com/attachment.cgi?id=64761&action=view
I have reproduced this on a second e100 machine and can get a serial
console log from this machine tomorrow if needed.
It did resume fine with 2.6.15-git12
-- 
Stefan Seyfried                  \ "I didn't want to write for pay. I
QA / R&D Team Mobile Devices      \ wanted to be paid for what I write."
SUSE LINUX Products GmbH, Nürnberg \                    -- Leonard Cohen

^ permalink raw reply

* Re: e100 oops on resume
From: Mattia Dongili @ 2006-01-24 23:21 UTC (permalink / raw)
  To: Stefan Seyfried; +Cc: Linux Kernel Mailing List, netdev
In-Reply-To: <20060124225919.GC12566@suse.de>

On Tue, Jan 24, 2006 at 11:59:19PM +0100, Stefan Seyfried wrote:
> Hi,
> since 2.6.16rc1-git3, e100 dies on resume (regardless if from disk, ram or
> runtime powermanagement). Unfortunately i only have a bad photo of
> the oops right now, it is available from
> https://bugzilla.novell.com/attachment.cgi?id=64761&action=view
> I have reproduced this on a second e100 machine and can get a serial
> console log from this machine tomorrow if needed.
> It did resume fine with 2.6.15-git12

I experienced the same today, I was planning to get a photo tomorrow :)
I'm running 2.6.16-rc1-mm2 and the last working kernel was 2.6.15-mm4
(didn't try .16-rc1-mm1 being scared of the reiserfs breakage).

-- 
mattia
:wq!

^ permalink raw reply

* [PATCH] ipw2200: fix ->eeprom[EEPROM_VERSION] check
From: Alexey Dobriyan @ 2006-01-25  0:44 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, netdev

priv->eeprom is a pointer.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 drivers/net/wireless/ipw2200.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -2456,7 +2456,7 @@ static void ipw_eeprom_init_sram(struct 
 	   copy.  Otherwise let the firmware know to perform the operation
 	   on it's own
 	 */
-	if ((priv->eeprom + EEPROM_VERSION) != 0) {
+	if (priv->eeprom[EEPROM_VERSION] != 0) {
 		IPW_DEBUG_INFO("Writing EEPROM data into SRAM\n");
 
 		/* write the eeprom data to sram */

^ permalink raw reply

* Re: [PATCH] ipw2200: fix ->eeprom[EEPROM_VERSION] check
From: Zhu Yi @ 2006-01-25  4:34 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Andrew Morton, linux-kernel, netdev
In-Reply-To: <20060125004429.GE3234@mipter.zuzino.mipt.ru>

Acked.

Thanks,
-yi

On Wed, 2006-01-25 at 03:44 +0300, Alexey Dobriyan wrote:
> priv->eeprom is a pointer.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  drivers/net/wireless/ipw2200.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/net/wireless/ipw2200.c
> +++ b/drivers/net/wireless/ipw2200.c
> @@ -2456,7 +2456,7 @@ static void ipw_eeprom_init_sram(struct 
>  	   copy.  Otherwise let the firmware know to perform the operation
>  	   on it's own
>  	 */
> -	if ((priv->eeprom + EEPROM_VERSION) != 0) {
> +	if (priv->eeprom[EEPROM_VERSION] != 0) {
>  		IPW_DEBUG_INFO("Writing EEPROM data into SRAM\n");
>  
>  		/* write the eeprom data to sram */
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: e100 oops on resume
From: Olaf Kirch @ 2006-01-25  9:02 UTC (permalink / raw)
  To: Stefan Seyfried, Linux Kernel Mailing List, netdev
In-Reply-To: <20060124232142.GB6174@inferi.kami.home>

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

On Wed, Jan 25, 2006 at 12:21:42AM +0100, Mattia Dongili wrote:
> I experienced the same today, I was planning to get a photo tomorrow :)
> I'm running 2.6.16-rc1-mm2 and the last working kernel was 2.6.15-mm4
> (didn't try .16-rc1-mm1 being scared of the reiserfs breakage).

I think that's because the latest driver version wants to wait for
the ucode download, and e100_exec_cb_wait before allocating any
control blocks.

static inline int e100_exec_cb_wait(struct nic *nic, struct sk_buff *skb,
        void (*cb_prepare)(struct nic *, struct cb *, struct sk_buff *))
{
        int err = 0, counter = 50;
        struct cb *cb = nic->cb_to_clean;

        if ((err = e100_exec_cb(nic, NULL, e100_setup_ucode)))
                DPRINTK(PROBE,ERR, "ucode cmd failed with error %d\n", err);
	/* NOTE: the oops shows that e100_exec_cb fails with ENOMEM,
  	 * which also means there are no cbs */

	/* ... other stuff...
	 * and then we die here because cb is NULL: */
        while (!(cb->status & cpu_to_le16(cb_complete))) {
                msleep(10);
                if (!--counter) break;
        }

I'm not sure what the right fix would be. e100_resume would probably
have to call e100_alloc_cbs early on, while e100_up should avoid
calling it a second time if nic->cbs_avail != 0. A tentative patch
for testing is attached.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

[-- Attachment #2: e100-resume-fix --]
[-- Type: text/plain, Size: 1830 bytes --]

[PATCH] e100: allocate cbs early on when resuming

Signed-off-by: Olaf Kirch <okir@suse.de>

 drivers/net/e100.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

Index: build/drivers/net/e100.c
===================================================================
--- build.orig/drivers/net/e100.c
+++ build/drivers/net/e100.c
@@ -1298,8 +1298,10 @@ static inline int e100_exec_cb_wait(stru
 	int err = 0, counter = 50;
 	struct cb *cb = nic->cb_to_clean;
 
-	if ((err = e100_exec_cb(nic, NULL, e100_setup_ucode)))
+	if ((err = e100_exec_cb(nic, NULL, e100_setup_ucode))) {
 		DPRINTK(PROBE,ERR, "ucode cmd failed with error %d\n", err);
+		return err;
+	}
 
 	/* must restart cuc */
 	nic->cuc_cmd = cuc_start;
@@ -1721,9 +1723,11 @@ static int e100_alloc_cbs(struct nic *ni
 	struct cb *cb;
 	unsigned int i, count = nic->params.cbs.count;
 
+	/* bail out if we've been here before */
+	if (nic->cbs_avail)
+		return 0;
+
 	nic->cuc_cmd = cuc_start;
-	nic->cb_to_use = nic->cb_to_send = nic->cb_to_clean = NULL;
-	nic->cbs_avail = 0;
 
 	nic->cbs = pci_alloc_consistent(nic->pdev,
 		sizeof(struct cb) * count, &nic->cbs_dma_addr);
@@ -2578,6 +2582,8 @@ static int __devinit e100_probe(struct p
 	nic->pdev = pdev;
 	nic->msg_enable = (1 << debug) - 1;
 	pci_set_drvdata(pdev, netdev);
+	nic->cb_to_use = nic->cb_to_send = nic->cb_to_clean = NULL;
+	nic->cbs_avail = 0;
 
 	if((err = pci_enable_device(pdev))) {
 		DPRINTK(PROBE, ERR, "Cannot enable PCI device, aborting.\n");
@@ -2752,6 +2758,8 @@ static int e100_resume(struct pci_dev *p
 	retval = pci_enable_wake(pdev, 0, 0);
 	if (retval)
 		DPRINTK(PROBE,ERR, "Error clearing wake events\n");
+	if ((retval = e100_alloc_cbs(nic)))
+		DPRINTK(PROBE,ERR, "No memory for cbs\n");
 	if(e100_hw_init(nic))
 		DPRINTK(HW, ERR, "e100_hw_init failed\n");
 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox