Netdev List
 help / color / mirror / Atom feed
* Re: WARN_ON() hit in fsl bitbanged phy driver
From: Scott Wood @ 2008-02-11 21:50 UTC (permalink / raw)
  To: Rune Torgersen; +Cc: linuxppc-dev, netdev
In-Reply-To: <DCEAAC0833DD314AB0B58112AD99B93B03F931E6@ismail.innsys.innovsys.com>

Rune Torgersen wrote:
> I hit the following WARN_ON when using mii-tools agains a ethernet
> interface using a bit-banged mii interface
> It is only diplayed once, and does not seem to impact usage at all
> 
> Does somebody know what is wrong, and how to fix it?

It looks like the kernel thinks it's in an interrupt, even though it 
clearly isn't from the backtrace.  Presumably, something slept from an 
interrupt handler; try turning on sleep-in-spinlock debugging.

The root cause was probably something other than the phy code.

-Scott

^ permalink raw reply

* RE: WARN_ON() hit in fsl bitbanged phy driver
From: Rune Torgersen @ 2008-02-11 22:12 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, netdev
In-Reply-To: <47B0C30A.1030801@freescale.com>

Scott Wood wrote:
> Rune Torgersen wrote:
>> I hit the following WARN_ON when using mii-tools agains a ethernet
>> interface using a bit-banged mii interface
> 
> It looks like the kernel thinks it's in an interrupt, even though it
> clearly isn't from the backtrace.  Presumably, something
> slept from an
> interrupt handler; try turning on sleep-in-spinlock debugging.

I turned on sleep-in-spinlock and it did not reveal anything. I'm trying
some other debug options.

> The root cause was probably something other than the phy code.
I'm not so sure, because it only happens when I run mii-tool agains the
interface useing the bit-banged driver, and then only the first time.




^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: James Chapman @ 2008-02-11 22:19 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: netdev
In-Reply-To: <47B09A90.7040508@gmail.com>

Jarek Poplawski wrote:
> James Chapman wrote, On 02/11/2008 10:22 AM:
> 
>> Fix locking issues in the pppol2tp driver which can cause a kernel
>> crash on SMP boxes when hundreds of L2TP sessions are created/deleted
>> simultaneously (ISP environment). The driver was violating read_lock()
>> and write_lock() scheduling rules so we now consistently use the _irq
>> variants of the lock functions.
> ... 
> 
> Hi,
> 
> Could you explain what exactly scheduling rules do you mean here,
> and why disabling interrupts is the best solution for this?

Below is example output from lockdep. The oops is reproducible when 
creating/deleting lots of sessions while passing data. The lock is being 
acquired for read and write in softirq contexts.

Is there a better way to fix this?

=================================
[ INFO: inconsistent lock state ]
2.6.24-core2 #1
---------------------------------
inconsistent {in-softirq-R} -> {softirq-on-W} usage.
openl2tpd/3215 [HC0[0]:SC0[0]:HE1:SE1] takes:
   (&tunnel->hlist_lock){---?}, at: [<f8eea157>]
pppol2tp_connect+0x517/0x6d0 [pppol2tp]
{in-softirq-R} state was registered at:
    [<c014edaf>] __lock_acquire+0x6bf/0x10a0
    [<c03ee75b>] fn_hash_lookup+0x1b/0xe0
    [<c014f804>] lock_acquire+0x74/0xa0
    [<f8ee859f>] pppol2tp_session_find+0x1f/0x80 [pppol2tp]
    [<c040427a>] _read_lock+0x2a/0x40
    [<f8ee859f>] pppol2tp_session_find+0x1f/0x80 [pppol2tp]
    [<f8ee859f>] pppol2tp_session_find+0x1f/0x80 [pppol2tp]
    [<f8ee8dc8>] pppol2tp_recv_core+0xd8/0x960 [pppol2tp]
    [<f8d3f72a>] ipt_do_table+0x23a/0x500 [ip_tables]
    [<f8ee967e>] pppol2tp_udp_encap_recv+0x2e/0x70 [pppol2tp]
    [<c0403fb4>] _read_unlock+0x14/0x20
    [<c03dd696>] udp_queue_rcv_skb+0x106/0x2a0
    [<c03ddc5a>] __udp4_lib_rcv+0x42a/0x7e0
    [<f8d57090>] ipt_hook+0x0/0x20 [iptable_filter]
    [<c03bc2da>] ip_local_deliver_finish+0xca/0x1c0
    [<c03bc23e>] ip_local_deliver_finish+0x2e/0x1c0
    [<c03bbfaf>] ip_rcv_finish+0xff/0x360
    [<c03bc6dc>] ip_rcv+0x20c/0x2a0
    [<c03bbeb0>] ip_rcv_finish+0x0/0x360
    [<c039ad87>] netif_receive_skb+0x317/0x4b0
    [<c039ab70>] netif_receive_skb+0x100/0x4b0
    [<f8d9627a>] e1000_clean_rx_irq_ps+0x28a/0x560 [e1000]
    [<f8d95ff0>] e1000_clean_rx_irq_ps+0x0/0x560 [e1000]
    [<f8d9384d>] e1000_clean+0x5d/0x290 [e1000]
    [<c039d580>] net_rx_action+0x1a0/0x2a0
    [<c039d43f>] net_rx_action+0x5f/0x2a0
    [<c0131e72>] __do_softirq+0x92/0x120
    [<c0131f78>] do_softirq+0x78/0x80
    [<c010b15a>] do_IRQ+0x4a/0xa0
    [<c0108dcc>] common_interrupt+0x24/0x34
    [<c0108dd6>] common_interrupt+0x2e/0x34
    [<c01062d6>] mwait_idle_with_hints+0x46/0x60
    [<c0106550>] mwait_idle+0x0/0x20
    [<c0106694>] cpu_idle+0x74/0xe0
    [<c0536a9a>] start_kernel+0x30a/0x3a0
    [<c0536150>] unknown_bootoption+0x0/0x1f0
    [<ffffffff>] 0xffffffff
irq event stamp: 275
hardirqs last  enabled at (275): [<c0132317>] local_bh_enable_ip+0xa7/0x120
hardirqs last disabled at (273): [<c01322a6>] local_bh_enable_ip+0x36/0x120
softirqs last  enabled at (274): [<f8eab8bc>]
ppp_register_channel+0xdc/0xf0 [ppp_generic]
softirqs last disabled at (272): [<c040410b>] _spin_lock_bh+0xb/0x40


-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development


^ permalink raw reply

* [PATCH] mlx4: needs vmalloc.h for vmap()
From: Kyle McMartin @ 2008-02-11 22:11 UTC (permalink / raw)
  To: netdev; +Cc: Roland Dreier, Jack Morgenstein

Commit 313abe55a87bc10e55d00f337d609e17ad5f8c9a added a vmap usage to
mlx4. Unfortunately on some platforms (ppc64) <linux/vmalloc.h> is not
implicitly included. Fix that.

Signed-off-by: Kyle McMartin <kmcmartin@redhat.com>

---
diff --git a/drivers/net/mlx4/alloc.c b/drivers/net/mlx4/alloc.c
index 521dc03..75ef9d0 100644
--- a/drivers/net/mlx4/alloc.c
+++ b/drivers/net/mlx4/alloc.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/bitmap.h>
 #include <linux/dma-mapping.h>
+#include <linux/vmalloc.h>
 
 #include "mlx4.h"
 

^ permalink raw reply related

* Re: WARN_ON() hit in fsl bitbanged phy driver
From: Scott Wood @ 2008-02-11 22:26 UTC (permalink / raw)
  To: Rune Torgersen; +Cc: linuxppc-dev, netdev
In-Reply-To: <DCEAAC0833DD314AB0B58112AD99B93B03F9325F@ismail.innsys.innovsys.com>

Rune Torgersen wrote:
> Scott Wood wrote:
>> Rune Torgersen wrote:
>>> I hit the following WARN_ON when using mii-tools agains a ethernet
>>> interface using a bit-banged mii interface
>> It looks like the kernel thinks it's in an interrupt, even though it
>> clearly isn't from the backtrace.  Presumably, something
>> slept from an
>> interrupt handler; try turning on sleep-in-spinlock debugging.
> 
> I turned on sleep-in-spinlock and it did not reveal anything. I'm trying
> some other debug options.
> 
>> The root cause was probably something other than the phy code.
> I'm not so sure, because it only happens when I run mii-tool agains the
> interface useing the bit-banged driver, and then only the first time.

OK, it seems I should have checked 2.6.24 instead of head-of-tree; 
softirq.c:139 is a different assertion than I thought.  It's not 
in_irq(), but irqs_disabled().  fs_ioctl() is disabling interrupts, and 
spin_unlock_bh() doesn't like that.  The current use of mutexes likes it 
even less.  The locking should be moved inside the phy bus 
implementation, if it's needed at all.

It only happens once because it's a WARN_ON_ONCE(). :-)

-Scott

^ permalink raw reply

* Re: [PATCH] mlx4: needs vmalloc.h for vmap()
From: Roland Dreier @ 2008-02-11 22:34 UTC (permalink / raw)
  To: Kyle McMartin; +Cc: netdev, Roland Dreier, Jack Morgenstein
In-Reply-To: <20080211221115.GH4516@phobos.i.cabal.ca>

thanks, just merged the same patch from Olof Johansson.

^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: Jarek Poplawski @ 2008-02-11 22:49 UTC (permalink / raw)
  To: James Chapman; +Cc: netdev
In-Reply-To: <47B0C9F7.5040200@katalix.com>

On Mon, Feb 11, 2008 at 10:19:35PM +0000, James Chapman wrote:
...
> Below is example output from lockdep. The oops is reproducible when  
> creating/deleting lots of sessions while passing data. The lock is being  
> acquired for read and write in softirq contexts.
>
> Is there a better way to fix this?
>
> =================================
> [ INFO: inconsistent lock state ]
> 2.6.24-core2 #1
> ---------------------------------
> inconsistent {in-softirq-R} -> {softirq-on-W} usage.
> openl2tpd/3215 [HC0[0]:SC0[0]:HE1:SE1] takes:
>   (&tunnel->hlist_lock){---?}, at: [<f8eea157>]
> pppol2tp_connect+0x517/0x6d0 [pppol2tp]
> {in-softirq-R} state was registered at:

IMHO, according to this, disabling bh should be enough. And if it's
like in this report: only read_lock is taken from softirqs, then this
should be necessary to change only all write_locks to write_lock_bh
(of course unless somewhere bhs are disabled already). Unless I miss
something?!

Cheers,
Jarek P.

^ permalink raw reply

* Re: [PATCH] Execute tasklets in the same order they were queued
From: Andrew Morton @ 2008-02-11 22:49 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linux-kernel, netdev
In-Reply-To: <20080211222813.GA14316@lixom.net>

On Mon, 11 Feb 2008 16:28:13 -0600
Olof Johansson <olof@lixom.net> wrote:

> I noticed this when looking at an openswan issue. Openswan (ab?)uses
> the tasklet API to defer processing of packets in some situations,
> with one packet per tasklet_action(). I started noticing sequences of
> reverse-ordered sequence numbers coming over the wire, since new tasklets
> are always queued at the head of the list but processed sequentially.
> 
> Convert it to instead append new entries to the tail of the list. As an
> extra bonus, the splicing code in takeover_tasklets() no longer has to
> iterate over the list.
> 

hm, I'd have thought that this would already have caused problems in
networking.  And perhaps this change might have effects on networking too?

Probably it won't have _much_ effect on networking because networking
probably isn't queueing one tasklet per packet(!) but perhaps with bonded
channels or something like that?


^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: Jarek Poplawski @ 2008-02-11 22:55 UTC (permalink / raw)
  To: James Chapman; +Cc: netdev
In-Reply-To: <20080211224924.GA2863@ami.dom.local>

On Mon, Feb 11, 2008 at 11:49:24PM +0100, Jarek Poplawski wrote:
> On Mon, Feb 11, 2008 at 10:19:35PM +0000, James Chapman wrote:
> ...
> > Below is example output from lockdep. The oops is reproducible when  
> > creating/deleting lots of sessions while passing data. The lock is being  
> > acquired for read and write in softirq contexts.

...Hmmm... And according to this, changing read_locks should be
necessary too.

Jarek P.

^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: James Chapman @ 2008-02-11 23:41 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: netdev
In-Reply-To: <20080211224924.GA2863@ami.dom.local>

Jarek Poplawski wrote:
> On Mon, Feb 11, 2008 at 10:19:35PM +0000, James Chapman wrote:
> ...
>> Below is example output from lockdep. The oops is reproducible when  
>> creating/deleting lots of sessions while passing data. The lock is being  
>> acquired for read and write in softirq contexts.
>>
>> Is there a better way to fix this?
>>
>> =================================
>> [ INFO: inconsistent lock state ]
>> 2.6.24-core2 #1
>> ---------------------------------
>> inconsistent {in-softirq-R} -> {softirq-on-W} usage.
>> openl2tpd/3215 [HC0[0]:SC0[0]:HE1:SE1] takes:
>>   (&tunnel->hlist_lock){---?}, at: [<f8eea157>]
>> pppol2tp_connect+0x517/0x6d0 [pppol2tp]
>> {in-softirq-R} state was registered at:
> 
> IMHO, according to this, disabling bh should be enough. And if it's
> like in this report: only read_lock is taken from softirqs, then this
> should be necessary to change only all write_locks to write_lock_bh
> (of course unless somewhere bhs are disabled already). Unless I miss
> something?!

I thought so too. I tried _bh locks first and the problem still 
occurred. Maybe I'll try it again in case I messed something up.

-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development


^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: James Chapman @ 2008-02-11 23:42 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: netdev
In-Reply-To: <20080211225533.GB2863@ami.dom.local>

Jarek Poplawski wrote:
> On Mon, Feb 11, 2008 at 11:49:24PM +0100, Jarek Poplawski wrote:
>> On Mon, Feb 11, 2008 at 10:19:35PM +0000, James Chapman wrote:
>> ...
>>> Below is example output from lockdep. The oops is reproducible when  
>>> creating/deleting lots of sessions while passing data. The lock is being  
>>> acquired for read and write in softirq contexts.
> 
> ...Hmmm... And according to this, changing read_locks should be
> necessary too.

The patch changes both read and write locks.

-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development


^ permalink raw reply

* Re: [Bugme-new] [Bug 9937] New: Bug in bonding driver - Kernel oops whenever driver is loaded with max_bonds parameter
From: Andrew Morton @ 2008-02-11 23:56 UTC (permalink / raw)
  To: netdev, Jay Vosburgh; +Cc: bugme-daemon, kantica
In-Reply-To: <bug-9937-10286@http.bugzilla.kernel.org/>

On Mon, 11 Feb 2008 15:04:03 -0800 (PST)
bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=9937
> 
>            Summary: Bug in bonding driver - Kernel oops whenever driver is
>                     loaded with max_bonds parameter
>            Product: Networking
>            Version: 2.5
>      KernelVersion: 2.6.24.2
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: high
>           Priority: P1
>          Component: IPV4
>         AssignedTo: shemminger@linux-foundation.org
>         ReportedBy: kantica@gmail.com
> 
> 
> Latest working kernel version:
> Earliest failing kernel version: 2.6.24.2
> Distribution: Slackware / Debian GNU/Linux
> Hardware Environment: HP ProLiant DL380 G5 (Debian), Slackware Acer TravelMate
> 4001 Laptop
> Software Environment: 
> Problem Description: Kernel oops whenever bonding driver with max_bonds=2 (or >
> 2) is loaded ...
> 
> Steps to reproduce:
> 
> modprobe bonding mode=0 miimon=100 max_bonds=2 
> or
> modprobe bonding max_bonds=2 
> 
> 
> dmesg output (from slackware laptop / x86):
> 
> BUG: unable to handle kernel NULL pointer dereference at virtual address
> 00000000
> printing eip: c028eeaf *pde = 00000000
> Oops: 0000 [#1] SMP
> Modules linked in: bonding snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq
> snd_seq_device snd_pcm_oss snd_mixer_oss ntfs pcmcia yenta_socket
> rsrc_nonstatic tifm_7xx1 tifm_core pcmcia_core snd_intel8x0 snd_ac97_codec
> ac97_bus snd_pcm i2c_i801 snd_timer snd i2c_core shpchp snd_page_alloc ehci_hcd
> uhci_hcd pci_hotplug
> 
> Pid: 2729, comm: modprobe Not tainted (2.6.24.2 #2)
> EIP: 0060:[<c028eeaf>] EFLAGS: 00010282 CPU: 0
> EIP is at strnicmp+0x17/0x61
> EAX: d8162800 EBX: 00000000 ECX: 00000010 EDX: 00000062
> ESI: 00000010 EDI: 00000000 EBP: d8162801 ESP: d82c9f60
>  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> Process modprobe (pid: 2729, ti=d82c8000 task=df926550 task.ti=d82c8000)
> Stack: d8162c80 00000000 e0c76814 00000000 e0c67170 00000001 df80b700 e0c77180
>        00000001 00000000 0000000c d82c8000 e0afe05e e0c6ed14 e0c6ce70 e0c76c00
>        0805c098 0000000c c014e355 b7e7a008 0805c098 c0106f12 b7e7a008 00019477
> Call Trace:
>  [<e0c67170>] bond_create+0x4a/0x162 [bonding]
>  [<e0afe05e>] bonding_init+0x5e/0xf0 [bonding]
>  [<c014e355>] sys_init_module+0x91/0x11b
>  [<c0106f12>] syscall_call+0x7/0xb
>  [<c0470000>] sctp_setsockopt_bindx+0xe8/0x127
>  =======================
> Code: 08 fe dc ba 98 c7 40 0c 76 54 32 10 c7 40 10 f0 e1 d2 c3 c3 55 89 c5 57
> 89 d7 31 d2 56 89 ce 53 31 db 85 c9 74 42 0f b6 55 00 45 <0f> b6 1f 47 84 d2 74
> 35 84 db 74 31 38 da 74 2a 0f b6 c2 88 d1
> EIP: [<c028eeaf>] strnicmp+0x17/0x61 SS:ESP 0068:d82c9f60
> ---[ end trace 75761717808bf4ee ]---
> 
> dmesg output (from Debian x86_64 - HP ProLiant DL380):
> 
> Unable to handle kernel NULL pointer dereference at 0000000000000000 RIP:
>  [<ffffffff8030271e>] strnicmp+0x12/0x5f
> PGD 223005067 PUD 223b22067 PMD 0
> Oops: 0000 [1] SMP
> CPU 7
> Modules linked in: bonding mptctl mptbase fan ac battery ipv6 dm_snapshot
> dm_mirror dm_mod loop usbhid ide_cd cdrom bnx2 generic thermal ipmi_si piix
> serio_raw evdev shpchp
> psmouse pci_hotplug container pcspkr ide_core ipmi_msghandler uhci_hcd button
> processor ehci_hcd e1000 ext3 jbd mbcache reiserfs cciss
> Pid: 12469, comm: modprobe Not tainted 2.6.24.2 #1
> RIP: 0010:[<ffffffff8030271e>]  [<ffffffff8030271e>] strnicmp+0x12/0x5f
> RSP: 0018:ffff81022339fe00  EFLAGS: 00010202
> RAX: ffff81022307e6c0 RBX: ffffffff88233918 RCX: 00000000000020e7
> RDX: 0000000000000010 RSI: 0000000000000000 RDI: ffff81022307e000
> RBP: 0000000000000000 R08: ffff810223b90362 R09: 0000000000000010
> R10: ffffffff8822d60b R11: 0000000000000001 R12: 0000000000000000
> R13: ffffffff88234b00 R14: ffff81022307e7c8 R15: 0000000000000000
> FS:  00002b07aa3166e0(0000) GS:ffff81022743bd00(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000000 CR3: 000000022339c000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process modprobe (pid: 12469, threadinfo ffff81022339e000, task
> ffff8102239aa000)
> Stack:  ffffffff882200ce ffff8102239ad000 0000000000000001 ffff8102274273c0
>  0000000000000000 0000000000000001 ffffc20011bef960 ffff810225c88540
>  ffffffff8809f7bf ffffffff882340c0 ffffffff882340c0 ffff8102263f7f00
> Call Trace:
>  [<ffffffff882200ce>] :bonding:bond_create+0x4e/0x30e
>  [<ffffffff8809f7bf>] :bonding:bonding_init+0x7bf/0x85d
>  [<ffffffff8024f752>] sys_init_module+0x176d/0x183f
>  [<ffffffff8020be8e>] system_call+0x7e/0x83
> 
> 
> Code: 8a 0e 48 ff c7 48 ff c6 45 84 c0 74 36 84 c9 74 32 41 38 c8
> RIP  [<ffffffff8030271e>] strnicmp+0x12/0x5f
>  RSP <ffff81022339fe00>
> CR2: 0000000000000000
> ---[ end trace ba3d7089e7da64fa ]---
> 


^ permalink raw reply

* Re: [PATCH] fib_trie: rcu_assign_pointer warning fix
From: David Miller @ 2008-02-12  1:16 UTC (permalink / raw)
  To: shemminger; +Cc: paulmck, netdev, linux-kernel
In-Reply-To: <20080211165954.2f1b3a9b@extreme>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 11 Feb 2008 16:59:54 -0800

linux-kernel added to CC:, any change to generic kernel infrastructure
should be posted there

> Eliminate warnings when rcu_assign_pointer is used with unsigned long.
> It is reasonable to use RCU with non-pointer values so allow it for general
> use.  Add a comment to explain the if test.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> ---
>  include/linux/rcupdate.h |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 37a642c..c44ac87 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -172,14 +172,15 @@ struct rcu_head {
>   * structure after the pointer assignment.  More importantly, this
>   * call documents which pointers will be dereferenced by RCU read-side
>   * code.
> + *
> + * If value is the NULL (constant 0), then no barrier is needed.
>   */
>  
> -#define rcu_assign_pointer(p, v) \
> -	({ \
> -		if (!__builtin_constant_p(v) || \
> -		    ((v) != NULL)) \
> -			smp_wmb(); \
> -		(p) = (v); \
> +#define rcu_assign_pointer(p, v)			\
> +	({						\
> +		if (!(__builtin_constant_p(v) && v))	\
> +			smp_wmb();			\
> +		(p) = (v);				\
>  	})
>  
>  /**
> -- 
> 1.5.3.8
> 

^ permalink raw reply

* Re: [Bugme-new] [Bug 9937] New: Bug in bonding driver - Kernel oops whenever driver is loaded with max_bonds parameter
From: Jay Vosburgh @ 2008-02-12  1:21 UTC (permalink / raw)
  To: Andrew Morton; +Cc: netdev, bugme-daemon, kantica, stable
In-Reply-To: <20080211155608.b60858d7.akpm@linux-foundation.org>

Andrew Morton <akpm@linux-foundation.org> wrote:

>> Problem Description: Kernel oops whenever bonding driver with max_bonds=2 (or >
>> 2) is loaded ...

	I believe this is fixed by the following (from linux-2.6):

From: Jay Vosburgh <fubar@us.ibm.com>
Date: Tue, 29 Jan 2008 18:07:45 -0800
Subject: [PATCH] bonding: fix NULL pointer deref in startup processing

	Fix the "are we creating a duplicate" check to not compare
the name if the name is NULL (meaning that the system should select
a name).  Bug reported by Benny Amorsen <benny+usenet@amorsen.dk>.

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/net/bonding/bond_main.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 65c7eba..81b4574 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4896,14 +4896,16 @@ int bond_create(char *name, struct bond_params *params, struct bonding **newbond
 	down_write(&bonding_rwsem);
 
 	/* Check to see if the bond already exists. */
-	list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list)
-		if (strnicmp(bond->dev->name, name, IFNAMSIZ) == 0) {
-			printk(KERN_ERR DRV_NAME
+	if (name) {
+		list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list)
+			if (strnicmp(bond->dev->name, name, IFNAMSIZ) == 0) {
+				printk(KERN_ERR DRV_NAME
 			       ": cannot add bond %s; it already exists\n",
-			       name);
-			res = -EPERM;
-			goto out_rtnl;
-		}
+				       name);
+				res = -EPERM;
+				goto out_rtnl;
+			}
+	}
 
 	bond_dev = alloc_netdev(sizeof(struct bonding), name ? name : "",
 				ether_setup);
-- 
1.5.2.4


^ permalink raw reply related

* Re: [PATCH] fib_trie: rcu_assign_pointer warning fix
From: Paul E. McKenney @ 2008-02-12  1:27 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, netdev
In-Reply-To: <20080211165954.2f1b3a9b@extreme>

On Mon, Feb 11, 2008 at 04:59:54PM -0800, Stephen Hemminger wrote:
> Eliminate warnings when rcu_assign_pointer is used with unsigned long.
> It is reasonable to use RCU with non-pointer values so allow it for general
> use.  Add a comment to explain the if test.

Good catch!!!  (An apologies for the hassle!!!)

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> ---
>  include/linux/rcupdate.h |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 37a642c..c44ac87 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -172,14 +172,15 @@ struct rcu_head {
>   * structure after the pointer assignment.  More importantly, this
>   * call documents which pointers will be dereferenced by RCU read-side
>   * code.
> + *
> + * If value is the NULL (constant 0), then no barrier is needed.
>   */
> 
> -#define rcu_assign_pointer(p, v) \
> -	({ \
> -		if (!__builtin_constant_p(v) || \
> -		    ((v) != NULL)) \
> -			smp_wmb(); \
> -		(p) = (v); \
> +#define rcu_assign_pointer(p, v)			\
> +	({						\
> +		if (!(__builtin_constant_p(v) && v))	\
> +			smp_wmb();			\
> +		(p) = (v);				\
>  	})
> 
>  /**
> -- 
> 1.5.3.8
> 

^ permalink raw reply

* [PATCH] [RFC] Smack: unlabeled outgoing ambient packets - v2
From: Casey Schaufler @ 2008-02-11 18:29 UTC (permalink / raw)
  To: paul.moore; +Cc: linux-kernel, netdev

From: Casey Schaufler <casey@schaufler-ca.com>

Smack uses CIPSO labeling, but allows for unlabeled packets
by specifying an "ambient" label that is applied to incoming
unlabeled packets. Because the other end of the connection
may dislike IP options, and ssh is one know application that
behaves thus, it is prudent to respond in kind. This patch
changes the network labeling behavior such that an outgoing
packet that would be given a CIPSO label that matches the
ambient label is left unlabeled. An "unlbl" domain is added
and the netlabel defaulting mechanism invoked rather than
assuming that everything is CIPSO. Locking has been added
around changes to the ambient label as the mechanisms used
to do so are more involved.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

---

This patch differs significantly from the previous version.
I think that I am using the netlbl interfaces more appropriately,
Paul, please let me know if there's a better approach.

It's inconvenient that netlbl_sock_setattr frees the domain passed.
I see that it makes sense for SELinux with the way SELinux treats
secctx's, but Smack is more careful about memory usage and I have
to do what I consider a gratuitous kalloc because of this behavior.
Would you be open to a patch to change this if it included the SELinux
changes?

Thank you.

 security/smack/smack_lsm.c |   20 ++++++------
 security/smack/smackfs.c   |   54 +++++++++++++++++++++++++----------
 2 files changed, 49 insertions(+), 25 deletions(-)

diff -uprN -X linux-2.6.25-g0210-base//Documentation/dontdiff linux-2.6.25-g0210-base/security/smack/smackfs.c linux-2.6.25-g0210/security/smack/smackfs.c
--- linux-2.6.25-g0210-base/security/smack/smackfs.c	2008-02-10 19:30:47.000000000 -0800
+++ linux-2.6.25-g0210/security/smack/smackfs.c	2008-02-11 07:14:54.000000000 -0800
@@ -45,6 +45,7 @@ enum smk_inos {
  */
 static DEFINE_MUTEX(smack_list_lock);
 static DEFINE_MUTEX(smack_cipso_lock);
+static DEFINE_MUTEX(smack_ambient_lock);
 
 /*
  * This is the "ambient" label for network traffic.
@@ -363,6 +364,27 @@ void smk_cipso_doi(void)
 		       __func__, __LINE__, rc);
 }
 
+/**
+ * smk_unlbl_ambient - initialize the unlabeled domain
+ */
+void smk_unlbl_ambient(char *oldambient)
+{
+	int rc;
+	struct netlbl_audit audit_info;
+
+	if (oldambient != NULL) {
+		rc = netlbl_cfg_map_del(oldambient, &audit_info);
+		if (rc != 0)
+			printk(KERN_WARNING "%s:%d remove rc = %d\n",
+			       __func__, __LINE__, rc);
+	}
+
+	rc = netlbl_cfg_unlbl_add_map(smack_net_ambient, &audit_info);
+	if (rc != 0)
+		printk(KERN_WARNING "%s:%d add rc = %d\n",
+		       __func__, __LINE__, rc);
+}
+
 /*
  * Seq_file read operations for /smack/cipso
  */
@@ -709,7 +731,6 @@ static ssize_t smk_read_ambient(struct f
 				size_t cn, loff_t *ppos)
 {
 	ssize_t rc;
-	char out[SMK_LABELLEN];
 	int asize;
 
 	if (*ppos != 0)
@@ -717,23 +738,18 @@ static ssize_t smk_read_ambient(struct f
 	/*
 	 * Being careful to avoid a problem in the case where
 	 * smack_net_ambient gets changed in midstream.
-	 * Since smack_net_ambient is always set with a value
-	 * from the label list, including initially, and those
-	 * never get freed, the worst case is that the pointer
-	 * gets changed just after this strncpy, in which case
-	 * the value passed up is incorrect. Locking around
-	 * smack_net_ambient wouldn't be any better than this
-	 * copy scheme as by the time the caller got to look
-	 * at the ambient value it would have cleared the lock
-	 * and been changed.
 	 */
-	strncpy(out, smack_net_ambient, SMK_LABELLEN);
-	asize = strlen(out) + 1;
+	mutex_lock(&smack_ambient_lock);
 
-	if (cn < asize)
-		return -EINVAL;
+	asize = strlen(smack_net_ambient) + 1;
 
-	rc = simple_read_from_buffer(buf, cn, ppos, out, asize);
+	if (cn >= asize)
+		rc = simple_read_from_buffer(buf, cn, ppos,
+					     smack_net_ambient, asize);
+	else
+		rc = -EINVAL;
+
+	mutex_unlock(&smack_ambient_lock);
 
 	return rc;
 }
@@ -751,6 +767,7 @@ static ssize_t smk_write_ambient(struct 
 				 size_t count, loff_t *ppos)
 {
 	char in[SMK_LABELLEN];
+	char *oldambient;
 	char *smack;
 
 	if (!capable(CAP_MAC_ADMIN))
@@ -766,7 +783,13 @@ static ssize_t smk_write_ambient(struct 
 	if (smack == NULL)
 		return -EINVAL;
 
+	mutex_lock(&smack_ambient_lock);
+
+	oldambient = smack_net_ambient;
 	smack_net_ambient = smack;
+	smk_unlbl_ambient(oldambient);
+
+	mutex_unlock(&smack_ambient_lock);
 
 	return count;
 }
@@ -974,6 +997,7 @@ static int __init init_smk_fs(void)
 
 	sema_init(&smack_write_sem, 1);
 	smk_cipso_doi();
+	smk_unlbl_ambient(NULL);
 
 	return err;
 }
diff -uprN -X linux-2.6.25-g0210-base//Documentation/dontdiff linux-2.6.25-g0210-base/security/smack/smack_lsm.c linux-2.6.25-g0210/security/smack/smack_lsm.c
--- linux-2.6.25-g0210-base/security/smack/smack_lsm.c	2008-02-10 19:30:47.000000000 -0800
+++ linux-2.6.25-g0210/security/smack/smack_lsm.c	2008-02-10 20:10:47.000000000 -0800
@@ -1251,7 +1251,7 @@ static void smack_to_secattr(char *smack
 
 	switch (smack_net_nltype) {
 	case NETLBL_NLTYPE_CIPSOV4:
-		nlsp->domain = NULL;
+		nlsp->domain = kstrdup(smack, GFP_ATOMIC);
 		nlsp->flags = NETLBL_SECATTR_DOMAIN;
 		nlsp->flags |= NETLBL_SECATTR_MLS_LVL;
 
@@ -1276,21 +1276,21 @@ static void smack_to_secattr(char *smack
  * Convert the outbound smack value (smk_out) to a
  * secattr and attach it to the socket.
  *
- * Returns 0 on success or an error code
+ * The return from netlbl_sock_setattr is ignored because
+ * the defaulting label behavior provided by netlbl takes
+ * care of the error cases.
  */
-static int smack_netlabel(struct sock *sk)
+static void smack_netlabel(struct sock *sk)
 {
 	struct socket_smack *ssp = sk->sk_security;
 	struct netlbl_lsm_secattr secattr;
-	int rc = 0;
+	int rc;
 
 	netlbl_secattr_init(&secattr);
 	smack_to_secattr(ssp->smk_out, &secattr);
 	if (secattr.flags != NETLBL_SECATTR_NONE)
 		rc = netlbl_sock_setattr(sk, &secattr);
-
 	netlbl_secattr_destroy(&secattr);
-	return rc;
 }
 
 /**
@@ -1340,7 +1340,7 @@ static int smack_inode_setsecurity(struc
 		ssp->smk_in = sp;
 	else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) {
 		ssp->smk_out = sp;
-		return smack_netlabel(sock->sk);
+		smack_netlabel(sock->sk);
 	} else
 		return -EOPNOTSUPP;
 
@@ -1367,7 +1367,8 @@ static int smack_socket_post_create(stru
 	/*
 	 * Set the outbound netlbl.
 	 */
-	return smack_netlabel(sock->sk);
+	smack_netlabel(sock->sk);
+	return 0;
 }
 
 /**
@@ -2199,7 +2200,6 @@ static int smack_socket_getpeersec_dgram
 static void smack_sock_graft(struct sock *sk, struct socket *parent)
 {
 	struct socket_smack *ssp;
-	int rc;
 
 	if (sk == NULL)
 		return;
@@ -2212,7 +2212,7 @@ static void smack_sock_graft(struct sock
 	ssp->smk_out = current->security;
 	ssp->smk_packet[0] = '\0';
 
-	rc = smack_netlabel(sk);
+	smack_netlabel(sk);
 }
 
 /**



^ permalink raw reply

* [PATCH 2/4] docbook: sunrpc filenames and notation fixes
From: Randy Dunlap @ 2008-02-12  1:28 UTC (permalink / raw)
  To: lkml; +Cc: netdev, davem, akpm

From: Randy Dunlap <randy.dunlap@oracle.com>

Use updated file list for docbook files and
fix kernel-doc warnings in sunrpc:
Warning(linux-2.6.24-git12//net/sunrpc/rpc_pipe.c:689): No description found for parameter 'rpc_client'
Warning(linux-2.6.24-git12//net/sunrpc/rpc_pipe.c:765): No description found for parameter 'flags'
Warning(linux-2.6.24-git12//net/sunrpc/clnt.c:584): No description found for parameter 'tk_ops'
Warning(linux-2.6.24-git12//net/sunrpc/clnt.c:618): No description found for parameter 'bufsize'

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 Documentation/DocBook/kernel-api.tmpl |    8 +++++++-
 net/sunrpc/clnt.c                     |   10 +++++-----
 net/sunrpc/rpc_pipe.c                 |    3 ++-
 net/sunrpc/xprt.c                     |    2 +-
 4 files changed, 15 insertions(+), 8 deletions(-)

--- linux-2.6.25-rc1.orig/Documentation/DocBook/kernel-api.tmpl
+++ linux-2.6.25-rc1/Documentation/DocBook/kernel-api.tmpl
@@ -231,8 +231,14 @@ X!Ilib/string.c
 !Dnet/sunrpc/sunrpc_syms.c
 -->
 !Enet/sunrpc/xdr.c
-!Enet/sunrpc/svcsock.c
+!Enet/sunrpc/svc_xprt.c
+!Enet/sunrpc/xprt.c
 !Enet/sunrpc/sched.c
+!Enet/sunrpc/socklib.c
+!Enet/sunrpc/stats.c
+!Enet/sunrpc/rpc_pipe.c
+!Enet/sunrpc/rpcb_clnt.c
+!Enet/sunrpc/clnt.c
      </sect1>
   </chapter>
 
--- linux-2.6.25-rc1.orig/net/sunrpc/clnt.c
+++ linux-2.6.25-rc1/net/sunrpc/clnt.c
@@ -464,9 +464,9 @@ rpc_release_client(struct rpc_clnt *clnt
 
 /**
  * rpc_bind_new_program - bind a new RPC program to an existing client
- * @old - old rpc_client
- * @program - rpc program to set
- * @vers - rpc program version
+ * @old: old rpc_client
+ * @program: rpc program to set
+ * @vers: rpc program version
  *
  * Clones the rpc client and sets up a new RPC program. This is mainly
  * of use for enabling different RPC programs to share the same transport.
@@ -575,7 +575,7 @@ EXPORT_SYMBOL_GPL(rpc_call_sync);
  * @clnt: pointer to RPC client
  * @msg: RPC call parameters
  * @flags: RPC call flags
- * @ops: RPC call ops
+ * @tk_ops: RPC call ops
  * @data: user call data
  */
 int
@@ -610,7 +610,7 @@ EXPORT_SYMBOL_GPL(rpc_call_start);
  * rpc_peeraddr - extract remote peer address from clnt's xprt
  * @clnt: RPC client structure
  * @buf: target buffer
- * @size: length of target buffer
+ * @bufsize: length of target buffer
  *
  * Returns the number of bytes that are actually in the stored address.
  */
--- linux-2.6.25-rc1.orig/net/sunrpc/xprt.c
+++ linux-2.6.25-rc1/net/sunrpc/xprt.c
@@ -124,7 +124,7 @@ EXPORT_SYMBOL_GPL(xprt_register_transpor
 
 /**
  * xprt_unregister_transport - unregister a transport implementation
- * transport: transport to unregister
+ * @transport: transport to unregister
  *
  * Returns:
  * 0:		transport successfully unregistered
--- linux-2.6.25-rc1.orig/net/sunrpc/rpc_pipe.c
+++ linux-2.6.25-rc1/net/sunrpc/rpc_pipe.c
@@ -677,7 +677,7 @@ rpc_lookup_negative(char *path, struct n
 /**
  * rpc_mkdir - Create a new directory in rpc_pipefs
  * @path: path from the rpc_pipefs root to the new directory
- * @rpc_clnt: rpc client to associate with this directory
+ * @rpc_client: rpc client to associate with this directory
  *
  * This creates a directory at the given @path associated with
  * @rpc_clnt, which will contain a file named "info" with some basic
@@ -748,6 +748,7 @@ rpc_rmdir(struct dentry *dentry)
  * @private: private data to associate with the pipe, for the caller's use
  * @ops: operations defining the behavior of the pipe: upcall, downcall,
  *	release_pipe, and destroy_msg.
+ * @flags: rpc_inode flags
  *
  * Data is made available for userspace to read by calls to
  * rpc_queue_upcall().  The actual reads will result in calls to

^ permalink raw reply

* [PATCH 4/4] docbook: make a networking book and fix a few errors
From: Randy Dunlap @ 2008-02-12  1:28 UTC (permalink / raw)
  To: lkml; +Cc: netdev, davem, akpm

From: Randy Dunlap <randy.dunlap@oracle.com>

Move networking (core and drivers) docbook to its own networking book.
Fix a few kernel-doc errors in header and source files.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 Documentation/DocBook/Makefile        |    2 
 Documentation/DocBook/kernel-api.tmpl |   65 --------------------
 Documentation/DocBook/networking.tmpl |  106 ++++++++++++++++++++++++++++++++++
 include/linux/etherdevice.h           |    3 
 net/core/dev.c                        |    3 
 net/core/skbuff.c                     |    4 -
 6 files changed, 111 insertions(+), 72 deletions(-)

--- linux-2.6.25-rc1.orig/include/linux/etherdevice.h
+++ linux-2.6.25-rc1/include/linux/etherdevice.h
@@ -70,8 +70,7 @@ static inline int is_multicast_ether_add
 }
 
 /**
- * is_local_ether_addr - Determine if the Ethernet address is locally-assigned
- * one (IEEE 802).
+ * is_local_ether_addr - Determine if the Ethernet address is locally-assigned one (IEEE 802).
  * @addr: Pointer to a six-byte array containing the Ethernet address
  *
  * Return true if the address is a local address.
--- linux-2.6.25-rc1.orig/net/core/dev.c
+++ linux-2.6.25-rc1/net/core/dev.c
@@ -3038,8 +3038,7 @@ int dev_unicast_sync(struct net_device *
 EXPORT_SYMBOL(dev_unicast_sync);
 
 /**
- *	dev_unicast_unsync - Remove synchronized addresses from the destination
- *			     device
+ *	dev_unicast_unsync - Remove synchronized addresses from the destination device
  *	@to: destination device
  *	@from: source device
  *
--- linux-2.6.25-rc1.orig/net/core/skbuff.c
+++ linux-2.6.25-rc1/net/core/skbuff.c
@@ -1907,11 +1907,11 @@ void skb_prepare_seq_read(struct sk_buff
  * of bytes already consumed and the next call to
  * skb_seq_read() will return the remaining part of the block.
  *
- * Note: The size of each block of data returned can be arbitary,
+ * Note 1: The size of each block of data returned can be arbitary,
  *       this limitation is the cost for zerocopy seqeuental
  *       reads of potentially non linear data.
  *
- * Note: Fragment lists within fragments are not implemented
+ * Note 2: Fragment lists within fragments are not implemented
  *       at the moment, state->root_skb could be replaced with
  *       a stack for this purpose.
  */
--- linux-2.6.25-rc1.orig/Documentation/DocBook/Makefile
+++ linux-2.6.25-rc1/Documentation/DocBook/Makefile
@@ -8,7 +8,7 @@
 
 DOCBOOKS := wanbook.xml z8530book.xml mcabook.xml videobook.xml \
 	    kernel-hacking.xml kernel-locking.xml deviceiobook.xml \
-	    procfs-guide.xml writing_usb_driver.xml \
+	    procfs-guide.xml writing_usb_driver.xml networking.xml \
 	    kernel-api.xml filesystems.xml lsm.xml usb.xml \
 	    gadget.xml libata.xml mtdnand.xml librs.xml rapidio.xml \
 	    genericirq.xml s390-drivers.xml uio-howto.xml scsi.xml
--- /dev/null
+++ linux-2.6.25-rc1/Documentation/DocBook/networking.tmpl
@@ -0,0 +1,106 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN"
+	"http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd" []>
+
+<book id="LinuxNetworking">
+ <bookinfo>
+  <title>Linux Networking and Network Devices APIs</title>
+
+  <legalnotice>
+   <para>
+     This documentation 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.
+   </para>
+
+   <para>
+     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.
+   </para>
+
+   <para>
+     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
+   </para>
+
+   <para>
+     For more details see the file COPYING in the source
+     distribution of Linux.
+   </para>
+  </legalnotice>
+ </bookinfo>
+
+<toc></toc>
+
+  <chapter id="netcore">
+     <title>Linux Networking</title>
+     <sect1><title>Networking Base Types</title>
+!Iinclude/linux/net.h
+     </sect1>
+     <sect1><title>Socket Buffer Functions</title>
+!Iinclude/linux/skbuff.h
+!Iinclude/net/sock.h
+!Enet/socket.c
+!Enet/core/skbuff.c
+!Enet/core/sock.c
+!Enet/core/datagram.c
+!Enet/core/stream.c
+     </sect1>
+     <sect1><title>Socket Filter</title>
+!Enet/core/filter.c
+     </sect1>
+     <sect1><title>Generic Network Statistics</title>
+!Iinclude/linux/gen_stats.h
+!Enet/core/gen_stats.c
+!Enet/core/gen_estimator.c
+     </sect1>
+     <sect1><title>SUN RPC subsystem</title>
+<!-- The !D functionality is not perfect, garbage has to be protected by comments
+!Dnet/sunrpc/sunrpc_syms.c
+-->
+!Enet/sunrpc/xdr.c
+!Enet/sunrpc/svc_xprt.c
+!Enet/sunrpc/xprt.c
+!Enet/sunrpc/sched.c
+!Enet/sunrpc/socklib.c
+!Enet/sunrpc/stats.c
+!Enet/sunrpc/rpc_pipe.c
+!Enet/sunrpc/rpcb_clnt.c
+!Enet/sunrpc/clnt.c
+     </sect1>
+  </chapter>
+
+  <chapter id="netdev">
+     <title>Network device support</title>
+     <sect1><title>Driver Support</title>
+!Enet/core/dev.c
+!Enet/ethernet/eth.c
+!Enet/sched/sch_generic.c
+!Iinclude/linux/etherdevice.h
+!Iinclude/linux/netdevice.h
+     </sect1>
+     <sect1><title>PHY Support</title>
+!Edrivers/net/phy/phy.c
+!Idrivers/net/phy/phy.c
+!Edrivers/net/phy/phy_device.c
+!Idrivers/net/phy/phy_device.c
+!Edrivers/net/phy/mdio_bus.c
+!Idrivers/net/phy/mdio_bus.c
+     </sect1>
+<!-- FIXME: Removed for now since no structured comments in source
+     <sect1><title>Wireless</title>
+X!Enet/core/wireless.c
+     </sect1>
+-->
+     <sect1><title>Synchronous PPP</title>
+!Edrivers/net/wan/syncppp.c
+     </sect1>
+  </chapter>
+
+</book>
--- linux-2.6.25-rc1.orig/Documentation/DocBook/kernel-api.tmpl
+++ linux-2.6.25-rc1/Documentation/DocBook/kernel-api.tmpl
@@ -204,71 +204,6 @@ X!Ilib/string.c
      </sect1>
   </chapter>
 
-  <chapter id="netcore">
-     <title>Linux Networking</title>
-     <sect1><title>Networking Base Types</title>
-!Iinclude/linux/net.h
-     </sect1>
-     <sect1><title>Socket Buffer Functions</title>
-!Iinclude/linux/skbuff.h
-!Iinclude/net/sock.h
-!Enet/socket.c
-!Enet/core/skbuff.c
-!Enet/core/sock.c
-!Enet/core/datagram.c
-!Enet/core/stream.c
-     </sect1>
-     <sect1><title>Socket Filter</title>
-!Enet/core/filter.c
-     </sect1>
-     <sect1><title>Generic Network Statistics</title>
-!Iinclude/linux/gen_stats.h
-!Enet/core/gen_stats.c
-!Enet/core/gen_estimator.c
-     </sect1>
-     <sect1><title>SUN RPC subsystem</title>
-<!-- The !D functionality is not perfect, garbage has to be protected by comments
-!Dnet/sunrpc/sunrpc_syms.c
--->
-!Enet/sunrpc/xdr.c
-!Enet/sunrpc/svc_xprt.c
-!Enet/sunrpc/xprt.c
-!Enet/sunrpc/sched.c
-!Enet/sunrpc/socklib.c
-!Enet/sunrpc/stats.c
-!Enet/sunrpc/rpc_pipe.c
-!Enet/sunrpc/rpcb_clnt.c
-!Enet/sunrpc/clnt.c
-     </sect1>
-  </chapter>
-
-  <chapter id="netdev">
-     <title>Network device support</title>
-     <sect1><title>Driver Support</title>
-!Enet/core/dev.c
-!Enet/ethernet/eth.c
-!Enet/sched/sch_generic.c
-!Iinclude/linux/etherdevice.h
-!Iinclude/linux/netdevice.h
-     </sect1>
-     <sect1><title>PHY Support</title>
-!Edrivers/net/phy/phy.c
-!Idrivers/net/phy/phy.c
-!Edrivers/net/phy/phy_device.c
-!Idrivers/net/phy/phy_device.c
-!Edrivers/net/phy/mdio_bus.c
-!Idrivers/net/phy/mdio_bus.c
-     </sect1>
-<!-- FIXME: Removed for now since no structured comments in source
-     <sect1><title>Wireless</title>
-X!Enet/core/wireless.c
-     </sect1>
--->
-     <sect1><title>Synchronous PPP</title>
-!Edrivers/net/wan/syncppp.c
-     </sect1>
-  </chapter>
-
   <chapter id="modload">
      <title>Module Support</title>
      <sect1><title>Module Loading</title>

^ permalink raw reply

* [PATCH 2.6.25] ixgbe/igb: correctly obtain protocol information on transmit
From: Andy Gospodarek @ 2008-02-12  1:35 UTC (permalink / raw)
  To: Auke Kok, netdev


I noticed a strange panic on a box with an ixgbe-based interface when
using bridging.  Protocol information ought to come from the frame
itself rather than anywhere else.

The igb driver has a similar coding error that would cause incorrect
flags to be set, so it would not panic.  It should still get fixed
though.

Checking the frame's payload on both drivers seems like a better
option.

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
---

 igb/igb_main.c     |    3 ++-
 ixgbe/ixgbe_main.c |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index f3c144d..5572923 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -2514,9 +2514,10 @@ static inline bool igb_tx_csum_adv(struct igb_adapter *adapter,
 		tu_cmd |= (E1000_TXD_CMD_DEXT | E1000_ADVTXD_DTYP_CTXT);
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
+			struct iphdr *iph = ip_hdr(skb);
 			if (skb->protocol == htons(ETH_P_IP))
 				tu_cmd |= E1000_ADVTXD_TUCMD_IPV4;
-			if (skb->sk && (skb->sk->sk_protocol == IPPROTO_TCP))
+			if (iph->protocol == IPPROTO_TCP)
 				tu_cmd |= E1000_ADVTXD_TUCMD_L4T_TCP;
 		}
 
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 3732dd6..4542870 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -2202,10 +2202,11 @@ static bool ixgbe_tx_csum(struct ixgbe_adapter *adapter,
 				    IXGBE_ADVTXD_DTYP_CTXT);
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
+			struct iphdr *iph = ip_hdr(skb);
 			if (skb->protocol == htons(ETH_P_IP))
 				type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV4;
 
-			if (skb->sk->sk_protocol == IPPROTO_TCP)
+			if (iph->protocol == IPPROTO_TCP)
 				type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_L4T_TCP;
 		}
 

^ permalink raw reply related

* [PATCH]: Remove IP_TOS privilege checks
From: David Miller @ 2008-02-12  2:03 UTC (permalink / raw)
  To: netdev


I've committing the following upstream, for the reasons outlined in
the commit message.

These DSCP value meanings are advisory and the kernel is not the
entity to enforce what values mean.  This change makes us behave
consistently with BSD and with our own IPV6 implementation, yet
without breaking existing setups.

[IPV4]: Remove IP_TOS setting privilege checks.

Various RFCs have all sorts of things to say about the CS field of the
DSCP value.  In particular they try to make the distinction between
values that should be used by "user applications" and things like
routing daemons.

This seems to have influenced the CAP_NET_ADMIN check which exists for
IP_TOS socket option settings, but in fact it has an off-by-one error
so it wasn't allowing CS5 which is meant for "user applications" as
well.

Further adding to the inconsistency and brokenness here, IPV6 does not
validate the DSCP values specified for the IPV6_TCLASS socket option.

The real actual uses of these TOS values are system specific in the
final analysis, and these RFC recommendations are just that, "a
recommendation".  In fact the standards very purposefully use
"SHOULD" and "SHOULD NOT" when describing how these values can be
used.

In the final analysis the only clean way to provide consistency here
is to remove the CAP_NET_ADMIN check.  The alternatives just don't
work out:

1) If we add the CAP_NET_ADMIN check to ipv6, this can break existing
   setups.

2) If we just fix the off-by-one error in the class comparison in
   IPV4, certain DSCP values can be used in IPV6 but not IPV4 by
   default.  So people will just ask for a sysctl asking to
   override that.

I checked several other freely available kernel trees and they
do not make any privilege checks in this area like we do.  For
the BSD stacks, this goes back all the way to Stevens Volume 2
and beyond.

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

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 754b0a5..de0572c 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -514,11 +514,6 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 			val &= ~3;
 			val |= inet->tos & 3;
 		}
-		if (IPTOS_PREC(val) >= IPTOS_PREC_CRITIC_ECP &&
-		    !capable(CAP_NET_ADMIN)) {
-			err = -EPERM;
-			break;
-		}
 		if (inet->tos != val) {
 			inet->tos = val;
 			sk->sk_priority = rt_tos2priority(val);

^ permalink raw reply related

* [PATCH] [RFC] Smack: unlabeled outgoing ambient packets - v2
From: Casey Schaufler @ 2008-02-12  0:00 UTC (permalink / raw)
  To: paul.moore; +Cc: netdev, linux-kernel

From: Casey Schaufler <casey@schaufler-ca.com>

Smack uses CIPSO labeling, but allows for unlabeled packets
by specifying an "ambient" label that is applied to incoming
unlabeled packets. Because the other end of the connection
may dislike IP options, and ssh is one know application that
behaves thus, it is prudent to respond in kind. This patch
changes the network labeling behavior such that an outgoing
packet that would be given a CIPSO label that matches the
ambient label is left unlabeled. An "unlbl" domain is added
and the netlabel defaulting mechanism invoked rather than
assuming that everything is CIPSO. Locking has been added
around changes to the ambient label as the mechanisms used
to do so are more involved.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

---

This patch differs significantly from the previous version.
I think that I am using the netlbl interfaces more appropriately,
Paul, please let me know if there's a better approach.

It's inconvenient that netlbl_sock_setattr frees the domain passed.
I see that it makes sense for SELinux with the way SELinux treats
secctx's, but Smack is more careful about memory usage and I have
to do what I consider a gratuitous kalloc because of this behavior.
Would you be open to a patch to change this if it included the SELinux
changes?

Thank you.

 security/smack/smack_lsm.c |   20 ++++++------
 security/smack/smackfs.c   |   54 +++++++++++++++++++++++++----------
 2 files changed, 49 insertions(+), 25 deletions(-)

diff -uprN -X linux-2.6.25-g0210-base//Documentation/dontdiff linux-2.6.25-g0210-base/security/smack/smackfs.c linux-2.6.25-g0210/security/smack/smackfs.c
--- linux-2.6.25-g0210-base/security/smack/smackfs.c	2008-02-10 19:30:47.000000000 -0800
+++ linux-2.6.25-g0210/security/smack/smackfs.c	2008-02-11 07:14:54.000000000 -0800
@@ -45,6 +45,7 @@ enum smk_inos {
  */
 static DEFINE_MUTEX(smack_list_lock);
 static DEFINE_MUTEX(smack_cipso_lock);
+static DEFINE_MUTEX(smack_ambient_lock);
 
 /*
  * This is the "ambient" label for network traffic.
@@ -363,6 +364,27 @@ void smk_cipso_doi(void)
 		       __func__, __LINE__, rc);
 }
 
+/**
+ * smk_unlbl_ambient - initialize the unlabeled domain
+ */
+void smk_unlbl_ambient(char *oldambient)
+{
+	int rc;
+	struct netlbl_audit audit_info;
+
+	if (oldambient != NULL) {
+		rc = netlbl_cfg_map_del(oldambient, &audit_info);
+		if (rc != 0)
+			printk(KERN_WARNING "%s:%d remove rc = %d\n",
+			       __func__, __LINE__, rc);
+	}
+
+	rc = netlbl_cfg_unlbl_add_map(smack_net_ambient, &audit_info);
+	if (rc != 0)
+		printk(KERN_WARNING "%s:%d add rc = %d\n",
+		       __func__, __LINE__, rc);
+}
+
 /*
  * Seq_file read operations for /smack/cipso
  */
@@ -709,7 +731,6 @@ static ssize_t smk_read_ambient(struct f
 				size_t cn, loff_t *ppos)
 {
 	ssize_t rc;
-	char out[SMK_LABELLEN];
 	int asize;
 
 	if (*ppos != 0)
@@ -717,23 +738,18 @@ static ssize_t smk_read_ambient(struct f
 	/*
 	 * Being careful to avoid a problem in the case where
 	 * smack_net_ambient gets changed in midstream.
-	 * Since smack_net_ambient is always set with a value
-	 * from the label list, including initially, and those
-	 * never get freed, the worst case is that the pointer
-	 * gets changed just after this strncpy, in which case
-	 * the value passed up is incorrect. Locking around
-	 * smack_net_ambient wouldn't be any better than this
-	 * copy scheme as by the time the caller got to look
-	 * at the ambient value it would have cleared the lock
-	 * and been changed.
 	 */
-	strncpy(out, smack_net_ambient, SMK_LABELLEN);
-	asize = strlen(out) + 1;
+	mutex_lock(&smack_ambient_lock);
 
-	if (cn < asize)
-		return -EINVAL;
+	asize = strlen(smack_net_ambient) + 1;
 
-	rc = simple_read_from_buffer(buf, cn, ppos, out, asize);
+	if (cn >= asize)
+		rc = simple_read_from_buffer(buf, cn, ppos,
+					     smack_net_ambient, asize);
+	else
+		rc = -EINVAL;
+
+	mutex_unlock(&smack_ambient_lock);
 
 	return rc;
 }
@@ -751,6 +767,7 @@ static ssize_t smk_write_ambient(struct 
 				 size_t count, loff_t *ppos)
 {
 	char in[SMK_LABELLEN];
+	char *oldambient;
 	char *smack;
 
 	if (!capable(CAP_MAC_ADMIN))
@@ -766,7 +783,13 @@ static ssize_t smk_write_ambient(struct 
 	if (smack == NULL)
 		return -EINVAL;
 
+	mutex_lock(&smack_ambient_lock);
+
+	oldambient = smack_net_ambient;
 	smack_net_ambient = smack;
+	smk_unlbl_ambient(oldambient);
+
+	mutex_unlock(&smack_ambient_lock);
 
 	return count;
 }
@@ -974,6 +997,7 @@ static int __init init_smk_fs(void)
 
 	sema_init(&smack_write_sem, 1);
 	smk_cipso_doi();
+	smk_unlbl_ambient(NULL);
 
 	return err;
 }
diff -uprN -X linux-2.6.25-g0210-base//Documentation/dontdiff linux-2.6.25-g0210-base/security/smack/smack_lsm.c linux-2.6.25-g0210/security/smack/smack_lsm.c
--- linux-2.6.25-g0210-base/security/smack/smack_lsm.c	2008-02-10 19:30:47.000000000 -0800
+++ linux-2.6.25-g0210/security/smack/smack_lsm.c	2008-02-10 20:10:47.000000000 -0800
@@ -1251,7 +1251,7 @@ static void smack_to_secattr(char *smack
 
 	switch (smack_net_nltype) {
 	case NETLBL_NLTYPE_CIPSOV4:
-		nlsp->domain = NULL;
+		nlsp->domain = kstrdup(smack, GFP_ATOMIC);
 		nlsp->flags = NETLBL_SECATTR_DOMAIN;
 		nlsp->flags |= NETLBL_SECATTR_MLS_LVL;
 
@@ -1276,21 +1276,21 @@ static void smack_to_secattr(char *smack
  * Convert the outbound smack value (smk_out) to a
  * secattr and attach it to the socket.
  *
- * Returns 0 on success or an error code
+ * The return from netlbl_sock_setattr is ignored because
+ * the defaulting label behavior provided by netlbl takes
+ * care of the error cases.
  */
-static int smack_netlabel(struct sock *sk)
+static void smack_netlabel(struct sock *sk)
 {
 	struct socket_smack *ssp = sk->sk_security;
 	struct netlbl_lsm_secattr secattr;
-	int rc = 0;
+	int rc;
 
 	netlbl_secattr_init(&secattr);
 	smack_to_secattr(ssp->smk_out, &secattr);
 	if (secattr.flags != NETLBL_SECATTR_NONE)
 		rc = netlbl_sock_setattr(sk, &secattr);
-
 	netlbl_secattr_destroy(&secattr);
-	return rc;
 }
 
 /**
@@ -1340,7 +1340,7 @@ static int smack_inode_setsecurity(struc
 		ssp->smk_in = sp;
 	else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) {
 		ssp->smk_out = sp;
-		return smack_netlabel(sock->sk);
+		smack_netlabel(sock->sk);
 	} else
 		return -EOPNOTSUPP;
 
@@ -1367,7 +1367,8 @@ static int smack_socket_post_create(stru
 	/*
 	 * Set the outbound netlbl.
 	 */
-	return smack_netlabel(sock->sk);
+	smack_netlabel(sock->sk);
+	return 0;
 }
 
 /**
@@ -2199,7 +2200,6 @@ static int smack_socket_getpeersec_dgram
 static void smack_sock_graft(struct sock *sk, struct socket *parent)
 {
 	struct socket_smack *ssp;
-	int rc;
 
 	if (sk == NULL)
 		return;
@@ -2212,7 +2212,7 @@ static void smack_sock_graft(struct sock
 	ssp->smk_out = current->security;
 	ssp->smk_packet[0] = '\0';
 
-	rc = smack_netlabel(sk);
+	smack_netlabel(sk);
 }
 
 /**



^ permalink raw reply

* Re: [PATCH 1/2] fib_trie: handle empty tree
From: David Miller @ 2008-02-12  5:14 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <20080211171250.60ad7bc5@extreme>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 11 Feb 2008 17:12:50 -0800

> To: David S. Miller <davem@davemloft.net>

When a user's name has "special" characters such as a "."  as mine
does, it must be surrounded by double-quotes or else it is a syntax
error by SMTP rules.  For this reason your patch postings were
rejected by vger.kernel.org

> This fixes possible problems when trie_firstleaf() returns NULL
> to trie_leafindex().
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied, thanks Stephen.

> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index f5fba3f..2d89527 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -1762,11 +1762,9 @@ static struct leaf *trie_leafindex(struct trie *t, int index)
>  {
>  	struct leaf *l = trie_firstleaf(t);
>  
> -	while (index-- > 0) {
> +	while (l && index-- > 0)
>  		l = trie_nextleaf(l);
> -		if (!l)
> -			break;
> -	}
> +
>  	return l;
>  }
>  
> -- 
> 1.5.3.8
> 

^ permalink raw reply

* Re: [PATCH 2/2] fib_trie: /proc/net/route performance improvement
From: David Miller @ 2008-02-12  5:14 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <20080211171659.28d96844@extreme>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 11 Feb 2008 17:16:59 -0800

> Use key/offset caching to change /proc/net/route (use by iputils route)
> from O(n^2) to O(n). This improves performance from 30sec with 160,000
> routes to 1sec.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied, thanks.

> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index 2d89527..1ff446d 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -2459,6 +2459,84 @@ static const struct file_operations fib_trie_fops = {
>  	.release = seq_release_net,
>  };
>  
> +struct fib_route_iter {
> +	struct seq_net_private p;
> +	struct trie *main_trie;
> +	loff_t	pos;
> +	t_key	key;
> +};
> +
> +static struct leaf *fib_route_get_idx(struct fib_route_iter *iter, loff_t pos)
> +{
> +	struct leaf *l = NULL;
> +	struct trie *t = iter->main_trie;
> +
> +	/* use cache location of last found key */
> +	if (iter->pos > 0 && pos >= iter->pos && (l = fib_find_node(t, iter->key)))
> +		pos -= iter->pos;
> +	else {
> +		iter->pos = 0;
> +		l = trie_firstleaf(t);
> +	}
> +
> +	while (l && pos-- > 0) {
> +		iter->pos++;
> +		l = trie_nextleaf(l);
> +	}
> +
> +	if (l)
> +		iter->key = pos;	/* remember it */
> +	else
> +		iter->pos = 0;		/* forget it */
> +
> +	return l;
> +}
> +
> +static void *fib_route_seq_start(struct seq_file *seq, loff_t *pos)
> +	__acquires(RCU)
> +{
> +	struct fib_route_iter *iter = seq->private;
> +	struct fib_table *tb;
> +
> +	rcu_read_lock();
> +	tb = fib_get_table(iter->p.net, RT_TABLE_MAIN);
> +	if (!tb)
> +		return NULL;
> +
> +	iter->main_trie = (struct trie *) tb->tb_data;
> +	if (*pos == 0)
> +		return SEQ_START_TOKEN;
> +	else
> +		return fib_route_get_idx(iter, *pos - 1);
> +}
> +
> +static void *fib_route_seq_next(struct seq_file *seq, void *v, loff_t *pos)
> +{
> +	struct fib_route_iter *iter = seq->private;
> +	struct leaf *l = v;
> +
> +	++*pos;
> +	if (v == SEQ_START_TOKEN) {
> +		iter->pos = 0;
> +		l = trie_firstleaf(iter->main_trie);
> +	} else {
> +		iter->pos++;
> +		l = trie_nextleaf(l);
> +	}
> +
> +	if (l)
> +		iter->key = l->key;
> +	else
> +		iter->pos = 0;
> +	return l;
> +}
> +
> +static void fib_route_seq_stop(struct seq_file *seq, void *v)
> +	__releases(RCU)
> +{
> +	rcu_read_unlock();
> +}
> +
>  static unsigned fib_flag_trans(int type, __be32 mask, const struct fib_info *fi)
>  {
>  	static unsigned type2flags[RTN_MAX + 1] = {
> @@ -2482,7 +2560,6 @@ static unsigned fib_flag_trans(int type, __be32 mask, const struct fib_info *fi)
>   */
>  static int fib_route_seq_show(struct seq_file *seq, void *v)
>  {
> -	const struct fib_trie_iter *iter = seq->private;
>  	struct leaf *l = v;
>  	struct leaf_info *li;
>  	struct hlist_node *node;
> @@ -2494,12 +2571,6 @@ static int fib_route_seq_show(struct seq_file *seq, void *v)
>  		return 0;
>  	}
>  
> -	if (iter->trie == iter->trie_local)
> -		return 0;
> -
> -	if (IS_TNODE(l))
> -		return 0;
> -
>  	hlist_for_each_entry_rcu(li, node, &l->list, hlist) {
>  		struct fib_alias *fa;
>  		__be32 mask, prefix;
> @@ -2542,16 +2613,16 @@ static int fib_route_seq_show(struct seq_file *seq, void *v)
>  }
>  
>  static const struct seq_operations fib_route_seq_ops = {
> -	.start  = fib_trie_seq_start,
> -	.next   = fib_trie_seq_next,
> -	.stop   = fib_trie_seq_stop,
> +	.start  = fib_route_seq_start,
> +	.next   = fib_route_seq_next,
> +	.stop   = fib_route_seq_stop,
>  	.show   = fib_route_seq_show,
>  };
>  
>  static int fib_route_seq_open(struct inode *inode, struct file *file)
>  {
>  	return seq_open_net(inode, file, &fib_route_seq_ops,
> -			    sizeof(struct fib_trie_iter));
> +			    sizeof(struct fib_route_iter));
>  }
>  
>  static const struct file_operations fib_route_fops = {
> -- 
> 1.5.3.8
> 

^ permalink raw reply

* Re: [PATCH] fib_trie: rcu_assign_pointer warning fix
From: David Miller @ 2008-02-12  5:15 UTC (permalink / raw)
  To: paulmck; +Cc: shemminger, netdev, linux-kernel
In-Reply-To: <20080212012741.GD29254@linux.vnet.ibm.com>

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date: Mon, 11 Feb 2008 17:27:41 -0800

> On Mon, Feb 11, 2008 at 04:59:54PM -0800, Stephen Hemminger wrote:
> > Eliminate warnings when rcu_assign_pointer is used with unsigned long.
> > It is reasonable to use RCU with non-pointer values so allow it for general
> > use.  Add a comment to explain the if test.
> 
> Good catch!!!  (An apologies for the hassle!!!)
> 
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Thanks for reviewing Paul, I'll apply this.

^ permalink raw reply

* Re: [PATCH v2][AX25] af_ax25: remove sock lock in ax25_info_show()
From: David Miller @ 2008-02-12  5:25 UTC (permalink / raw)
  To: jarkao2; +Cc: f6bvp, ralf, jann, netdev
In-Reply-To: <20080210131051.GA7750@ami.dom.local>

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Sun, 10 Feb 2008 14:10:51 +0100

> [AX25] af_ax25: remove sock lock in ax25_info_show()
>  
> This lockdep warning:
> 
> > =======================================================
> > [ INFO: possible circular locking dependency detected ]
> > 2.6.24 #3
> > -------------------------------------------------------
> > swapper/0 is trying to acquire lock:
> >  (ax25_list_lock){-+..}, at: [<f91dd3b1>] ax25_destroy_socket+0x171/0x1f0 [ax25]
> > 
> > but task is already holding lock:
> >  (slock-AF_AX25){-+..}, at: [<f91dbabc>] ax25_std_heartbeat_expiry+0x1c/0xe0 [ax25]
> > 
> > which lock already depends on the new lock.
> ...
> 
> shows that ax25_list_lock and slock-AF_AX25 are taken in different
> order: ax25_info_show() takes slock (bh_lock_sock(ax25->sk)) while
> ax25_list_lock is held, so reversely to other functions. To fix this
> the sock lock should be moved to ax25_info_start(), and there would
> be still problem with breaking ax25_list_lock (it seems this "proper"
> order isn't optimal yet). But, since it's only for reading proc info
> it seems this is not necessary (e.g.  ax25_send_to_raw() does similar
> reading without this lock too).
> 
> So, this patch removes sock lock to avoid deadlock possibility; there
> is also used sock_i_ino() function, which reads sk_socket under proper
> read lock. Additionally printf format of this i_ino is changed to %lu.
> 
> Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

Applied, thanks Jarek.

> +			   sock_i_ino(ax25->sk));

Note that this taks the sk callback lock, it should be OK but
let's keep a watch out for any new lockdep warnings this
ends up causing :-)

^ 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