* Re: [PATCH] USB VID/PID clash between pegasus and hci_usb drivers
From: Marcel Holtmann @ 2007-12-31 20:54 UTC (permalink / raw)
To: Chris Rankin; +Cc: netdev, gregkh
In-Reply-To: <520598.94496.qm@web52904.mail.re2.yahoo.com>
Hi Chris,
> As requested, here is the patch for the pegasus driver to resolve its VID/PID clash with the
> Belkin bluetooth adaptor.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: 2.6.24-rc6-mm1
From: Torsten Kaiser @ 2007-12-31 20:15 UTC (permalink / raw)
To: Herbert Xu
Cc: Andrew Morton, linux-kernel, Neil Brown, J. Bruce Fields, netdev,
Tom Tucker
In-Reply-To: <64bb37e0712291934o77a3d365h56c9c31ac8437469@mail.gmail.com>
On Dec 30, 2007 4:34 AM, Torsten Kaiser <just.for.lkml@googlemail.com> wrote:
> On Dec 30, 2007 2:30 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > On Sat, Dec 29, 2007 at 05:51:13PM +0100, Torsten Kaiser wrote:
> > >
> > > > > The cause, why I am resending this: I just got a crash with
> > > > > 2.6.24-rc6-mm1, again looking network related:
> > > > >
> > > > > [93436.933356] WARNING: at include/net/dst.h:165 dst_release()
> > > > > [93436.936685] Pid: 8079, comm: konqueror Not tainted 2.6.24-rc6-mm1 #11
> > > > > [93436.939292]
> > > > > [93436.939293] Call Trace:
> > > > > [93436.939304] [<ffffffff80531d2d>] skb_release_all+0xdd/0x110
> > > > > [93436.939307] [<ffffffff80531311>] __kfree_skb+0x11/0xa0
> > > > > [93436.939309] [<ffffffff805313b7>] kfree_skb+0x17/0x30
> > > > > [93436.939312] [<ffffffff805a0b48>] unix_release_sock+0x128/0x250
> > > > > [93436.939315] [<ffffffff805a0c91>] unix_release+0x21/0x30
> > > > > [93436.939318] [<ffffffff8052b144>] sock_release+0x24/0x90
> > > > > [93436.939320] [<ffffffff8052b656>] sock_close+0x26/0x50
> > > > > [93436.939324] [<ffffffff8029f921>] __fput+0xc1/0x230
> > > > > [93436.939327] [<ffffffff8029fe46>] fput+0x16/0x20
> > > > > [93436.939329] [<ffffffff8029c576>] filp_close+0x56/0x90
> > > > > [93436.939331] [<ffffffff8029de46>] sys_close+0xa6/0x110
> > > > > [93436.939335] [<ffffffff8020b57b>] system_call_after_swapgs+0x7b/0x80
> > >
> > > >From code inspection I would blame the patch "[SKBUFF]: Free old skb
> > > properly in skb_morph" from Herbert Xu. (CC added)
> >
> > I doubt it. skb_morph is only used on IP fragments so I don't see how
> > you could attribute an error from a Unix domain socket to this patch.
>
> That's why I wrote that I do not know much about the network core...
>
> > In any case, Unix socket packets should not have a dst at all so the
> > very fact that you're in that path means that you have some sort of
> > memory corruption.
>
> ... I did not know about the fact that there should not have been an dst.
>
> Its just that this warning was the first nice clue about the memory
> corruption related to networking that I see since 2.6.24-rc3-mm2.
> The time of the patch (Mon, 26 Nov 2007 15:11:19) even fits into the
> window between -rc3-mm1 and -rc3-mm2.
>
> I doubt that the memory corruption is a hardware problem, because the
> system in question is using ECC ram and I did not see any messages
> about corrected/detected errors.
>
> > Is this the very first OOPS/warning that you see? If not you should
> > ignore all but the very first one as that may have left your system
> > in an inconsistent state which may render all subsequent OOPSes and
> > warnings useless.
>
> I looked into the log in question and the only other warning was a
> circular locking dependency that lockdep detected around 1.5 hour
> before this warning.
>
> As reported in my original mail immeadeatly after the warning the
> system OOPSed and hang:
> [93436.947241] general protection fault: 0000 [1] SMP
> -> first OOPS
> [93436.947243] last sysfs file:
> /sys/devices/pci0000:00/0000:00:0f.0/0000:01:00.1/irq
> [93436.947245] CPU 1
> [93436.947246] Modules linked in: radeon drm nfsd exportfs w83792d
> ipv6 tuner tea5767 tda8290 tuner_xc2
> 028 tda9887 tuner_simple mt20xx tea5761 tvaudio msp3400 bttv ir_common
> compat_ioctl32 videobuf_dma_sg v
> ideobuf_core btcx_risc tveeprom usbhid videodev v4l2_common hid
> v4l1_compat pata_amd sg i2c_nforce2
> [93436.947257] Pid: 8079, comm: konqueror Not tainted 2.6.24-rc6-mm1 #11
> -> not tainted by a previous OOPS
>
> [93436.947259] RIP: 0010:[<ffffffff80531438>] [<ffffffff80531438>]
> skb_drop_list+0x18/0x30
> [93436.947262] RSP: 0018:ffff810005f4fda8 EFLAGS: 00010286
> [93436.947263] RAX: ab1ed5ca5b74e7de RBX: ab1ed5ca5b74e7de RCX: 000000000000d135
> [93436.947265] RDX: ffff81011d089a80 RSI: 0000000000000001 RDI: ffff81011d089a88
> [93436.947266] RBP: ffff810005f4fdb8 R08: 0000000000000001 R09: 0000000000000006
> [93436.947268] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8100de02c500
> [93436.947269] R13: ffff81011c188a00 R14: 0000000000000001 R15: ffff81011c189198
> [93436.947271] FS: 00007fb5bde0d700(0000) GS:ffff81007ff22000(0000)
> knlGS:0000000000000000
> [93436.947273] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [93436.947274] CR2: 00007fb5bdd76000 CR3: 00000000664d5000 CR4: 00000000000006e0
> [93436.947276] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [93436.947277] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [93436.947279] Process konqueror (pid: 8079, threadinfo
> ffff810005f4e000, task ffff8100a1dec000)
> [93436.947281] Stack: ffff810005f4fdd8 ffff810116c86140
> ffff810005f4fdd8 ffffffff805314ae
> [93436.947284] ffff810116c86140 ffff8100de02c500 ffff810005f4fdf8
> ffffffff80531cf0
> [93436.947286] ffff8100de02c500 ffff81011c188b48 ffff810005f4fe18
> ffffffff80531311
> [93436.947288] Call Trace:
> [93436.947290] [<ffffffff805314ae>] skb_release_data+0x5e/0xa0
> [93436.947293] [<ffffffff80531cf0>] skb_release_all+0xa0/0x110
> [93436.947295] [<ffffffff80531311>] __kfree_skb+0x11/0xa0
> [93436.947297] [<ffffffff805313b7>] kfree_skb+0x17/0x30
> [93436.947299] [<ffffffff805a0b48>] unix_release_sock+0x128/0x250
> [93436.947302] [<ffffffff805a0c91>] unix_release+0x21/0x30
> [93436.947304] [<ffffffff8052b144>] sock_release+0x24/0x90
> [93436.947307] [<ffffffff8052b656>] sock_close+0x26/0x50
> [93436.947309] [<ffffffff8029f921>] __fput+0xc1/0x230
> [93436.947312] [<ffffffff8029fe46>] fput+0x16/0x20
> [93436.947314] [<ffffffff8029c576>] filp_close+0x56/0x90
> [93436.947316] [<ffffffff8029de46>] sys_close+0xa6/0x110
> [93436.947319] [<ffffffff8020b57b>] system_call_after_swapgs+0x7b/0x80
> [93436.947322]
> [93436.947322]
> [93436.947323] Code: 48 8b 18 48 89 c7 e8 5d ff ff ff 48 85 db 75 ed 48 83 c4 08
> [93436.947328] RIP [<ffffffff80531438>] skb_drop_list+0x18/0x30
> [93436.947330] RSP <ffff810005f4fda8>
> [93436.947332] ---[ end trace befb7cc3528ab3b1 ]---
>
> Your patch just fit so "good" to my problems:
> * it had the correct time frame for 2.6.24-rc3-mm2
> * it looked guilty at changing the refcounting of __refcnt because of
> the added dst_release()
> * it added other release / freeing operations so that a use-after-free
> memory corruption seemed possible
>
> I just have no better idea to what caused this OOPS and the other
> hangs in -rc3-mm2.
After testing the patch from http://lkml.org/lkml/2007/12/30/210 the
system hung again after building ~10 packages from the last kde4
release candidate. (see other mail)
I then tried to "fix" it with this suspect.
I changed "skb_release_all(dst);" back to "skb_release_data(dst);" in
skb_morph() (net/core/skbuff.c).
I'm now at 205 of 210 packages completed without a further hang. I
also do not see an obvious memory leak.
(All of these tests where done on 2.6.24-rc3-mm2, as I'm relative
sure, that doing these compiles will trigger the error on that kernel
version)
Torsten
^ permalink raw reply
* Re: 2.6.24-rc6-mm1 - git-lblnet.patch and networking horkage
From: Paul Moore @ 2007-12-31 20:06 UTC (permalink / raw)
To: James Morris; +Cc: Valdis.Kletnieks, akpm, linux-kernel, sds, netdev
In-Reply-To: <200712311213.32515.paul.moore@hp.com>
On Monday 31 December 2007 12:13:32 pm Paul Moore wrote:
> On Wednesday 26 December 2007 4:52:03 pm James Morris wrote:
> > On Thu, 26 Dec 2007, Paul Moore wrote:
> > > As James said I'm away right now and computer access is limited.
> > > However, I'm stuck in the airport right now and spent some time looking
> > > at the code ... Based on what has been found so far I wonder if the
> > > problem isn't a race but a problem of skb->iif never being initialized
> > > correctly? To my untrained eye it looks like __netdev_alloc_skb()
> > > should be setting skb->iif (like it does for skb->dev) but it currently
> > > doesn't.
> >
> > ->iif will be zeroed during skb allocation, then set during
> > netif_receive_skb().
>
> I was able to reproduce this bug this morning by running avahi as James did
> and did a little more digging. I don't have a fix yet, but thought I would
> pass along what I've found in case this triggers a moment of clarity to
> someone out there ...
>
> The skb->iif value appears to be messed up as early as netif_receive_skb(),
> in my case it is set to 196611 (trust me, I don't have that many interfaces
> in my test machine) which causes the ->iif initialization code in
> netif_receive_skb() to be skipped because ->iif is greater than zero. This
> particular packet is locally generated and locally consumed.
>
> Hopefully I'll have a fix later this afternoon but if someone has a bright
> idea I'd love to hear it ...
[NOTE: I added netdev to this thread to gather some input. @netdev folks, the
problem is that the skb->iif field contains garbage in some cases which is
causing problems for some new SELinux network code. The exact problem
probably isn't too important for this discussion, what is important is that
the skb->iif field contains a non-zero garbage value some of the time on
incoming packets.]
I'm pretty certain this is an uninitialized value problem now and not a
use-after-free issue. The invalid/garbage ->iif value seems to only happen
on packets that are generated locally and sent back into the stack for local
consumption, e.g. loopback. These local packets also need to have been
cloned at some point, either on the output or input path.
The problem appears to be a skb_clone() function which does not clear the skb
structure properly and fails to copy the ->iif value from the original skb to
the cloned skb. From what I can tell, there are two possible solutions to
this problem:
1. Clear all of the cloned skb fields in skb_clone() via memset()
2. Copy the ->iif field in __copy_skb_header()
I don't have a good enough understanding of all the details involving skb
memory management to know if option #1 is a Good Idea or not, but option #2
seems much simpler and solves the problem of garbage in the ->iif field. My
preference is to go with option #2 but before I submit a patch does anyone
think this is the wrong solution?
--
paul moore
linux security @ hp
^ permalink raw reply
* Re: [PATCH] Force UNIX domain sockets to be built in
From: Al Viro @ 2007-12-31 19:38 UTC (permalink / raw)
To: Bodo Eggert; +Cc: David Miller, jengelh, devzero, linux-kernel, bunk, netdev
In-Reply-To: <alpine.LSU.0.999.0712311426520.3761@be1.lrz>
On Mon, Dec 31, 2007 at 03:03:20PM +0100, Bodo Eggert wrote:
> On Mon, 31 Dec 2007, David Miller wrote:
> > From: Bodo Eggert <7eggert@gmx.de>
>
> > > As suggested by Adrian Bunk, UNIX domain sockets should always be built in
> > > on normal systems. This is especially true since udev needs these sockets
> > > and fails to run if UNIX=m.
> > >
> > > Signed-Off-By: Bodo Eggert <7eggert@gmx.de>
> >
> > People who use udev can make sure they have it built into their kernel
> > if they have such a dependency.
> >
> > Not everyone uses udev, and therefore needs AF_UNIX non-modular.
>
> That's why I kept this option for embedded folks.
>
> Is there any benefit for non-embedded systems from having UNIX=m?
udev-free != embedded.
^ permalink raw reply
* Re: [usb regression] Re: [PATCH 2.6.24-rc3] Fix /proc/net breakage
From: Greg KH @ 2007-12-31 19:26 UTC (permalink / raw)
To: Alan Stern
Cc: Andreas Mohr, Ingo Molnar, Alexey Dobriyan, Andrew Morton,
David Woodhouse, Eric W. Biederman, Linus Torvalds,
Rafael J. Wysocki, Pavel Machek, kernel list, netdev,
Pavel Emelyanov, Denis V. Lunev, USB list
In-Reply-To: <Pine.LNX.4.44L0.0712311237150.7182-100000@iolanthe.rowland.org>
On Mon, Dec 31, 2007 at 12:49:52PM -0500, Alan Stern wrote:
> On Sun, 30 Dec 2007, Greg KH wrote:
>
> > > It looks like Greg misused the debugfs API -- which is ironic, because
> > > he wrote debugfs in the first place! :-)
> >
> > Oh crap, sorry, I did mess that up :(
> >
> > > Let me know if this patch fixes the problem. If it does, I'll submit
> > > it to Greg with all the proper accoutrements.
> >
> > This isn't going to work if CONFIG_DEBUGFS is not enabled either :(
>
> Why not? The debugging files won't be created, true, but the driver
> should work just fine regardless. This is exactly the way uhci-hcd
> behaves, and it hasn't caused any problems.
Ok, you are right, it was late and after some wine and I shouldn't have
been responding to email :)
I'll rework this, and send out a patch tonight or tomorrow when I get a
chance (am on the road right now). We should only care about NULL being
returned here, that's the only "real" error.
And in thinking about it some more, I should go audit all the debugfs
callers to make sure they are doing something sane too, it shouldn't be
this complex at all...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] [IPROUTE2] IPT action compatibility with iptables 1.4.0
From: Stephen Hemminger @ 2007-12-31 19:16 UTC (permalink / raw)
To: hadi; +Cc: Denys Fedoryshchenko, netdev, Pablo Neira Ayuso
In-Reply-To: <1198515305.4427.12.camel@localhost>
On Mon, 24 Dec 2007 11:55:05 -0500
jamal <hadi@cyberus.ca> wrote:
>
> Stephen,
>
> Please apply this patch from Denys Fedoryshchenko to make the ipt action
> work with latest iptables.
>
> cheers,
> jamal
>
applied and pushed
--
Stephen Hemminger <stephen.hemminger@vyatta.com>
^ permalink raw reply
* Re: [PATCH] Force UNIX domain sockets to be built in
From: Michael Buesch @ 2007-12-31 19:05 UTC (permalink / raw)
To: Torsten Kaiser
Cc: Alan Cox, Adrian Bunk, Bodo Eggert, Jan Engelhardt, devzero,
linux-kernel, netdev
In-Reply-To: <64bb37e0712311037w42bae43ayb3c10b2e0310586d@mail.gmail.com>
On Monday 31 December 2007 19:37:43 Torsten Kaiser wrote:
> The base problem is that there already are many options to break
> external modules. (CONFIG_MODULES=n ;) )
Exactly. There already are enough ways to break external modules.
No need to introduce more. ;)
> The question I can't answer in this context is: Do distributions want
> to support external modules?
> Only if yes, your argument is valid. But then they could just disable
> this feature and prevent this kind of bugreports.
That's my point. Nobody will risk bugs to save a few bytes of memory.
--
Greetings Michael.
^ permalink raw reply
* Re: [PATCH 0/4] New interface for memory accounting (take 1)
From: Hideo AOKI @ 2007-12-31 19:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, herbert, vladislav.yasevich, netdev,
lksctp-developers, tyasui, mhiramat, satoshi.oshima.fk, billfink,
andi, johnpol, shemminger, yoshfuji, yumiko.sugita.yf, haoki
In-Reply-To: <47790811.9080004@cosmosbay.com>
Eric Dumazet wrote:
> Hi David
>
> Could you add the following patch, because it apparently was lost
> during the battle :)
>
> Thank you
>
> [PATCH] use SK_MEM_QUANTUM_SHIFT in __sk_mem_reclaim()
>
> Avoid an expensive divide (as done in commit
> 18030477e70a826b91608aee40a987bbd368fec6 but lost in commit
> 23821d2653111d20e75472c8c5003df1a55309a8)
Hello Eric,
Thank you for catching this. I'm sorry about the lost.
Best regards,
Hideo
--
Hitachi Computer Products (America) Inc.
^ permalink raw reply
* Re: [PATCH 2/3] [UDP]: memory accounting in IPv4
From: Eric Dumazet @ 2007-12-31 18:58 UTC (permalink / raw)
To: Hideo AOKI
Cc: David Miller, Herbert Xu, netdev, Takahiro Yasui,
Masami Hiramatsu, Satoshi Oshima, billfink, Andi Kleen,
Evgeniy Polyakov, Stephen Hemminger, yoshfuji, Yumiko Sugita
In-Reply-To: <47793836.10407@redhat.com>
Hideo AOKI a écrit :
> Eric Dumazet wrote:
>> Hideo AOKI a écrit :
>>> This patch adds UDP memory usage accounting in IPv4. Currently,
>>> receiving buffer accounting is only supported.
>>>
>>> This patch is also introduced memory_allocated variable for UDP protocol.
>> OK, but sockstat_seq_show() should be updated so that this
>> udp_memory_allocated value can be read from /proc/net/sockstat ?
>
> Yes, and I updated it in the first patch of the patch set.
>
> I thought dividing patch made easy to review. But I was wrong.
> Sorry about that.
No problem Hideo, I think you did a good job with this UDP memory accounting
and were very patient.
Thank you
^ permalink raw reply
* Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min
From: Hideo AOKI @ 2007-12-31 18:58 UTC (permalink / raw)
To: David Miller
Cc: herbert, netdev, tyasui, mhiramat, satoshi.oshima.fk, billfink,
andi, johnpol, shemminger, yoshfuji, yumiko.sugita.yf, haoki
In-Reply-To: <20071231.001925.151533664.davem@davemloft.net>
David Miller wrote:
> From: Hideo AOKI <haoki@redhat.com>
> Date: Sun, 30 Dec 2007 04:01:46 -0500
>
>> diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
>> --- net-2.6.25-t12t19m-p4/net/ipv4/proc.c 2007-12-27 10:19:02.000000000 -0500
>> +++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c 2007-12-29 21:09:21.000000000 -0500
>> @@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
>> sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
>> tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
>> atomic_read(&tcp_memory_allocated));
>> - seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
>> + seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
>> + atomic_read(&udp_memory_allocated));
>> seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
>> seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
>> seq_printf(seq, "FRAG: inuse %d memory %d\n",
>
> More careless patch creation. :-/
>
> This breaks the build because udp_memory_allocated is not added until
> patch 2.
Hello David,
You are absolutely right.
> Once again I'll combine all three patches into one but I am extremely
> angry about how careless and broken these two patch submissions were.
I really apologize for wasting your time twice.
I do not do this kind mess again.
Many thanks for your support.
Best regards,
Hideo
--
Hitachi Computer Products (America) Inc.
^ permalink raw reply
* Re: [PATCH] net: santize headers for iproute2
From: Stephen Hemminger @ 2007-12-31 18:55 UTC (permalink / raw)
To: David Miller; +Cc: vgusev, netdev
In-Reply-To: <20071229.192212.149111967.davem@davemloft.net>
The Glib 2.7 version of netinet/tcp.h works for building iproute2,
but since iproute needs to build on older versions of glibc;
for now, I'll just put a corrected version of netinet/tcp.h in the iproute2
build tree.
^ permalink raw reply
* Re: [PATCH 2/4] [CORE]: adding memory accounting points
From: Hideo AOKI @ 2007-12-31 18:52 UTC (permalink / raw)
To: David Miller
Cc: herbert, vladislav.yasevich, netdev, lksctp-developers, tyasui,
mhiramat, satoshi.oshima.fk, billfink, andi, johnpol, shemminger,
yoshfuji, yumiko.sugita.yf, haoki
In-Reply-To: <20071230.235806.171376126.davem@davemloft.net>
David Miller wrote:
> This patch would not apply, because is contained changes
> present in the first patch, specifically:
<snip>
> And now I see exactly what you did, and it is quite careless.
>
> You wrote one big patch then tried to split it up by hand. This
> proves to me that you did not test the patches individually. Even
> worse, you did not even try to apply each patch nor compile the tree
> each step along the way as a basic sanity check.
Hello David,
You are right. Since I felt the patch was big, I divided
into three for review. And I mistook during the dividing.
> This wastes a lot of my time, as well as the time of other developers
> who might want to try out and test your changes.
I apologize for wasting your time.
> I will fix it up this time, but please do not ever do this again.
I really appreciate the fix. And I understood this.
Best regards,
Hideo
--
Hitachi Computer Products (America) Inc.
^ permalink raw reply
* Re: [PATCH 0/4] New interface for memory accounting (take 1)
From: Hideo AOKI @ 2007-12-31 18:46 UTC (permalink / raw)
To: David Miller
Cc: herbert, vladislav.yasevich, netdev, lksctp-developers, tyasui,
mhiramat, satoshi.oshima.fk, billfink, andi, johnpol, shemminger,
yoshfuji, yumiko.sugita.yf, haoki
In-Reply-To: <20071230.233413.117211235.davem@davemloft.net>
Hello David,
David Miller wrote:
> From: Hideo AOKI <haoki@redhat.com>
> Date: Sun, 30 Dec 2007 03:47:33 -0500
>
>> The patch set consists of the following 4 patches.
>>
>> [1/4] introducing new memory accounting interface
>> [2/4] adding memory accounting points to consolidate functions
>> [3/4] updating TCP to use new interface
>> [4/4] updating SCTP to use new interface
>
> I like this work very much and will add this to net-2.6.25
> But I will have to combine it all into one patch.
>
> You cannot have one patch which breaks the build in any way. All of
> the kernel must build properly after each patch in your patchset is
> applied.
>
> Since patch 1 renames all of the sk_stream_*() functions, TCP and SCTP
> stop building.
That's correct.
> We enforce this rule, otherwise when users try to use "git bisect" to
> find out where regressions are added, they will get stuck in places
> like this where the tree will not build due to such careless
> changesets.
Thank you for your explanation. To be honest, I didn't know the rule
exactly. I am so sorry for sending inconvenient patch set.
Best regards,
Hideo
--
Hitachi Computer Products (America) Inc.
^ permalink raw reply
* Re: [PATCH 2/3] [UDP]: memory accounting in IPv4
From: Hideo AOKI @ 2007-12-31 18:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Herbert Xu, netdev, Takahiro Yasui,
Masami Hiramatsu, Satoshi Oshima, billfink, Andi Kleen,
Evgeniy Polyakov, Stephen Hemminger, yoshfuji, Yumiko Sugita,
haoki
In-Reply-To: <477764B4.2030200@cosmosbay.com>
Eric Dumazet wrote:
> Hideo AOKI a écrit :
>> This patch adds UDP memory usage accounting in IPv4. Currently,
>> receiving buffer accounting is only supported.
>>
>> This patch is also introduced memory_allocated variable for UDP protocol.
>
> OK, but sockstat_seq_show() should be updated so that this
> udp_memory_allocated value can be read from /proc/net/sockstat ?
Yes, and I updated it in the first patch of the patch set.
I thought dividing patch made easy to review. But I was wrong.
Sorry about that.
Regards,
Hideo
--
Hitachi Computer Products (America) Inc.
^ permalink raw reply
* Re: [PATCH] Force UNIX domain sockets to be built in
From: Torsten Kaiser @ 2007-12-31 18:37 UTC (permalink / raw)
To: Michael Buesch
Cc: Alan Cox, Adrian Bunk, Bodo Eggert, Jan Engelhardt, devzero,
linux-kernel, netdev
In-Reply-To: <200712311818.12825.mb@bu3sch.de>
On Dec 31, 2007 6:18 PM, Michael Buesch <mb@bu3sch.de> wrote:
> On Monday 31 December 2007 17:38:03 Alan Cox wrote:
> > On Mon, 31 Dec 2007 17:17:19 +0100
> > "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> >
> > > a) this could be disabled during development if you want this
> > > b) this would even only affect development if you add new code that
> > > now needs a EXPORT_SYMBOL that was removed on an earlier build. And
> > > right now this would also need to trigger a rerun of depmod. And the
> > > same trigger could redo this garbage collect.
> > >
> > > Or am I missing something obvious?
> >
> > Development is not a phase seperate from use or distribution. A lot of
> > module testers for distributions will not be compiling their own modules
> > but loading in ones to test provided by their vendor - which may of
> > course then need different ksyms
I understand that point.
I just always assumed that kernel tests meant 'please test this patch'
and doing the compile yourself.
But I'm not convinced be the following:
> As an example, the whole purpose wireless-compat package is
> to load latest bleeding edge wireless stuff into a distribution kernel.
> So people are not required to recompile their kernels for using
> drivers that support their hardware.
> And guess what, it is used a _lot_. And lots of bugs are found with it.
> It increases our testing community a lot.
This looks more like a "regular" out-of-tree module for the purpose of
the suggested symbol garbage collector.
And for that case I already a 'don't use it then'-note.
The base problem is that there already are many options to break
external modules. (CONFIG_MODULES=n ;) )
Or in the case of this wireless module: CONFIG_CRYPTO_ARC4=n
(Without 'arc4' ieee80211_wep_init() will fail, that will fail
ieee80211_register_hw() and so no mac80211 driver could be loaded)
> So, all this wouldn't work, if kernel symbols could randomly get
> nuked by some "garbage collector".
>
> In practice, no distribution would use symbol garbage collection, as the
> only benefit from it would be an increased level of bugreports.
The question I can't answer in this context is: Do distributions want
to support external modules?
Only if yes, your argument is valid. But then they could just disable
this feature and prevent this kind of bugreports.
Torsten
^ permalink raw reply
* Re: [PATCH] Force UNIX domain sockets to be built in
From: Adrian Bunk @ 2007-12-31 18:05 UTC (permalink / raw)
To: Bodo Eggert; +Cc: Jan Engelhardt, devzero, linux-kernel, netdev
In-Reply-To: <alpine.LSU.0.999.0712311553530.4211@be1.lrz>
On Mon, Dec 31, 2007 at 04:19:23PM +0100, Bodo Eggert wrote:
> On Mon, 31 Dec 2007, Adrian Bunk wrote:
> > On Mon, Dec 31, 2007 at 02:26:42PM +0100, Bodo Eggert wrote:
> > > On Mon, 31 Dec 2007, Adrian Bunk wrote:
> > > > On Mon, Dec 31, 2007 at 01:09:43PM +0100, Bodo Eggert wrote:
>
> > > > > As suggested by Adrian Bunk, UNIX domain sockets should always be built in
> > > > > on normal systems. This is especially true since udev needs these sockets
> > > > > and fails to run if UNIX=m.
> > > > >
> > > > > Signed-Off-By: Bodo Eggert <7eggert@gmx.de>
> > > > >
> > > > > ---
> > > > > Last minute change: I decided against making it a bool because embedded
> > > > > folks might depend on a small kernel image. Edited in the patch below.
> > > > >...
> > > >
> > > > Is this just a purely theoretical thought or is this a reasonable use
> > > > case people actually use in practice?
> > >
> > > For now, it's a theoretical thought, but having an embedded device, I can
> > > see the reason for $EVERYTHING=m there.
> >
> > The only advantage I see is that the kernel image you have to flash
> > can be made smaller - with the disadvantage that the running kernel
> > is bigger by more than 10%.
> >
> > If you don't believe me, try it yourself:
> > Build all drivers statically into your kernel, and then compare the
> > vmlinux sizes with CONFIG_MODULES=n and CONFIG_MODULES=y.
>
> If you'd aim for a small kernel image, you would build anything as a module
> that is not requred for booting.
>...
Small kernel image to flash (e.g. due to a size limit there), if you
would aim for a small running kernel you wouldn't use modules.
And I'm not so sure about the image size without seeing actual numbers,
e.g. for your device.
Especially comparing your all-modular approach with a CONFIG_MODULES=n
kernel using Denys' section garbage collection patches in both cases
would be interesting.
I'm not claiming you are wrong, all I say is that I'm not convinced
without actual numbers that you are right.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* Re: [patch 7/7] netxen: fix byte-swapping in tx and rx
From: Dhananjay Phadke @ 2007-12-31 18:08 UTC (permalink / raw)
To: netdev; +Cc: Al Viro, jeff
In-Reply-To: <20071226235334.GF27894@ZenIV.linux.org.uk>
Here's the reworked patch.
This cleans up some unnecessary byte-swapping while setting up tx and
interpreting rx desc. The 64 bit rx status data should be converted
to host endian format only once and the macros just need to extract
bitfields.
This saves a spate of interrupts on pseries blades caused by buggy
(non) processing rx status ring.
Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Index: netdev-2.6/drivers/net/netxen/netxen_nic.h
===================================================================
--- netdev-2.6.orig/drivers/net/netxen/netxen_nic.h
+++ netdev-2.6/drivers/net/netxen/netxen_nic.h
@@ -309,23 +309,26 @@ struct netxen_ring_ctx {
((cmd_desc)->port_ctxid |= ((var) & 0xF0))
#define netxen_set_cmd_desc_flags(cmd_desc, val) \
- ((cmd_desc)->flags_opcode &= ~cpu_to_le16(0x7f), \
- (cmd_desc)->flags_opcode |= cpu_to_le16((val) & 0x7f))
+ (cmd_desc)->flags_opcode = ((cmd_desc)->flags_opcode & \
+ ~cpu_to_le16(0x7f)) | cpu_to_le16((val) & 0x7f)
#define netxen_set_cmd_desc_opcode(cmd_desc, val) \
- ((cmd_desc)->flags_opcode &= ~cpu_to_le16(0x3f<<7), \
- (cmd_desc)->flags_opcode |= cpu_to_le16(((val & 0x3f)<<7)))
+ (cmd_desc)->flags_opcode = ((cmd_desc)->flags_opcode & \
+ ~cpu_to_le16((u16)0x3f << 7)) | cpu_to_le16(((val) & 0x3f) << 7)
#define netxen_set_cmd_desc_num_of_buff(cmd_desc, val) \
- ((cmd_desc)->num_of_buffers_total_length &= ~cpu_to_le32(0xff), \
- (cmd_desc)->num_of_buffers_total_length |= cpu_to_le32((val) & 0xff))
+ (cmd_desc)->num_of_buffers_total_length = \
+ ((cmd_desc)->num_of_buffers_total_length & \
+ ~cpu_to_le32(0xff)) | cpu_to_le32((val) & 0xff)
#define netxen_set_cmd_desc_totallength(cmd_desc, val) \
- ((cmd_desc)->num_of_buffers_total_length &= ~cpu_to_le32(0xffffff00), \
- (cmd_desc)->num_of_buffers_total_length |= cpu_to_le32(val << 8))
+ (cmd_desc)->num_of_buffers_total_length = \
+ ((cmd_desc)->num_of_buffers_total_length & \
+ ~cpu_to_le32((u32)0xffffff << 8)) | \
+ cpu_to_le32(((val) & 0xffffff) << 8)
#define netxen_get_cmd_desc_opcode(cmd_desc) \
- ((le16_to_cpu((cmd_desc)->flags_opcode) >> 7) & 0x003F)
+ ((le16_to_cpu((cmd_desc)->flags_opcode) >> 7) & 0x003f)
#define netxen_get_cmd_desc_totallength(cmd_desc) \
- (le32_to_cpu((cmd_desc)->num_of_buffers_total_length) >> 8)
+ ((le32_to_cpu((cmd_desc)->num_of_buffers_total_length) >> 8) & 0xffffff)
struct cmd_desc_type0 {
u8 tcp_hdr_offset; /* For LSO only */
@@ -412,29 +415,29 @@ struct rcv_desc {
#define netxen_get_sts_desc_lro_last_frag(status_desc) \
(((status_desc)->lro & 0x80) >> 7)
-#define netxen_get_sts_port(status_desc) \
- (le64_to_cpu((status_desc)->status_desc_data) & 0x0F)
-#define netxen_get_sts_status(status_desc) \
- ((le64_to_cpu((status_desc)->status_desc_data) >> 4) & 0x0F)
-#define netxen_get_sts_type(status_desc) \
- ((le64_to_cpu((status_desc)->status_desc_data) >> 8) & 0x0F)
-#define netxen_get_sts_totallength(status_desc) \
- ((le64_to_cpu((status_desc)->status_desc_data) >> 12) & 0xFFFF)
-#define netxen_get_sts_refhandle(status_desc) \
- ((le64_to_cpu((status_desc)->status_desc_data) >> 28) & 0xFFFF)
-#define netxen_get_sts_prot(status_desc) \
- ((le64_to_cpu((status_desc)->status_desc_data) >> 44) & 0x0F)
+#define netxen_get_sts_port(sts_data) \
+ ((sts_data) & 0x0F)
+#define netxen_get_sts_status(sts_data) \
+ (((sts_data) >> 4) & 0x0F)
+#define netxen_get_sts_type(sts_data) \
+ (((sts_data) >> 8) & 0x0F)
+#define netxen_get_sts_totallength(sts_data) \
+ (((sts_data) >> 12) & 0xFFFF)
+#define netxen_get_sts_refhandle(sts_data) \
+ (((sts_data) >> 28) & 0xFFFF)
+#define netxen_get_sts_prot(sts_data) \
+ (((sts_data) >> 44) & 0x0F)
+#define netxen_get_sts_opcode(sts_data) \
+ (((sts_data) >> 58) & 0x03F)
+
#define netxen_get_sts_owner(status_desc) \
((le64_to_cpu((status_desc)->status_desc_data) >> 56) & 0x03)
-#define netxen_get_sts_opcode(status_desc) \
- ((le64_to_cpu((status_desc)->status_desc_data) >> 58) & 0x03F)
-
-#define netxen_clear_sts_owner(status_desc) \
- ((status_desc)->status_desc_data &= \
- ~cpu_to_le64(((unsigned long long)3) << 56 ))
-#define netxen_set_sts_owner(status_desc, val) \
- ((status_desc)->status_desc_data |= \
- cpu_to_le64(((unsigned long long)((val) & 0x3)) << 56 ))
+#define netxen_set_sts_owner(status_desc, val) { \
+ (status_desc)->status_desc_data = \
+ ((status_desc)->status_desc_data & \
+ ~cpu_to_le64(0x3ULL << 56)) | \
+ cpu_to_le64((u64)((val) & 0x3) << 56); \
+}
struct status_desc {
/* Bit pattern: 0-3 port, 4-7 status, 8-11 type, 12-27 total_length
Index: netdev-2.6/drivers/net/netxen/netxen_nic_init.c
===================================================================
--- netdev-2.6.orig/drivers/net/netxen/netxen_nic_init.c
+++ netdev-2.6/drivers/net/netxen/netxen_nic_init.c
@@ -1053,16 +1053,17 @@ static void netxen_process_rcv(struct ne
{
struct pci_dev *pdev = adapter->pdev;
struct net_device *netdev = adapter->netdev;
- int index = netxen_get_sts_refhandle(desc);
+ u64 sts_data = le64_to_cpu(desc->status_desc_data);
+ int index = netxen_get_sts_refhandle(sts_data);
struct netxen_recv_context *recv_ctx = &(adapter->recv_ctx[ctxid]);
struct netxen_rx_buffer *buffer;
struct sk_buff *skb;
- u32 length = netxen_get_sts_totallength(desc);
+ u32 length = netxen_get_sts_totallength(sts_data);
u32 desc_ctx;
struct netxen_rcv_desc_ctx *rcv_desc;
int ret;
- desc_ctx = netxen_get_sts_type(desc);
+ desc_ctx = netxen_get_sts_type(sts_data);
if (unlikely(desc_ctx >= NUM_RCV_DESC_RINGS)) {
printk("%s: %s Bad Rcv descriptor ring\n",
netxen_nic_driver_name, netdev->name);
@@ -1102,7 +1103,7 @@ static void netxen_process_rcv(struct ne
skb = (struct sk_buff *)buffer->skb;
if (likely(adapter->rx_csum &&
- netxen_get_sts_status(desc) == STATUS_CKSUM_OK)) {
+ netxen_get_sts_status(sts_data) == STATUS_CKSUM_OK)) {
adapter->stats.csummed++;
skb->ip_summed = CHECKSUM_UNNECESSARY;
} else
@@ -1191,7 +1192,6 @@ u32 netxen_process_rcv_ring(struct netxe
break;
}
netxen_process_rcv(adapter, ctxid, desc);
- netxen_clear_sts_owner(desc);
netxen_set_sts_owner(desc, STATUS_OWNER_PHANTOM);
consumer = (consumer + 1) & (adapter->max_rx_desc_count - 1);
count++;
Index: netdev-2.6/drivers/net/netxen/netxen_nic_main.c
===================================================================
--- netdev-2.6.orig/drivers/net/netxen/netxen_nic_main.c
+++ netdev-2.6/drivers/net/netxen/netxen_nic_main.c
@@ -1144,16 +1144,8 @@ static int netxen_nic_xmit_frame(struct
}
}
- i = netxen_get_cmd_desc_totallength(&hw->cmd_desc_head[saved_producer]);
-
- hw->cmd_desc_head[saved_producer].flags_opcode =
- cpu_to_le16(hw->cmd_desc_head[saved_producer].flags_opcode);
- hw->cmd_desc_head[saved_producer].num_of_buffers_total_length =
- cpu_to_le32(hw->cmd_desc_head[saved_producer].
- num_of_buffers_total_length);
-
spin_lock_bh(&adapter->tx_lock);
- adapter->stats.txbytes += i;
+ adapter->stats.txbytes += skb->len;
/* Code to update the adapter considering how many producer threads
are currently working */
^ permalink raw reply
* Re: [PATCH] ip[6]_tables.c: remove some inlines
From: Denys Vlasenko @ 2007-12-31 17:55 UTC (permalink / raw)
To: Patrick McHardy
Cc: Harald Welte, netdev, linux-kernel,
Netfilter Development Mailinglist
In-Reply-To: <47792032.3020609@trash.net>
On Monday 31 December 2007 17:00, Patrick McHardy wrote:
> Denys Vlasenko wrote:
> > ip[6]_tables.c seem to abuse inline.
> >
> > This patch removes inlines except those which are used
> > by packet matching code and thus are performance-critical.
> >
>
> Some people also consider the ruleset replacement path performance
> critical, but overall I agree with your patch. I'm travelling currently
> though so it will take a few days until I'll apply it.
>
> Do you have some numbers that show the actual difference these
> changes make?
Before:
$ size */*/*/ip*tables*.o
text data bss dec hex filename
6402 500 16 6918 1b06 net/ipv4/netfilter/ip_tables.o
7130 500 16 7646 1dde net/ipv6/netfilter/ip6_tables.o
After:
$ size */*/*/ip*tables*.o
text data bss dec hex filename
6307 500 16 6823 1aa7 net/ipv4/netfilter/ip_tables.o
7010 500 16 7526 1d66 net/ipv6/netfilter/ip6_tables.o
--
vda
^ permalink raw reply
* Re: [PATCH] Re: Nested VLAN causes recursive locking error
From: Jarek Poplawski @ 2007-12-31 17:54 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Chuck Ebbert, Netdev
In-Reply-To: <20071231174555.GA3097@ami.dom.local>
On Mon, Dec 31, 2007 at 06:45:55PM +0100, Jarek Poplawski wrote:
> Patric, [...]
OOPS... Sorry Patrick!
Jarek P.
^ permalink raw reply
* Re: [PATCH] Force UNIX domain sockets to be built in
From: Adrian Bunk @ 2007-12-31 17:51 UTC (permalink / raw)
To: Bodo Eggert; +Cc: Jan Engelhardt, devzero, linux-kernel, netdev
In-Reply-To: <alpine.LSU.0.999.0712311553530.4211@be1.lrz>
On Mon, Dec 31, 2007 at 04:19:23PM +0100, Bodo Eggert wrote:
> On Mon, 31 Dec 2007, Adrian Bunk wrote:
> > On Mon, Dec 31, 2007 at 02:26:42PM +0100, Bodo Eggert wrote:
> > > On Mon, 31 Dec 2007, Adrian Bunk wrote:
> > > > On Mon, Dec 31, 2007 at 01:09:43PM +0100, Bodo Eggert wrote:
>
> > > > > As suggested by Adrian Bunk, UNIX domain sockets should always be built in
> > > > > on normal systems. This is especially true since udev needs these sockets
> > > > > and fails to run if UNIX=m.
> > > > >
> > > > > Signed-Off-By: Bodo Eggert <7eggert@gmx.de>
> > > > >
> > > > > ---
> > > > > Last minute change: I decided against making it a bool because embedded
> > > > > folks might depend on a small kernel image. Edited in the patch below.
> > > > >...
> > > >
> > > > Is this just a purely theoretical thought or is this a reasonable use
> > > > case people actually use in practice?
> > >
> > > For now, it's a theoretical thought, but having an embedded device, I can
> > > see the reason for $EVERYTHING=m there.
> >
> > The only advantage I see is that the kernel image you have to flash
> > can be made smaller - with the disadvantage that the running kernel
> > is bigger by more than 10%.
> >
> > If you don't believe me, try it yourself:
> > Build all drivers statically into your kernel, and then compare the
> > vmlinux sizes with CONFIG_MODULES=n and CONFIG_MODULES=y.
>
> If you'd aim for a small kernel image, you would build anything as a module
> that is not requred for booting.
>
> > > > After all, changing it to a bool will allow us to make the kernel image
> > > > for nearly everyone smaller by a few hundred bytes...
> > >
> > > I can't see why optionally building it as a module would force us to make
> > > the kernel bigger. It may be a little more ugly to support =m, but thats it,
> > > isn't it?
> >
> > On architectures like x86 where __exit code is freed at runtime
> > af_unix_exit() makes your kernel image (but not the running kernel)
> > bigger.
> >
> > With CONFIG_MODULES=y the 13 EXPORT_SYMBOL's that only exist for the
> > theoretical possibility of CONIG_UNIX=m waste a few hundred bytes
> > of memory.
>
> #define m='m'
> #if CONIG_UNIX=='m'
> #define EXPORT_SYMBOL_AF_UNIX EXPORT_SYMBOL
> #else
> #define EXPORT_SYMBOL_AF_UNIX()
> #endif
> #undef m
>
> You could also use "#if defined(C_U) && (C_U == m)".
Look at the number of exports in the kernel and the number of lines
changed per months, and it becomes obvious that somwthing like that
would not be maintainable.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* Re: [usb regression] Re: [PATCH 2.6.24-rc3] Fix /proc/net breakage
From: Alan Stern @ 2007-12-31 17:49 UTC (permalink / raw)
To: Greg KH
Cc: Andreas Mohr, Ingo Molnar, Alexey Dobriyan, Andrew Morton,
David Woodhouse, Eric W. Biederman, Linus Torvalds,
Rafael J. Wysocki, Pavel Machek, kernel list, netdev,
Pavel Emelyanov, Denis V. Lunev, USB list
In-Reply-To: <20071231052500.GB4187@kroah.com>
On Sun, 30 Dec 2007, Greg KH wrote:
> > It looks like Greg misused the debugfs API -- which is ironic, because
> > he wrote debugfs in the first place! :-)
>
> Oh crap, sorry, I did mess that up :(
>
> > Let me know if this patch fixes the problem. If it does, I'll submit
> > it to Greg with all the proper accoutrements.
>
> This isn't going to work if CONFIG_DEBUGFS is not enabled either :(
Why not? The debugging files won't be created, true, but the driver
should work just fine regardless. This is exactly the way uhci-hcd
behaves, and it hasn't caused any problems.
> > Index: 2.6.24-rc6-mm1/drivers/usb/host/ohci-hcd.c
> > ===================================================================
> > --- 2.6.24-rc6-mm1.orig/drivers/usb/host/ohci-hcd.c
> > +++ 2.6.24-rc6-mm1/drivers/usb/host/ohci-hcd.c
> > @@ -1067,14 +1067,8 @@ static int __init ohci_hcd_mod_init(void
> >
> > #ifdef DEBUG
> > ohci_debug_root = debugfs_create_dir("ohci", NULL);
> > - if (!ohci_debug_root || IS_ERR(ohci_debug_root)) {
> > - if (!ohci_debug_root)
> > - retval = -ENOENT;
> > - else
> > - retval = PTR_ERR(ohci_debug_root);
> > -
> > - goto error_debug;
> > - }
> > + if (!ohci_debug_root)
> > + return -ENOENT;
>
> It needs to check for ERR_PTR(-ENODEV) which is the return value if
> debugfs is not enabled, and if so, just ignore things.
No. That is the mistake you made before. If debugfs isn't enabled
then the driver should just ignore things, yes -- and in particular it
should ignore the fact that the return code is ERR_PTR(-ENODEV). No
extra checking is required.
> If NULL is returned, or anything else, it's a real error.
If NULL is returned, it's a real error, agreed. But if anything else
is returned then the call was successful as far as the driver is
concerned.
> So, try something like:
> if (!ohci_debug_root) {
> retval = -ENOENT;
> goto error_debug;
> }
> if (IS_ERR(ohci_debug_root) && PTR_ERR(ohci_debug_root) != -ENODEV) {
> retval = PTR_ERR(ohci_debug_root);
> goto error_debug;
> }
>
> and let me know of that works for you.
Greg, it sounds like you have forgotten the rationale you originally
used for specifying the return codes in debugfs! The idea was to
return non-NULL if CONFIG_DEBUGFS was disabled, so that drivers could
pretend the calls had succeeded and not need to worry about matters
beyond their control.
Only a NULL return indicates a genuine error. The kerneldoc for
debugfs_create_dir says this very plainly.
> Although the combination of CONFIG_USB_DEBUG and not CONFIG_DEBUGFS is
> strange, we could just enable CONFIG_DEBUGFS is USB_DEBUG is enabled and
> then simplify this logic a bunch at the same time.
Most distributions enable CONFIG_DEBUGFS by default, don't they? So
this problem only comes up when people make up their own configs.
Having USB_DEBUG enabled and DEBUGFS disabled seems like a perfectly
reasonable combination to me. I wouldn't change any aspect of the
configuration logic.
Alan Stern
^ permalink raw reply
* Re: [IPSEC]: Move all calls to xfrm_audit_state_icvfail to xfrm_input
From: Paul Moore @ 2007-12-31 17:48 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20071231042355.GA9215@gondor.apana.org.au>
On Sunday 30 December 2007 11:23:55 pm Herbert Xu wrote:
> Hi Dave:
>
> While refreshing the async IPsec patches I noticed some fresh code
> duplication.
>
> [IPSEC]: Move all calls to xfrm_audit_state_icvfail to xfrm_input
>
> Let's nip the code duplication in the bud :)
Thanks, not sure why I didn't see that in the first place :)
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
> index ec8de0a..d76803a 100644
> --- a/net/ipv4/ah4.c
> +++ b/net/ipv4/ah4.c
> @@ -179,10 +179,8 @@ static int ah_input(struct xfrm_state *x, struct
> sk_buff *skb) err = ah_mac_digest(ahp, skb, ah->auth_data);
> if (err)
> goto unlock;
> - if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len)) {
> - xfrm_audit_state_icvfail(x, skb, IPPROTO_AH);
> + if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len))
> err = -EBADMSG;
> - }
> }
> unlock:
> spin_unlock(&x->lock);
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index b334c76..28ea5c7 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -191,7 +191,6 @@ static int esp_input(struct xfrm_state *x, struct
> sk_buff *skb) BUG();
>
> if (unlikely(memcmp(esp->auth.work_icv, sum, alen))) {
> - xfrm_audit_state_icvfail(x, skb, IPPROTO_ESP);
> err = -EBADMSG;
> goto unlock;
> }
> diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
> index 2d32772..fb0d07a 100644
> --- a/net/ipv6/ah6.c
> +++ b/net/ipv6/ah6.c
> @@ -380,10 +380,8 @@ static int ah6_input(struct xfrm_state *x, struct
> sk_buff *skb) err = ah_mac_digest(ahp, skb, ah->auth_data);
> if (err)
> goto unlock;
> - if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len)) {
> - xfrm_audit_state_icvfail(x, skb, IPPROTO_AH);
> + if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len))
> err = -EBADMSG;
> - }
> }
> unlock:
> spin_unlock(&x->lock);
> diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> index e10f10b..5bd5292 100644
> --- a/net/ipv6/esp6.c
> +++ b/net/ipv6/esp6.c
> @@ -186,7 +186,6 @@ static int esp6_input(struct xfrm_state *x, struct
> sk_buff *skb) BUG();
>
> if (unlikely(memcmp(esp->auth.work_icv, sum, alen))) {
> - xfrm_audit_state_icvfail(x, skb, IPPROTO_ESP);
> ret = -EBADMSG;
> goto unlock;
> }
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index 1b250f3..039e701 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -186,8 +186,11 @@ int xfrm_input(struct sk_buff *skb, int nexthdr,
> __be32 spi, int encap_type) resume:
> spin_lock(&x->lock);
> if (nexthdr <= 0) {
> - if (nexthdr == -EBADMSG)
> + if (nexthdr == -EBADMSG) {
> + xfrm_audit_state_icvfail(x, skb,
> + x->type->proto);
> x->stats.integrity_failed++;
> + }
> XFRM_INC_STATS(LINUX_MIB_XFRMINSTATEPROTOERROR);
> goto drop_unlock;
> }
>
> Thanks,
--
paul moore
linux security @ hp
^ permalink raw reply
* Re: [PATCH] Re: Nested VLAN causes recursive locking error
From: Jarek Poplawski @ 2007-12-31 17:45 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Chuck Ebbert, Netdev
In-Reply-To: <477904F1.6090403@trash.net>
On Mon, Dec 31, 2007 at 04:04:17PM +0100, Patrick McHardy wrote:
> Jarek Poplawski wrote:
>> diff -Nurp linux-2.6.24-rc5-/net/8021q/vlan.c linux-2.6.24-rc5+/net/8021q/vlan.c
>> --- linux-2.6.24-rc5-/net/8021q/vlan.c 2007-12-17 13:29:19.000000000 +0100
>> +++ linux-2.6.24-rc5+/net/8021q/vlan.c 2007-12-20 14:21:02.000000000 +0100
>> @@ -307,12 +307,15 @@ int unregister_vlan_device(struct net_de
>> return ret;
>> }
>> +#ifdef CONFIG_LOCKDEP
>> /*
>> * vlan network devices have devices nesting below it, and are a special
>> * "super class" of normal network devices; split their locks off into a
>> * separate class since they always nest.
>> */
>> static struct lock_class_key vlan_netdev_xmit_lock_key;
>> +static int subclass; /* vlan nesting vlan */
>> +#endif
>> static const struct header_ops vlan_header_ops = {
>> .create = vlan_dev_hard_header,
>> @@ -349,7 +352,14 @@ static int vlan_dev_init(struct net_devi
>> dev->hard_start_xmit = vlan_dev_hard_start_xmit;
>> }
>> - lockdep_set_class(&dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
>> +#ifdef CONFIG_LOCKDEP
>> + if ((real_dev->priv_flags & IFF_802_1Q_VLAN) &&
>> + subclass < MAX_LOCKDEP_SUBCLASSES - 1)
>> + subclass++;
>> +
>
> That will increment the subclass globally, but it should actually just
> use real_dev->subclass + 1. Otherwise we'll permenently fail after
> registering 8 nested devices and unregistering them again.
>
Very good point! (As usual...) Actually, this shouldn't fail but work
with this one, last subclass only, so similarly like now, without this
patch.
Patric, currently I'm not sure this patch is very necessary... Since
I don't use vlans, I don't know how much it's real or theoretical
only problem. Probably the easiest solution would be limiting this
to two subclasses - any nested vlan gets second. Otherwise, it seems
there is some place needed to store these subclasses or use some
unofficial checks on lockdep's structures?
So, if you think this is useful and have better idea how this should
be done, I would be very glad if you re-do this your way (plus there
should be no problem with testing). Otherwise, give me some hint...
Thanks & Happy New Year!
Jarek P.
^ permalink raw reply
* Re: [PATCH] Force UNIX domain sockets to be built in
From: Patrick Mau @ 2007-12-31 17:43 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: Bodo Eggert, Adrian Bunk, devzero, linux-kernel, netdev
In-Reply-To: <Pine.LNX.4.64.0712311628330.28123@fbirervta.pbzchgretzou.qr>
On Mon, Dec 31, 2007 at 04:34:55PM +0100, Jan Engelhardt wrote:
> >
> >If you'd aim for a small kernel image, you would build anything as a module
> >that is not requred for booting.
> >
> Yes, there is a tradeoff for both.
>
> Example:
> 16:30 ichi:../net/802 > l fc.o fc.ko
> -rw-r--r-- 1 jengelh users 7961 Dec 27 15:19 fc.ko
> -rw-r--r-- 1 jengelh users 2453 Dec 28 23:58 fc.o
> (from a recent not-so-complete patch turning CONFIG_FC etc. into =m)
>
> If fc was modular, it might save the 2453 bytes off the core kernel image,
> but adds ~5508 bytes to disk.
> So one has to pick =y or =m depending on whatever suits his/her situation.
May I ask something that might be obvious for most of the
development community:
Modules have to be loaded in seperate pages, right ?
Does that mean that each module wastes partially used
pages of memory at runtime ?
I've always tried to build as much into the kernel image as
possible, because all of my systems have only 512M memory.
Thanks,
Patrick
^ permalink raw reply
* Re: [PATCH] Force UNIX domain sockets to be built in
From: Michael Buesch @ 2007-12-31 17:18 UTC (permalink / raw)
To: Alan Cox
Cc: Torsten Kaiser, Adrian Bunk, Bodo Eggert, Jan Engelhardt, devzero,
linux-kernel, netdev
In-Reply-To: <20071231163803.33acb647@the-village.bc.nu>
On Monday 31 December 2007 17:38:03 Alan Cox wrote:
> On Mon, 31 Dec 2007 17:17:19 +0100
> "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
>
> > a) this could be disabled during development if you want this
> > b) this would even only affect development if you add new code that
> > now needs a EXPORT_SYMBOL that was removed on an earlier build. And
> > right now this would also need to trigger a rerun of depmod. And the
> > same trigger could redo this garbage collect.
> >
> > Or am I missing something obvious?
>
> Development is not a phase seperate from use or distribution. A lot of
> module testers for distributions will not be compiling their own modules
> but loading in ones to test provided by their vendor - which may of
> course then need different ksyms
As an example, the whole purpose wireless-compat package is
to load latest bleeding edge wireless stuff into a distribution kernel.
So people are not required to recompile their kernels for using
drivers that support their hardware.
And guess what, it is used a _lot_. And lots of bugs are found with it.
It increases our testing community a lot.
So, all this wouldn't work, if kernel symbols could randomly get
nuked by some "garbage collector".
In practice, no distribution would use symbol garbage collection, as the
only benefit from it would be an increased level of bugreports.
--
Greetings Michael.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox