Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] IPv6:Send an ICMPv6 "Fragment Reassembly Timeout" message when enabling connection track
From: Patrick McHardy @ 2010-02-03 16:42 UTC (permalink / raw)
  To: Shan Wei
  Cc: David Miller, Yasuyuki KOZAKAI, eric.dumazet, randy.dunlap, mst,
	johannes, kuznet, pekkas, jmorris, yoshfuji, pablo, ebiederm,
	adobriyan, brian.haley, shemminger, akpm, netfilter-devel,
	netdev@vger.kernel.org
In-Reply-To: <4B62A338.6020106@cn.fujitsu.com>

Shan Wei wrote:
> @@ -349,17 +378,20 @@ static int nf_ct_frag6_queue(struct nf_ct_frag6_queue *fq, struct sk_buff *skb,
>  	else
>  		fq->q.fragments = skb;
>  
> -	skb->dev = NULL;
>  	fq->q.stamp = skb->tstamp;
>  	fq->q.meat += skb->len;
>  	atomic_add(skb->truesize, &nf_init_frags.mem);
>  
>  	/* The first fragment.
>  	 * nhoffset is obtained from the first fragment, of course.
> +	 * Reserve dev for sending an ICMP "Fragment Reassembly Timeout"
> +	 * message.
>  	 */
>  	if (offset == 0) {
>  		fq->nhoffset = nhoff;
>  		fq->q.last_in |= INET_FRAG_FIRST_IN;
> +	} else {
> +		skb->dev = NULL;
>  	}

We need to store the iif and perform a lookup later just as in IPv4
because the device is not reference counted and might disappear while
the fragments are queued.

Besides this, the patch looks good.

^ permalink raw reply

* Re: [PATCHv2 08/11] fec: fix uninitialized rx buffer usage
From: Grant Likely @ 2010-02-03 16:46 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: List Linux Kernel, Rob Herring, David S. Miller, netdev,
	linux-arm-kernel, linux, s.hauer, valentin.longchamp, daniel,
	Dinh.Nguyen, bryan.wu
In-Reply-To: <e46b06510f7eb6ca51ea8b96335b2d32ec8bf6f8.1265173480.git.amit.kucheria@canonical.com>

On Tue, Feb 2, 2010 at 10:16 PM, Amit Kucheria
<amit.kucheria@canonical.com> wrote:
> From: Rob Herring <r.herring@freescale.com>
>
> The fec driver was enabling receive buffer descriptor without allocating
> the buffers. Make sure the buffer descriptors are initialized to not
> start receiving packets.
>
> Open also calls fec_restart after the rx buffers are allocated. With the code
> in fec_restart, it zeroes out the buffer descriptors that have just been
> setup.

fec related patches 8 & 9 look okay to me.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH] sky2:  receive dma mapping error handling
From: Michael Breuer @ 2010-02-03 16:47 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jarek Poplawski, David Miller, akpm, flyboy, linux-kernel, netdev,
	Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <4B68F664.6050007@majjas.com>

On 02/02/2010 11:07 PM, Michael Breuer wrote:
> Just a brief update - this has been up and stable for about 32 hours - 
> I've been periodically generating load on the system. No kernel errors 
> of any sort so far. Actually, in retrospect, I believe the dma issue 
> was triggering other bad things - including an rcu lockup (patched in 
> tip - sched.c).
>
> Just as an FYI - (and this should probably be in a new thread) I am 
> seeing an large number (>9,000,000) of dropped rx packets, however at 
> this time I see no errors resulting from that (on this or client 
> machines). As the # of dropped packets hasn't incremented at any time 
> I was observing things, I can't say what this is about. Probably 
> nothing, but I'll see if I can track down what is going on. I did see 
> some of this earlier on while troubleshooting the sky2 issues that now 
> seem resolved.  Quick crosschecking of other machines do not show high 
> error or retransmission rates. I'm also not seeing any evidence of 
> other errors (no errors reported by ifconfig, or ethtool, or printk 
> (debug is enabled).
>
> I'm wondering whether these dropped packets are due mostly to hitting 
> GMR_FS_RX_OK in sky2_receive. I'm also guessing that the high numbers 
> of this that I'm seeing is an artifact of being able to pump more 
> traffic through with the above patch. Given the description of the 
> status code in sky2.h (receive ok) I'm wondering whether a) this 
> should be reported as dropped, b) whether resubmit is necessary, c) 
> whether it's possible that eth1 events coinciding with eth0 events are 
> the cause and d) whether or not there's another issue entirely.
>
Tracked this down. The status being returned is 0x3c0080 - good flow 
control packets. Nothing is actually being dropped (confirmed by packet 
trace on switch compared with packet trace on server).

I whipped up a trivial patch to not count these as dropped packets and 
will post to netdev.

I'm not really sure what the driver should be doing in this case, but 
resubmit seems to work.

^ permalink raw reply

