Netdev List
 help / color / mirror / Atom feed
* [net-next-2.6 PATCH 5/5] be2net: delete unnecessary code from be_cmd_POST()
From: Sathya Perla @ 2009-07-28  8:53 UTC (permalink / raw)
  To: netdev

When the driver loads the POST stage of the card is expected to be
POST_STAGE_ARMFW_RDY.

Signed-off-by: Sathya Perla <sathyap@serverengines.com>
---
 drivers/net/benet/be_cmds.c |   45 +++---------------------------------------
 1 files changed, 4 insertions(+), 41 deletions(-)

diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index ebf65c7..de10773 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -227,54 +227,17 @@ static int be_POST_stage_get(struct be_adapter *adapter, u16 *stage)
 		return 0;
 }
 
-static int be_POST_stage_poll(struct be_adapter *adapter, u16 poll_stage)
-{
-	u16 stage, cnt, error;
-	for (cnt = 0; cnt < 5000; cnt++) {
-		error = be_POST_stage_get(adapter, &stage);
-		if (error)
-			return -1;
-
-		if (stage == poll_stage)
-			break;
-		udelay(1000);
-	}
-	if (stage != poll_stage)
-		return -1;
-	return 0;
-}
-
-
 int be_cmd_POST(struct be_adapter *adapter)
 {
 	u16 stage, error;
 
 	error = be_POST_stage_get(adapter, &stage);
-	if (error)
-		goto err;
-
-	if (stage == POST_STAGE_ARMFW_RDY)
-		return 0;
-
-	if (stage != POST_STAGE_AWAITING_HOST_RDY)
-		goto err;
-
-	/* On awaiting host rdy, reset and again poll on awaiting host rdy */
-	iowrite32(POST_STAGE_BE_RESET, adapter->csr + MPU_EP_SEMAPHORE_OFFSET);
-	error = be_POST_stage_poll(adapter, POST_STAGE_AWAITING_HOST_RDY);
-	if (error)
-		goto err;
-
-	/* Now kickoff POST and poll on armfw ready */
-	iowrite32(POST_STAGE_HOST_RDY, adapter->csr + MPU_EP_SEMAPHORE_OFFSET);
-	error = be_POST_stage_poll(adapter, POST_STAGE_ARMFW_RDY);
-	if (error)
-		goto err;
+	if (error || stage != POST_STAGE_ARMFW_RDY) {
+		dev_err(&adapter->pdev->dev, "POST failed.\n");
+		return -1;
+	}
 
 	return 0;
-err:
-	printk(KERN_WARNING DRV_NAME ": ERROR, stage=%d\n", stage);
-	return -1;
 }
 
 static inline void *embedded_payload(struct be_mcc_wrb *wrb)
-- 
1.6.0.4


^ permalink raw reply related
* Re: [RFC] [PATCH] Don't run __qdisc_run() on a stopped TX queue
From: Jarek Poplawski @ 2009-07-28  9:02 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David Miller, krkumar2, netdev
In-Reply-To: <20090728084451.GA26299@gondor.apana.org.au>

On Tue, Jul 28, 2009 at 04:44:51PM +0800, Herbert Xu wrote:
> On Tue, Jul 28, 2009 at 08:37:13AM +0000, Jarek Poplawski wrote:
> >
> > Hmm.. would something IMHO_more_readable/less_fixing like:
> > (qdisc->flags & TCQ_F_DEFAULT) be too much? (Btw, IIRC there was an
> > idea long time ago to treat all queues equally.)
> 
> Well the fact that as it stands the only true multiqueue qdisc
> is the default is a mere coincidence.  There is no fundamental
> reason why non-default qdiscs cannot be made multiqueue aware,
> well, for some of them anyway.
> 
> For example, sfq can naturally be enhanced as a multiqueue qdisc
> just like the default qdisc.
> 
> So I don't think we want to prejudice what a future non-default
> qdisc may support.

If it's only about the name I'm OK with: (qdisc->flags &
TCQ_F_DEFAULT_WITHOUT_PREJUDICE_MAYBE_NON_DEFAULT_IN_THE_FUTURE)
or any other, too ;-)

Cheers,
Jarek P.

^ permalink raw reply
* Re: Kernel oops on setting sky2 interfaces down
From: Rene Mayrhofer @ 2009-07-28  9:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Richard Leitner
In-Reply-To: <20090727153548.7c0d9f85@nehalam>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Stephen Hemminger wrote:
> Does this help?
> 
> --- a/drivers/net/sky2.c	2009-07-27 15:28:27.653757064 -0700
> +++ b/drivers/net/sky2.c	2009-07-27 15:34:24.358730966 -0700
> @@ -2763,6 +2763,11 @@ static int sky2_poll(struct napi_struct 
>  	int work_done = 0;
>  	u16 idx;
>  
> +	if (unlikely(status == ~0)) {
> +		dev_info(&hw->pdev->dev, "device status error\n");
> +		goto clear_napi;
> +	}
> +
>  	if (unlikely(status & Y2_IS_ERROR))
>  		sky2_err_intr(hw, status);
>  
> @@ -2779,6 +2784,7 @@ static int sky2_poll(struct napi_struct 
>  			goto done;
>  	}
>  
> +clear_napi:
>  	napi_complete(napi);
>  	sky2_read32(hw, B0_Y2_SP_LISR);
>  done:

With this applied, the behaviour is certainly different: On first
networking restart, the kernel still continues to run although there are
some phy errors. On the second restart, there is no Oops, but a BUG.


[~]# /etc/init.d/networking restart
Reconfiguring network interfaces...Removed VLAN -:quara.6:-
[  269.681295] sky2 0000:03:00.0: dmz: phy I/O error
[  269.686079] sky2 0000:03:00.0: dmz: phy I/O error
[  269.691000] sky2 0000:03:00.0: dmz: phy I/O error
[  269.695880] sky2 0000:03:00.0: dmz: phy I/O error
[  269.700751] sky2 0000:03:00.0: dmz: phy I/O error
[  269.705613] sky2 0000:03:00.0: dmz: phy I/O error
[  269.710519] sky2 0000:03:00.0: dmz: phy I/O error
[  269.715420] sky2 0000:03:00.0: dmz: phy I/O error
[  269.720290] sky2 0000:03:00.0: dmz: phy I/O error
[  269.725203] sky2 0000:03:00.0: dmz: phy I/O error
Set name-type for VLAN subsystem. Should be visible in /proc/net/vlan/config
Added VLAN with VID == 6 to IF -:testnet:-


[~]# /etc/init.d/networking restart
Reconfiguring network interfaces...[  298.296616] ICMPv6 NA: someone
advertises our address on lan!

[  299.360719] lan: hw csum failure.

[  299.364169] Pid: 11563, comm: sh Not tainted 2.6.28.10 #3

[  299.369763] Call Trace:

[  299.372317]  [<c09eb603>] __skb_checksum_complete_head+0x3e/0x4f

[  299.378536]  [<f827f1f7>] udp_error+0x124/0x198 [nf_conntrack]

[  299.384571]  [<f827f0d3>] udp_error+0x0/0x198 [nf_conntrack]

[  299.390429]  [<f827b7bd>] nf_conntrack_in+0x117/0x72a [nf_conntrack]

[  299.397138]  [<c08718e1>] handle_mm_fault+0x54a/0xbfa

[  299.402464]  [<c0a04da4>] nf_iterate+0x30/0x61

[  299.407148]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af

[  299.412108]  [<c0a04f17>] nf_hook_slow+0x49/0xbd

[  299.416893]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af

[  299.421760]  [<c0a0a5a9>] ip_rcv+0x1d6/0x20e

[  299.426185]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af

[  299.431067]  [<c09ef01d>] netif_receive_skb+0x3f7/0x435

[  299.436479]  [<f8085143>] sky2_poll+0x844/0xc21 [sky2]

[  299.441780]  [<f817f52e>] au_reval_and_lock_fdi+0x7e/0x540 [aufs]

[  299.448088]  [<f817f3aa>] au_reopen_nondir+0x0/0x106 [aufs]

[  299.453825]  [<c09eda45>] net_rx_action+0xb8/0x1f6

[  299.458886]  [<c082f954>] __do_softirq+0x95/0x142

[  299.463850]  [<c082fa49>] do_softirq+0x48/0x57

[  299.468547]  [<c082fbc9>] irq_exit+0x3b/0x78

[  299.473053]  [<c0806642>] do_IRQ+0x7a/0x8c

[  299.477306]  [<c0804e23>] common_interrupt+0x23/0x30

[  299.482543]  [<c089f0f1>] fsstack_copy_inode_size+0x1d/0x3f

[  299.488314]  [<f817b3a4>] au_cpup_attr_timesizes+0x4e/0x58 [aufs]

[  299.494615]  [<f8180b06>] aufs_flush+0x93/0xc9 [aufs]

[  299.499837]  [<c0884a24>] filp_close+0x2e/0x53

[  299.504559]  [<c0884ab4>] sys_close+0x6b/0xa4

[  299.509178]  [<c0803cb2>] syscall_call+0x7/0xb

[  299.513893]  [<c0a60000>] rwsem_down_failed_common+0xa4/0x175

[  299.621662] ------------[ cut here ]------------

[  299.625591] kernel BUG at drivers/net/sky2.c:1781!

[  299.625591] invalid opcode: 0000 [#1] PREEMPT SMP

[  299.625591] last sysfs file:
/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed

[  299.625591] Modules linked in: xt_multiport cpufreq_userspace xt_DSCP
xt_length xt_mark xt_dscp xt_MARK xt_CONNMARK xt_comment xt_policy
ipt_REDIRECT ip6t_LOG xt_tcpudp ip6table_mangle iptable_mangle
ip6table_filter ip6_tables sit tunnel4 8021q garp stp llc ipt_LOG
xt_limit xt_state iptable_nat iptable_filter ip_tables x_tables dm_mod
p4_clockmod speedstep_lib freq_table tun imq nf_nat_ftp nf_nat
nf_conntrack_ftp nf_conntrack_ipv6 nf_conntrack_ipv4 nf_conntrack
nf_defrag_ipv4 ipv6 evdev parport_pc parport pcspkr serio_raw i2c_i801
i2c_core iTCO_wdt rng_core intel_agp agpgart squashfs sqlzma unlzma loop
aufs exportfs nls_utf8 nls_cp437 ide_generic sd_mod ide_gd_mod
ata_generic pata_acpi ata_piix piix ide_pci_generic ide_core skge sky2
thermal_sys
[  299.625591]

[  299.625591] Pid: 11626, comm: ip Not tainted (2.6.28.10 #3)

[  299.625591] EIP: 0060:[<f80836a5>] EFLAGS: 00010256 CPU: 0

[  299.625591] EIP is at sky2_down+0x84/0x5c3 [sky2]

[  299.625591] EAX: f8010000 EBX: 00000280 ECX: 000006b4 EDX: f7060000

[  299.625591] ESI: 00000000 EDI: f684e980 EBP: 00000001 ESP: f5e35b78

[  299.625591]  DS: 0068 ES: 0068 FS: 00d8 GS: 0033 SS: 0068

[  299.625591] Process ip (pid: 11626, ti=f5e34000 task=f4a7e140
task.ti=f5e34000)

[  299.625591] Stack:

[  299.625591]  f7060000 00000303 00000004 f7060500 00000004 c09fc8c5
00000000 f7060290

[  299.625591]  f7060000 00001002 00001003 00000001 c09f00bd f7060000
c09efe15 00000000

[  299.625591]  ffffffef 00000000 00000000 f5e35ce4 c09f66b8 f5e35c28
f5e8f410 f7060000

[  299.625591] Call Trace:

[  299.625591]  [<c09fc8c5>] dev_deactivate+0x116/0x13b

[  299.625591]  [<c09f00bd>] dev_close+0x5f/0x7b

[  299.625591]  [<c09efe15>] dev_change_flags+0x9e/0x14f

[  299.625591]  [<c09f66b8>] do_setlink+0x28b/0x349

[  299.625591]  [<f80438b2>] skge_get_stats+0x2f/0x7b [skge]

[  299.625591]  [<c09f77a2>] rtnl_newlink+0x292/0x3f7

[  299.625591]  [<c09f756a>] rtnl_newlink+0x5a/0x3f7

[  299.625591]  [<c09f75aa>] rtnl_newlink+0x9a/0x3f7

[  299.625591]  [<c0862c3d>] find_get_page+0x87/0xaa

[  299.625591]  [<c09f7510>] rtnl_newlink+0x0/0x3f7

[  299.625591]  [<c09f74f6>] rtnetlink_rcv_msg+0x188/0x1a2

[  299.625591]  [<c09f736e>] rtnetlink_rcv_msg+0x0/0x1a2

[  299.625591]  [<c0a0390e>] netlink_rcv_skb+0x2d/0x73

[  299.625591]  [<c09f7368>] rtnetlink_rcv+0x19/0x1f

[  299.625591]  [<c0a03477>] netlink_unicast+0x1c7/0x229

[  299.625591]  [<c0a03729>] netlink_sendmsg+0x250/0x25d

[  299.625591]  [<c09e497a>] sock_sendmsg+0xc7/0xe1

[  299.625591]  [<c083b5fc>] autoremove_wake_function+0x0/0x2d

[  299.625591]  [<c083b5fc>] autoremove_wake_function+0x0/0x2d

[  299.625591]  [<c083b5fc>] autoremove_wake_function+0x0/0x2d

[  299.625591]  [<c09eb21b>] verify_iovec+0x3e/0x6b

[  299.625591]  [<c09e4b18>] sys_sendmsg+0x184/0x1e3

[  299.625591]  [<c09e574f>] sys_recvmsg+0x147/0x1e5

[  299.625591]  [<c09e5413>] sys_sendto+0xf9/0x124

[  299.625591]  [<c0a0298c>] netlink_insert+0xd2/0xef

[  299.625591]  [<c08757bb>] vma_merge+0x1d7/0x3ac

[  299.625591]  [<c09e5dd8>] sys_socketcall+0x177/0x1a9

[  299.625591]  [<c0875f3b>] sys_brk+0xd1/0xd9

[  299.625591]  [<c0803cb2>] syscall_call+0x7/0xb

[  299.625591]  [<c0a60000>] rwsem_down_failed_common+0xa4/0x175

[  299.625591] Code: b5 00 66 08 f8 b8 00 02 00 00 8d 8b 34 04 00 00 89
ca 03 17 89 02 8b 14 24 8b 82 00 05 00 00 8b 00 83 c0 04 66 8b 00 66 40
75 04 <0f> 0b eb fe 89 c8 03 07 8b 00 ba 05 00 00 00 8d 83 28 08 00 00

[  299.625591] EIP: [<f80836a5>] sky2_down+0x84/0x5c3 [sky2] SS:ESP
0068:f5e35b78

[  299.969972] sky2 0000:03:00.0: device status error


Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel[  299.982399] ---[ end trace c27f023d76d6060f ]---
:[  299.621662] ------------[ cut here ]------------

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591] invalid opcode: 0000 [#1] PREEMPT SMP

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591] last sysfs file:
/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed


Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591] Process ip (pid: 11626, ti=f5e34000 task=f4a7e140
task.ti=f5e34000)


Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591] Stack:

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  f7060000 00001002 00001003 00000001 c09f00bd
f7060000 c09efe15 00000000


Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  ffffffef 00000000 00000000 f5e35ce4 c09f66b8
f5e35c28 f5e8f410 f7060000


Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591] Call Trace:

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09fc8c5>] dev_deactivate+0x116/0x13b

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09f00bd>] dev_close+0x5f/0x7b

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09efe15>] dev_change_flags+0x9e/0x14f

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.6[  300.125238] lan: hw csum failure.
25591]  [<c09f66[  300.128944] Pid: 0, comm: swapper Tainted: G      D
  2.6.28.10 #3

b8>] do_setlink+[  300.136882] Call Trace:

0x28b/0x349

[  300.140827]  [<c09eb603>] __skb_checksum_complete_head+0x3e/0x4f


Message from s[  300.148422]  [<f827f1f7>] udp_error+0x124/0x198
[nf_conntrack]
yslogd@gibraltar[  300.155814]  [<f827f0d3>] udp_error+0x0/0x198
[nf_conntrack]
3-esys-master at[  300.163067]  [<f827b7bd>] nf_conntrack_in+0x117/0x72a
[nf_conntrack]

 Jul 28 11:46:45[  300.170988]  [<c08620a9>] cpupri_set+0xcf/0xeb

 ...

 kernel:[[  300.177061]  [<c08232cf>] enqueue_task_rt+0xfd/0x1ba

  299.625591]  [[  300.183686]  [<c081f9e3>] enqueue_task+0x52/0x5d

<f80438b2>] skge[  300.189847]  [<c0a604e8>]
_spin_unlock_irqrestore+0x22/0x39
_get_stats+0x2f/[  300.196991]  [<c0827013>] try_to_wake_up+0x158/0x162

0x7b [skge]

[  300.203501]  [<c0a04da4>] nf_iterate+0x30/0x61


Message from s[  300.209498]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
yslogd@gibraltar[  300.215661]  [<c0a04f17>] nf_hook_slow+0x49/0xbd
3-esys-master at[  300.221842]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
 Jul 28 11:46:45[  300.228078]  [<c0a0a5a9>] ip_rcv+0x1d6/0x20e
 ...
 kernel:[[  300.233875]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
  299.625591]  [[  300.240132]  [<c09ef01d>] netif_receive_skb+0x3f7/0x435
<c09f77a2>] rtnl[  300.246911]  [<f8085143>] sky2_poll+0x844/0xc21 [sky2]
_newlink+0x292/0[  300.253594]  [<c09eda45>] net_rx_action+0xb8/0x1f6
x3f7

Messa[  300.259939]  [<c082f954>] __do_softirq+0x95/0x142
ge from syslogd@[  300.266166]  [<c082fa49>] do_softirq+0x48/0x57
gibraltar3-esys-[  300.272137]  [<c082fbc9>] irq_exit+0x3b/0x78
master at Jul 28[  300.277945]  [<c0806642>] do_IRQ+0x7a/0x8c
 11:46:45 ...
[  300.283559]  [<c0804e23>] common_interrupt+0x23/0x30
 kernel:[  299.6[  300.290055]  [<c080a2c6>] mwait_idle+0x2f/0x3b
25591]  [<c09f75[  300.296043]  [<c0802ac9>] cpu_idle+0x7a/0xad
6a>] rtnl_newlink+0x5a/0x3f7

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09f75aa>] rtnl_newlink+0x9a/0x3f7

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c0862c3d>] find_get_page+0x87/0xaa

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09f7510>] rtnl_newlink+0x0/0x3f7

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09f74f6>] rtnetlink_rcv_msg+0x188/0x1a2

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09f736e>] rtnetlink_rcv_msg+0x0/0x1a2

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c0a0390e>] netlink_rcv_skb+0x2d/0x73

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09f7368>] rtnetlink_rcv+0x19/0x1f

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c0a03477>] netlink_unicast+0x1c7/0x229

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c0a03729>] netlink_sendmsg+0x250/0x25d

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09e497a>] sock_sendmsg+0xc7/0xe1

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c083b5fc>] autoremove_wake_function+0x0/0x2d

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:last message repeated 2 times

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09eb21b>] verify_iovec+0x3e/0x6b

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09e4b18>] sys_sendmsg+0x184/0x1e3

Message from syslogd@gibraltar3-esys-master at Jul 28 11:46:45 ...
 kernel:[  299.625591]  [<c09e574f>] sys_recvmsg+0x
Message from/etc/network/if-down.d/60address: line 25: 11626
Segmentation fault      ip link set dev $DEV down

run-parts: /etc/network/if-down.d/60address exited with return code 139

[  300.968019] sky2 0000:03:00.0: device status error

^C

[~]# [  302.000024] sky2 0000:03:00.0: device status error
[  302.746708] lan: hw csum failure.
[  302.750282] Pid: 0, comm: swapper Tainted: G      D    2.6.28.10 #3
[  302.757023] Call Trace:
[  302.759602]  [<c09eb603>] __skb_checksum_complete_head+0x3e/0x4f
[  302.765863]  [<f827f1f7>] udp_error+0x124/0x198 [nf_conntrack]
[  302.771934]  [<f827f0d3>] udp_error+0x0/0x198 [nf_conntrack]
[  302.777799]  [<f827b7bd>] nf_conntrack_in+0x117/0x72a [nf_conntrack]
[  302.784380]  [<c08210dd>] __wake_up_sync+0x2a/0x3e
[  302.789356]  [<c0a604e8>] _spin_unlock_irqrestore+0x22/0x39
[  302.795129]  [<c09e6ca5>] sock_def_readable+0x32/0x5b
[  302.800379]  [<c0a6050d>] _read_unlock+0xe/0x21
[  302.805080]  [<c09e8064>] sock_queue_rcv_skb+0xb5/0xbd
[  302.810421]  [<c0a25e9f>] __udp_queue_rcv_skb+0x12/0x86
[  302.815849]  [<c0a04da4>] nf_iterate+0x30/0x61
[  302.820442]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  302.825356]  [<c0a04f17>] nf_hook_slow+0x49/0xbd
[  302.830416]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  302.835510]  [<c0a0a5a9>] ip_rcv+0x1d6/0x20e
[  302.840117]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  302.845167]  [<c09ef01d>] netif_receive_skb+0x3f7/0x435
[  302.850791]  [<f8085143>] sky2_poll+0x844/0xc21 [sky2]
[  302.856337]  [<c0811a18>] lapic_next_event+0x10/0x13
[  302.861672]  [<c09eda45>] net_rx_action+0xb8/0x1f6
[  302.866654]  [<c082f954>] __do_softirq+0x95/0x142
[  302.871590]  [<c082fa49>] do_softirq+0x48/0x57
[  302.876335]  [<c082fbc9>] irq_exit+0x3b/0x78
[  302.880861]  [<c0806642>] do_IRQ+0x7a/0x8c
[  302.885184]  [<c0804e23>] common_interrupt+0x23/0x30
[  302.890422]  [<c080a2c6>] mwait_idle+0x2f/0x3b
[  302.896051]  [<c0802ac9>] cpu_idle+0x7a/0xad
[  303.000027] sky2 0000:03:00.0: device status error
[  303.058758] lan: hw csum failure.
[  303.062367] Pid: 0, comm: swapper Tainted: G      D    2.6.28.10 #3
[  303.068861] Call Trace:
[  303.071406]  [<c09eb603>] __skb_checksum_complete_head+0x3e/0x4f
[  303.077789]  [<f827f1f7>] udp_error+0x124/0x198 [nf_conntrack]
[  303.083871]  [<c0811a18>] lapic_next_event+0x10/0x13
[  303.089068]  [<f827f0d3>] udp_error+0x0/0x198 [nf_conntrack]
[  303.094929]  [<f827b7bd>] nf_conntrack_in+0x117/0x72a [nf_conntrack]
[  303.101514]  [<c0844533>] tick_program_event+0x2c/0x32
[  303.106851]  [<c083e621>] hrtimer_interrupt+0x146/0x16e
[  303.112313]  [<c082fb99>] irq_exit+0xb/0x78
[  303.116669]  [<c081218f>] smp_apic_timer_interrupt+0x75/0x7f
[  303.122585]  [<c0a04da4>] nf_iterate+0x30/0x61
[  303.127184]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  303.132067]  [<c0a04f17>] nf_hook_slow+0x49/0xbd
[  303.136911]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  303.141785]  [<c0a0a5a9>] ip_rcv+0x1d6/0x20e
[  303.146239]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  303.151112]  [<c09ef01d>] netif_receive_skb+0x3f7/0x435
[  303.156552]  [<f8085143>] sky2_poll+0x844/0xc21 [sky2]
[  303.161923]  [<c08417f0>] getnstimeofday+0x4f/0xd5
[  303.166879]  [<c09eda45>] net_rx_action+0xb8/0x1f6
[  303.171889]  [<c082f954>] __do_softirq+0x95/0x142
[  303.176758]  [<c082fa49>] do_softirq+0x48/0x57
[  303.181403]  [<c082fbc9>] irq_exit+0x3b/0x78
[  303.185850]  [<c0806642>] do_IRQ+0x7a/0x8c
[  303.190128]  [<c0804e23>] common_interrupt+0x23/0x30
[  303.195241]  [<c080a2c6>] mwait_idle+0x2f/0x3b
[  303.199879]  [<c0802ac9>] cpu_idle+0x7a/0xad
[  304.000039] sky2 0000:03:00.0: device status error
[  304.150934] lan: hw csum failure.
[  304.154546] Pid: 0, comm: swapper Tainted: G      D    2.6.28.10 #3
[  304.161288] Call Trace:
[  304.163876]  [<c09eb603>] __skb_checksum_complete_head+0x3e/0x4f
[  304.170102]  [<f827f1f7>] udp_error+0x124/0x198 [nf_conntrack]
[  304.176113]  [<c0811a18>] lapic_next_event+0x10/0x13
[  304.181260]  [<f827f0d3>] udp_error+0x0/0x198 [nf_conntrack]
[  304.187188]  [<f827b7bd>] nf_conntrack_in+0x117/0x72a [nf_conntrack]
[  304.193794]  [<c0844533>] tick_program_event+0x2c/0x32
[  304.199092]  [<c083e621>] hrtimer_interrupt+0x146/0x16e
[  304.204545]  [<c082fb99>] irq_exit+0xb/0x78
[  304.208887]  [<c081218f>] smp_apic_timer_interrupt+0x75/0x7f
[  304.214774]  [<c0a04da4>] nf_iterate+0x30/0x61
[  304.219390]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  304.224251]  [<c0a04f17>] nf_hook_slow+0x49/0xbd
[  304.229045]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  304.233922]  [<c0a0a5a9>] ip_rcv+0x1d6/0x20e
[  304.238348]  [<c0a0a124>] ip_rcv_finish+0x0/0x2af
[  304.243199]  [<c09ef01d>] netif_receive_skb+0x3f7/0x435
[  304.248634]  [<f8085143>] sky2_poll+0x844/0xc21 [sky2]
[  304.253961]  [<c0811a18>] lapic_next_event+0x10/0x13
[  304.259076]  [<c09eda45>] net_rx_action+0xb8/0x1f6
[  304.264063]  [<c082f954>] __do_softirq+0x95/0x142
[  304.268988]  [<c082fa49>] do_softirq+0x48/0x57
[  304.273595]  [<c082fbc9>] irq_exit+0x3b/0x78
[  304.278053]  [<c0806642>] do_IRQ+0x7a/0x8c
[  304.282326]  [<c0804e23>] common_interrupt+0x23/0x30
[  304.287467]  [<c080a2c6>] mwait_idle+0x2f/0x3b
[  304.292095]  [<c0802ac9>] cpu_idle+0x7a/0xad
[  305.000039] sky2 0000:03:00.0: device status error
[  306.000049] sky2 0000:03:00.0: device status error
[  307.000058] sky2 0000:03:00.0: device status error
[  308.000090] sky2 0000:03:00.0: device status error
[  309.000145] sky2 0000:03:00.0: device status error

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpuyU0ACgkQq7SPDcPCS95M+gCg/Sc8kO3FmHrQTdIqeKIzq1XI
gEwAoJez4jZCId+81exvRH6jF4Lzj922
=ryPP
-----END PGP SIGNATURE-----

^ permalink raw reply
* Re: [Bugme-new] [Bug 13760] New: 2.6.30 kernel locks up with pppoe in  back trace (regression)
From: Pavel Emelyanov @ 2009-07-28  9:51 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Igor M Podlesny, Andrew Morton, bugzilla-daemon, bugme-daemon,
	netdev, Pavel Emelyanov, Paul E. McKenney, David S. Miller
In-Reply-To: <4A6EBA88.8030205@cosmosbay.com>

Eric Dumazet wrote:
> Igor M Podlesny a écrit :
>> [...]
>>> Could have been a problem in net core, perhaps.
>>>
>>> Below is a ppp fix from 2.6.31, but it seems unlikely to fix your problem.
>>>
>>> It would help if we could see that trace, please.  A digital photo
>>> would suit.
>> 	Here it is:
>>
>> 		http://bugzilla.kernel.org/attachment.cgi?id=22516
>>
>> 	(It's 2.6.30.3)
>> 	
> 
> Looking at this, I believe net_assign_generic() is not safe.
> 
> Two cpus could try to expand/update the array at same time, one update could be lost.
> 
> register_pernet_gen_device() has a mutex to guard against concurrent
> calls, but net_assign_generic() has no locking at all.
> 
> I doubt this is the reason of the crash, still worth to mention it...
> 
> [PATCH] net: net_assign_generic() is not SMP safe
> 
> Two cpus could try to expand/update the array at same time, one update
> could be lost during the copy of old array.

How can this happen? The array is updated only during ->init routines
of the pernet_operations, which are called from under the net_mutex.

Do I miss anything?

> Re-using net_mutex is an easy way to fix this, it was used right
> before to allocate the 'id'
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> 
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index b7292a2..9c31ad1 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -467,15 +467,17 @@ int net_assign_generic(struct net *net, int id, void *data)
>  	BUG_ON(!mutex_is_locked(&net_mutex));
>  	BUG_ON(id == 0);
>  
> +	mutex_lock(&net_mutex);
>  	ng = old_ng = net->gen;
>  	if (old_ng->len >= id)
>  		goto assign;
>  
>  	ng = kzalloc(sizeof(struct net_generic) +
>  			id * sizeof(void *), GFP_KERNEL);
> -	if (ng == NULL)
> +	if (ng == NULL) {
> +		mutex_unlock(&net_mutex);
>  		return -ENOMEM;
> -
> +	}
>  	/*
>  	 * Some synchronisation notes:
>  	 *
> @@ -494,6 +496,7 @@ int net_assign_generic(struct net *net, int id, void *data)
>  	call_rcu(&old_ng->rcu, net_generic_release);
>  assign:
>  	ng->ptr[id - 1] = data;
> +	mutex_unlock(&net_mutex);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(net_assign_generic);
> 
> 


^ permalink raw reply
* Re: [RFC] [PATCH] Don't run __qdisc_run() on a stopped TX queue
From: Herbert Xu @ 2009-07-28 10:53 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David Miller, krkumar2, netdev
In-Reply-To: <20090728090229.GA6593@ff.dom.local>

On Tue, Jul 28, 2009 at 09:02:29AM +0000, Jarek Poplawski wrote:
>
> If it's only about the name I'm OK with: (qdisc->flags &
> TCQ_F_DEFAULT_WITHOUT_PREJUDICE_MAYBE_NON_DEFAULT_IN_THE_FUTURE)
> or any other, too ;-)

I see.  Yes a flag should work.

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
* Re: [RFC][PATCH 1/5] IPVS: prefix EnterFunction and LeaveFunction msg with "IPVS:"
From: Hannes Eder @ 2009-07-28 11:15 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: lvs-devel, netdev, netfilter-devel, linux-kernel
In-Reply-To: <alpine.LSU.2.00.0907272013070.20984@fbirervta.pbzchgretzou.qr>

On Mon, Jul 27, 2009 at 20:14, Jan Engelhardt<jengelh@medozas.de> wrote:
>
> On Monday 2009-07-27 15:46, Hannes Eder wrote:
>>
>>Now all printk messages from IPVS are prefixed with "IPVS:".
>>
>>+#define EnterFunction(level)                                                 \
>>+      do {                                                                   \
>>+              if (level <= ip_vs_get_debug_level())                          \
>>+                      printk(KERN_DEBUG "IPVS: Enter: %s, %s line %i\n",     \
>>+                              __func__, __FILE__, __LINE__);                 \
>>+      } while (0)
>>+#define LeaveFunction(level)                                                 \
>>+      do {                                                                   \
>>+              if (level <= ip_vs_get_debug_level())                          \
>>+                      printk(KERN_DEBUG "IPVS: Leave: %s, %s line %i\n",     \
>>+                              __func__, __FILE__, __LINE__);                 \
>>+      } while (0)
>
> I think you should rather make use of pr_fmt:
>
> <before any #includes>
> #define pr_fmt(x) "IPVS: " x
>
> And then use pr_<level>("Elvis has left the building") in code. This
> will add IPVS: automatically to all pr_* calls, alleviating the need
> to manually type it into all printks.

I like this idea.  I'll come up with an extra patch, it does not fit
into this series anyway.

> Of course, if you only want it for the two defines here, scrap
> my idea :)
>

Cheers,
-Hannes

^ permalink raw reply
* Re: [RFC] [PATCH] Don't run __qdisc_run() on a stopped TX queue
From: Jarek Poplawski @ 2009-07-28 11:24 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David Miller, krkumar2, netdev
In-Reply-To: <20090728105329.GA27453@gondor.apana.org.au>

On Tue, Jul 28, 2009 at 06:53:29PM +0800, Herbert Xu wrote:
> On Tue, Jul 28, 2009 at 09:02:29AM +0000, Jarek Poplawski wrote:
> >
> > If it's only about the name I'm OK with: (qdisc->flags &
> > TCQ_F_DEFAULT_WITHOUT_PREJUDICE_MAYBE_NON_DEFAULT_IN_THE_FUTURE)
> > or any other, too ;-)
> 
> I see.  Yes a flag should work.

So, I hope Krishna will try if these requested comments save any gains
of the original patch...

Thanks,
Jarek P.

^ permalink raw reply
* [PATCH 0/1] mISDN: Fix handling of receive buffer size in L1oIP
From: Karsten Keil @ 2009-07-28 11:37 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, netdev, David Miller, i4ldeveloper,
	Andreas Eversberg, Arnaldo Carvalho de Melo

This patch fix a regression introduced in 2.6.31 with the commit
8c90e11e3543d7de612194a042a148caeaab5f1d "mISDN: Use kernel_{send,recv}msg instead of open coding".

Linus: you can pull the fix from
git://git.kernel.org/pub/scm/linux/kernel/git/kkeil/ISDN-2.6-net-next for_linus

David for net-next-2.6 you can get it from:
git://git.kernel.org/pub/scm/linux/kernel/git/kkeil/ISDN-2.6-net-next for_david

Andreas Eversberg (1):
  mISDN: Fix handling of receive buffer size in L1oIP

 drivers/isdn/mISDN/l1oip_core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

^ permalink raw reply
* Re: [RFC] [PATCH] Don't run __qdisc_run() on a stopped TX queue
From: Krishna Kumar2 @ 2009-07-28 11:43 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David Miller, Herbert Xu, netdev
In-Reply-To: <20090728112445.GB6593@ff.dom.local>

Sure, I will test and give my findings here. I guess setting the
flag in qdisc_create_dflt is the right place - that will cover pfifo,
bfifo and pfifo_fast (instead of attach_one_default_qdisc)?

Thanks for all the suggestions.

- KK

Jarek Poplawski <jarkao2@gmail.com> wrote on 07/28/2009 04:54:45 PM:

> On Tue, Jul 28, 2009 at 06:53:29PM +0800, Herbert Xu wrote:
> > On Tue, Jul 28, 2009 at 09:02:29AM +0000, Jarek Poplawski wrote:
> > >
> > > If it's only about the name I'm OK with: (qdisc->flags &
> > > TCQ_F_DEFAULT_WITHOUT_PREJUDICE_MAYBE_NON_DEFAULT_IN_THE_FUTURE)
> > > or any other, too ;-)
> >
> > I see.  Yes a flag should work.
>
> So, I hope Krishna will try if these requested comments save any gains
> of the original patch...
>
> Thanks,
> Jarek P.


^ permalink raw reply
* Re: [RFC] [PATCH] Don't run __qdisc_run() on a stopped TX queue
From: Herbert Xu @ 2009-07-28 11:48 UTC (permalink / raw)
  To: Krishna Kumar2; +Cc: Jarek Poplawski, David Miller, netdev
In-Reply-To: <OFD1BE2D07.6BDFA87B-ON65257601.003F8F24-65257601.00406E62@in.ibm.com>

On Tue, Jul 28, 2009 at 05:13:45PM +0530, Krishna Kumar2 wrote:
> Sure, I will test and give my findings here. I guess setting the
> flag in qdisc_create_dflt is the right place - that will cover pfifo,
> bfifo and pfifo_fast (instead of attach_one_default_qdisc)?

No I think you want to put it in attach_one_default_qdisc.

Despite its name, qdisc_create_dflt is used by non-default qdiscs
too.

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
* [PATCH 1/1] mISDN: Fix handling of receive buffer size in L1oIP
From: Karsten Keil @ 2009-07-27 17:24 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, netdev, David Miller, i4ldeveloper,
	Andreas Eversberg, Arnaldo Carvalho de Melo
In-Reply-To: <cover.1248781021.git.keil@b1-systems.de>

From: Andreas Eversberg <andreas@eversberg.eu>

The size of receive buffer pointer was used to get size of
receive buffer instead of recvbuf_size itself, so only 4/8
bytes could be transfered.

This is a regression to 2.6.30 introduced by commit 8c90e11e3543d7de612194a042a148caeaab5f1d
mISDN: Use kernel_{send,recv}msg instead of open coding

Signed-off-by: Andreas Eversberg <andreas@eversberg.eu>
Signed-off-by: Karsten Keil <keil@b1-systems.de>
---
 drivers/isdn/mISDN/l1oip_core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/isdn/mISDN/l1oip_core.c b/drivers/isdn/mISDN/l1oip_core.c
index 990e6a7..c3b661a 100644
--- a/drivers/isdn/mISDN/l1oip_core.c
+++ b/drivers/isdn/mISDN/l1oip_core.c
@@ -731,10 +731,10 @@ l1oip_socket_thread(void *data)
 	while (!signal_pending(current)) {
 		struct kvec iov = {
 			.iov_base = recvbuf,
-			.iov_len = sizeof(recvbuf),
+			.iov_len = recvbuf_size,
 		};
 		recvlen = kernel_recvmsg(socket, &msg, &iov, 1,
-					 sizeof(recvbuf), 0);
+					 recvbuf_size, 0);
 		if (recvlen > 0) {
 			l1oip_socket_parse(hc, &sin_rx, recvbuf, recvlen);
 		} else {
-- 
1.6.0.2


^ permalink raw reply related
* [PATCH V2] CAN: Add Flexcan CAN controller driver
From: Sascha Hauer @ 2009-07-28 12:06 UTC (permalink / raw)
  To: Socketcan-core; +Cc: Linux Netdev List

Hi,

Here is the second version of the flexcan driver.

Sascha


>From a1ea20bc862778f2b64efc18d21d05e5debb9562 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 21 Jul 2009 10:47:19 +0200
Subject: [PATCH] CAN: Add Flexcan CAN driver

This core is found on some Freescale SoCs and also some Coldfire
SoCs. Support for Coldfire is missing though at the moment as
they have an older revision of the core which does not have RX FIFO
support.

V2: integrated comments from Wolfgang Grandegger

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/can/Kconfig   |    6 +
 drivers/net/can/Makefile  |    1 +
 drivers/net/can/flexcan.c |  692 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 699 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/flexcan.c

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 33821a8..99c6da4 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -74,6 +74,12 @@ config CAN_KVASER_PCI
 	  This driver is for the the PCIcanx and PCIcan cards (1, 2 or
 	  4 channel) from Kvaser (http://www.kvaser.com).
 
+config CAN_FLEXCAN
+	tristate "Support for Freescale FLEXCAN based chips"
+	depends on CAN_DEV
+	---help---
+	  Say Y here if you want to support for Freescale FlexCAN.
+
 config CAN_DEBUG_DEVICES
 	bool "CAN devices debugging messages"
 	depends on CAN
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 523a941..25f2032 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -8,5 +8,6 @@ obj-$(CONFIG_CAN_DEV)		+= can-dev.o
 can-dev-y			:= dev.o
 
 obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
+obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
new file mode 100644
index 0000000..8a2e820
--- /dev/null
+++ b/drivers/net/can/flexcan.c
@@ -0,0 +1,692 @@
+/*
+ * flexcan.c - FLEXCAN CAN controller driver
+ *
+ * Copyright (c) 2005-2006 Varma Electronics Oy
+ * Copyright (c) 2009 Sascha Hauer, Pengutronix
+ *
+ * Based on code originally by Andrey Volkov <avolkov@varma-el.com>
+ *
+ * LICENCE:
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/can/netlink.h>
+#include <linux/can/error.h>
+#include <linux/interrupt.h>
+#include <linux/netdevice.h>
+#include <linux/if_ether.h>
+#include <linux/can/dev.h>
+#include <linux/if_arp.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/list.h>
+#include <linux/can.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME "flexcan"
+
+/* FLEXCAN module configuration register (CANMCR) bits */
+#define CANMCR_MDIS				(1 << 31)
+#define CANMCR_FRZ				(1 << 30)
+#define CANMCR_FEN				(1 << 29)
+#define CANMCR_HALT				(1 << 28)
+#define CANMCR_SOFTRST				(1 << 25)
+#define CANMCR_FRZACK				(1 << 24)
+#define CANMCR_SUPV				(1 << 23)
+#define CANMCR_SRX_DIS				(1 << 17)
+#define CANMCR_MAXMB(x)				((x) & 0x0f)
+#define CANMCR_IDAM_A				(0 << 8)
+#define CANMCR_IDAM_B				(1 << 8)
+#define CANMCR_IDAM_C				(2 << 8)
+
+/* FLEXCAN control register (CANCTRL) bits */
+#define CANCTRL_PRESDIV(x)			(((x) & 0xff) << 24)
+#define CANCTRL_RJW(x)				(((x) & 0x03) << 22)
+#define CANCTRL_PSEG1(x)			(((x) & 0x07) << 19)
+#define CANCTRL_PSEG2(x)			(((x) & 0x07) << 16)
+#define CANCTRL_BOFFMSK				(1 << 15)
+#define CANCTRL_ERRMSK				(1 << 14)
+#define CANCTRL_CLKSRC				(1 << 13)
+#define CANCTRL_LPB				(1 << 12)
+#define CANCTRL_TWRN_MSK			(1 << 11)
+#define CANCTRL_RWRN_MSK			(1 << 10)
+#define CANCTRL_SAMP				(1 << 7)
+#define CANCTRL_BOFFREC				(1 << 6)
+#define CANCTRL_TSYNC				(1 << 5)
+#define CANCTRL_LBUF				(1 << 4)
+#define CANCTRL_LOM				(1 << 3)
+#define CANCTRL_PROPSEG(x)			((x) & 0x07)
+
+/* FLEXCAN error counter register (ERRCNT) bits */
+#define ERRCNT_REXECTR(x)			(((x) & 0xff) << 8)
+#define ERRCNT_TXECTR(x)			((x) & 0xff)
+
+/* FLEXCAN error and status register (ERRSTAT) bits */
+#define ERRSTAT_TWRNINT				(1 << 17)
+#define ERRSTAT_RWRNINT				(1 << 16)
+#define ERRSTAT_BIT1ERR				(1 << 15)
+#define ERRSTAT_BIT0ERR				(1 << 14)
+#define ERRSTAT_ACKERR				(1 << 13)
+#define ERRSTAT_CRCERR				(1 << 12)
+#define ERRSTAT_FRMERR				(1 << 11)
+#define ERRSTAT_STFERR				(1 << 10)
+#define ERRSTAT_TXWRN				(1 << 9)
+#define ERRSTAT_RXWRN				(1 << 8)
+#define ERRSTAT_IDLE 				(1 << 7)
+#define ERRSTAT_TXRX				(1 << 6)
+#define ERRSTAT_FLTCONF_MASK			(3 << 4)
+#define ERRSTAT_FLTCONF_ERROR_ACTIVE		(0 << 4)
+#define ERRSTAT_FLTCONF_ERROR_PASSIVE		(1 << 4)
+#define ERRSTAT_FLTCONF_ERROR_BUS_OFF		(2 << 4)
+#define ERRSTAT_BOFFINT				(1 << 2)
+#define ERRSTAT_ERRINT          		(1 << 1)
+#define ERRSTAT_WAKINT				(1 << 0)
+#define ERRSTAT_INT	(ERRSTAT_BOFFINT | ERRSTAT_ERRINT | ERRSTAT_TWRNINT | \
+		ERRSTAT_RWRNINT)
+
+/* FLEXCAN interrupt flag register (IFLAG) bits */
+#define IFLAG_BUF(x)				(1 << (x))
+#define IFLAG_RX_FIFO_OVERFLOW			(1 << 7)
+#define IFLAG_RX_FIFO_WARN			(1 << 6)
+#define IFLAG_RX_FIFO_AVAILABLE			(1 << 5)
+
+/* FLEXCAN message buffers */
+#define MB_CNT_CODE(x)				(((x) & 0xf) << 24)
+#define MB_CNT_SRR				(1 << 22)
+#define MB_CNT_IDE				(1 << 21)
+#define MB_CNT_RTR				(1 << 20)
+#define MB_CNT_LENGTH(x)			(((x) & 0xf) << 16)
+#define MB_CNT_TIMESTAMP(x)			((x) & 0xffff)
+
+#define MB_ID_STD				(0x7ff << 18)
+#define MB_ID_EXT				0x1fffffff
+#define MB_CODE_MASK				0xf0ffffff
+
+/* Structure of the message buffer */
+struct flexcan_mb {
+	u32	can_dlc;
+	u32	can_id;
+	u32	data[2];
+};
+
+/* Structure of the hardware registers */
+struct flexcan_regs {
+	u32	canmcr;		/* 0x00 */
+	u32	canctrl;	/* 0x04 */
+	u32	timer;		/* 0x08 */
+	u32	reserved1;	/* 0x0c */
+	u32 	rxgmask;	/* 0x10 */
+	u32 	rx14mask;	/* 0x14 */
+	u32 	rx15mask;	/* 0x18 */
+	u32	errcnt;		/* 0x1c */
+	u32 	errstat;	/* 0x20 */
+	u32	imask2;		/* 0x24 */
+	u32	imask1;		/* 0x28 */
+	u32	iflag2;		/* 0x2c */
+	u32 	iflag1;		/* 0x30 */
+	u32	reserved4[19];
+	struct	flexcan_mb cantxfg[64];
+};
+
+struct flexcan_priv {
+	struct can_priv can;
+	void __iomem *base;
+
+	struct net_device *dev;
+	struct clk *clk;
+};
+
+static struct can_bittiming_const flexcan_bittiming_const = {
+	.name = DRIVER_NAME,
+	.tseg1_min = 1,
+	.tseg1_max = 16,
+	.tseg2_min = 2,
+	.tseg2_max = 8,
+	.sjw_max = 4,
+	.brp_min = 1,
+	.brp_max = 256,
+	.brp_inc = 1,
+};
+
+#define TX_BUF_ID	8
+
+static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct can_frame *frame = (struct can_frame *)skb->data;
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 can_id;
+	u32 dlc = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
+	u32 *can_data;
+
+	netif_stop_queue(dev);
+
+	if (frame->can_id & CAN_EFF_FLAG) {
+		can_id = frame->can_id & MB_ID_EXT;
+		dlc |= MB_CNT_IDE | MB_CNT_SRR;
+	} else {
+		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
+	}
+
+	if (frame->can_id & CAN_RTR_FLAG)
+		dlc |= MB_CNT_RTR;
+
+	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
+	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
+
+	can_data = (u32 *)frame->data;
+	writel(cpu_to_be32(*can_data), &regs->cantxfg[TX_BUF_ID].data[0]);
+	writel(cpu_to_be32(*(can_data + 1)), &regs->cantxfg[TX_BUF_ID].data[1]);
+
+	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
+
+	kfree_skb(skb);
+
+	return NETDEV_TX_OK;
+}
+
+static void flexcan_rx_frame(struct net_device *ndev,
+		struct flexcan_mb __iomem *mb)
+{
+	struct net_device_stats *stats = &ndev->stats;
+	struct sk_buff *skb;
+	struct can_frame *frame;
+	int ctrl = readl(&mb->can_dlc);
+	int length = (ctrl >> 16) & 0x0f;
+	u32 id;
+
+	skb = dev_alloc_skb(sizeof(struct can_frame));
+	if (!skb) {
+		stats->rx_dropped++;
+		return;
+	}
+
+	frame = (struct can_frame *)skb_put(skb,
+			sizeof(struct can_frame));
+
+	frame->can_dlc = length;
+	id = readl(&mb->can_id) & MB_ID_EXT;
+
+	if (ctrl & MB_CNT_IDE) {
+		frame->can_id = id & CAN_EFF_MASK;
+		frame->can_id |= CAN_EFF_FLAG;
+		if (ctrl & MB_CNT_RTR)
+			frame->can_id |= CAN_RTR_FLAG;
+	} else {
+		frame->can_id  = id >> 18;
+		if (ctrl & MB_CNT_RTR)
+			frame->can_id |= CAN_RTR_FLAG;
+	}
+
+	*(u32 *)frame->data = be32_to_cpu(readl(&mb->data[0]));
+	*((u32 *)frame->data + 1) = be32_to_cpu(readl(&mb->data[1]));
+
+	stats->rx_packets++;
+	stats->rx_bytes += frame->can_dlc;
+	skb->dev = ndev;
+	skb->protocol = __constant_htons(ETH_P_CAN);
+	skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	netif_rx(skb);
+}
+
+static void flexcan_error(struct net_device *ndev, u32 stat)
+{
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct net_device_stats *stats = &ndev->stats;
+	enum can_state state = priv->can.state;
+	int error_warning = 0, rx_errors = 0, tx_errors = 0;
+
+	skb = dev_alloc_skb(sizeof(struct can_frame));
+	if (!skb)
+		return;
+
+	skb->dev = ndev;
+	skb->protocol = __constant_htons(ETH_P_CAN);
+	skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	cf = (struct can_frame *)skb_put(skb, sizeof(*cf));
+	memset(cf, 0, sizeof(*cf));
+
+	cf->can_id = CAN_ERR_FLAG;
+	cf->can_dlc = CAN_ERR_DLC;
+
+	if (stat & ERRSTAT_RWRNINT) {
+		error_warning = 1;
+		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
+	}
+
+	if (stat & ERRSTAT_TWRNINT) {
+		error_warning = 1;
+		cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
+	}
+
+	switch ((stat >> 4) & 0x3) {
+	case 0:
+		state = CAN_STATE_ERROR_ACTIVE;
+		break;
+	case 1:
+		state = CAN_STATE_ERROR_PASSIVE;
+		break;
+	default:
+		state = CAN_STATE_BUS_OFF;
+		break;
+	}
+
+	if (stat & ERRSTAT_BOFFINT) {
+		cf->can_id |= CAN_ERR_BUSOFF;
+		can_bus_off(ndev);
+	}
+
+	if (stat & ERRSTAT_BIT1ERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_BIT1;
+	}
+
+	if (stat & ERRSTAT_BIT0ERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_BIT0;
+	}
+
+	if (stat & ERRSTAT_FRMERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_FORM;
+	}
+
+	if (stat & ERRSTAT_STFERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_STUFF;
+	}
+
+
+	if (stat & ERRSTAT_ACKERR) {
+		tx_errors = 1;
+		cf->can_id |= CAN_ERR_ACK;
+	}
+
+	if (error_warning)
+		priv->can.can_stats.error_warning++;
+	if (rx_errors)
+		stats->rx_errors++;
+	if (tx_errors)
+		stats->tx_errors++;
+
+	priv->can.state = state;
+
+	netif_rx(skb);
+
+	ndev->last_rx = jiffies;
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+}
+
+static irqreturn_t flexcan_isr(int irq, void *dev_id)
+{
+	struct net_device *ndev = dev_id;
+	struct net_device_stats *stats = &ndev->stats;
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 iflags, errstat;
+
+	errstat = readl(&regs->errstat);
+	if (errstat & ERRSTAT_INT) {
+		flexcan_error(ndev, errstat);
+		writel(errstat & ERRSTAT_INT, &regs->errstat);
+	}
+
+	iflags = readl(&regs->iflag1);
+
+	if (iflags & IFLAG_RX_FIFO_OVERFLOW) {
+		writel(IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
+		stats->rx_over_errors++;
+		stats->rx_errors++;
+	}
+
+	while (iflags & IFLAG_RX_FIFO_AVAILABLE) {
+		struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
+
+		flexcan_rx_frame(ndev, mb);
+		writel(IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+		readl(&regs->timer);
+		iflags = readl(&regs->iflag1);
+	}
+
+	if (iflags & (1 << TX_BUF_ID)) {
+		stats->tx_packets++;
+		writel((1 << TX_BUF_ID), &regs->iflag1);
+		netif_wake_queue(ndev);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int flexcan_set_bittiming(struct net_device *ndev)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct can_bittiming *bt = &priv->can.bittiming;
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->canctrl);
+	reg &= ~(CANCTRL_SAMP | CANCTRL_PRESDIV(0xff) |
+			CANCTRL_PSEG1(7) | CANCTRL_PSEG2(7));
+	reg |= CANCTRL_PRESDIV(bt->brp - 1) |
+		CANCTRL_PSEG1(bt->phase_seg1 - 1) |
+		CANCTRL_PSEG2(bt->phase_seg2 - 1) |
+		CANCTRL_RJW(3) |
+		CANCTRL_PROPSEG(bt->prop_seg - 1);
+	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+		reg |= CANCTRL_SAMP;
+	writel(reg, &regs->canctrl);
+
+	dev_dbg(&ndev->dev, "setting canctrl=0x%08x\n", reg);
+
+	return 0;
+}
+
+static int flexcan_open(struct net_device *ndev)
+{
+	int ret, i;
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	ret = open_candev(ndev);
+	if (ret)
+		return ret;
+
+	ret = request_irq(ndev->irq, flexcan_isr, 0, DRIVER_NAME, ndev);
+	if (ret)
+		goto err_irq;
+
+	clk_enable(priv->clk);
+
+	reg = CANMCR_SOFTRST | CANMCR_HALT;
+	writel(CANMCR_SOFTRST, &regs->canmcr);
+	udelay(10);
+
+	if (readl(&regs->canmcr) & CANMCR_SOFTRST) {
+		dev_err(&ndev->dev, "Failed to softreset can module.\n");
+		ret = -ENODEV;
+		goto err_reset;
+	}
+
+	/* Enable error and bus off interrupt */
+	reg = readl(&regs->canctrl);
+	reg |= CANCTRL_CLKSRC | CANCTRL_ERRMSK | CANCTRL_BOFFMSK |
+		CANCTRL_BOFFREC | CANCTRL_TWRN_MSK | CANCTRL_TWRN_MSK;
+	writel(reg, &regs->canctrl);
+
+	/* Set lowest buffer transmitted first */
+	reg |= CANCTRL_LBUF;
+	writel(reg, &regs->canctrl);
+
+	for (i = 0; i < 64; i++) {
+		writel(0, &regs->cantxfg[i].can_dlc);
+		writel(0, &regs->cantxfg[i].can_id);
+		writel(0, &regs->cantxfg[i].data[0]);
+		writel(0, &regs->cantxfg[i].data[1]);
+
+		/* Put MB into rx queue */
+		writel(MB_CNT_CODE(0x04), &regs->cantxfg[i].can_dlc);
+	}
+
+	/* acceptance mask/acceptance code (accept everything) */
+	writel(0x0, &regs->rxgmask);
+	writel(0x0, &regs->rx14mask);
+	writel(0x0, &regs->rx15mask);
+
+	/* Enable flexcan module */
+	reg = readl(&regs->canmcr);
+	reg &= ~(CANMCR_MDIS | CANMCR_FRZ);
+	reg |= CANMCR_IDAM_C | CANMCR_FEN;
+	writel(reg, &regs->canmcr);
+
+	/* Synchronize with the can bus */
+	reg &= ~CANMCR_HALT;
+	writel(reg, &regs->canmcr);
+
+	/* Enable interrupts */
+	writel(IFLAG_RX_FIFO_OVERFLOW | IFLAG_RX_FIFO_AVAILABLE |
+			IFLAG_BUF(TX_BUF_ID),
+			&regs->imask1);
+
+	netif_start_queue(ndev);
+
+	return 0;
+
+err_reset:
+	free_irq(ndev->irq, ndev);
+err_irq:
+	close_candev(ndev);
+
+	return ret;
+}
+
+static int flexcan_close(struct net_device *ndev)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+
+	netif_stop_queue(ndev);
+
+	/* Disable all interrupts */
+	writel(0, &regs->imask1);
+	free_irq(ndev->irq, ndev);
+
+	close_candev(ndev);
+
+	/* Disable module */
+	writel(CANMCR_MDIS, &regs->canmcr);
+	clk_disable(priv->clk);
+	return 0;
+}
+
+static int flexcan_set_mode(struct net_device *ndev, enum can_mode mode)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	switch (mode) {
+	case CAN_MODE_START:
+		reg = readl(&regs->canctrl);
+		reg &= ~CANCTRL_BOFFREC;
+		writel(reg, &regs->canctrl);
+		reg |= CANCTRL_BOFFREC;
+		writel(reg, &regs->canctrl);
+		priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+		if (netif_queue_stopped(ndev))
+			netif_wake_queue(ndev);
+		break;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static const struct net_device_ops flexcan_netdev_ops = {
+       .ndo_open		= flexcan_open,
+       .ndo_stop		= flexcan_close,
+       .ndo_start_xmit		= flexcan_start_xmit,
+};
+
+static int register_flexcandev(struct net_device *ndev)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->canmcr);
+	reg &= ~CANMCR_MDIS;
+	writel(reg, &regs->canmcr);
+	udelay(100);
+	reg |= CANMCR_FRZ | CANMCR_HALT | CANMCR_FEN;
+	writel(reg, &regs->canmcr);
+
+	reg = readl(&regs->canmcr);
+
+	/* Currently we only support newer versions of this core featuring
+	 * a RX FIFO. Older cores found on some Coldfire derivates are not
+	 * yet supported.
+	 */
+	if (!(reg & CANMCR_FEN)) {
+		dev_err(&ndev->dev, "Could not enable RX FIFO, unsupported "
+				"core");
+		return -ENODEV;
+	}
+
+	ndev->flags |= IFF_ECHO; /* we support local echo in hardware */
+	ndev->netdev_ops = &flexcan_netdev_ops;
+
+	return register_candev(ndev);
+}
+
+static void unregister_flexcandev(struct net_device *ndev)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->canmcr);
+	reg |= CANMCR_FRZ | CANMCR_HALT | CANMCR_MDIS;
+	writel(reg, &regs->canmcr);
+
+	unregister_candev(ndev);
+}
+
+static int __devinit flexcan_probe(struct platform_device *pdev)
+{
+	struct resource *mem;
+	struct net_device *ndev;
+	struct flexcan_priv *priv;
+	u32 mem_size;
+	int ret;
+
+	ndev = alloc_candev(sizeof(struct flexcan_priv));
+	if (!ndev)
+		return -ENOMEM;
+
+	priv = netdev_priv(ndev);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	ndev->irq = platform_get_irq(pdev, 0);
+	if (!mem || !ndev->irq) {
+		ret = -ENODEV;
+		goto failed_req;
+	}
+
+	mem_size = resource_size(mem);
+
+	if (!request_mem_region(mem->start, mem_size, DRIVER_NAME)) {
+		ret = -EBUSY;
+		goto failed_req;
+	}
+
+	SET_NETDEV_DEV(ndev, &pdev->dev);
+
+	priv->base = ioremap(mem->start, mem_size);
+	if (!priv->base) {
+		ret = -ENOMEM;
+		goto failed_map;
+	}
+
+	priv->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		ret = PTR_ERR(priv->clk);
+		goto failed_clock;
+	}
+	priv->can.clock.freq = clk_get_rate(priv->clk);
+
+	platform_set_drvdata(pdev, ndev);
+
+	priv->can.do_set_bittiming = flexcan_set_bittiming;
+	priv->can.bittiming_const = &flexcan_bittiming_const;
+	priv->can.do_set_mode = flexcan_set_mode;
+
+	ret = register_flexcandev(ndev);
+	if (ret)
+		goto failed_register;
+
+	return 0;
+
+failed_register:
+	clk_put(priv->clk);
+failed_clock:
+	iounmap(priv->base);
+failed_map:
+	release_mem_region(mem->start, mem_size);
+failed_req:
+	free_candev(ndev);
+
+	return ret;
+}
+
+static int __devexit flexcan_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct resource *mem;
+
+	unregister_flexcandev(ndev);
+	platform_set_drvdata(pdev, NULL);
+	iounmap(priv->base);
+	clk_put(priv->clk);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(mem->start, resource_size(mem));
+	free_candev(ndev);
+
+	return 0;
+}
+
+static struct platform_driver flexcan_driver = {
+	.driver = {
+		   .name = DRIVER_NAME,
+		   },
+	.probe = flexcan_probe,
+	.remove = __devexit_p(flexcan_remove),
+};
+
+static int __init flexcan_init(void)
+{
+	return platform_driver_register(&flexcan_driver);
+}
+
+static void __exit flexcan_exit(void)
+{
+	platform_driver_unregister(&flexcan_driver);
+}
+
+module_init(flexcan_init);
+module_exit(flexcan_exit);
+
+MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("CAN port driver for flexcan based chip");
-- 
1.6.3.3

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply related
* Re: [RFC] [PATCH] Don't run __qdisc_run() on a stopped TX queue
From: Krishna Kumar2 @ 2009-07-28 12:24 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David Miller, Jarek Poplawski, netdev
In-Reply-To: <20090728114803.GA28001@gondor.apana.org.au>

Hi Herbert,

Herbert Xu <herbert@gondor.apana.org.au> wrote on 07/28/2009 05:18:03 PM:
>
> Re: [RFC] [PATCH] Don't run __qdisc_run() on a stopped TX queue
>
> On Tue, Jul 28, 2009 at 05:13:45PM +0530, Krishna Kumar2 wrote:
> > Sure, I will test and give my findings here. I guess setting the
> > flag in qdisc_create_dflt is the right place - that will cover pfifo,
> > bfifo and pfifo_fast (instead of attach_one_default_qdisc)?
>
> No I think you want to put it in attach_one_default_qdisc.
>
> Despite its name, qdisc_create_dflt is used by non-default qdiscs
> too.

Right, thanks for pointing this out.

BTW, I am testing another patch that does "Do not enqueue skb for
default qdisc if there are no skbs already on the queue" (to avoid
enqueue followed by immediate dequeue of the same skb for most xmit
operations on default qdiscs). I am planning to use the same flag
for both. I hope there is nothing wrong with that idea (excuse me
if this has already been discussed on the list but I haven't been
following for almost a year).

Thanks,

- KK


^ permalink raw reply
* Re: [Bugme-new] [Bug 13760] New: 2.6.30 kernel locks up with pppoe in  back trace (regression)
From: Eric Dumazet @ 2009-07-28 12:30 UTC (permalink / raw)
  To: Pavel Emelyanov
  Cc: Igor M Podlesny, Andrew Morton, bugzilla-daemon, bugme-daemon,
	netdev, Paul E. McKenney, David S. Miller