* Re: [PATCH 2.6.27.7-9-pae #7 SMP 1/1] networking tcp: Writing tcp socket be atomic
From: David Miller @ 2010-02-03 16:52 UTC (permalink / raw)
  To: johny; +Cc: kuznet, netdev, jmorris, kaber, linux-kernel
In-Reply-To: <474C17045AA24E3F8519EE76BAB47439@JOHNYE1>


Locking belongs inside of the application, not in the kernel.

As evidenced by the fact that you had to add new socket options, the
application needs to be modified anyways.

It's therefore just as easy to add the appropriate locking to the
sequences where the application writes over the TCP socket.  And
it avoids having to modify the kernel at all.

^ permalink raw reply

* Re: [PATCH] sky2:  receive dma mapping error handling
From: Stephen Hemminger @ 2010-02-03 16:56 UTC (permalink / raw)
  To: Michael Breuer
  Cc: Jarek Poplawski, David Miller, akpm, flyboy, linux-kernel, netdev,
	Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <4B69A897.7030408@majjas.com>

On Wed, 03 Feb 2010 11:47:19 -0500
Michael Breuer <mbreuer@majjas.com> wrote:

> Tracked this down. The status being returned is 0x3c0080 - good flow 
> control packets. Nothing is actually being dropped (confirmed by packet 
> trace on switch compared with packet trace on server).
> 
> I whipped up a trivial patch to not count these as dropped packets and 
> will post to netdev.
> 
> I'm not really sure what the driver should be doing in this case, but 
> resubmit seems to work.

Looks like a flow control negotiation issue. You probably turned off
flow control on the Linux side, but the switch is still doing flow
control.

-- 

^ permalink raw reply

* Re: [PATCH] hrtimer, softirq: Fix hrtimer->softirq trampoline
From: David Miller @ 2010-02-03 16:56 UTC (permalink / raw)
  To: peterz; +Cc: ypolyans, herbert, yjwei, netdev, polyanskiy, tglx, linux-kernel
In-Reply-To: <1265120401.24455.306.camel@laptop>

From: Peter Zijlstra <peterz@infradead.org>
Date: Tue, 02 Feb 2010 15:20:01 +0100

> Subject: hrtimer, softirq: Fix hrtimer->softirq trampoline
> 
> hrtimers callbacks are always done from hardirq context, either the
> jiffy tick interrupt or the hrtimer device interrupt.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Acked-by: David S. Miller <davem@davemloft.net>

It would be nice to give mention of the bug reporter et al.
in the final commit message.

Thanks.

^ permalink raw reply

* [PATCH] sky2: Flow control frames recorded as dropped packets
From: Michael Breuer @ 2010-02-03 16:56 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

When a flow control packet is received, sky2_receive resubmits the i/o. 
Upon exit, sky2_status_intr counts these as dropped packets.

This fix just skips the rx_dropped accounting in this one case. It 
didn't seem worthwhile to change sky2_receive to deal differently.

This is my first patch submission - comments more than welcome.

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 95fd10f..bcb035c 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -2525,7 +2517,8 @@ static int sky2_status_intr(struct sky2_hw *hw, 
int to_do, u16 idx)
                         total_bytes[port] += length;
                         skb = sky2_receive(dev, length, status);
                         if (unlikely(!skb)) {
-                               dev->stats.rx_dropped++;
+                               if (!(status & GMR_FS_GOOD_FC) )
+                                       dev->stats.rx_dropped++;
                                 break;
                         }


^ permalink raw reply related

* Re: [PATCH] sky2:  receive dma mapping error handling
From: Michael Breuer @ 2010-02-03 17:07 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jarek Poplawski, David Miller, akpm, flyboy, linux-kernel, netdev,
	Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100203085618.6662cc6f@nehalam>

On 02/03/2010 11:56 AM, Stephen Hemminger wrote:
> On Wed, 03 Feb 2010 11:47:19 -0500
> Michael Breuer<mbreuer@majjas.com>  wrote:
>
>    
>> Tracked this down. The status being returned is 0x3c0080 - good flow
>> control packets. Nothing is actually being dropped (confirmed by packet
>> trace on switch compared with packet trace on server).
>>
>> I whipped up a trivial patch to not count these as dropped packets and
>> will post to netdev.
>>
>> I'm not really sure what the driver should be doing in this case, but
>> resubmit seems to work.
>>      
> Looks like a flow control negotiation issue. You probably turned off
> flow control on the Linux side, but the switch is still doing flow
> control.
>
>    
According to the driver:
Feb  3 12:03:02 mail kernel: sky2 eth0: Link is up at 1000 Mbps, full 
duplex, flow control both

So if the rx flow control packet status is due to flow control being 
disabled, then there's a different issue.

^ permalink raw reply

* Re: [PATCH] sky2: Flow control frames recorded as dropped packets
From: Stephen Hemminger @ 2010-02-03 17:10 UTC (permalink / raw)
  To: Michael Breuer; +Cc: netdev
In-Reply-To: <4B69AAA9.20505@majjas.com>

On Wed, 03 Feb 2010 11:56:09 -0500
Michael Breuer <mbreuer@majjas.com> wrote:

> When a flow control packet is received, sky2_receive resubmits the i/o. 
> Upon exit, sky2_status_intr counts these as dropped packets.
> 
> This fix just skips the rx_dropped accounting in this one case. It 
> didn't seem worthwhile to change sky2_receive to deal differently.
> 
> This is my first patch submission - comments more than welcome.
> 
> diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
> index 95fd10f..bcb035c 100644
> --- a/drivers/net/sky2.c
> +++ b/drivers/net/sky2.c
> @@ -2525,7 +2517,8 @@ static int sky2_status_intr(struct sky2_hw *hw, 
> int to_do, u16 idx)
>                          total_bytes[port] += length;
>                          skb = sky2_receive(dev, length, status);
>                          if (unlikely(!skb)) {
> -                               dev->stats.rx_dropped++;
> +                               if (!(status & GMR_FS_GOOD_FC) )
> +                                       dev->stats.rx_dropped++;
>                                  break;
>                          }
> 

Since FC should be absorbed by MAC, it would be better to never receive
send them up stream. I'll make a patch for that
-- 

^ permalink raw reply

* Re: [PATCH] sky2:  receive dma mapping error handling
From: Justin P. Mattock @ 2010-02-03 17:16 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Michael Breuer, Jarek Poplawski, David Miller, akpm, flyboy,
	linux-kernel, netdev, Michael Chan, Don Fry, Francois Romieu,
	Matt Carlson
In-Reply-To: <20100203085618.6662cc6f@nehalam>

On 02/03/10 08:56, Stephen Hemminger wrote:
> On Wed, 03 Feb 2010 11:47:19 -0500
> Michael Breuer<mbreuer@majjas.com>  wrote:
>
>> Tracked this down. The status being returned is 0x3c0080 - good flow
>> control packets. Nothing is actually being dropped (confirmed by packet
>> trace on switch compared with packet trace on server).
>>
>> I whipped up a trivial patch to not count these as dropped packets and
>> will post to netdev.
>>
>> I'm not really sure what the driver should be doing in this case, but
>> resubmit seems to work.
>
> Looks like a flow control negotiation issue. You probably turned off
> flow control on the Linux side, but the switch is still doing flow
> control.
>

I noticed this in a hotel last week, I can try
at the home to see if this fires of, if so
I can test any patches you have.

Justin P. Mattock

^ permalink raw reply

* lockdep complaining about btusb during hibernation
From: Oliver Neukum @ 2010-02-03 17:28 UTC (permalink / raw)
  To: Alan Stern, Marcel Holtmann, linux-pm; +Cc: netdev, Rafael J. Wysocki, teheo

Hi,

I've got lockdep complaining in the latest Linus tree when doing STD.

	Regards
		Oliver

Feb  3 17:02:59 vanamonde kernel: =======================================================
Feb  3 17:02:59 vanamonde kernel: [ INFO: possible circular locking dependency detected ]
Feb  3 17:02:59 vanamonde kernel: 2.6.33-rc6-0.1-default #50
Feb  3 17:02:59 vanamonde kernel: -------------------------------------------------------
Feb  3 17:03:00 vanamonde kernel: bash/2406 is trying to acquire lock:
Feb  3 17:03:00 vanamonde kernel:  (rtnl_mutex){+.+.+.}, at: [<ffffffff812b74c6>] rtnl_lock+0x12/0x14
Feb  3 17:03:00 vanamonde kernel: 
Feb  3 17:03:00 vanamonde kernel: but task is already holding lock:
Feb  3 17:03:00 vanamonde kernel:  (pm_mutex){+.+.+.}, at: [<ffffffff81077e1c>] hibernate+0x17/0x16d
Feb  3 17:03:00 vanamonde kernel: 
Feb  3 17:03:00 vanamonde kernel: which lock already depends on the new lock.
Feb  3 17:03:00 vanamonde kernel: 
Feb  3 17:03:00 vanamonde kernel: 
Feb  3 17:03:00 vanamonde kernel: the existing dependency chain (in reverse order) is:
Feb  3 17:03:01 vanamonde bluetoothd[1780]: Stopping security manager 0
Feb  3 17:03:04 vanamonde bluetoothd[1780]: HCI dev 0 down
Feb  3 17:03:04 vanamonde kernel: 
Feb  3 17:03:04 vanamonde kernel: -> #2 (pm_mutex){+.+.+.}:
Feb  3 17:03:04 vanamonde kernel:        [<ffffffff8106ab91>] __lock_acquire+0x135e/0x16eb
Feb  3 17:03:04 vanamonde kernel:        [<ffffffff8106afea>] lock_acquire+0xcc/0xe9
Feb  3 17:03:04 vanamonde kernel:        [<ffffffff81331e2a>] mutex_lock_nested+0x54/0x2b5
Feb  3 17:03:04 vanamonde kernel:        [<ffffffff81077c06>] resume_store+0x61/0xa2
Feb  3 17:03:04 vanamonde kernel:        [<ffffffff811d3fef>] kobj_attr_store+0x17/0x19
Feb  3 17:03:04 vanamonde kernel:        [<ffffffff8114332a>] sysfs_write_file+0x103/0x13f
Feb  3 17:03:04 vanamonde bluetoothd[1780]: Adapter /org/bluez/1775/hci0 has been disabled
Feb  3 17:03:05 vanamonde bluetoothd[1780]: HCI dev 0 unregistered
Feb  3 17:03:05 vanamonde bluetoothd[1780]: Unregister path: /org/bluez/1775/hci0
Feb  3 17:03:05 vanamonde bluetoothd[1780]: HCI dev 0 registered
Feb  3 17:03:05 vanamonde kernel:        [<ffffffff810ed676>] vfs_write+0xad/0x172
Feb  3 17:03:05 vanamonde kernel:        [<ffffffff810ed7f4>] sys_write+0x45/0x6c
Feb  3 17:03:05 vanamonde kernel:        [<ffffffff810029ab>] system_call_fastpath+0x16/0x1b
Feb  3 17:03:05 vanamonde kernel: 
Feb  3 17:03:05 vanamonde kernel: -> #1 (s_active){++++.+}:
Feb  3 17:03:06 vanamonde bluetoothd[1780]: HCI dev 0 up
Feb  3 17:03:06 vanamonde bluetoothd[1780]: Starting security manager 0
Feb  3 17:03:06 vanamonde kernel:        [<ffffffff8106ab91>] __lock_acquire+0x135e/0x16eb
Feb  3 17:03:06 vanamonde kernel:        [<ffffffff8106afea>] lock_acquire+0xcc/0xe9
Feb  3 17:03:06 vanamonde kernel:        [<ffffffff81143f56>] sysfs_deactivate+0x86/0xc3
Feb  3 17:03:06 vanamonde kernel:        [<ffffffff81144633>] sysfs_addrm_finish+0x31/0x5a
Feb  3 17:03:06 vanamonde kernel:        [<ffffffff81142931>] sysfs_hash_and_remove+0x4e/0x65
Feb  3 17:03:06 vanamonde kernel:        [<ffffffff81145814>] sysfs_remove_group+0x8c/0xc5
Feb  3 17:03:06 vanamonde kernel:        [<ffffffff81269570>] dpm_sysfs_remove+0x14/0x16
Feb  3 17:03:06 vanamonde kernel:        [<ffffffff81263d37>] device_del+0x47/0x181
Feb  3 17:03:06 vanamonde bluetoothd[1780]: Parsing /etc/bluetooth/serial.conf failed: No such file or directory
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff812ba61c>] netdev_unregister_kobject+0x24/0x28
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff812ae837>] rollback_registered_many+0x29b/0x300
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff812ae97d>] unregister_netdevice_queue+0x7f/0xb2
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff812ae9cc>] unregister_netdev+0x1c/0x23
Feb  3 17:03:07 vanamonde kernel:        [<ffffffffa04730c4>] usbnet_disconnect+0x87/0xba [usbnet]
Feb  3 17:03:07 vanamonde kernel:        [<ffffffffa00bb6ac>] usb_unbind_interface+0x69/0xf4 [usbcore]
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff8126617e>] __device_release_driver+0x76/0xcc
Feb  3 17:03:07 vanamonde bluetoothd[1780]: Adapter /org/bluez/1775/hci0 has been enabled
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff812662a2>] device_release_driver+0x1e/0x2b
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff81265797>] bus_remove_device+0xa8/0xbd
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff81263e22>] device_del+0x132/0x181
Feb  3 17:03:07 vanamonde kernel:        [<ffffffffa00b8446>] usb_disable_device+0x101/0x17e [usbcore]
Feb  3 17:03:07 vanamonde kernel:        [<ffffffffa00b2c24>] usb_disconnect+0xdb/0x150 [usbcore]
Feb  3 17:03:07 vanamonde kernel:        [<ffffffffa00b434e>] hub_thread+0x6e6/0x1268 [usbcore]
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff8105837b>] kthread+0x7a/0x82
Feb  3 17:03:07 vanamonde kernel:        [<ffffffff810037d4>] kernel_thread_helper+0x4/0x10
Feb  3 17:03:07 vanamonde kernel: 
Feb  3 17:03:08 vanamonde kernel: -> #0 (rtnl_mutex){+.+.+.}:
Feb  3 17:03:08 vanamonde kernel:        [<ffffffff8106a841>] __lock_acquire+0x100e/0x16eb
Feb  3 17:03:08 vanamonde kernel:        [<ffffffff8106afea>] lock_acquire+0xcc/0xe9
Feb  3 17:03:08 vanamonde kernel:        [<ffffffff81331e2a>] mutex_lock_nested+0x54/0x2b5
Feb  3 17:03:08 vanamonde kernel:        [<ffffffff812b74c6>] rtnl_lock+0x12/0x14
Feb  3 17:03:08 vanamonde kernel:        [<ffffffffa020c452>] wiphy_suspend+0x35/0x58 [cfg80211]
Feb  3 17:03:08 vanamonde kernel:        [<ffffffff81269c34>] legacy_suspend+0x29/0x63
Feb  3 17:03:08 vanamonde kernel:        [<ffffffff81269faa>] dpm_suspend_start+0x2e5/0x444
Feb  3 17:03:08 vanamonde kernel:        [<ffffffff81077c8b>] hibernation_snapshot+0x44/0x1be
Feb  3 17:03:08 vanamonde kernel:        [<ffffffff81077ece>] hibernate+0xc9/0x16d
Feb  3 17:03:09 vanamonde kernel:        [<ffffffff81076d33>] state_store+0x57/0xce
Feb  3 17:03:09 vanamonde kernel:        [<ffffffff811d3fef>] kobj_attr_store+0x17/0x19
Feb  3 17:03:09 vanamonde kernel:        [<ffffffff8114332a>] sysfs_write_file+0x103/0x13f
Feb  3 17:03:09 vanamonde kernel:        [<ffffffff810ed676>] vfs_write+0xad/0x172
Feb  3 17:03:09 vanamonde kernel:        [<ffffffff810ed7f4>] sys_write+0x45/0x6c
Feb  3 17:03:09 vanamonde kernel:        [<ffffffff810029ab>] system_call_fastpath+0x16/0x1b
Feb  3 17:03:09 vanamonde kernel: 
Feb  3 17:03:09 vanamonde kernel: other info that might help us debug this:
Feb  3 17:03:09 vanamonde kernel: 
Feb  3 17:03:09 vanamonde kernel: 4 locks held by bash/2406:
Feb  3 17:03:09 vanamonde kernel:  #0:  (&buffer->mutex){+.+.+.}, at: [<ffffffff8114325e>] sysfs_write_file+0x37/0x13f
Feb  3 17:03:09 vanamonde kernel:  #1:  (s_active){++++.+}, at: [<ffffffff8114489f>] sysfs_get_active_two+0x1f/0x44
Feb  3 17:03:09 vanamonde kernel:  #2:  (s_active){++++.+}, at: [<ffffffff811448ac>] sysfs_get_active_two+0x2c/0x44
Feb  3 17:03:09 vanamonde kernel:  #3:  (pm_mutex){+.+.+.}, at: [<ffffffff81077e1c>] hibernate+0x17/0x16d
Feb  3 17:03:09 vanamonde kernel: 
Feb  3 17:03:09 vanamonde kernel: stack backtrace:
Feb  3 17:03:09 vanamonde kernel: Pid: 2406, comm: bash Not tainted 2.6.33-rc6-0.1-default #50
Feb  3 17:03:09 vanamonde kernel: Call Trace:
Feb  3 17:03:09 vanamonde kernel:  [<ffffffff813336ef>] ? _raw_spin_unlock_irqrestore+0x33/0x41
Feb  3 17:03:09 vanamonde kernel:  [<ffffffff810692f5>] print_circular_bug+0xaf/0xbe
Feb  3 17:03:10 vanamonde kernel:  [<ffffffff8106a841>] __lock_acquire+0x100e/0x16eb
Feb  3 17:03:10 vanamonde kernel:  [<ffffffff8106afea>] lock_acquire+0xcc/0xe9
Feb  3 17:03:10 vanamonde kernel:  [<ffffffff812b74c6>] ? rtnl_lock+0x12/0x14
Feb  3 17:03:10 vanamonde kernel:  [<ffffffffa020c41d>] ? wiphy_suspend+0x0/0x58 [cfg80211]
Feb  3 17:03:10 vanamonde kernel:  [<ffffffff81331e2a>] mutex_lock_nested+0x54/0x2b5
Feb  3 17:03:10 vanamonde kernel:  [<ffffffff812b74c6>] ? rtnl_lock+0x12/0x14
Feb  3 17:03:10 vanamonde kernel:  [<ffffffffa020c41d>] ? wiphy_suspend+0x0/0x58 [cfg80211]
Feb  3 17:03:10 vanamonde kernel:  [<ffffffff812b74c6>] rtnl_lock+0x12/0x14
Feb  3 17:03:10 vanamonde kernel:  [<ffffffffa020c452>] wiphy_suspend+0x35/0x58 [cfg80211]
Feb  3 17:03:10 vanamonde kernel:  [<ffffffff81269c34>] legacy_suspend+0x29/0x63
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff81269faa>] dpm_suspend_start+0x2e5/0x444
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff81077c8b>] hibernation_snapshot+0x44/0x1be
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff81077ece>] hibernate+0xc9/0x16d
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff81076d33>] state_store+0x57/0xce
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff811d3fef>] kobj_attr_store+0x17/0x19
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff8114332a>] sysfs_write_file+0x103/0x13f
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff810ed676>] vfs_write+0xad/0x172
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff81068c13>] ? trace_hardirqs_on_caller+0x10c/0x130
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff810ed7f4>] sys_write+0x45/0x6c
Feb  3 17:03:11 vanamonde kernel:  [<ffffffff810029ab>] system_call_fastpath+0x16/0x1b
Feb  3 17:03:11 vanamonde kernel: btusb_intr_complete: hci0 urb ffff8801357f1080 failed to resubmit (1)
Feb  3 17:03:11 vanamonde kernel: uhci_hcd 0000:00:1a.2: release dev 2 ep81-INT, period 1, phase 0, 23 us
Feb  3 17:03:11 vanamonde kernel: btusb_bulk_complete: hci0 urb ffff88013967be40 failed to resubmit (1)
Feb  3 17:03:11 vanamonde kernel: btusb_bulk_complete: hci0 urb ffff88013967bf00 failed to resubmit (1)


^ permalink raw reply

* [net-next PATCH 0/3] qlge: EEH fixes.
From: Ron Mercer @ 2010-02-03 17:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer


1) Move asic reset from io_resume() to io_reset.
2) Fix case where close() can hang if eeh did not recover.
3) Fix stall in eeh recovery process.



^ permalink raw reply

* [net-next PATCH 1/3] qlge: Move reset from eeh io_resume to slot_reset.
From: Ron Mercer @ 2010-02-03 17:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1265217853-26959-1-git-send-email-ron.mercer@qlogic.com>

Issue asic reset and verify functionality before continuing to the
resume call.  This allows proper error code to be returned in the case
the asic does not recover.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 5be3ae2..87ec720 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4717,6 +4717,12 @@ static pci_ers_result_t qlge_io_slot_reset(struct pci_dev *pdev)
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 	pci_set_master(pdev);
+
+	if (ql_adapter_reset(qdev)) {
+		QPRINTK(qdev, DRV, ERR, "reset FAILED!\n");
+		return PCI_ERS_RESULT_DISCONNECT;
+	}
+
 	return PCI_ERS_RESULT_RECOVERED;
 }
 
@@ -4726,8 +4732,6 @@ static void qlge_io_resume(struct pci_dev *pdev)
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	int err = 0;
 
-	if (ql_adapter_reset(qdev))
-		QPRINTK(qdev, DRV, ERR, "reset FAILED!\n");
 	if (netif_running(ndev)) {
 		err = qlge_open(ndev);
 		if (err) {
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 3/3] qlge: Add watchdog timer.
From: Ron Mercer @ 2010-02-03 17:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1265217853-26959-1-git-send-email-ron.mercer@qlogic.com>

Add periodic heartbeat register read to trigger the eeh
recovery process.
We see cases where an eeh error was injected and the slot was
suspended.  An asic access attempt is required to flush the recovery process,
but without interrupts the process can stall.
Adding this periodic register read causes the recovery process to begin.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    1 +
 drivers/net/qlge/qlge_main.c |   26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 780a387..ebfd177 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2145,6 +2145,7 @@ struct ql_adapter {
 	struct completion ide_completion;
 	struct nic_operations *nic_ops;
 	u16 device_id;
+	struct timer_list timer;
 	atomic_t lb_count;
 };
 
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 7e00029..8f2a5ae 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4574,6 +4574,20 @@ static const struct net_device_ops qlge_netdev_ops = {
 	.ndo_vlan_rx_kill_vid	= qlge_vlan_rx_kill_vid,
 };
 
+static void ql_timer(unsigned long data)
+{
+	struct ql_adapter *qdev = (struct ql_adapter *)data;
+	u32 var = 0;
+
+	var = ql_read32(qdev, STS);
+	if (pci_channel_offline(qdev->pdev)) {
+		QPRINTK(qdev, IFUP, ERR, "EEH STS = 0x%.08x.\n", var);
+		return;
+	}
+
+	mod_timer(&qdev->timer, jiffies + HZ);
+}
+
 static int __devinit qlge_probe(struct pci_dev *pdev,
 				const struct pci_device_id *pci_entry)
 {
@@ -4625,6 +4639,13 @@ static int __devinit qlge_probe(struct pci_dev *pdev,
 		pci_disable_device(pdev);
 		return err;
 	}
+	/* Start up the timer to trigger EEH if
+	 * the bus goes dead
+	 */
+	init_timer(&qdev->timer);
+	qdev->timer.data = (unsigned long)qdev;
+	qdev->timer.function = ql_timer;
+	mod_timer(&qdev->timer, jiffies + HZ);
 	ql_link_off(qdev);
 	ql_display_dev_info(ndev);
 	atomic_set(&qdev->lb_count, 0);
@@ -4645,6 +4666,8 @@ int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget)
 static void __devexit qlge_remove(struct pci_dev *pdev)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
+	struct ql_adapter *qdev = netdev_priv(ndev);
+	del_timer_sync(&qdev->timer);
 	unregister_netdev(ndev);
 	ql_release_all(pdev);
 	pci_disable_device(pdev);
@@ -4757,6 +4780,7 @@ static void qlge_io_resume(struct pci_dev *pdev)
 		QPRINTK(qdev, IFUP, ERR,
 			"Device was not running prior to EEH.\n");
 	}
+	mod_timer(&qdev->timer, jiffies + HZ);
 	netif_device_attach(ndev);
 }
 
@@ -4773,6 +4797,7 @@ static int qlge_suspend(struct pci_dev *pdev, pm_message_t state)
 	int err;
 
 	netif_device_detach(ndev);
+	del_timer_sync(&qdev->timer);
 
 	if (netif_running(ndev)) {
 		err = ql_adapter_down(qdev);
@@ -4817,6 +4842,7 @@ static int qlge_resume(struct pci_dev *pdev)
 			return err;
 	}
 
+	mod_timer(&qdev->timer, jiffies + HZ);
 	netif_device_attach(ndev);
 
 	return 0;
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 2/3] qlge: Add check for eeh failure when closing device.
From: Ron Mercer @ 2010-02-03 17:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1265217853-26959-1-git-send-email-ron.mercer@qlogic.com>

Fix crash where resources are freed twice on an eeh recovery failure.
If eeh recovery fails we set a flag to indicate to close() that
resources have been freed.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    1 +
 drivers/net/qlge/qlge_main.c |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 9169c4c..780a387 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -2005,6 +2005,7 @@ enum {
 	QL_SELFTEST = 9,
 	QL_LB_LINK_UP = 10,
 	QL_FRC_COREDUMP = 11,
+	QL_EEH_FATAL = 12,
 };
 
 /* link_status bit definitions */
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 87ec720..7e00029 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -3929,6 +3929,16 @@ static int qlge_close(struct net_device *ndev)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 
+	/* If we hit pci_channel_io_perm_failure
+	 * failure condition, then we already
+	 * brought the adapter down.
+	 */
+	if (test_bit(QL_EEH_FATAL, &qdev->flags)) {
+		QPRINTK(qdev, DRV, ERR, "EEH fatal did unload.\n");
+		clear_bit(QL_EEH_FATAL, &qdev->flags);
+		return 0;
+	}
+
 	/*
 	 * Wait for device to recover from a reset.
 	 * (Rarely happens, but possible.)
@@ -4677,6 +4687,7 @@ static pci_ers_result_t qlge_io_error_detected(struct pci_dev *pdev,
 					       enum pci_channel_state state)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
+	struct ql_adapter *qdev = netdev_priv(ndev);
 
 	switch (state) {
 	case pci_channel_io_normal:
@@ -4690,6 +4701,8 @@ static pci_ers_result_t qlge_io_error_detected(struct pci_dev *pdev,
 	case pci_channel_io_perm_failure:
 		dev_err(&pdev->dev,
 			"%s: pci_channel_io_perm_failure.\n", __func__);
+		ql_eeh_close(ndev);
+		set_bit(QL_EEH_FATAL, &qdev->flags);
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 
@@ -4720,6 +4733,7 @@ static pci_ers_result_t qlge_io_slot_reset(struct pci_dev *pdev)
 
 	if (ql_adapter_reset(qdev)) {
 		QPRINTK(qdev, DRV, ERR, "reset FAILED!\n");
+		set_bit(QL_EEH_FATAL, &qdev->flags);
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 
-- 
1.6.0.2


^ permalink raw reply related

* Re: [PATCH net-next-2.6 v2] can: mcp251x: Move to threaded interrupts instead of workqueues.
From: christian pellegrin @ 2010-02-03 17:39 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4B692A8D.2040000-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

On Wed, Feb 3, 2010 at 8:49 AM, Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org> wrote:
>
> CAN experts told me/us, that the bus-errors might be important
> information for an apps. I just started a thread on how to improve our
> current bus-error handling implementation.
>

OK, I'll follow that thread to see how can additional error
information be exported to user space efficiently. For now I'm
sending, in reply to this email, the v3 patch that disables one shot
mode too as was discussed in the socketcan ml. There are some
important bug fixes in it so I hope it could be approved before next
merge window.

-- 
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."

^ permalink raw reply

* [PATCH net-next-2.6 v3] can: mcp251x: Move to threaded interrupts instead of workqueues.
From: Christian Pellegrin @ 2010-02-03 17:39 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Christian Pellegrin
In-Reply-To: <cabda6421002030939w3788a40en38955d31dd765583-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

This patch addresses concerns about efficiency of handling incoming
packets. Handling of interrupts is done in a threaded interrupt handler
which has a smaller latency than workqueues. This change needed a rework
of the locking scheme that was much simplified. Some other (more or less
longstanding) bugs are fixed: utilization of just half of the RX
buffers, useless wait for interrupt on open, more reliable reset
sequence. The MERR interrupt is not used anymore: it overloads the CPU
in error-passive state without any additional information. One shot mode
is disabled because it's not clear if it can be handled efficiently on
this CAN controller.

Signed-off-by: Christian Pellegrin <chripell-VaTbYqLCNhc@public.gmane.org>
---
 drivers/net/can/mcp251x.c |  421 ++++++++++++++++++++++-----------------------
 1 files changed, 203 insertions(+), 218 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index bbe186b..f8cc168 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -180,6 +180,14 @@
 #define RXBEID0_OFF 4
 #define RXBDLC_OFF  5
 #define RXBDAT_OFF  6
+#define RXFSIDH(n) ((n) * 4)
+#define RXFSIDL(n) ((n) * 4 + 1)
+#define RXFEID8(n) ((n) * 4 + 2)
+#define RXFEID0(n) ((n) * 4 + 3)
+#define RXMSIDH(n) ((n) * 4 + 0x20)
+#define RXMSIDL(n) ((n) * 4 + 0x21)
+#define RXMEID8(n) ((n) * 4 + 0x22)
+#define RXMEID0(n) ((n) * 4 + 0x23)
 
 #define GET_BYTE(val, byte)			\
 	(((val) >> ((byte) * 8)) & 0xff)
@@ -219,7 +227,8 @@ struct mcp251x_priv {
 	struct net_device *net;
 	struct spi_device *spi;
 
-	struct mutex spi_lock; /* SPI buffer lock */
+	struct mutex mcp_lock; /* SPI device lock */
+
 	u8 *spi_tx_buf;
 	u8 *spi_rx_buf;
 	dma_addr_t spi_tx_dma;
@@ -227,11 +236,11 @@ struct mcp251x_priv {
 
 	struct sk_buff *tx_skb;
 	int tx_len;
+
 	struct workqueue_struct *wq;
 	struct work_struct tx_work;
-	struct work_struct irq_work;
-	struct completion awake;
-	int wake;
+	struct work_struct restart_work;
+
 	int force_quit;
 	int after_suspend;
 #define AFTER_SUSPEND_UP 1
@@ -245,7 +254,8 @@ static void mcp251x_clean(struct net_device *net)
 {
 	struct mcp251x_priv *priv = netdev_priv(net);
 
-	net->stats.tx_errors++;
+	if (priv->tx_skb || priv->tx_len)
+		net->stats.tx_errors++;
 	if (priv->tx_skb)
 		dev_kfree_skb(priv->tx_skb);
 	if (priv->tx_len)
@@ -300,16 +310,12 @@ static u8 mcp251x_read_reg(struct spi_device *spi, uint8_t reg)
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
 	u8 val = 0;
 
-	mutex_lock(&priv->spi_lock);
-
 	priv->spi_tx_buf[0] = INSTRUCTION_READ;
 	priv->spi_tx_buf[1] = reg;
 
 	mcp251x_spi_trans(spi, 3);
 	val = priv->spi_rx_buf[2];
 
-	mutex_unlock(&priv->spi_lock);
-
 	return val;
 }
 
@@ -317,15 +323,11 @@ static void mcp251x_write_reg(struct spi_device *spi, u8 reg, uint8_t val)
 {
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
 
-	mutex_lock(&priv->spi_lock);
-
 	priv->spi_tx_buf[0] = INSTRUCTION_WRITE;
 	priv->spi_tx_buf[1] = reg;
 	priv->spi_tx_buf[2] = val;
 
 	mcp251x_spi_trans(spi, 3);
-
-	mutex_unlock(&priv->spi_lock);
 }
 
 static void mcp251x_write_bits(struct spi_device *spi, u8 reg,
@@ -333,16 +335,12 @@ static void mcp251x_write_bits(struct spi_device *spi, u8 reg,
 {
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
 
-	mutex_lock(&priv->spi_lock);
-
 	priv->spi_tx_buf[0] = INSTRUCTION_BIT_MODIFY;
 	priv->spi_tx_buf[1] = reg;
 	priv->spi_tx_buf[2] = mask;
 	priv->spi_tx_buf[3] = val;
 
 	mcp251x_spi_trans(spi, 4);
-
-	mutex_unlock(&priv->spi_lock);
 }
 
 static void mcp251x_hw_tx_frame(struct spi_device *spi, u8 *buf,
@@ -358,10 +356,8 @@ static void mcp251x_hw_tx_frame(struct spi_device *spi, u8 *buf,
 			mcp251x_write_reg(spi, TXBCTRL(tx_buf_idx) + i,
 					  buf[i]);
 	} else {
-		mutex_lock(&priv->spi_lock);
 		memcpy(priv->spi_tx_buf, buf, TXBDAT_OFF + len);
 		mcp251x_spi_trans(spi, TXBDAT_OFF + len);
-		mutex_unlock(&priv->spi_lock);
 	}
 }
 
@@ -408,13 +404,9 @@ static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *buf,
 		for (; i < (RXBDAT_OFF + len); i++)
 			buf[i] = mcp251x_read_reg(spi, RXBCTRL(buf_idx) + i);
 	} else {
-		mutex_lock(&priv->spi_lock);
-
 		priv->spi_tx_buf[RXBCTRL_OFF] = INSTRUCTION_READ_RXB(buf_idx);
 		mcp251x_spi_trans(spi, SPI_TRANSFER_BUF_LEN);
 		memcpy(buf, priv->spi_rx_buf, SPI_TRANSFER_BUF_LEN);
-
-		mutex_unlock(&priv->spi_lock);
 	}
 }
 
@@ -467,21 +459,6 @@ static void mcp251x_hw_sleep(struct spi_device *spi)
 	mcp251x_write_reg(spi, CANCTRL, CANCTRL_REQOP_SLEEP);
 }
 
-static void mcp251x_hw_wakeup(struct spi_device *spi)
-{
-	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
-
-	priv->wake = 1;
-
-	/* Can only wake up by generating a wake-up interrupt. */
-	mcp251x_write_bits(spi, CANINTE, CANINTE_WAKIE, CANINTE_WAKIE);
-	mcp251x_write_bits(spi, CANINTF, CANINTF_WAKIF, CANINTF_WAKIF);
-
-	/* Wait until the device is awake */
-	if (!wait_for_completion_timeout(&priv->awake, HZ))
-		dev_err(&spi->dev, "MCP251x didn't wake-up\n");
-}
-
 static netdev_tx_t mcp251x_hard_start_xmit(struct sk_buff *skb,
 					   struct net_device *net)
 {
@@ -490,7 +467,6 @@ static netdev_tx_t mcp251x_hard_start_xmit(struct sk_buff *skb,
 
 	if (priv->tx_skb || priv->tx_len) {
 		dev_warn(&spi->dev, "hard_xmit called while tx busy\n");
-		netif_stop_queue(net);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -511,12 +487,13 @@ static int mcp251x_do_set_mode(struct net_device *net, enum can_mode mode)
 
 	switch (mode) {
 	case CAN_MODE_START:
+		mcp251x_clean(net);
 		/* We have to delay work since SPI I/O may sleep */
 		priv->can.state = CAN_STATE_ERROR_ACTIVE;
 		priv->restart_tx = 1;
 		if (priv->can.restart_ms == 0)
 			priv->after_suspend = AFTER_SUSPEND_RESTART;
-		queue_work(priv->wq, &priv->irq_work);
+		queue_work(priv->wq, &priv->restart_work);
 		break;
 	default:
 		return -EOPNOTSUPP;
@@ -525,7 +502,7 @@ static int mcp251x_do_set_mode(struct net_device *net, enum can_mode mode)
 	return 0;
 }
 
-static void mcp251x_set_normal_mode(struct spi_device *spi)
+static int mcp251x_set_normal_mode(struct spi_device *spi)
 {
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
 	unsigned long timeout;
@@ -533,8 +510,7 @@ static void mcp251x_set_normal_mode(struct spi_device *spi)
 	/* Enable interrupts */
 	mcp251x_write_reg(spi, CANINTE,
 			  CANINTE_ERRIE | CANINTE_TX2IE | CANINTE_TX1IE |
-			  CANINTE_TX0IE | CANINTE_RX1IE | CANINTE_RX0IE |
-			  CANINTF_MERRF);
+			  CANINTE_TX0IE | CANINTE_RX1IE | CANINTE_RX0IE);
 
 	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
 		/* Put device into loopback mode */
@@ -544,9 +520,7 @@ static void mcp251x_set_normal_mode(struct spi_device *spi)
 		mcp251x_write_reg(spi, CANCTRL, CANCTRL_REQOP_LISTEN_ONLY);
 	} else {
 		/* Put device into normal mode */
-		mcp251x_write_reg(spi, CANCTRL, CANCTRL_REQOP_NORMAL |
-				  (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT ?
-				   CANCTRL_OSM : 0));
+		mcp251x_write_reg(spi, CANCTRL, CANCTRL_REQOP_NORMAL);
 
 		/* Wait for the device to enter normal mode */
 		timeout = jiffies + HZ;
@@ -555,11 +529,12 @@ static void mcp251x_set_normal_mode(struct spi_device *spi)
 			if (time_after(jiffies, timeout)) {
 				dev_err(&spi->dev, "MCP251x didn't"
 					" enter in normal mode\n");
-				return;
+				return -EBUSY;
 			}
 		}
 	}
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+	return 0;
 }
 
 static int mcp251x_do_set_bittiming(struct net_device *net)
@@ -590,33 +565,39 @@ static int mcp251x_setup(struct net_device *net, struct mcp251x_priv *priv,
 {
 	mcp251x_do_set_bittiming(net);
 
-	/* Enable RX0->RX1 buffer roll over and disable filters */
-	mcp251x_write_bits(spi, RXBCTRL(0),
-			   RXBCTRL_BUKT | RXBCTRL_RXM0 | RXBCTRL_RXM1,
-			   RXBCTRL_BUKT | RXBCTRL_RXM0 | RXBCTRL_RXM1);
-	mcp251x_write_bits(spi, RXBCTRL(1),
-			   RXBCTRL_RXM0 | RXBCTRL_RXM1,
-			   RXBCTRL_RXM0 | RXBCTRL_RXM1);
+	mcp251x_write_reg(spi, RXBCTRL(0),
+			  RXBCTRL_BUKT | RXBCTRL_RXM0 | RXBCTRL_RXM1);
+	mcp251x_write_reg(spi, RXBCTRL(1),
+			  RXBCTRL_RXM0 | RXBCTRL_RXM1);
 	return 0;
 }
 
-static void mcp251x_hw_reset(struct spi_device *spi)
+static int mcp251x_hw_reset(struct spi_device *spi)
 {
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
 	int ret;
-
-	mutex_lock(&priv->spi_lock);
+	unsigned long timeout;
 
 	priv->spi_tx_buf[0] = INSTRUCTION_RESET;
-
 	ret = spi_write(spi, priv->spi_tx_buf, 1);
-
-	mutex_unlock(&priv->spi_lock);
-
-	if (ret)
+	if (ret) {
 		dev_err(&spi->dev, "reset failed: ret = %d\n", ret);
+		return -EIO;
+	}
+
 	/* Wait for reset to finish */
+	timeout = jiffies + HZ;
 	mdelay(10);
+	while ((mcp251x_read_reg(spi, CANSTAT) & CANCTRL_REQOP_MASK)
+	       != CANCTRL_REQOP_CONF) {
+		schedule();
+		if (time_after(jiffies, timeout)) {
+			dev_err(&spi->dev, "MCP251x didn't"
+				" enter in conf mode after reset\n");
+			return -EBUSY;
+		}
+	}
+	return 0;
 }
 
 static int mcp251x_hw_probe(struct spi_device *spi)
@@ -640,63 +621,17 @@ static int mcp251x_hw_probe(struct spi_device *spi)
 	return (st1 == 0x80 && st2 == 0x07) ? 1 : 0;
 }
 
-static irqreturn_t mcp251x_can_isr(int irq, void *dev_id)
-{
-	struct net_device *net = (struct net_device *)dev_id;
-	struct mcp251x_priv *priv = netdev_priv(net);
-
-	/* Schedule bottom half */
-	if (!work_pending(&priv->irq_work))
-		queue_work(priv->wq, &priv->irq_work);
-
-	return IRQ_HANDLED;
-}
-
-static int mcp251x_open(struct net_device *net)
+static void mcp251x_open_clean(struct net_device *net)
 {
 	struct mcp251x_priv *priv = netdev_priv(net);
 	struct spi_device *spi = priv->spi;
 	struct mcp251x_platform_data *pdata = spi->dev.platform_data;
-	int ret;
-
-	ret = open_candev(net);
-	if (ret) {
-		dev_err(&spi->dev, "unable to set initial baudrate!\n");
-		return ret;
-	}
 
+	free_irq(spi->irq, priv);
+	mcp251x_hw_sleep(spi);
 	if (pdata->transceiver_enable)
-		pdata->transceiver_enable(1);
-
-	priv->force_quit = 0;
-	priv->tx_skb = NULL;
-	priv->tx_len = 0;
-
-	ret = request_irq(spi->irq, mcp251x_can_isr,
-			  IRQF_TRIGGER_FALLING, DEVICE_NAME, net);
-	if (ret) {
-		dev_err(&spi->dev, "failed to acquire irq %d\n", spi->irq);
-		if (pdata->transceiver_enable)
-			pdata->transceiver_enable(0);
-		close_candev(net);
-		return ret;
-	}
-
-	mcp251x_hw_wakeup(spi);
-	mcp251x_hw_reset(spi);
-	ret = mcp251x_setup(net, priv, spi);
-	if (ret) {
-		free_irq(spi->irq, net);
-		mcp251x_hw_sleep(spi);
-		if (pdata->transceiver_enable)
-			pdata->transceiver_enable(0);
-		close_candev(net);
-		return ret;
-	}
-	mcp251x_set_normal_mode(spi);
-	netif_wake_queue(net);
-
-	return 0;
+		pdata->transceiver_enable(0);
+	close_candev(net);
 }
 
 static int mcp251x_stop(struct net_device *net)
@@ -707,17 +642,19 @@ static int mcp251x_stop(struct net_device *net)
 
 	close_candev(net);
 
+	priv->force_quit = 1;
+	free_irq(spi->irq, priv);
+	destroy_workqueue(priv->wq);
+	priv->wq = NULL;
+
+	mutex_lock(&priv->mcp_lock);
+
 	/* Disable and clear pending interrupts */
 	mcp251x_write_reg(spi, CANINTE, 0x00);
 	mcp251x_write_reg(spi, CANINTF, 0x00);
 
-	priv->force_quit = 1;
-	free_irq(spi->irq, net);
-	flush_workqueue(priv->wq);
-
 	mcp251x_write_reg(spi, TXBCTRL(0), 0);
-	if (priv->tx_skb || priv->tx_len)
-		mcp251x_clean(net);
+	mcp251x_clean(net);
 
 	mcp251x_hw_sleep(spi);
 
@@ -726,9 +663,27 @@ static int mcp251x_stop(struct net_device *net)
 
 	priv->can.state = CAN_STATE_STOPPED;
 
+	mutex_unlock(&priv->mcp_lock);
+
 	return 0;
 }
 
+static void mcp251x_error_skb(struct net_device *net, int can_id, int data1)
+{
+	struct sk_buff *skb;
+	struct can_frame *frame;
+
+	skb = alloc_can_err_skb(net, &frame);
+	if (skb) {
+		frame->can_id = can_id;
+		frame->data[1] = data1;
+		netif_rx(skb);
+	} else {
+		dev_err(&net->dev,
+			"cannot allocate error skb\n");
+	}
+}
+
 static void mcp251x_tx_work_handler(struct work_struct *ws)
 {
 	struct mcp251x_priv *priv = container_of(ws, struct mcp251x_priv,
@@ -737,33 +692,32 @@ static void mcp251x_tx_work_handler(struct work_struct *ws)
 	struct net_device *net = priv->net;
 	struct can_frame *frame;
 
+	mutex_lock(&priv->mcp_lock);
 	if (priv->tx_skb) {
-		frame = (struct can_frame *)priv->tx_skb->data;
-
 		if (priv->can.state == CAN_STATE_BUS_OFF) {
 			mcp251x_clean(net);
-			netif_wake_queue(net);
-			return;
+		} else {
+			frame = (struct can_frame *)priv->tx_skb->data;
+
+			if (frame->can_dlc > CAN_FRAME_MAX_DATA_LEN)
+				frame->can_dlc = CAN_FRAME_MAX_DATA_LEN;
+			mcp251x_hw_tx(spi, frame, 0);
+			priv->tx_len = 1 + frame->can_dlc;
+			can_put_echo_skb(priv->tx_skb, net, 0);
+			priv->tx_skb = NULL;
 		}
-		if (frame->can_dlc > CAN_FRAME_MAX_DATA_LEN)
-			frame->can_dlc = CAN_FRAME_MAX_DATA_LEN;
-		mcp251x_hw_tx(spi, frame, 0);
-		priv->tx_len = 1 + frame->can_dlc;
-		can_put_echo_skb(priv->tx_skb, net, 0);
-		priv->tx_skb = NULL;
 	}
+	mutex_unlock(&priv->mcp_lock);
 }
 
-static void mcp251x_irq_work_handler(struct work_struct *ws)
+static void mcp251x_restart_work_handler(struct work_struct *ws)
 {
 	struct mcp251x_priv *priv = container_of(ws, struct mcp251x_priv,
-						 irq_work);
+						 restart_work);
 	struct spi_device *spi = priv->spi;
 	struct net_device *net = priv->net;
-	u8 txbnctrl;
-	u8 intf;
-	enum can_state new_state;
 
+	mutex_lock(&priv->mcp_lock);
 	if (priv->after_suspend) {
 		mdelay(10);
 		mcp251x_hw_reset(spi);
@@ -772,45 +726,54 @@ static void mcp251x_irq_work_handler(struct work_struct *ws)
 			mcp251x_set_normal_mode(spi);
 		} else if (priv->after_suspend & AFTER_SUSPEND_UP) {
 			netif_device_attach(net);
-			/* Clean since we lost tx buffer */
-			if (priv->tx_skb || priv->tx_len) {
-				mcp251x_clean(net);
-				netif_wake_queue(net);
-			}
+			mcp251x_clean(net);
 			mcp251x_set_normal_mode(spi);
+			netif_wake_queue(net);
 		} else {
 			mcp251x_hw_sleep(spi);
 		}
 		priv->after_suspend = 0;
+		priv->force_quit = 0;
 	}
 
-	if (priv->can.restart_ms == 0 && priv->can.state == CAN_STATE_BUS_OFF)
-		return;
+	if (priv->restart_tx) {
+		priv->restart_tx = 0;
+		mcp251x_write_reg(spi, TXBCTRL(0), 0);
+		mcp251x_clean(net);
+		netif_wake_queue(net);
+		mcp251x_error_skb(net, CAN_ERR_RESTARTED, 0);
+	}
+	mutex_unlock(&priv->mcp_lock);
+}
 
-	while (!priv->force_quit && !freezing(current)) {
-		u8 eflag = mcp251x_read_reg(spi, EFLG);
-		int can_id = 0, data1 = 0;
+static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
+{
+	struct mcp251x_priv *priv = dev_id;
+	struct spi_device *spi = priv->spi;
+	struct net_device *net = priv->net;
 
-		mcp251x_write_reg(spi, EFLG, 0x00);
+	mutex_lock(&priv->mcp_lock);
+	while (!priv->force_quit) {
+		enum can_state new_state;
+		u8 intf = mcp251x_read_reg(spi, CANINTF);
+		u8 eflag;
+		int can_id = 0, data1 = 0;
 
-		if (priv->restart_tx) {
-			priv->restart_tx = 0;
-			mcp251x_write_reg(spi, TXBCTRL(0), 0);
-			if (priv->tx_skb || priv->tx_len)
-				mcp251x_clean(net);
-			netif_wake_queue(net);
-			can_id |= CAN_ERR_RESTARTED;
+		if (intf & CANINTF_RX0IF) {
+			mcp251x_hw_rx(spi, 0);
+			/* Free one buffer ASAP */
+			mcp251x_write_bits(spi, CANINTF, intf & CANINTF_RX0IF,
+					   0x00);
 		}
 
-		if (priv->wake) {
-			/* Wait whilst the device wakes up */
-			mdelay(10);
-			priv->wake = 0;
-		}
+		if (intf & CANINTF_RX1IF)
+			mcp251x_hw_rx(spi, 1);
 
-		intf = mcp251x_read_reg(spi, CANINTF);
 		mcp251x_write_bits(spi, CANINTF, intf, 0x00);
 
+		eflag = mcp251x_read_reg(spi, EFLG);
+		mcp251x_write_reg(spi, EFLG, 0x00);
+
 		/* Update can state */
 		if (eflag & EFLG_TXBO) {
 			new_state = CAN_STATE_BUS_OFF;
@@ -851,59 +814,31 @@ static void mcp251x_irq_work_handler(struct work_struct *ws)
 		}
 		priv->can.state = new_state;
 
-		if ((intf & CANINTF_ERRIF) || (can_id & CAN_ERR_RESTARTED)) {
-			struct sk_buff *skb;
-			struct can_frame *frame;
-
-			/* Create error frame */
-			skb = alloc_can_err_skb(net, &frame);
-			if (skb) {
-				/* Set error frame flags based on bus state */
-				frame->can_id = can_id;
-				frame->data[1] = data1;
-
-				/* Update net stats for overflows */
-				if (eflag & (EFLG_RX0OVR | EFLG_RX1OVR)) {
-					if (eflag & EFLG_RX0OVR)
-						net->stats.rx_over_errors++;
-					if (eflag & EFLG_RX1OVR)
-						net->stats.rx_over_errors++;
-					frame->can_id |= CAN_ERR_CRTL;
-					frame->data[1] |=
-						CAN_ERR_CRTL_RX_OVERFLOW;
-				}
-
-				netif_rx(skb);
-			} else {
-				dev_info(&spi->dev,
-					 "cannot allocate error skb\n");
+		if (intf & CANINTF_ERRIF) {
+			/* Handle overflow counters */
+			if (eflag & (EFLG_RX0OVR | EFLG_RX1OVR)) {
+				if (eflag & EFLG_RX0OVR)
+					net->stats.rx_over_errors++;
+				if (eflag & EFLG_RX1OVR)
+					net->stats.rx_over_errors++;
+				can_id |= CAN_ERR_CRTL;
+				data1 |= CAN_ERR_CRTL_RX_OVERFLOW;
 			}
+			mcp251x_error_skb(net, can_id, data1);
 		}
 
 		if (priv->can.state == CAN_STATE_BUS_OFF) {
 			if (priv->can.restart_ms == 0) {
+				priv->force_quit = 1;
 				can_bus_off(net);
 				mcp251x_hw_sleep(spi);
-				return;
+				break;
 			}
 		}
 
 		if (intf == 0)
 			break;
 
-		if (intf & CANINTF_WAKIF)
-			complete(&priv->awake);
-
-		if (intf & CANINTF_MERRF) {
-			/* If there are pending Tx buffers, restart queue */
-			txbnctrl = mcp251x_read_reg(spi, TXBCTRL(0));
-			if (!(txbnctrl & TXBCTRL_TXREQ)) {
-				if (priv->tx_skb || priv->tx_len)
-					mcp251x_clean(net);
-				netif_wake_queue(net);
-			}
-		}
-
 		if (intf & (CANINTF_TX2IF | CANINTF_TX1IF | CANINTF_TX0IF)) {
 			net->stats.tx_packets++;
 			net->stats.tx_bytes += priv->tx_len - 1;
@@ -914,12 +849,66 @@ static void mcp251x_irq_work_handler(struct work_struct *ws)
 			netif_wake_queue(net);
 		}
 
-		if (intf & CANINTF_RX0IF)
-			mcp251x_hw_rx(spi, 0);
+	}
+	mutex_unlock(&priv->mcp_lock);
+	return IRQ_HANDLED;
+}
 
-		if (intf & CANINTF_RX1IF)
-			mcp251x_hw_rx(spi, 1);
+static int mcp251x_open(struct net_device *net)
+{
+	struct mcp251x_priv *priv = netdev_priv(net);
+	struct spi_device *spi = priv->spi;
+	struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+	int ret;
+
+	ret = open_candev(net);
+	if (ret) {
+		dev_err(&spi->dev, "unable to set initial baudrate!\n");
+		return ret;
+	}
+
+	mutex_lock(&priv->mcp_lock);
+	if (pdata->transceiver_enable)
+		pdata->transceiver_enable(1);
+
+	priv->force_quit = 0;
+	priv->tx_skb = NULL;
+	priv->tx_len = 0;
+
+	ret = request_threaded_irq(spi->irq, NULL, mcp251x_can_ist,
+			  IRQF_TRIGGER_FALLING, DEVICE_NAME, priv);
+	if (ret) {
+		dev_err(&spi->dev, "failed to acquire irq %d\n", spi->irq);
+		if (pdata->transceiver_enable)
+			pdata->transceiver_enable(0);
+		close_candev(net);
+		goto open_unlock;
+	}
+
+	priv->wq = create_freezeable_workqueue("mcp251x_wq");
+	INIT_WORK(&priv->tx_work, mcp251x_tx_work_handler);
+	INIT_WORK(&priv->restart_work, mcp251x_restart_work_handler);
+
+	ret = mcp251x_hw_reset(spi);
+	if (ret) {
+		mcp251x_open_clean(net);
+		goto open_unlock;
+	}
+	ret = mcp251x_setup(net, priv, spi);
+	if (ret) {
+		mcp251x_open_clean(net);
+		goto open_unlock;
 	}
+	ret = mcp251x_set_normal_mode(spi);
+	if (ret) {
+		mcp251x_open_clean(net);
+		goto open_unlock;
+	}
+	netif_wake_queue(net);
+
+open_unlock:
+	mutex_unlock(&priv->mcp_lock);
+	return ret;
 }
 
 static const struct net_device_ops mcp251x_netdev_ops = {
@@ -955,13 +944,11 @@ static int __devinit mcp251x_can_probe(struct spi_device *spi)
 	priv->can.clock.freq = pdata->oscillator_frequency / 2;
 	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
 		CAN_CTRLMODE_LOOPBACK | CAN_CTRLMODE_LISTENONLY;
-	if (pdata->model == CAN_MCP251X_MCP2515)
-		priv->can.ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT;
 	priv->net = net;
 	dev_set_drvdata(&spi->dev, priv);
 
 	priv->spi = spi;
-	mutex_init(&priv->spi_lock);
+	mutex_init(&priv->mcp_lock);
 
 	/* If requested, allocate DMA buffers */
 	if (mcp251x_enable_dma) {
@@ -1010,18 +997,12 @@ static int __devinit mcp251x_can_probe(struct spi_device *spi)
 
 	SET_NETDEV_DEV(net, &spi->dev);
 
-	priv->wq = create_freezeable_workqueue("mcp251x_wq");
-
-	INIT_WORK(&priv->tx_work, mcp251x_tx_work_handler);
-	INIT_WORK(&priv->irq_work, mcp251x_irq_work_handler);
-
-	init_completion(&priv->awake);
-
 	/* Configure the SPI bus */
 	spi->mode = SPI_MODE_0;
 	spi->bits_per_word = 8;
 	spi_setup(spi);
 
+	/* Here is OK to not lock the MCP, no one knows about it yet */
 	if (!mcp251x_hw_probe(spi)) {
 		dev_info(&spi->dev, "Probe failed\n");
 		goto error_probe;
@@ -1064,10 +1045,6 @@ static int __devexit mcp251x_can_remove(struct spi_device *spi)
 	unregister_candev(net);
 	free_candev(net);
 
-	priv->force_quit = 1;
-	flush_workqueue(priv->wq);
-	destroy_workqueue(priv->wq);
-
 	if (mcp251x_enable_dma) {
 		dma_free_coherent(&spi->dev, PAGE_SIZE,
 				  priv->spi_tx_buf, priv->spi_tx_dma);
@@ -1089,6 +1066,12 @@ static int mcp251x_can_suspend(struct spi_device *spi, pm_message_t state)
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
 	struct net_device *net = priv->net;
 
+	priv->force_quit = 1;
+	disable_irq(spi->irq);
+	/*
+	 * Note: at this point neither IST nor workqueues are running.
+	 * open/stop cannot be called anyway so locking is not needed
+	 */
 	if (netif_running(net)) {
 		netif_device_detach(net);
 
@@ -1115,16 +1098,18 @@ static int mcp251x_can_resume(struct spi_device *spi)
 
 	if (priv->after_suspend & AFTER_SUSPEND_POWER) {
 		pdata->power_enable(1);
-		queue_work(priv->wq, &priv->irq_work);
+		queue_work(priv->wq, &priv->restart_work);
 	} else {
 		if (priv->after_suspend & AFTER_SUSPEND_UP) {
 			if (pdata->transceiver_enable)
 				pdata->transceiver_enable(1);
-			queue_work(priv->wq, &priv->irq_work);
+			queue_work(priv->wq, &priv->restart_work);
 		} else {
 			priv->after_suspend = 0;
 		}
 	}
+	priv->force_quit = 0;
+	enable_irq(spi->irq);
 	return 0;
 }
 #else
-- 
1.5.6.5

^ permalink raw reply related

* [RFC] Connection-Oriented Multicasting
From: Michael Chan @ 2010-02-03 17:55 UTC (permalink / raw)
  To: netdev

This RFC is for transport layer people:

I am implementing a TCP single-source multicast optimization (SMO):

Ref: http://www-dsg.stanford.edu/sliang/tcpm_infocom.pdf

In a nutshell, a source server enables the "SMO" mode of a TCP
listener socket, which creates a master socket through which multicast
data is sent. The server still maintains a connection with each client.
These individual sockets are responsible for retransmissions to
clients. They use the master socket's retransmission queue and
snd_nxt. The multicast clients enables "SMO" mode of its TCP
socket and joins a multicast group.

While coding the client side, I discovered that since 2007 multicast
joins are not allowed for connection-oriented sockets.There isn't any
explanation for the change in the commit that effected the change:

http://www.mail-archive.com/git-commits-head@vger.kernel.org/msg21013.html

Is it a policy to forbid any connection-oriented protocol from
participating in multicast?

Conventional wisdom is against such an optimization due to the
complexity in handling multiple clients from a TCP perspective, but
I don't see what is wrong with a connection-oriented protocol departing
from the well-established point-to-point abstraction given by TCP.
After all, tons of application-level protocols are built to provide
connection services on top of (UDP) multicast.

I believe it is worth optimizing that by putting connection services
in the transport level, given that:

1. There are already in-order delivery, retransmission and connection
management mechanisms in TCP.

2. There are performance benefits in a kernel-mode implementation.
such as timing precision, over using either select() or threads
for individual connections.

3. The additions to TCP are modest and may be enabled /
disabled via kernel configurations. An application that wants
to use reliable single-source multicast needs only add two
setsockopt() calls to the code. Further customization may
be provided with additional socket options, but the benefits
of reliable multicast is available with minimal
application-level changes.

I understand this is a rather intrusive modification to the
existing TCP implementation, since it adds to
struct tcp_sock and modifies the segment processing routines.
Is it a fool's errand to even attempt to get this accepted?

Thanks,
Michael

^ permalink raw reply

* Re: [PATCH] sky2:  receive dma mapping error handling
From: Justin P. Mattock @ 2010-02-03 18:23 UTC (permalink / raw)
  To: Michael Breuer
  Cc: Stephen Hemminger, Jarek Poplawski, David Miller, akpm, flyboy,
	linux-kernel, netdev, Michael Chan, Don Fry, Francois Romieu,
	Matt Carlson
In-Reply-To: <4B69AD5C.5030601@majjas.com>

On 02/03/10 09:07, Michael Breuer wrote:
> On 02/03/2010 11:56 AM, Stephen Hemminger wrote:
>> On Wed, 03 Feb 2010 11:47:19 -0500
>> Michael Breuer<mbreuer@majjas.com> wrote:
>>
>>> Tracked this down. The status being returned is 0x3c0080 - good flow
>>> control packets. Nothing is actually being dropped (confirmed by packet
>>> trace on switch compared with packet trace on server).
>>>
>>> I whipped up a trivial patch to not count these as dropped packets and
>>> will post to netdev.
>>>
>>> I'm not really sure what the driver should be doing in this case, but
>>> resubmit seems to work.
>> Looks like a flow control negotiation issue. You probably turned off
>> flow control on the Linux side, but the switch is still doing flow
>> control.
>>
> According to the driver:
> Feb 3 12:03:02 mail kernel: sky2 eth0: Link is up at 1000 Mbps, full
> duplex, flow control both
>
> So if the rx flow control packet status is due to flow control being
> disabled, then there's a different issue.
>
>
> --
> 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/
>


hmm.. after an hour or so I'm not seeing anything.
from what I remember I turned the machine on in the
hotel, then left the system there as I went out for a few hours
(so maybe I need to wait).

Anyways I did keep dmesg of when this occurred, basically
the log was spammed with these:

[  863.294057] sky2 eth0: rx error, status 0x580002 length 88
[  865.646645] sky2 eth0: rx error, status 0x600002 length 96
[ 1286.420471] sky2 eth0: rx error, status 0x600002 length 96
[ 1286.499459] sky2 eth0: rx error, status 0x600002 length 96
[ 1746.903826] sky2 eth0: rx error, status 0x600002 length 96
[ 1754.263692] sky2 eth0: rx error, status 0x600002 length 96
[ 1755.309360] sky2 eth0: rx error, status 0x680002 length 104
[ 2213.256294] sky2 eth0: rx error, status 0x600002 length 96
[ 2219.653342] sky2 eth0: rx error, status 0x580002 length 88
[ 2221.673601] sky2 eth0: rx error, status 0x600002 length 96
[ 2679.654655] sky2 eth0: rx error, status 0x680002 length 104
[ 2692.315058] sky2 eth0: rx error, status 0x500002 length 80
[ 2694.349612] sky2 eth0: rx error, status 0x580002 length 88
[ 2703.676717] sky2 eth0: rx error, status 0x700002 length 112
[ 2703.826375] sky2 eth0: rx error, status 0x600002 length 96
[ 3187.504843] sky2 eth0: rx error, status 0x600002 length 96
[ 3189.560744] sky2 eth0: rx error, status 0x600002 length 96
[ 3672.475719] sky2 eth0: rx error, status 0x680002 length 104
[ 3676.696959] sky2 eth0: rx error, status 0x680002 length 104

but while using the system with this, I didn't notice anything
out of the ordinary.
(if this fires off I can try a bisect for you guys, but right now
since I'm not seeing anything, might be a different story);

Justin P. Mattock

^ permalink raw reply

* Re: [PATCH] sky2:  receive dma mapping error handling
From: Stephen Hemminger @ 2010-02-03 18:25 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: Michael Breuer, Jarek Poplawski, David Miller, akpm, flyboy,
	linux-kernel, netdev, Michael Chan, Don Fry, Francois Romieu,
	Matt Carlson
In-Reply-To: <4B69BF05.6020802@gmail.com>

On Wed, 03 Feb 2010 10:23:01 -0800
"Justin P. Mattock" <justinmattock@gmail.com> wrote:

> On 02/03/10 09:07, Michael Breuer wrote:
> > On 02/03/2010 11:56 AM, Stephen Hemminger wrote:
> >> On Wed, 03 Feb 2010 11:47:19 -0500
> >> Michael Breuer<mbreuer@majjas.com> wrote:
> >>
> >>> Tracked this down. The status being returned is 0x3c0080 - good flow
> >>> control packets. Nothing is actually being dropped (confirmed by packet
> >>> trace on switch compared with packet trace on server).
> >>>
> >>> I whipped up a trivial patch to not count these as dropped packets and
> >>> will post to netdev.
> >>>
> >>> I'm not really sure what the driver should be doing in this case, but
> >>> resubmit seems to work.
> >> Looks like a flow control negotiation issue. You probably turned off
> >> flow control on the Linux side, but the switch is still doing flow
> >> control.
> >>
> > According to the driver:
> > Feb 3 12:03:02 mail kernel: sky2 eth0: Link is up at 1000 Mbps, full
> > duplex, flow control both
> >
> > So if the rx flow control packet status is due to flow control being
> > disabled, then there's a different issue.
> >
> >
> > --
> > 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/
> >
> 
> 
> hmm.. after an hour or so I'm not seeing anything.
> from what I remember I turned the machine on in the
> hotel, then left the system there as I went out for a few hours
> (so maybe I need to wait).
> 
> Anyways I did keep dmesg of when this occurred, basically
> the log was spammed with these:
> 
> [  863.294057] sky2 eth0: rx error, status 0x580002 length 88
> [  865.646645] sky2 eth0: rx error, status 0x600002 length 96
> [ 1286.420471] sky2 eth0: rx error, status 0x600002 length 96
> [ 1286.499459] sky2 eth0: rx error, status 0x600002 length 96
> [ 1746.903826] sky2 eth0: rx error, status 0x600002 length 96
> [ 1754.263692] sky2 eth0: rx error, status 0x600002 length 96
> [ 1755.309360] sky2 eth0: rx error, status 0x680002 length 104
> [ 2213.256294] sky2 eth0: rx error, status 0x600002 length 96
> [ 2219.653342] sky2 eth0: rx error, status 0x580002 length 88
> [ 2221.673601] sky2 eth0: rx error, status 0x600002 length 96
> [ 2679.654655] sky2 eth0: rx error, status 0x680002 length 104
> [ 2692.315058] sky2 eth0: rx error, status 0x500002 length 80
> [ 2694.349612] sky2 eth0: rx error, status 0x580002 length 88
> [ 2703.676717] sky2 eth0: rx error, status 0x700002 length 112
> [ 2703.826375] sky2 eth0: rx error, status 0x600002 length 96
> [ 3187.504843] sky2 eth0: rx error, status 0x600002 length 96
> [ 3189.560744] sky2 eth0: rx error, status 0x600002 length 96
> [ 3672.475719] sky2 eth0: rx error, status 0x680002 length 104
> [ 3676.696959] sky2 eth0: rx error, status 0x680002 length 104
> 
> but while using the system with this, I didn't notice anything
> out of the ordinary.
> (if this fires off I can try a bisect for you guys, but right now
> since I'm not seeing anything, might be a different story);
> 

You were on a crappy hotel switch. Those are all CRC errors.



-- 

^ permalink raw reply

* Re: [PATCH] sky2: Flow control frames recorded as dropped packets
From: Stephen Hemminger @ 2010-02-03 18:31 UTC (permalink / raw)
  To: Michael Breuer; +Cc: netdev
In-Reply-To: <4B69AAA9.20505@majjas.com>

Thanks for your patch.  A more general solution would be to move the
rx_dropped up into sky2_receive.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/sky2.c	2010-02-03 09:10:27.863367226 -0800
+++ b/drivers/net/sky2.c	2010-02-03 09:17:01.602741445 -0800
@@ -2404,6 +2404,9 @@ okay:
 		skb = receive_copy(sky2, re, length);
 	else
 		skb = receive_new(sky2, re, length);
+
+	dev->stats.rx_dropped += (skb == NULL);
+
 resubmit:
 	sky2_rx_submit(sky2, re);
 
@@ -2515,11 +2518,10 @@ static int sky2_status_intr(struct sky2_
 		case OP_RXSTAT:
 			total_packets[port]++;
 			total_bytes[port] += length;
+
 			skb = sky2_receive(dev, length, status);
-			if (unlikely(!skb)) {
-				dev->stats.rx_dropped++;
+			if (!skb)
 				break;
-			}
 
 			/* This chip reports checksum status differently */
 			if (hw->flags & SKY2_HW_NEW_LE) {

^ permalink raw reply

* Re: [PATCHv2 08/11] fec: fix uninitialized rx buffer usage
From: Amit Kucheria @ 2010-02-03 18:33 UTC (permalink / raw)
  To: Grant Likely
  Cc: List Linux Kernel, Rob Herring, David S. Miller, netdev,
	linux-arm-kernel, linux, s.hauer, valentin.longchamp, daniel,
	Dinh.Nguyen, bryan.wu
In-Reply-To: <fa686aa41002030846v5044d585w7f4291f89c4b7ad@mail.gmail.com>

On Wed, Feb 3, 2010 at 8:46 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Tue, Feb 2, 2010 at 10:16 PM, Amit Kucheria
> <amit.kucheria@canonical.com> wrote:
>> From: Rob Herring <r.herring@freescale.com>
>>
>> The fec driver was enabling receive buffer descriptor without allocating
>> the buffers. Make sure the buffer descriptors are initialized to not
>> start receiving packets.
>>
>> Open also calls fec_restart after the rx buffers are allocated. With the code
>> in fec_restart, it zeroes out the buffer descriptors that have just been
>> setup.
>
> fec related patches 8 & 9 look okay to me.
>
> g.
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.

Can I take that as an Acked-by?

^ permalink raw reply

* Re: [PATCHv2 08/11] fec: fix uninitialized rx buffer usage
From: Grant Likely @ 2010-02-03 18:38 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: List Linux Kernel, Rob Herring, David S. Miller, netdev,
	linux-arm-kernel, linux, s.hauer, valentin.longchamp, daniel,
	Dinh.Nguyen, bryan.wu
In-Reply-To: <37786d4b1002031033s15d0b24et82c3f0e45112ae65@mail.gmail.com>

On Wed, Feb 3, 2010 at 11:33 AM, Amit Kucheria
<amit.kucheria@canonical.com> wrote:
> On Wed, Feb 3, 2010 at 8:46 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Tue, Feb 2, 2010 at 10:16 PM, Amit Kucheria
>> <amit.kucheria@canonical.com> wrote:
>>> From: Rob Herring <r.herring@freescale.com>
>>>
>>> The fec driver was enabling receive buffer descriptor without allocating
>>> the buffers. Make sure the buffer descriptors are initialized to not
>>> start receiving packets.
>>>
>>> Open also calls fec_restart after the rx buffers are allocated. With the code
>>> in fec_restart, it zeroes out the buffer descriptors that have just been
>>> setup.
>>
>> fec related patches 8 & 9 look okay to me.
>>
>> g.
>>
>> --
>> Grant Likely, B.Sc., P.Eng.
>> Secret Lab Technologies Ltd.
>
> Can I take that as an Acked-by?

of course.

g.

^ permalink raw reply

* Re: [PATCH] netfilter: per netns nf_conntrack_cachep
From: Jon Masters @ 2010-02-03 18:38 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Alexey Dobriyan, Eric Dumazet, linux-kernel, netdev,
	netfilter-devel, Paul E. McKenney
In-Reply-To: <4B6967BC.600@trash.net>

On Wed, 2010-02-03 at 13:10 +0100, Patrick McHardy wrote:
> Patrick McHardy wrote:
> > Jon Masters wrote:
> >> On Tue, 2010-02-02 at 19:58 +0200, Alexey Dobriyan wrote:
> >>
> >>> Yes, moving to init_net-only function is fine.
> >> So moving the "setup up fake conntrack" bits to init_init_net from
> >> init_net still results in the panic, which means that the use count
> >> really is dropping to zero and we really are trying to free it when
> >> using multiple namespaces. Per ns is probably an easier way to go.
> > 
> > Agreed, that will also avoid problems in the future with the
> > ct_net pointer pointing to &init_net. I'll take care of this
> > tommorrow.
> 
> Unfortunately a per-namespace conntrack is not easily possible without
> larger changes (most of which are already queued in nf-next-2.6.git
> though). So for now I just moved the untrack handling to the init_net
> setup and cleanup functions and we can try to fix the remainder in
> 2.6.34.

Ok. I'd love to help out actually, given that I've been poking at this,
and it's quite fun. So please at least send me patches. The only other
thing I consider a priority issue at the moment for this is that writing
into /sys/module/nf_conntrack/parameters/hashsize on a running system
with multiple namespaces will cause the system to corrupt random memory
silently and fall over. That probably needs fixing until there is
per-namespace hashsize tracking, and this isn't a global tunable.

Also, some other things I think are required before 2.6.34:

*). Per namespace cacheing allocation (the cachep bits). We know it's
still possible for weirdness to happen in the SLAB cache here.
*). Per namespace hashsize tracking. Existing code corrupts hashtables
if the global size is changed when there is more than one netns
*). Per namespace expectations. This is for similar reasons to the need
for multiple hashtables, though I haven't poked at that.

I also think it is necessary to expose net namespace layout and
configuration via sysfs or some other interface, add a net->id parameter
(and may even an optional name), etc. Where does netns discussion
happen, on netdev I would presume?

> Jon, could you give this patch a try please?

Yup. Box is stable and boots multiple virtual machines as it did with
the quick hack from yesterday, so this has now fixed the problem.

Can you let me know if this is the final patch you want to post? If so,
we should get this into stable asap (and I have a couple of vendor
kernels that will need a version of this fix also).

Jon.



^ permalink raw reply

* Re: [PATCH] sky2:  receive dma mapping error handling
From: Justin P. Mattock @ 2010-02-03 18:48 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Michael Breuer, Jarek Poplawski, David Miller, akpm, flyboy,
	linux-kernel, netdev, Michael Chan, Don Fry, Francois Romieu,
	Matt Carlson
In-Reply-To: <20100203102558.75d7a919@nehalam>

On 02/03/10 10:25, Stephen Hemminger wrote:
> On Wed, 03 Feb 2010 10:23:01 -0800
> "Justin P. Mattock"<justinmattock@gmail.com>  wrote:
>
>> On 02/03/10 09:07, Michael Breuer wrote:
>>> On 02/03/2010 11:56 AM, Stephen Hemminger wrote:
>>>> On Wed, 03 Feb 2010 11:47:19 -0500
>>>> Michael Breuer<mbreuer@majjas.com>  wrote:
>>>>
>>>>> Tracked this down. The status being returned is 0x3c0080 - good flow
>>>>> control packets. Nothing is actually being dropped (confirmed by packet
>>>>> trace on switch compared with packet trace on server).
>>>>>
>>>>> I whipped up a trivial patch to not count these as dropped packets and
>>>>> will post to netdev.
>>>>>
>>>>> I'm not really sure what the driver should be doing in this case, but
>>>>> resubmit seems to work.
>>>> Looks like a flow control negotiation issue. You probably turned off
>>>> flow control on the Linux side, but the switch is still doing flow
>>>> control.
>>>>
>>> According to the driver:
>>> Feb 3 12:03:02 mail kernel: sky2 eth0: Link is up at 1000 Mbps, full
>>> duplex, flow control both
>>>
>>> So if the rx flow control packet status is due to flow control being
>>> disabled, then there's a different issue.
>>>
>>>
>>> --
>>> 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/
>>>
>>
>>
>> hmm.. after an hour or so I'm not seeing anything.
>> from what I remember I turned the machine on in the
>> hotel, then left the system there as I went out for a few hours
>> (so maybe I need to wait).
>>
>> Anyways I did keep dmesg of when this occurred, basically
>> the log was spammed with these:
>>
>> [  863.294057] sky2 eth0: rx error, status 0x580002 length 88
>> [  865.646645] sky2 eth0: rx error, status 0x600002 length 96
>> [ 1286.420471] sky2 eth0: rx error, status 0x600002 length 96
>> [ 1286.499459] sky2 eth0: rx error, status 0x600002 length 96
>> [ 1746.903826] sky2 eth0: rx error, status 0x600002 length 96
>> [ 1754.263692] sky2 eth0: rx error, status 0x600002 length 96
>> [ 1755.309360] sky2 eth0: rx error, status 0x680002 length 104
>> [ 2213.256294] sky2 eth0: rx error, status 0x600002 length 96
>> [ 2219.653342] sky2 eth0: rx error, status 0x580002 length 88
>> [ 2221.673601] sky2 eth0: rx error, status 0x600002 length 96
>> [ 2679.654655] sky2 eth0: rx error, status 0x680002 length 104
>> [ 2692.315058] sky2 eth0: rx error, status 0x500002 length 80
>> [ 2694.349612] sky2 eth0: rx error, status 0x580002 length 88
>> [ 2703.676717] sky2 eth0: rx error, status 0x700002 length 112
>> [ 2703.826375] sky2 eth0: rx error, status 0x600002 length 96
>> [ 3187.504843] sky2 eth0: rx error, status 0x600002 length 96
>> [ 3189.560744] sky2 eth0: rx error, status 0x600002 length 96
>> [ 3672.475719] sky2 eth0: rx error, status 0x680002 length 104
>> [ 3676.696959] sky2 eth0: rx error, status 0x680002 length 104
>>
>> but while using the system with this, I didn't notice anything
>> out of the ordinary.
>> (if this fires off I can try a bisect for you guys, but right now
>> since I'm not seeing anything, might be a different story);
>>
>
> You were on a crappy hotel switch. Those are all CRC errors.
>
>
>


alright.. makes sense cause I'm not getting
any of this now at home.

Justin P. Mattock

^ 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