In-Reply-To: <4A6ECA3A.4050309@openvz.org>

Pavel Emelyanov a écrit :
> Eric Dumazet wrote:
>> Igor M Podlesny a écrit :
>>> [...]
>>>> Could have been a problem in net core, perhaps.
>>>>
>>>> Below is a ppp fix from 2.6.31, but it seems unlikely to fix your problem.
>>>>
>>>> It would help if we could see that trace, please.  A digital photo
>>>> would suit.
>>> 	Here it is:
>>>
>>> 		http://bugzilla.kernel.org/attachment.cgi?id=22516
>>>
>>> 	(It's 2.6.30.3)
>>> 	
>> Looking at this, I believe net_assign_generic() is not safe.
>>
>> Two cpus could try to expand/update the array at same time, one update could be lost.
>>
>> register_pernet_gen_device() has a mutex to guard against concurrent
>> calls, but net_assign_generic() has no locking at all.
>>
>> I doubt this is the reason of the crash, still worth to mention it...
>>
>> [PATCH] net: net_assign_generic() is not SMP safe
>>
>> Two cpus could try to expand/update the array at same time, one update
>> could be lost during the copy of old array.
> 
> How can this happen? The array is updated only during ->init routines
> of the pernet_operations, which are called from under the net_mutex.
> 
> Do I miss anything?
> 

Oops, I missed the obvious "BUG_ON(!mutex_is_locked(&net_mutex));"

Sorry for the noise and untested patch as well :)

>> Re-using net_mutex is an easy way to fix this, it was used right
>> before to allocate the 'id'
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
>>
>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>> index b7292a2..9c31ad1 100644
>> --- a/net/core/net_namespace.c
>> +++ b/net/core/net_namespace.c
>> @@ -467,15 +467,17 @@ int net_assign_generic(struct net *net, int id, void *data)
>>  	BUG_ON(!mutex_is_locked(&net_mutex));
>>  	BUG_ON(id == 0);
>>  
>> +	mutex_lock(&net_mutex);

^ permalink raw reply
* Re: [RFC][PATCH 5/5] libxt_ipvs: user space lib for netfilter matcher xt_ipvs
From: Hannes Eder @ 2009-07-28 12:34 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: lvs-devel, netdev, netfilter-devel, linux-kernel
In-Reply-To: <alpine.LSU.2.00.0907272037500.20984@fbirervta.pbzchgretzou.qr>

On Mon, Jul 27, 2009 at 20:40, Jan Engelhardt<jengelh@medozas.de> wrote:
>
> On Monday 2009-07-27 15:48, Hannes Eder wrote:
>>+
>>+      switch (c) {
>>+      case '0': /* --ipvs */
>>+              /* Nothing to do here. */
>
>                Then why add it?

In the 'default' branch is an assert(false);  Call it defensive programming.

>>+      char buf[BUFSIZ];
>>+
>>+      if (family == NFPROTO_IPV4) {
>>+              if (!numeric && addr->ip == 0) {
>>+                      printf("anywhere ");
>>+                      return;
>>+              }
>>+              if (numeric)
>>+                      strcpy(buf, xtables_ipaddr_to_numeric(&addr->in));
>>+              else
>>+                      strcpy(buf, xtables_ipaddr_to_anyname(&addr->in));
>>+              strcat(buf, xtables_ipmask_to_numeric(&mask->in));
>>+              printf("%s ", buf);
>
> There is no need to use the strcpy/strcat hacks. Just directly printf it.

As the comment says: "Shamelessly copied from libxt_conntrack.c". ;)

Furthermore I think it is good that way, because
xtables_ipaddr_to_numeric writes to a local static buffer, and
xtables_ipaddr_to_numeric might get called by
xtables_ipmask_to_numeric.

>>--- /dev/null
>>+++ b/extensions/libxt_ipvs.man
>>@@ -0,0 +1,7 @@
>>+ipvs tests where the packet was modified by IPVS, i.e. is the
>>+skb_buff->ipvs_property set.
>>+.TP
>>+[\fB!\fP] \fB--ipvs
>>+Does the packet have to IPVS property?
>>+
>>+TODO: Write proper documentation.
>
> Yes.

Sir, yes, sir ;) I am working on that.

Thanks,
-Hannes
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply
* [PATCH] net: net_assign_generic()  fix
From: Eric Dumazet @ 2009-07-28 12:36 UTC (permalink / raw)
  To: Pavel Emelyanov
  Cc: Igor M Podlesny, Andrew Morton, bugzilla-daemon, bugme-daemon,
	netdev, Paul E. McKenney, David S. Miller
In-Reply-To: <4A6EEF69.1050001@cosmosbay.com>

Eric Dumazet a écrit :
> Pavel Emelyanov a écrit :
>> Eric Dumazet wrote:
>>> Igor M Podlesny a écrit :
>>>> [...]
>>>>> Could have been a problem in net core, perhaps.
>>>>>
>>>>> Below is a ppp fix from 2.6.31, but it seems unlikely to fix your problem.
>>>>>
>>>>> It would help if we could see that trace, please.  A digital photo
>>>>> would suit.
>>>> 	Here it is:
>>>>
>>>> 		http://bugzilla.kernel.org/attachment.cgi?id=22516
>>>>
>>>> 	(It's 2.6.30.3)
>>>> 	
>>> Looking at this, I believe net_assign_generic() is not safe.
>>>
>>> Two cpus could try to expand/update the array at same time, one update could be lost.
>>>
>>> register_pernet_gen_device() has a mutex to guard against concurrent
>>> calls, but net_assign_generic() has no locking at all.
>>>
>>> I doubt this is the reason of the crash, still worth to mention it...
>>>
>>> [PATCH] net: net_assign_generic() is not SMP safe
>>>
>>> Two cpus could try to expand/update the array at same time, one update
>>> could be lost during the copy of old array.
>> How can this happen? The array is updated only during ->init routines
>> of the pernet_operations, which are called from under the net_mutex.
>>
>> Do I miss anything?
>>
> 
> Oops, I missed the obvious "BUG_ON(!mutex_is_locked(&net_mutex));"
> 
> Sorry for the noise and untested patch as well :)
>

Hmm...

Real bug may be fixed by followed patch ? (yet untested, sorry...)

[PATCH] net: net_assign_generic() fix 

memcpy() should take into account size of pointers,
not only number of pointers to copy.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index b7292a2..1972830 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -488,7 +488,7 @@ int net_assign_generic(struct net *net, int id, void *data)
 	 */
 
 	ng->len = id;
-	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len);
+	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len * sizeof(void*));
 
 	rcu_assign_pointer(net->gen, ng);
 	call_rcu(&old_ng->rcu, net_generic_release);

^ permalink raw reply related
* [PATCH] smsc95xx: remove EEPROM loaded check
From: Steve Glendinning @ 2009-07-28 12:37 UTC (permalink / raw)
  To: netdev; +Cc: Ian Saturley, David Miller, Steve Glendinning

The eeprom read & write commands currently check the E2P_CMD_LOADED_ bit is
set before allowing any operations.  This prevents any reading or writing
unless a correctly programmed EEPROM is installed.

This patch removes the check, so it is possible to program blank EEPROMS
via ethtool.

Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
---
 drivers/net/usb/smsc95xx.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index fe04589..09bd635 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -220,11 +220,6 @@ static int smsc95xx_eeprom_confirm_not_busy(struct usbnet *dev)
 	do {
 		smsc95xx_read_reg(dev, E2P_CMD, &val);
 
-		if (!(val & E2P_CMD_LOADED_)) {
-			devwarn(dev, "No EEPROM present");
-			return -EIO;
-		}
-
 		if (!(val & E2P_CMD_BUSY_))
 			return 0;
 
-- 
1.6.2.5


^ permalink raw reply related
* Re: [Bug #13838] kernel BUG at include/net/netns/generic.h:41!
From: Eric Dumazet @ 2009-07-28 12:46 UTC (permalink / raw)
  To: Luca Tettamanti
  Cc: Américo Wang, Rafael J. Wysocki, Linux Kernel Mailing List,
	Kernel Testers List, netdev, herbert, Alexey Dobriyan
In-Reply-To: <68676e00907280130tbbf5672x83182641b0c977b3@mail.gmail.com>

Luca Tettamanti a écrit :
> On Mon, Jul 27, 2009 at 7:29 AM, Américo Wang<xiyou.wangcong@gmail.com> wrote:
>> On Mon, Jul 27, 2009 at 4:28 AM, Rafael J. Wysocki<rjw@sisk.pl> wrote:
>>> This message has been generated automatically as a part of a report
>>> of recent regressions.
>>>
>>> The following bug entry is on the current list of known regressions
>>> from 2.6.30.  Please verify if it still should be listed and let me know
>>> (either way).
>>>
>>>
>>> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=13838
>>> Subject         : kernel BUG at include/net/netns/generic.h:41!
>>> Submitter       : Luca Tettamanti <kronos.it@gmail.com>
>>> Date            : 2009-07-20 15:27 (7 days old)
>>> References      : http://lkml.org/lkml/2009/7/20/105
>> Hmm, it looks like that 'pfkey_net_id' is still zero after ipsec_pfkey_init()...
>>
>> Add Herbert and net-dev into Cc.
>>
>> Luca, would you mind to provide your .config and the steps to reproduce this?
> 
> I cannot reproduce it anymore with the current git kernel. The BUG was
> triggered by racoon at startup.
> Should I go back to an older kernel to investigate or can we consider
> it "fixed"?
> 

This should be fixed by following patch (submitted for bug 13760, but should apply)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index b7292a2..1972830 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -488,7 +488,7 @@ int net_assign_generic(struct net *net, int id, void *data)
 	 */
 
 	ng->len = id;
-	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len);
+	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len * sizeof(void *));
 
 	rcu_assign_pointer(net->gen, ng);
 	call_rcu(&old_ng->rcu, net_generic_release);

^ permalink raw reply related
* Re: [PATCH] net: net_assign_generic()  fix
From: Pavel Emelyanov @ 2009-07-28 13:03 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Igor M Podlesny, Andrew Morton, bugzilla-daemon, bugme-daemon,
	netdev, Paul E. McKenney, David S. Miller
In-Reply-To: <4A6EF0BF.2050801@gmail.com>

> Hmm...
> 
> Real bug may be fixed by followed patch ? (yet untested, sorry...)
> 
> [PATCH] net: net_assign_generic() fix 
> 
> memcpy() should take into account size of pointers,
> not only number of pointers to copy.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Acked-by: Pavel Emelyanov <xemul@openvz.org>

> ---
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index b7292a2..1972830 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -488,7 +488,7 @@ int net_assign_generic(struct net *net, int id, void *data)
>  	 */
>  
>  	ng->len = id;
> -	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len);
> +	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len * sizeof(void*));
>  
>  	rcu_assign_pointer(net->gen, ng);
>  	call_rcu(&old_ng->rcu, net_generic_release);
> 
> 


^ permalink raw reply
* Re: [PATCH] net: net_assign_generic()  fix
From: Eric Dumazet @ 2009-07-28 13:16 UTC (permalink / raw)
  To: Pavel Emelyanov
  Cc: Igor M Podlesny, Andrew Morton, bugzilla-daemon, bugme-daemon,
	netdev, Paul E. McKenney, David S. Miller
In-Reply-To: <4A6EF705.6070403@openvz.org>

Pavel Emelyanov a écrit :
>> Hmm...
>>
>> Real bug may be fixed by followed patch ? (yet untested, sorry...)
>>
>> [PATCH] net: net_assign_generic() fix 
>>
>> memcpy() should take into account size of pointers,
>> not only number of pointers to copy.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> Acked-by: Pavel Emelyanov <xemul@openvz.org>

Thanks.

Still this doesnt explain the crash, because initial number of pointers is 13
(INITIAL_NET_GEN_PTRS)

We probably never realloc this array, unless a module forgets to
unregister_pernet_gen_device() and we load/unload it many times ?


^ permalink raw reply
* Re: [PATCH V2] CAN: Add Flexcan CAN controller driver
From: Oliver Hartkopp @ 2009-07-28 13:21 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Socketcan-core, Linux Netdev List
In-Reply-To: <20090728120624.GS2714@pengutronix.de>

Sascha Hauer wrote:
> Hi,
> 
> Here is the second version of the flexcan driver.

Hi Sascha,

some more points i forgot to mention, sorry ...


> +/* Structure of the message buffer */
> +struct flexcan_mb {
> +	u32	can_dlc;
> +	u32	can_id;
> +	u32	data[2];
> +};

This looks really hackish and does not reflect the structure of a flexcan
message buffer! The data is 'u8' and the name of 'dlc' for the
description/flag register is bad.


> +
> +static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct can_frame *frame = (struct can_frame *)skb->data;
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;

> +	u32 can_id;
> +	u32 dlc = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);

Naming this variable 'dlc' does not hit the point. See below.

> +	u32 *can_data;

Really this needs to be fixed up by defining a proper mailbox struct.


> +
> +	netif_stop_queue(dev);
> +
> +	if (frame->can_id & CAN_EFF_FLAG) {
> +		can_id = frame->can_id & MB_ID_EXT;

Please use CAN_EFF_MASK here.


> +		dlc |= MB_CNT_IDE | MB_CNT_SRR;
> +	} else {
> +		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
> +	}

Just nitpicking for Kernel coding style:
remove the last '{' and '}' pair.

> +
> +	if (frame->can_id & CAN_RTR_FLAG)
> +		dlc |= MB_CNT_RTR;
> +
> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
> +
> +	can_data = (u32 *)frame->data;
> +	writel(cpu_to_be32(*can_data), &regs->cantxfg[TX_BUF_ID].data[0]);
> +	writel(cpu_to_be32(*(can_data + 1)), &regs->cantxfg[TX_BUF_ID].data[1]);

IMHO it is not really transparent, that this is a correct handling to copy the
  can_frame.data[] on all architectures. I bet creating a for-statement
regarding the dlc is not slower and makes really clear, what's going on here.

> +
> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> +
> +	kfree_skb(skb);
> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static void flexcan_rx_frame(struct net_device *ndev,
> +		struct flexcan_mb __iomem *mb)
> +{
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct sk_buff *skb;
> +	struct can_frame *frame;
> +	int ctrl = readl(&mb->can_dlc);

'ctrl' is much better than 'dlc' naming above :-)

> +	int length = (ctrl >> 16) & 0x0f;

is probably not enough ... use %9 or an additional check.

> +	u32 id;
> +
> +	skb = dev_alloc_skb(sizeof(struct can_frame));
> +	if (!skb) {
> +		stats->rx_dropped++;
> +		return;
> +	}
> +
> +	frame = (struct can_frame *)skb_put(skb,
> +			sizeof(struct can_frame));
> +
> +	frame->can_dlc = length;
> +	id = readl(&mb->can_id) & MB_ID_EXT;

like above use CAN_EFF_MASK

or at least rename MB_ID_EXT to MB_ID_EXT_MASK

> +
> +	if (ctrl & MB_CNT_IDE) {
> +		frame->can_id = id & CAN_EFF_MASK;
> +		frame->can_id |= CAN_EFF_FLAG;
> +		if (ctrl & MB_CNT_RTR)
> +			frame->can_id |= CAN_RTR_FLAG;
> +	} else {
> +		frame->can_id  = id >> 18;
> +		if (ctrl & MB_CNT_RTR)
> +			frame->can_id |= CAN_RTR_FLAG;
> +	}
> +
> +	*(u32 *)frame->data = be32_to_cpu(readl(&mb->data[0]));
> +	*((u32 *)frame->data + 1) = be32_to_cpu(readl(&mb->data[1]));

Same as above.

Please write readable an maintainable code and let the compiler do his job.

Thanks,
Oliver

^ permalink raw reply
* Re: [PATCH] net: net_assign_generic()  fix
From: Eric Dumazet @ 2009-07-28 13:22 UTC (permalink / raw)
  To: Pavel Emelyanov
  Cc: Igor M Podlesny, Andrew Morton, netdev, Paul E. McKenney,
	David S. Miller
In-Reply-To: <4A6EFA35.3060309@gmail.com>

Eric Dumazet a écrit :
> Pavel Emelyanov a écrit :
>>> Hmm...
>>>
>>> Real bug may be fixed by followed patch ? (yet untested, sorry...)
>>>
>>> [PATCH] net: net_assign_generic() fix 
>>>
>>> memcpy() should take into account size of pointers,
>>> not only number of pointers to copy.
>>>
>>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> Acked-by: Pavel Emelyanov <xemul@openvz.org>
> 
> Thanks.
> 
> Still this doesnt explain the crash, because initial number of pointers is 13
> (INITIAL_NET_GEN_PTRS)
> 
> We probably never realloc this array, unless a module forgets to
> unregister_pernet_gen_device() and we load/unload it many times ?
> 

Seems drivers/net/pppol2tp.c is a suspect...

It uses register_pernet_gen_device() from pppol2tp_init()
but doesnt call unregister_pernet_gen_device()

^ permalink raw reply
* [PATCH] ppp: fix lost fragments in ppp_mp_explode()
From: Ben McKeegan @ 2009-07-28 13:35 UTC (permalink / raw)
  To: davem, paulus; +Cc: gabriele.paoloni, netdev, linux-ppp


This patch fixes the corner cases where the sum of MTU of the free 
channels (adjusted for fragmentation overheads) is less than the MTU of 
PPP link.  There are at least 3 situations where this case might arise:

- some of the channels are busy
- the multilink session is running in a degraded state (i.e. with less 
than its full complement of active channels)
- by design, where multilink protocol is being used to artificially 
increase the effective link MTU of a single link.

Without this patch, at most 1 fragment is ever sent per free channel for 
a given PPP frame and any remaining part of the PPP frame that does not 
fit into those fragments is silently discarded.

This patch restores the original behaviour which was broken by commit 
9c705260feea6ae329bc6b6d5f6d2ef0227eda0a 'ppp:ppp_mp_explode() 
redesign'.  Once all 'free' channels have been given a fragment, an 
additional fragment is queued to each available channel in turn, as many 
times as necessary, until the entire PPP frame has been consumed.

Signed-off-by: Ben McKeegan <ben@netservers.co.uk>
---
diff -uprN linux-2.6.31-rc4/drivers/net/ppp_generic.c 
linux-2.6.31-rc4-multilink-fix/drivers/net/ppp_generic.c
--- linux-2.6.31-rc4/drivers/net/ppp_generic.c	2009-07-23 
03:32:59.000000000 +0100
+++ linux-2.6.31-rc4-multilink-fix/drivers/net/ppp_generic.c	2009-07-28 
13:54:49.000000000 +0100
@@ -1384,7 +1384,7 @@ static int ppp_mp_explode(struct ppp *pp

  	/* create a	fragment for each channel */
  	bits = B;
-	while (nfree > 0 &&	len	> 0) {
+	while (len	> 0) {
  		list = list->next;
  		if (list ==	&ppp->channels)	{
  			i =	0;
@@ -1431,29 +1431,31 @@ static int ppp_mp_explode(struct ppp *pp
  		*otherwise divide it according to the speed
  		*of the channel we are going to transmit on
  		*/
-		if (pch->speed == 0) {
-			flen = totlen/nfree	;
-			if (nbigger > 0) {
-				flen++;
-				nbigger--;
-			}
-		} else {
-			flen = (((totfree - nzero)*(totlen + hdrlen*totfree)) /
-				((totspeed*totfree)/pch->speed)) - hdrlen;
-			if (nbigger > 0) {
-				flen += ((totfree - nzero)*pch->speed)/totspeed;
-				nbigger -= ((totfree - nzero)*pch->speed)/
+		if (nfree > 0) {
+			if (pch->speed == 0) {
+				flen = totlen/nfree	;
+				if (nbigger > 0) {
+					flen++;
+					nbigger--;
+				}
+			} else {
+				flen = (((totfree - nzero)*(totlen + hdrlen*totfree)) /
+					((totspeed*totfree)/pch->speed)) - hdrlen;
+				if (nbigger > 0) {
+					flen += ((totfree - nzero)*pch->speed)/totspeed;
+					nbigger -= ((totfree - nzero)*pch->speed)/
  							totspeed;
+				}
  			}
+			nfree--;
  		}
-		nfree--;

  		/*
  		 *check	if we are on the last channel or
  		 *we exceded the lenght	of the data	to
  		 *fragment
  		 */
-		if ((nfree == 0) || (flen > len))
+		if ((nfree <= 0) || (flen > len))
  			flen = len;
  		/*
  		 *it is not worth to tx on slow channels:
@@ -1467,7 +1469,7 @@ static int ppp_mp_explode(struct ppp *pp
  			continue;
  		}

-		mtu	= pch->chan->mtu + 2 - hdrlen;
+		mtu	= pch->chan->mtu - hdrlen;
  		if (mtu	< 4)
  			mtu	= 4;
  		if (flen > mtu)


^ permalink raw reply
* Re: [PATCH V2] CAN: Add Flexcan CAN controller driver
From: Sascha Hauer @ 2009-07-28 13:37 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: Socketcan-core, Linux Netdev List
In-Reply-To: <4A6EFB64.8070804@hartkopp.net>

Hi Oliver,

On Tue, Jul 28, 2009 at 03:21:40PM +0200, Oliver Hartkopp wrote:
> Sascha Hauer wrote:
> > Hi,
> > 
> > Here is the second version of the flexcan driver.
> 
> Hi Sascha,
> 
> some more points i forgot to mention, sorry ...
> 
> 
> > +/* Structure of the message buffer */
> > +struct flexcan_mb {
> > +	u32	can_dlc;
> > +	u32	can_id;
> > +	u32	data[2];
> > +};
> 
> This looks really hackish and does not reflect the structure of a flexcan
> message buffer! The data is 'u8' and the name of 'dlc' for the
> description/flag register is bad.
> 

see below..

> 
> > +
> > +static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > +{
> > +	struct can_frame *frame = (struct can_frame *)skb->data;
> > +	struct flexcan_priv *priv = netdev_priv(dev);
> > +	struct flexcan_regs __iomem *regs = priv->base;
> 
> > +	u32 can_id;
> > +	u32 dlc = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
> 
> Naming this variable 'dlc' does not hit the point. See below.
> 
> > +	u32 *can_data;
> 
> Really this needs to be fixed up by defining a proper mailbox struct.
> 
> 
> > +
> > +	netif_stop_queue(dev);
> > +
> > +	if (frame->can_id & CAN_EFF_FLAG) {
> > +		can_id = frame->can_id & MB_ID_EXT;
> 
> Please use CAN_EFF_MASK here.

I used MX_ID_EXT intentionally because it it flexcan specific and just
happens to be the same as CAN_EFF_MASK. I can change it if you like.

> 
> 
> > +		dlc |= MB_CNT_IDE | MB_CNT_SRR;
> > +	} else {
> > +		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
> > +	}
> 
> Just nitpicking for Kernel coding style:
> remove the last '{' and '}' pair.

No, Documentation/CondingStyle suggests that if one branch needs braces
the other branch should use them, too.

> 
> > +
> > +	if (frame->can_id & CAN_RTR_FLAG)
> > +		dlc |= MB_CNT_RTR;
> > +
> > +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> > +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
> > +
> > +	can_data = (u32 *)frame->data;
> > +	writel(cpu_to_be32(*can_data), &regs->cantxfg[TX_BUF_ID].data[0]);
> > +	writel(cpu_to_be32(*(can_data + 1)), &regs->cantxfg[TX_BUF_ID].data[1]);
> 
> IMHO it is not really transparent, that this is a correct handling to copy the
>   can_frame.data[] on all architectures. I bet creating a for-statement
> regarding the dlc is not slower and makes really clear, what's going on here.

This is indeed a problem here. The original Coldfire code I used as a
template used a loop around unsigned char * which did the wrong thing
for me. So yes, this is not generic here, but I have no idea how the
generic code looks like. As Coldfire is big endian this doesn't seem
that wrong.

> 
> > +
> > +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> > +
> > +	kfree_skb(skb);
> > +
> > +	return NETDEV_TX_OK;
> > +}
> > +
> > +static void flexcan_rx_frame(struct net_device *ndev,
> > +		struct flexcan_mb __iomem *mb)
> > +{
> > +	struct net_device_stats *stats = &ndev->stats;
> > +	struct sk_buff *skb;
> > +	struct can_frame *frame;
> > +	int ctrl = readl(&mb->can_dlc);
> 
> 'ctrl' is much better than 'dlc' naming above :-)

ok, will change it above.

> 
> > +	int length = (ctrl >> 16) & 0x0f;
> 
> is probably not enough ... use %9 or an additional check.

ok, I'll add a sanity check.

> 
> > +	u32 id;
> > +
> > +	skb = dev_alloc_skb(sizeof(struct can_frame));
> > +	if (!skb) {
> > +		stats->rx_dropped++;
> > +		return;
> > +	}
> > +
> > +	frame = (struct can_frame *)skb_put(skb,
> > +			sizeof(struct can_frame));
> > +
> > +	frame->can_dlc = length;
> > +	id = readl(&mb->can_id) & MB_ID_EXT;
> 
> like above use CAN_EFF_MASK
> 
> or at least rename MB_ID_EXT to MB_ID_EXT_MASK
> 
> > +
> > +	if (ctrl & MB_CNT_IDE) {
> > +		frame->can_id = id & CAN_EFF_MASK;
> > +		frame->can_id |= CAN_EFF_FLAG;
> > +		if (ctrl & MB_CNT_RTR)
> > +			frame->can_id |= CAN_RTR_FLAG;
> > +	} else {
> > +		frame->can_id  = id >> 18;
> > +		if (ctrl & MB_CNT_RTR)
> > +			frame->can_id |= CAN_RTR_FLAG;
> > +	}
> > +
> > +	*(u32 *)frame->data = be32_to_cpu(readl(&mb->data[0]));
> > +	*((u32 *)frame->data + 1) = be32_to_cpu(readl(&mb->data[1]));
> 
> Same as above.
> 
> Please write readable an maintainable code and let the compiler do his job.
> 
> Thanks,
> Oliver
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply
* [PATCH] pppol2tp: calls unregister_pernet_gen_device() at unload time
From: Eric Dumazet @ 2009-07-28 13:47 UTC (permalink / raw)
  To: David S. Miller
  Cc: Pavel Emelyanov, Igor M Podlesny, Andrew Morton, netdev,
	Cyrill Gorcunov
In-Reply-To: <4A6EFB81.4090105@gmail.com>

Eric Dumazet a écrit :
> Seems drivers/net/pppol2tp.c is a suspect...
> 
> It uses register_pernet_gen_device() from pppol2tp_init()
> but doesnt call unregister_pernet_gen_device()

OK patch seems really easy...

This bug was added in commit 4e9fb8016a351b5b9da7fea32bcfdbc9d836e421
net: pppol2tp - introduce net-namespace functionality

So this is a stable candidate I guess ?

Thank you


[PATCH] pppol2tp: calls unregister_pernet_gen_device() at unload time

Failure to call unregister_pernet_gen_device() can exhaust memory
if module is loaded/unloaded many times.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
index e7935d0..e0f9219 100644
--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -2680,6 +2680,7 @@ out_unregister_pppol2tp_proto:
 static void __exit pppol2tp_exit(void)
 {
 	unregister_pppox_proto(PX_PROTO_OL2TP);
+	unregister_pernet_gen_device(pppol2tp_net_id, &pppol2tp_net_ops);
 	proto_unregister(&pppol2tp_sk_proto);
 }
 

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