* [PROBLEM] linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic
From: emin ak @ 2010-10-03 6:20 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Kumar Gala
In-Reply-To: <AANLkTi=Kvi3u5bRp5DtRH-Pr6ALew60cPgeVEZ8V-Dnu@mail.gmail.com>
Hi all,
My problem is kernel crash under full line rate random packet length
ip network traffic.
I'am using default unmodified kernel and default SMP kernel
configuration, MPC8572DS development board and also using a hardware
packet generator.
My test is ip forwarding between eth0 and eth1, and Hardware packet
generator produces full duplex, full line rate traffic with random
packet length and random payload . After a few millions of packets
passed, kernel produces this bellow two different crash messages . I
have retry this scenario many times, crash occurs sometimes on
skb_put, but mostly occurs on ip_rcv function. I have aplied same
test to latest stable linux 2.6.35.6 kernel. Same errors produced.
Any comment and help are appreciated.
Here is crash logs:
Thanks.
Emin
First type of crash:
root@mpc8572ds:~# skb_over_panic: text:c0226280 len:1171 put:1171
head:eed6d000 data:eed63040 tail:0xeed6d4d3 end:0xeed63660 dev:<NULL>
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:127!
Oops: Exception in kernel mode, sig: 5 [#1]
SMP NR_CPUS=2 MPC8572 DS
last sysfs file: /sys/devices/pci0002:03/0002:03:00.0/subsystem_device
Modules linked in:
NIP: c023bdcc LR: c023bdcc CTR: c01f3ff8
REGS: effe7d70 TRAP: 0700 Not tainted (2.6.36-rc5)
MSR: 00029000 <EE,ME,CE> CR: 22028024 XER: 20000000
TASK = ef83e9a0[9] 'ksoftirqd/1' THREAD: ef856000 CPU: 1
GPR00: c023bdcc effe7e20 ef83e9a0 0000007c 00021000 ffffffff c01f7b98 c03ccf1c
GPR08: c03c69d4 c03f94b4 00c4e000 00000004 20028048 1001a108 ef211000 efb52d90
GPR16: efb52e38 efb52870 00000000 ef211800 00000008 00000009 efb52800 00000037
GPR24: ef24e180 ef2be040 00000000 ef211948 efb52b80 00000493 ef015940 ef386600
NIP [c023bdcc] skb_put+0x8c/0x94
LR [c023bdcc] skb_put+0x8c/0x94
Call Trace:
[effe7e20] [c023bdcc] skb_put+0x8c/0x94 (unreliable)
[effe7e30] [c0226280] gfar_clean_rx_ring+0x104/0x4b8
[effe7e90] [c02269dc] gfar_poll+0x3a8/0x60c
[effe7f60] [c024928c] net_rx_action+0xf8/0x1a4
[effe7fa0] [c0042524] __do_softirq+0xe0/0x178
[effe7ff0] [c000e59c] call_do_softirq+0x14/0x24
[ef857f50] [c0004840] do_softirq+0x90/0xa0
[ef857f70] [c00430e4] run_ksoftirqd+0xb4/0x164
[ef857fb0] [c00586b4] kthread+0x7c/0x80
[ef857ff0] [c000e9a8] kernel_thread+0x4c/0x68
Instruction dump:
81030098 2f800000 409e000c 3d20c037 3809a19c 3c60c037 7c8802a6 7d695b78
3863b010 90010008 4cc63182 4be016c5 <0fe00000> 48000000 9421fff0 7c0802a6
Kernel panic - not syncing: Fatal exception in interrupt
---------------
second type of crash:
Faulting instruction address: 0xc026c1dc
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2 MPC8572 DS
last sysfs file: /sys/devices/pci0002:03/0002:03:00.0/subsystem_device
Modules linked in:
NIP: c026c1dc LR: c026bfac CTR: 00000000
REGS: effebd00 TRAP: 0300 Not tainted (2.6.36-rc5)
MSR: 00029000 <EE,ME,CE> CR: 42028042 XER: 00000000
DEAR: 0000cad8, ESR: 00000000
TASK = ef83cde0[3] 'ksoftirqd/0' THREAD: ef84a000 CPU: 0
GPR00: 00000005 effebdb0 ef83cde0 00000000 000001b9 00000000 c1008060 00000000
GPR08: 02c3f605 0000ca00 000005b9 0000ca00 b653a6c7 7af823f0 ef217000 efbab590
GPR16: efbab638 efbab070 00000000 ef217800 00000008 00000018 efbab000 00000028
GPR24: c03f971c c0410000 c0400000 c03f94b4 effea000 ef316e40 00000000 eecb685e
NIP [c026c1dc] ip_rcv+0x3f8/0x808
LR [c026bfac] ip_rcv+0x1c8/0x808
Call Trace:
[effebdb0] [c026c204] ip_rcv+0x420/0x808 (unreliable)
[effebde0] [c02482dc] __netif_receive_skb+0x2f8/0x324
[effebe10] [c02483a4] netif_receive_skb+0x9c/0xb0
[effebe30] [c0226308] gfar_clean_rx_ring+0x18c/0x4b8
[effebe90] [c02269dc] gfar_poll+0x3a8/0x60c
[effebf60] [c024928c] net_rx_action+0xf8/0x1a4
[effebfa0] [c0042524] __do_softirq+0xe0/0x178
[effebff0] [c000e59c] call_do_softirq+0x14/0x24
[ef84bf50] [c0004840] do_softirq+0x90/0xa0
[ef84bf70] [c00430e4] run_ksoftirqd+0xb4/0x164
[ef84bfb0] [c00586b4] kthread+0x7c/0x80
[ef84bff0] [c000e9a8] kernel_thread+0x4c/0x68
Instruction dump:
8148003c 318a0001 7d690194 91680038 9188003c 4bfffd78 7fa3eb78 48002a29
2f830000 40beff50 817d0048 5569003c <a00900d8> 2f800005 419e0034 2f800003
Kernel panic - not syncing: Fatal exception in interrupt
^ permalink raw reply
* Re: [patch v3 00/12] IPVS: SIP Persistence Engine
From: Simon Horman @ 2010-10-03 5:07 UTC (permalink / raw)
To: Julian Anastasov
Cc: lvs-devel, netdev, netfilter, netfilter-devel, Jan Engelhardt,
Stephen Hemminger, Wensong Zhang, Patrick McHardy
In-Reply-To: <alpine.LFD.2.00.1010021155520.2055@ja.ssi.bg>
On Sat, Oct 02, 2010 at 12:00:14PM +0300, Julian Anastasov wrote:
>
> Hello,
>
> On Sat, 2 Oct 2010, Simon Horman wrote:
>
> >This patch series adds load-balancing of UDP SIP based on Call-ID to
> >IPVS as well as a frame-work for extending IPVS to handle alternate
> >persistence requirements.
> >
> >REVISIONS
> >
> >This is v3 of the patch series with addresses serveral problems
> >raised by Julian Anastasov on the lvs-devel mailing list.
>
> No other obvious problems in v3 1-12. So,
> Acked-by: Julian Anastasov <ja@ssi.bg>
Thanks.
> May be next days I'll test my changes on top of PE v3
I think that could save a bit of porting work later
and perhaps you might notice some more bugs too :-)
^ permalink raw reply
* Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
From: Eric Dumazet @ 2010-10-03 4:53 UTC (permalink / raw)
To: David Miller; +Cc: tomer_iisc, netdev, linux-kernel
In-Reply-To: <20101002.170654.193719563.davem@davemloft.net>
Le samedi 02 octobre 2010 à 17:06 -0700, David Miller a écrit :
> From: Nagendra Tomar <tomer_iisc@yahoo.com>
> Date: Sat, 2 Oct 2010 16:54:23 -0700 (PDT)
>
> > I had done the exercise of sending the patch to myself and
> > applying it (copy-pasting just the patch). One thing that I see
> > is the long line in the description. If you are referring to
> > that, I've fixed it and submitted it again. If not this, I'm at
> > loss.
>
> This new submission looks good, thank you.
Yes, but the email address in the "Signed-odd-by: ...." is mangled :(
^ permalink raw reply
* Re: Ask For Comment: add routines for exchanging data between sock buffer and scatter list
From: Hillf Danton @ 2010-10-03 3:10 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-kernel, axboe, robert.w.love, James.Bottomley
In-Reply-To: <20101002.132056.226767861.davem@davemloft.net>
There seems no routines provided for exchanging data
directly between sock buffer and scatter list in
both scatterlist.c and skbuff.c, so comes this work.
And it is hard to determine into which file these
routines should be added, then a header file is added.
Signed-off-by: Hillf Danton <dhillf@gmail.com>
---
diff -Npur o/linux-2.6.36-rc4/include/skb_sg.h
m/linux-2.6.36-rc4/include/skb_sg.h
--- o/linux-2.6.36-rc4/include/skb_sg.h 1970-01-01 08:00:00.000000000 +0800
+++ m/linux-2.6.36-rc4/include/skb_sg.h 2010-10-03 10:03:54.000000000 +0800
@@ -0,0 +1,113 @@
+/*
+ Definition for exchanging data between sock buffer and scatter list
+
+ Copyright (C) Oct 2010 Hillf Danton <dhillf@gmail.com>
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ MA 02110-1301 USA
+*/
+
+#ifndef __LINUX_SKB_SG_H
+#define __LINUX_SKB_SG_H
+
+#include <linux/scatterlist.h>
+#include <linux/skbuff.h>
+
+/*
+ * sg_fill_skb_page_desc - fill skb frags with info in sg list
+ * @index the start index to fill
+ *
+ * return the number of filled frags
+ */
+
+static int sg_fill_skb_page_desc(struct sk_buff *skb, int index,
+ struct scatterlist *sg)
+{
+ int old = index;
+ struct page *page;
+
+ for (; sg && index < MAX_SKB_FRAGS; index++) {
+ page = sg_page(sg);
+ get_page(page);
+ skb_add_rx_frag(skb, index, page, sg->offset, sg->length);
+ sg = sg_next(sg);
+ }
+
+ return index - old;
+}
+
+/*
+ * skb_copy_bits_to_sg - copy data from skb to sg list
+ * @len length of data to be copied
+ *
+ * return the number of copied bytes
+ */
+
+static int skb_copy_bits_to_sg(struct sk_buff *skb, int offset_in_skb,
+ struct scatterlist *sg, int offset_in_sg,
+ int len)
+{
+ int old = len;
+ struct sg_mapping_iter miter;
+
+ if (offset_in_skb >= skb->len)
+ return 0;
+
+ if (len > skb->len - offset_in_skb)
+ old = len = skb->len - offset_in_skb;
+
+ /* skip offset in sg */
+ while (sg && offset_in_sg >= sg->length) {
+ offset_in_sg -= sg->length;
+ sg = sg_next(sg);
+ }
+ if (! sg)
+ return 0;
+
+ /* and go thru sg list */
+ while (len > 0 && sg) {
+ int this_len;
+ int err;
+
+ sg_miter_start(&miter, sg, 1, SG_MITER_ATOMIC|SG_MITER_TO_SG);
+
+ if (offset_in_sg) {
+ /* we have to count this residual */
+ miter.__offset = offset_in_sg;
+ offset_in_sg = 0;
+ }
+
+ if (! sg_miter_next(&miter))
+ break;
+
+ this_len = min(miter.length, len);
+
+ err = skb_copy_bits(skb, offset_in_skb, miter.addr, this_len);
+
+ sg_miter_stop(&miter);
+
+ if (err)
+ break;
+
+ offset_in_skb += this_len;
+ len -= this_len;
+
+ sg = sg_next(sg);
+ }
+
+ return old - len;
+}
+
+#endif /* __LINUX_SKB_SG_H */
^ permalink raw reply
* Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
From: Nagendra Tomar @ 2010-10-03 1:30 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, davem
Dave,
Thinking more about it, we need to check for sk->sk_err, thus the
existing code behaves fine. Just that we might incur an additional sleep
even while we know that the socket already has an error, but that should
be ok.
We only need the other patch. Pls ignore this, and sorry for the confusion.
Thanks,
Tomar
--- On Sun, 3/10/10, Nagendra Tomar <tomer_iisc@yahoo.com> wrote:
> From: Nagendra Tomar <tomer_iisc@yahoo.com>
> Subject: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
> To: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org, davem@davemloft.net
> Date: Sunday, 3 October, 2010, 5:19
> This patch fixes the sk_wait_event()
> condition in the sk_stream_wait_connect()
> function. With this change, we correctly check for the
> TCPF_ESTABLISHED and
> TCPF_CLOSE_WAIT states and avoid potentially returning
> success when there
> might be an error on the socket.
>
> Signed-off-by: Nagendra Singh Tomar
> <tomer_iisc@xxxxxxxxx>
> ---
> --- linux-2.6.35.7/net/core/stream.c.orig
> 2010-03-24 09:30:00.000000000 +0530
> +++ linux-2.6.35.7/net/core/stream.c
> 2010-03-24 09:30:17.000000000 +0530
> @@ -73,9 +73,8 @@ int sk_stream_wait_connect(struct sock *
>
> prepare_to_wait(sk_sleep(sk), &wait,
> TASK_INTERRUPTIBLE);
>
> sk->sk_write_pending++;
> done =
> sk_wait_event(sk, timeo_p,
> -
> !sk->sk_err
> &&
> -
> !((1 <<
> sk->sk_state) &
> -
>
> ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
> +
> ((1 <<
> sk->sk_state) &
> +
>
> (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
>
> finish_wait(sk_sleep(sk), &wait);
>
> sk->sk_write_pending--;
> } while (!done);
>
> ---
>
>
>
>
>
^ permalink raw reply
* Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
From: David Miller @ 2010-10-03 0:06 UTC (permalink / raw)
To: tomer_iisc; +Cc: netdev, linux-kernel
In-Reply-To: <970819.25375.qm@web53706.mail.re2.yahoo.com>
From: Nagendra Tomar <tomer_iisc@yahoo.com>
Date: Sat, 2 Oct 2010 16:54:23 -0700 (PDT)
> I had done the exercise of sending the patch to myself and
> applying it (copy-pasting just the patch). One thing that I see
> is the long line in the description. If you are referring to
> that, I've fixed it and submitted it again. If not this, I'm at
> loss.
This new submission looks good, thank you.
^ permalink raw reply
* Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
From: Nagendra Tomar @ 2010-10-02 23:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20101002.132659.71124680.davem@davemloft.net>
Dave,
I had done the exercise of sending the patch to myself and applying it (copy-pasting just the patch). One thing that I see is the long line in the description. If you are referring to that, I've fixed it and submitted it again. If not this, I'm at loss.
Thanks,
Tomar
--- On Sun, 3/10/10, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Subject: Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
> To: tomer_iisc@yahoo.com
> Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Sunday, 3 October, 2010, 1:56
>
> Your patch is still corrupted, your email client is
> splitting up
> long lines.
>
> Please, save us a lot of time by test emailing the patch to
> yourself,
> and then trying to apply the patch as you receive it.
> Do this until
> you're fixed all of the formatting problems and then you
> can send it
> here.
>
> Do not resend the patch by simply replying again to this
> thread,
> send a fresh posting so that "Re: " doesn't show up in the
> subject
> and this way I can apply it directly without any editing.
>
> Thank you.
>
^ permalink raw reply
* [PATCH 2/2] net: Fix the condition passed to sk_wait_event()
From: Nagendra Tomar @ 2010-10-02 23:51 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, davem
This patch fixes the condition (3rd arg) passed to sk_wait_event() in
sk_stream_wait_memory(). The incorrect check in sk_stream_wait_memory()
causes the following soft lockup in tcp_sendmsg() when the global tcp
memory pool has exhausted.
>>> snip <<<
localhost kernel: BUG: soft lockup - CPU#3 stuck for 11s! [sshd:6429]
localhost kernel: CPU 3:
localhost kernel: RIP: 0010:[sk_stream_wait_memory+0xcd/0x200] [sk_stream_wait_memory+0xcd/0x200] sk_stream_wait_memory+0xcd/0x200
localhost kernel:
localhost kernel: Call Trace:
localhost kernel: [sk_stream_wait_memory+0x1b1/0x200] sk_stream_wait_memory+0x1b1/0x200
localhost kernel: [<ffffffff802557c0>] autoremove_wake_function+0x0/0x40
localhost kernel: [ipv6:tcp_sendmsg+0x6e6/0xe90] tcp_sendmsg+0x6e6/0xce0
localhost kernel: [sock_aio_write+0x126/0x140] sock_aio_write+0x126/0x140
localhost kernel: [xfs:do_sync_write+0xf1/0x130] do_sync_write+0xf1/0x130
localhost kernel: [<ffffffff802557c0>] autoremove_wake_function+0x0/0x40
localhost kernel: [hrtimer_start+0xe3/0x170] hrtimer_start+0xe3/0x170
localhost kernel: [vfs_write+0x185/0x190] vfs_write+0x185/0x190
localhost kernel: [sys_write+0x50/0x90] sys_write+0x50/0x90
localhost kernel: [system_call+0x7e/0x83] system_call+0x7e/0x83
>>> snip <<<
What is happening is, that the sk_wait_event() condition passed from
sk_stream_wait_memory() evaluates to true for the case of tcp global memory
exhaustion. This is because both sk_stream_memory_free() and vm_wait are true
which causes sk_wait_event() to *not* call schedule_timeout().
Hence sk_stream_wait_memory() returns immediately to the caller w/o sleeping.
This causes the caller to again try allocation, which again fails and again
calls sk_stream_wait_memory(), and so on.
Signed-off-by: Nagendra Singh Tomar <tomer_iisc@xxxxxxxxx>
---
--- linux-2.6.35.7/net/core/stream.c.orig 2010-03-24 09:31:00.000000000 +0530
+++ linux-2.6.35.7/net/core/stream.c 2010-03-24 09:31:08.000000000 +0530
@@ -143,10 +143,9 @@ int sk_stream_wait_memory(struct sock *s
set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
sk->sk_write_pending++;
- sk_wait_event(sk, ¤t_timeo, !sk->sk_err &&
- !(sk->sk_shutdown & SEND_SHUTDOWN) &&
- sk_stream_memory_free(sk) &&
- vm_wait);
+ sk_wait_event(sk, ¤t_timeo, sk->sk_err ||
+ (sk->sk_shutdown & SEND_SHUTDOWN) ||
+ (sk_stream_memory_free(sk) && !vm_wait));
sk->sk_write_pending--;
if (vm_wait) {
---
^ permalink raw reply
* [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
From: Nagendra Tomar @ 2010-10-02 23:49 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, davem
This patch fixes the sk_wait_event() condition in the sk_stream_wait_connect()
function. With this change, we correctly check for the TCPF_ESTABLISHED and
TCPF_CLOSE_WAIT states and avoid potentially returning success when there
might be an error on the socket.
Signed-off-by: Nagendra Singh Tomar <tomer_iisc@xxxxxxxxx>
---
--- linux-2.6.35.7/net/core/stream.c.orig 2010-03-24 09:30:00.000000000 +0530
+++ linux-2.6.35.7/net/core/stream.c 2010-03-24 09:30:17.000000000 +0530
@@ -73,9 +73,8 @@ int sk_stream_wait_connect(struct sock *
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
sk->sk_write_pending++;
done = sk_wait_event(sk, timeo_p,
- !sk->sk_err &&
- !((1 << sk->sk_state) &
- ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
+ ((1 << sk->sk_state) &
+ (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
finish_wait(sk_sleep(sk), &wait);
sk->sk_write_pending--;
} while (!done);
---
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: Daniel Lezcano @ 2010-10-02 21:13 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: hadi, Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Linus Torvalds, Michael Kerrisk,
Ulrich Drepper, Al Viro, David Miller, Serge E. Hallyn,
Pavel Emelyanov, Ben Greear, Matt Helsley, Jonathan Corbet,
Sukadev Bhattiprolu, Jan Engelhardt, Patrick McHardy
In-Reply-To: <4C9B3F9C.8080506@parallels.com>
On 09/23/2010 01:53 PM, Pavel Emelyanov wrote:
> On 09/23/2010 03:40 PM, jamal wrote:
>
>> On Thu, 2010-09-23 at 15:33 +0400, Pavel Emelyanov wrote:
>>
>>
>>> This particular usecase is unneeded once you have the "enter" ability.
>>>
>> Is that cheaper from a syscall count/cost?
>>
> Why does it matter? You told, that the usage scenario was to
> add routes to container. If I do 2 syscalls instead of 1, is
> it THAT worse?
>
>
>> i.e do I have to enter every time i want to write/read this fd?
>>
> No - you enter once, create a socket and do whatever you need
> withing the enterned namespace.
>
Just to clarify this point. You enter the namespace, create the socket
and go back to the initial namespace (or create a new one). Further
operations can be made against this fd because it is the network
namespace stored in the sock struct which is used, not the current
process network namespace which is used at the socket creation only.
We can actually already do that by unsharing and then create a socket.
This socket will pin the namespace and can be used as a control socket
for the namespace (assuming the socket domain will be ok for all the
operations).
Jamal, I don't know what kind of application you want to use but if I
assume you want to create a process controlling 1024 netns, let's try to
identificate what happen with setns and with socketat :
With setns:
* open /proc/self/ns/net (1)
* unshare the netns
* open /proc/self/ns/net (2)
* setns (1)
* create a virtual network device
* move the virtual device to (2) (using the set netns by fd)
* unshare the netns
...
With socketat:
* open a socket (1)
* unshare the netns
* open a netlink with socketat(1) => (2)
* create a virtual device using (2) (at this point it is init_net_ns)
* move the virtual device to the current netns (using the set netns
by pid)
* open a socket (3)
* unshare the netns
...
We have the same number of file descriptors kept opened. Except, with
setns we can bind mount the directory somewhere, that will pin the
namespace and then we can close the /proc/self/ns/net file descriptors
and reopen them later.
If your application has to do a lot of specific network processing,
during its life cycle, in different namespaces, the socketat syscall
will be better because it will reduce the number of syscalls but at the
cost of keeping the file descriptors opened (potentially a big number).
Otherwise, setns should fit your needs.
>> How does poll/select work in that enter scenario?
>>
> Just like it used to before the enter.
>
>
>> cheers,
>> jamal
>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
^ permalink raw reply
* Re: [PATCH 2.6.35.7] net: Fix the condition passed to sk_wait_event()
From: David Miller @ 2010-10-02 20:27 UTC (permalink / raw)
To: tomer_iisc; +Cc: netdev, linux-kernel
In-Reply-To: <667106.4951.qm@web53706.mail.re2.yahoo.com>
From: Nagendra Tomar <tomer_iisc@yahoo.com>
Date: Sat, 2 Oct 2010 01:22:16 -0700 (PDT)
> Resending ...
It's still corrupted, see my other reply.
^ permalink raw reply
* Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
From: David Miller @ 2010-10-02 20:26 UTC (permalink / raw)
To: tomer_iisc; +Cc: netdev, linux-kernel
In-Reply-To: <590816.4455.qm@web53707.mail.re2.yahoo.com>
Your patch is still corrupted, your email client is splitting up
long lines.
Please, save us a lot of time by test emailing the patch to yourself,
and then trying to apply the patch as you receive it. Do this until
you're fixed all of the formatting problems and then you can send it
here.
Do not resend the patch by simply replying again to this thread,
send a fresh posting so that "Re: " doesn't show up in the subject
and this way I can apply it directly without any editing.
Thank you.
^ permalink raw reply
* Re: problem with flowi structure
From: David Miller @ 2010-10-02 20:18 UTC (permalink / raw)
To: nicola.padovano
Cc: jengelh, eric.dumazet, aijazbaig1, netfilter-devel, netdev
In-Reply-To: <AANLkTinObV+176prSdCL0RoVx7a74quUM5H58xR2_WLG@mail.gmail.com>
From: Nicola Padovano <nicola.padovano@gmail.com>
Date: Sat, 2 Oct 2010 10:08:56 +0200
> i.e. like a wildcard?
Please do not top-post, because when you top-post people need
to scroll down and read the context you're replying to, then
scroll back up to read your reply.
If, instead of top-posting, you reply after the context you're
replying to, people need to do less work to read your email.
Thanks.
^ permalink raw reply
* INVESTMENT PROJECT IN YOUR COUNTRY. (HOW CAN I INVEST IN YOUR COUNTRY)?
From: kunihira barbara @ 2010-10-01 7:59 UTC (permalink / raw)
Dear Sir,
I am Mr.Sam Gapke, The Managing Director of the REA. I am interested in investing the sum of US $20M (Twenty Million US Dollars) in any Lucrative Business in the area related to your Business line. Thanks for anticipate cooperation, your urgent response will be highly appreciated.
Feel free to contact me with this my personal email address:sam.pk01@gmail.com
Best regards,
Mr. Sam Gapke
^ permalink raw reply
* [PATCH] ibmtr: fix tr%d in dmesg
From: Meelis Roos @ 2010-10-02 18:35 UTC (permalink / raw)
To: netdev
ibmtr and ibmtr_cs show tr%d in dmesg after alloc_netdev() but before
register_netdev(). Fix it like e100 does - put a different name into
dev->name until the device gets registered. I/O port seems to be unique
enough and is available for the time of printk messages. With the fix,
dmesg shows
ibmtr@0a20: ISA P&P 16/4 Adapter/A (short) | 16/4 ISA-16 Adapter found
ibmtr@0a20: using irq 3, PIOaddr a20, 64K shared RAM.
ibmtr@0a20: Hardware address : 00:40:aa:a9:03:98
ibmtr@0a20: Shared RAM paging disabled. ti->page_mask 0
ibmtr@0a20: Maximum Receive Internet Protocol MTU 16Mbps: 16344, 4Mbps: 6104
tr0: port 0xa20, irq 3, mmio 0xd4850000, sram 0xd0000, hwaddr=00:40:aa:a9:03:98
Signed-off-by: Meelis Roos <mroos@linux.ee>
diff --git a/drivers/net/tokenring/ibmtr.c b/drivers/net/tokenring/ibmtr.c
index 91e6c78..5de281f 100644
--- a/drivers/net/tokenring/ibmtr.c
+++ b/drivers/net/tokenring/ibmtr.c
@@ -368,6 +368,7 @@ int __devinit ibmtr_probe_card(struct net_device *dev)
{
int err = ibmtr_probe(dev);
if (!err) {
+ strcpy(dev->name, "tr%d");
err = register_netdev(dev);
if (err)
ibmtr_cleanup_card(dev);
@@ -699,6 +700,7 @@ static int __devinit ibmtr_probe1(struct net_device *dev, int PIOaddr)
printk(version);
}
#endif /* !PCMCIA */
+ sprintf(dev->name, "ibmtr@%04x", PIOaddr);
DPRINTK("%s %s found\n",
channel_def[cardpresent - 1], adapter_def(ti->adapter_type));
DPRINTK("using irq %d, PIOaddr %hx, %dK shared RAM.\n",
--
Meelis Roos (mroos@linux.ee)
^ permalink raw reply related
* Urgent Write Back
From: Dr. Raymond Kuo Fung CH'IEN. @ 2010-10-02 17:02 UTC (permalink / raw)
Dear Friend,
I am Dr. Raymond Kuo Fung CHIEN Executive Director and Chief Financial
Officer of the operations of the Hang Seng Bank Ltd.I have a secured
business suggestion for you. Before the U.S and Iraqi war our client Mr
Fayez Abdulaziz Mohammed who was with the Iraqi forces and also a business
man made a numbered fixed deposit for 18 calendar months, with a value of
Thirty Million United State Dollars($30,000,000.00) only in my
branch.Upon maturity several notices was sent to him but there was no
word. Again after the war another notification was sent and still no
response came from him. We later find out that Mr Fayez Abdulaziz Mohammed
and his family had been killed during the war in bomb blast that hit their
home.
After further investigation it was also discovered that Mr Fayez
Abdulaziz Mohammed did not declare any next of kin in his official papers
including the paper work of his bank deposit. And he also confided in me
the last time he was at my office that no one except me knew of his
deposit in my bank. So,Thirty Million United State
Dollars($30,000,000.00)is still lying in my bank and no one will ever
come forward to claim it.What bothers me most is that according to the
laws of my country at the expiration of 8 years the funds will revert to
the ownership of the Hong Kong Government if nobody applies to claim the
funds.
Against this backdrop, my suggestion to you is that I will like you as a
foreigner to stand as the next of kin to Mr Fayez Abdulaziz Mohammed so
that you will be able to receive his funds.
WHAT IS TO BE DONE:
I want you to know that I have had everything planned out so that we shall
come out successful.I have contacted an attorney that will prepare the
necessary document that will back you up as the next of kin to Mr Fayez
Abdulaziz Mohammed, all that is required from you at this stage is for you
to provide me with your Full Names and Address so that the attorney can
commence his job. After you have been made the next of kin to the late Mr
Fayez Abdulaziz Mohammed, the Attorney will also file in for claims on
your behalf and secure the necessary approval and letter of probate in
your favor for the move of the funds to an account that will be provided
by you.There is no risk involved at all in the matter as we are going to
adopt a legalized method and the attorney will prepare all the necessary
documents. Please endeavor to observe utmost discretion in all matters
concerning this issue.Once the funds have been transferred to your
nominated bank account we shall share in the ratio of 70% for me, 30% for
you, Should you be interested please send me the following informations
below,
1. Full name and Age.
2. Occupation
3. Private/office phone number.
4. Current residential address.
And finally after that i shall provide you with more details of this
transaction.Your earliest response to this letter will be highly
appreciated. Write back if intrested at
Email:drraymondkuochien3001@gmail.com or
Email:drraymondkuochien@w.cn
Kind Regards,
Dr.Raymond Kuo Fung CHIEN.
-----------------------------------------------------------------------------------------------------------------------------
Please consider the environmental impact of needlessly printing this e-mail. Epyllion dreams new world with green environment.
^ permalink raw reply
* Urgent Write Back
From: Dr. Raymond Kuo Fung CH'IEN. @ 2010-10-02 16:12 UTC (permalink / raw)
Dear Friend,
I am Dr. Raymond Kuo Fung CHIEN Executive Director and Chief Financial
Officer of the operations of the Hang Seng Bank Ltd.I have a secured
business suggestion for you. Before the U.S and Iraqi war our client Mr
Fayez Abdulaziz Mohammed who was with the Iraqi forces and also a business
man made a numbered fixed deposit for 18 calendar months, with a value of
Thirty Million United State Dollars($30,000,000.00) only in my
branch.Upon maturity several notices was sent to him but there was no
word. Again after the war another notification was sent and still no
response came from him. We later find out that Mr Fayez Abdulaziz Mohammed
and his family had been killed during the war in bomb blast that hit their
home.
After further investigation it was also discovered that Mr Fayez
Abdulaziz Mohammed did not declare any next of kin in his official papers
including the paper work of his bank deposit. And he also confided in me
the last time he was at my office that no one except me knew of his
deposit in my bank. So,Thirty Million United State
Dollars($30,000,000.00)is still lying in my bank and no one will ever
come forward to claim it.What bothers me most is that according to the
laws of my country at the expiration of 8 years the funds will revert to
the ownership of the Hong Kong Government if nobody applies to claim the
funds.
Against this backdrop, my suggestion to you is that I will like you as a
foreigner to stand as the next of kin to Mr Fayez Abdulaziz Mohammed so
that you will be able to receive his funds.
WHAT IS TO BE DONE:
I want you to know that I have had everything planned out so that we shall
come out successful.I have contacted an attorney that will prepare the
necessary document that will back you up as the next of kin to Mr Fayez
Abdulaziz Mohammed, all that is required from you at this stage is for you
to provide me with your Full Names and Address so that the attorney can
commence his job. After you have been made the next of kin to the late Mr
Fayez Abdulaziz Mohammed, the Attorney will also file in for claims on
your behalf and secure the necessary approval and letter of probate in
your favor for the move of the funds to an account that will be provided
by you.There is no risk involved at all in the matter as we are going to
adopt a legalized method and the attorney will prepare all the necessary
documents. Please endeavor to observe utmost discretion in all matters
concerning this issue.Once the funds have been transferred to your
nominated bank account we shall share in the ratio of 70% for me, 30% for
you, Should you be interested please send me the following informations
below,
1. Full name and Age.
2. Occupation
3. Private/office phone number.
4. Current residential address.
And finally after that i shall provide you with more details of this
transaction.Your earliest response to this letter will be highly
appreciated. Write back if intrested at
Email:drraymondkuochien3001@gmail.com or
Email:drraymondkuochien@w.cn
Kind Regards,
Dr.Raymond Kuo Fung CHIEN.
-----------------------------------------------------------------------------------------------------------------------------
Please consider the environmental impact of needlessly printing this e-mail. Epyllion dreams new world with green environment.
^ permalink raw reply
* [PATCH net-next V3] net: dynamic ingress_queue allocation
From: Eric Dumazet @ 2010-10-02 16:11 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: hadi, David Miller, netdev
In-Reply-To: <20101002093255.GA2049@del.dom.local>
Le samedi 02 octobre 2010 à 11:32 +0200, Jarek Poplawski a écrit :
> On Fri, Oct 01, 2010 at 03:56:28PM +0200, Eric Dumazet wrote:
>
> > static void netdev_init_queue_locks(struct net_device *dev)
> > {
> > netdev_for_each_tx_queue(dev, __netdev_init_queue_locks_one, NULL);
> > - __netdev_init_queue_locks_one(dev, &dev->ingress_queue, NULL);
> > + __netdev_init_queue_locks_one(dev, dev_ingress_queue(dev), NULL);
>
> Is dev_ingress_queue(dev) not NULL anytime here?
Yes, but I felt this could be removed later. If you feel its OK right
now, I am OK too ;)
>
> > }
> >
> > unsigned long netdev_fix_features(unsigned long features, const char *name)
> > @@ -5447,16 +5448,37 @@ static void netdev_init_one_queue(struct net_device *dev,
> > struct netdev_queue *queue,
> > void *_unused)
> > {
> > - queue->dev = dev;
> > + if (queue)
> > + queue->dev = dev;
> > }
> >
> > static void netdev_init_queues(struct net_device *dev)
> > {
> > - netdev_init_one_queue(dev, &dev->ingress_queue, NULL);
> > + netdev_init_one_queue(dev, dev_ingress_queue(dev), NULL);
>
> Is dev_ingress_queue(dev) not NULL anytime here?
>
Yes
> > netdev_for_each_tx_queue(dev, netdev_init_one_queue, NULL);
> > spin_lock_init(&dev->tx_global_lock);
> > }
> >
> > +struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
> > +{
> > + struct netdev_queue *queue = dev_ingress_queue(dev);
> > +
> > +#ifdef CONFIG_NET_CLS_ACT
> > + if (queue)
> > + return queue;
> > + queue = kzalloc(sizeof(*queue), GFP_KERNEL);
> > + if (!queue)
> > + return NULL;
> > + netdev_init_one_queue(dev, queue, NULL);
> > + __netdev_init_queue_locks_one(dev, queue, NULL);
> > + queue->qdisc = &noop_qdisc;
> > + queue->qdisc_sleeping = &noop_qdisc;
> > + smp_wmb();
>
> Why don't we need smp_rmb() in handle_ing()?
>
I only wanted to see if Al Viro was using ingress on its Alpha
machine ;)
I am going to use regular rcu api to ease code understanding :)
> > + dev->ingress_queue = queue;
> > +#endif
> > + return queue;
> > +}
> > +
> > /**
> > * alloc_netdev_mq - allocate network device
> > * @sizeof_priv: size of private data to allocate space for
> > @@ -5559,6 +5581,8 @@ void free_netdev(struct net_device *dev)
> >
> > kfree(dev->_tx);
> >
> > + kfree(dev_ingress_queue(dev));
> > +
> > /* Flush device addresses */
> > dev_addr_flush(dev);
> >
> > diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> > index b802078..8635110 100644
> > --- a/net/sched/sch_api.c
> > +++ b/net/sched/sch_api.c
> > @@ -240,7 +240,10 @@ struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle)
> > if (q)
> > goto out;
> >
> > - q = qdisc_match_from_root(dev->ingress_queue.qdisc_sleeping, handle);
> > + if (!dev_ingress_queue(dev))
> > + goto out;
> > + q = qdisc_match_from_root(dev_ingress_queue(dev)->qdisc_sleeping,
> > + handle);
>
> I'd prefer:
> + if (dev_ingress_queue(dev))
> + q = qdisc_match_from_root(dev_ingress_queue(dev)->qdisc_sleeping,
>
Yes
> > out:
> > return q;
> > }
> > @@ -690,6 +693,8 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
> > (new && new->flags & TCQ_F_INGRESS)) {
> > num_q = 1;
> > ingress = 1;
> > + if (!dev_ingress_queue(dev))
> > + return -ENOENT;
>
> Is this test really needed here?
To avoid a NULL dereference some lines later.
Do I have a guarantee its not NULL here ?
>
> > }
> >
> > if (dev->flags & IFF_UP)
> > @@ -701,7 +706,7 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
> > }
> >
> > for (i = 0; i < num_q; i++) {
> > - struct netdev_queue *dev_queue = &dev->ingress_queue;
> > + struct netdev_queue *dev_queue = dev_ingress_queue(dev);
> >
> > if (!ingress)
> > dev_queue = netdev_get_tx_queue(dev, i);
> > @@ -979,7 +984,8 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
> > return -ENOENT;
> > q = qdisc_leaf(p, clid);
> > } else { /* ingress */
> > - q = dev->ingress_queue.qdisc_sleeping;
> > + if (dev_ingress_queue(dev))
> > + q = dev_ingress_queue(dev)->qdisc_sleeping;
> > }
> > } else {
> > q = dev->qdisc;
> > @@ -1044,7 +1050,8 @@ replay:
> > return -ENOENT;
> > q = qdisc_leaf(p, clid);
> > } else { /*ingress */
> > - q = dev->ingress_queue.qdisc_sleeping;
> > + if (dev_ingress_queue_create(dev))
> > + q = dev_ingress_queue(dev)->qdisc_sleeping;
>
> I wonder if doing dev_ingress_queue_create() just before qdisc_create()
> (and the test here) isn't more readable.
Sorry, I dont understand. I want to create ingress_queue only if user
wants it. If we setup (egress) trafic shaping, no need to setup
ingress_queue.
> >
> > @@ -753,7 +755,7 @@ void dev_activate(struct net_device *dev)
> >
> > need_watchdog = 0;
> > netdev_for_each_tx_queue(dev, transition_one_qdisc, &need_watchdog);
> > - transition_one_qdisc(dev, &dev->ingress_queue, NULL);
> > + transition_one_qdisc(dev, dev_ingress_queue(dev), NULL);
>
> I'd prefer here and similarly later:
>
> + if (dev_ingress_queue(dev))
> + transition_one_qdisc(dev, dev_ingress_queue(dev), NULL);
>
> to show NULL dev_queue is only legal in this one case.
OK, thanks a lot for the extended review Jarek (and Jamal of course)
Here is the V3 then.
[PATCH net-next V3] net: dynamic ingress_queue allocation
ingress being not used very much, and net_device->ingress_queue being
quite a big object (128 or 256 bytes), use a dynamic allocation if
needed (tc qdisc add dev eth0 ingress ...)
dev_ingress_queue(dev) helper should be used only with RTNL taken.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
V3: add rcu notations & address Jarek comments
include/linux/netdevice.h | 2 -
include/linux/rtnetlink.h | 8 ++++++
net/core/dev.c | 34 ++++++++++++++++++++++-------
net/sched/sch_api.c | 42 ++++++++++++++++++++++++------------
net/sched/sch_generic.c | 12 ++++++----
5 files changed, 71 insertions(+), 27 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ceed347..92d81ed 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -986,7 +986,7 @@ struct net_device {
rx_handler_func_t *rx_handler;
void *rx_handler_data;
- struct netdev_queue ingress_queue; /* use two cache lines */
+ struct netdev_queue __rcu *ingress_queue;
/*
* Cache lines mostly used on transmit path
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 68c436b..0bb7b48 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -6,6 +6,7 @@
#include <linux/if_link.h>
#include <linux/if_addr.h>
#include <linux/neighbour.h>
+#include <linux/netdevice.h>
/* rtnetlink families. Values up to 127 are reserved for real address
* families, values above 128 may be used arbitrarily.
@@ -769,6 +770,13 @@ extern int lockdep_rtnl_is_held(void);
#define rtnl_dereference(p) \
rcu_dereference_check(p, lockdep_rtnl_is_held())
+static inline struct netdev_queue *dev_ingress_queue(struct net_device *dev)
+{
+ return rtnl_dereference(dev->ingress_queue);
+}
+
+extern struct netdev_queue *dev_ingress_queue_create(struct net_device *dev);
+
extern void rtnetlink_init(void);
extern void __rtnl_unlock(void);
diff --git a/net/core/dev.c b/net/core/dev.c
index a313bab..b078ec8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2702,11 +2702,10 @@ EXPORT_SYMBOL_GPL(br_fdb_test_addr_hook);
* the ingress scheduler, you just cant add policies on ingress.
*
*/
-static int ing_filter(struct sk_buff *skb)
+static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
{
struct net_device *dev = skb->dev;
u32 ttl = G_TC_RTTL(skb->tc_verd);
- struct netdev_queue *rxq;
int result = TC_ACT_OK;
struct Qdisc *q;
@@ -2720,8 +2719,6 @@ static int ing_filter(struct sk_buff *skb)
skb->tc_verd = SET_TC_RTTL(skb->tc_verd, ttl);
skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_INGRESS);
- rxq = &dev->ingress_queue;
-
q = rxq->qdisc;
if (q != &noop_qdisc) {
spin_lock(qdisc_lock(q));
@@ -2737,7 +2734,9 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb,
struct packet_type **pt_prev,
int *ret, struct net_device *orig_dev)
{
- if (skb->dev->ingress_queue.qdisc == &noop_qdisc)
+ struct netdev_queue *rxq = rcu_dereference(skb->dev->ingress_queue);
+
+ if (!rxq || rxq->qdisc == &noop_qdisc)
goto out;
if (*pt_prev) {
@@ -2745,7 +2744,7 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb,
*pt_prev = NULL;
}
- switch (ing_filter(skb)) {
+ switch (ing_filter(skb, rxq)) {
case TC_ACT_SHOT:
case TC_ACT_STOLEN:
kfree_skb(skb);
@@ -4940,7 +4939,6 @@ static void __netdev_init_queue_locks_one(struct net_device *dev,
static void netdev_init_queue_locks(struct net_device *dev)
{
netdev_for_each_tx_queue(dev, __netdev_init_queue_locks_one, NULL);
- __netdev_init_queue_locks_one(dev, &dev->ingress_queue, NULL);
}
unsigned long netdev_fix_features(unsigned long features, const char *name)
@@ -5452,11 +5450,29 @@ static void netdev_init_one_queue(struct net_device *dev,
static void netdev_init_queues(struct net_device *dev)
{
- netdev_init_one_queue(dev, &dev->ingress_queue, NULL);
netdev_for_each_tx_queue(dev, netdev_init_one_queue, NULL);
spin_lock_init(&dev->tx_global_lock);
}
+struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
+{
+ struct netdev_queue *queue = dev_ingress_queue(dev);
+
+#ifdef CONFIG_NET_CLS_ACT
+ if (queue)
+ return queue;
+ queue = kzalloc(sizeof(*queue), GFP_KERNEL);
+ if (!queue)
+ return NULL;
+ netdev_init_one_queue(dev, queue, NULL);
+ __netdev_init_queue_locks_one(dev, queue, NULL);
+ queue->qdisc = &noop_qdisc;
+ queue->qdisc_sleeping = &noop_qdisc;
+ rcu_assign_pointer(dev->ingress_queue, queue);
+#endif
+ return queue;
+}
+
/**
* alloc_netdev_mq - allocate network device
* @sizeof_priv: size of private data to allocate space for
@@ -5559,6 +5575,8 @@ void free_netdev(struct net_device *dev)
kfree(dev->_tx);
+ kfree(rcu_dereference_raw(dev->ingress_queue));
+
/* Flush device addresses */
dev_addr_flush(dev);
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b802078..b22ca2d 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -240,7 +240,10 @@ struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle)
if (q)
goto out;
- q = qdisc_match_from_root(dev->ingress_queue.qdisc_sleeping, handle);
+ if (dev_ingress_queue(dev))
+ q = qdisc_match_from_root(
+ dev_ingress_queue(dev)->qdisc_sleeping,
+ handle);
out:
return q;
}
@@ -690,6 +693,8 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
(new && new->flags & TCQ_F_INGRESS)) {
num_q = 1;
ingress = 1;
+ if (!dev_ingress_queue(dev))
+ return -ENOENT;
}
if (dev->flags & IFF_UP)
@@ -701,7 +706,7 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
}
for (i = 0; i < num_q; i++) {
- struct netdev_queue *dev_queue = &dev->ingress_queue;
+ struct netdev_queue *dev_queue = dev_ingress_queue(dev);
if (!ingress)
dev_queue = netdev_get_tx_queue(dev, i);
@@ -979,7 +984,8 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
return -ENOENT;
q = qdisc_leaf(p, clid);
} else { /* ingress */
- q = dev->ingress_queue.qdisc_sleeping;
+ if (dev_ingress_queue(dev))
+ q = dev_ingress_queue(dev)->qdisc_sleeping;
}
} else {
q = dev->qdisc;
@@ -1043,8 +1049,9 @@ replay:
if ((p = qdisc_lookup(dev, TC_H_MAJ(clid))) == NULL)
return -ENOENT;
q = qdisc_leaf(p, clid);
- } else { /*ingress */
- q = dev->ingress_queue.qdisc_sleeping;
+ } else { /* ingress */
+ if (dev_ingress_queue_create(dev))
+ q = dev_ingress_queue(dev)->qdisc_sleeping;
}
} else {
q = dev->qdisc;
@@ -1123,11 +1130,14 @@ replay:
create_n_graft:
if (!(n->nlmsg_flags&NLM_F_CREATE))
return -ENOENT;
- if (clid == TC_H_INGRESS)
- q = qdisc_create(dev, &dev->ingress_queue, p,
- tcm->tcm_parent, tcm->tcm_parent,
- tca, &err);
- else {
+ if (clid == TC_H_INGRESS) {
+ if (dev_ingress_queue(dev))
+ q = qdisc_create(dev, dev_ingress_queue(dev), p,
+ tcm->tcm_parent, tcm->tcm_parent,
+ tca, &err);
+ else
+ err = -ENOENT;
+ } else {
struct netdev_queue *dev_queue;
if (p && p->ops->cl_ops && p->ops->cl_ops->select_queue)
@@ -1304,8 +1314,10 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
if (tc_dump_qdisc_root(dev->qdisc, skb, cb, &q_idx, s_q_idx) < 0)
goto done;
- dev_queue = &dev->ingress_queue;
- if (tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb, &q_idx, s_q_idx) < 0)
+ dev_queue = dev_ingress_queue(dev);
+ if (dev_queue &&
+ tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb,
+ &q_idx, s_q_idx) < 0)
goto done;
cont:
@@ -1595,8 +1607,10 @@ static int tc_dump_tclass(struct sk_buff *skb, struct netlink_callback *cb)
if (tc_dump_tclass_root(dev->qdisc, skb, tcm, cb, &t, s_t) < 0)
goto done;
- dev_queue = &dev->ingress_queue;
- if (tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb, &t, s_t) < 0)
+ dev_queue = dev_ingress_queue(dev);
+ if (dev_queue &&
+ tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb,
+ &t, s_t) < 0)
goto done;
done:
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 545278a..3d57681 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -753,7 +753,8 @@ void dev_activate(struct net_device *dev)
need_watchdog = 0;
netdev_for_each_tx_queue(dev, transition_one_qdisc, &need_watchdog);
- transition_one_qdisc(dev, &dev->ingress_queue, NULL);
+ if (dev_ingress_queue(dev))
+ transition_one_qdisc(dev, dev_ingress_queue(dev), NULL);
if (need_watchdog) {
dev->trans_start = jiffies;
@@ -812,7 +813,8 @@ static bool some_qdisc_is_busy(struct net_device *dev)
void dev_deactivate(struct net_device *dev)
{
netdev_for_each_tx_queue(dev, dev_deactivate_queue, &noop_qdisc);
- dev_deactivate_queue(dev, &dev->ingress_queue, &noop_qdisc);
+ if (dev_ingress_queue(dev))
+ dev_deactivate_queue(dev, dev_ingress_queue(dev), &noop_qdisc);
dev_watchdog_down(dev);
@@ -838,7 +840,8 @@ void dev_init_scheduler(struct net_device *dev)
{
dev->qdisc = &noop_qdisc;
netdev_for_each_tx_queue(dev, dev_init_scheduler_queue, &noop_qdisc);
- dev_init_scheduler_queue(dev, &dev->ingress_queue, &noop_qdisc);
+ if (dev_ingress_queue(dev))
+ dev_init_scheduler_queue(dev, dev_ingress_queue(dev), &noop_qdisc);
setup_timer(&dev->watchdog_timer, dev_watchdog, (unsigned long)dev);
}
@@ -861,7 +864,8 @@ static void shutdown_scheduler_queue(struct net_device *dev,
void dev_shutdown(struct net_device *dev)
{
netdev_for_each_tx_queue(dev, shutdown_scheduler_queue, &noop_qdisc);
- shutdown_scheduler_queue(dev, &dev->ingress_queue, &noop_qdisc);
+ if (dev_ingress_queue(dev))
+ shutdown_scheduler_queue(dev, dev_ingress_queue(dev), &noop_qdisc);
qdisc_destroy(dev->qdisc);
dev->qdisc = &noop_qdisc;
^ permalink raw reply related
* Re: [ANNOUNCE] ipset-4.4 released
From: Christophe Ngo Van Duc @ 2010-10-02 15:39 UTC (permalink / raw)
To: netdev
In-Reply-To: <alpine.DEB.2.00.1010012214250.19669@blackhole.kfki.hu>
Thanks so much,
I was just facing that very same problem of mixed src,dst with ipportiphash.
Christophe
On Fri, Oct 1, 2010 at 6:02 PM, Jozsef Kadlecsik
<kadlec@blackhole.kfki.hu> wrote:
> Hi,
>
> ipset-4.4 has just been released with one important fix and some small
> corrections:
>
> Kernel part changes:
> - The ipporthash, ipportiphash and ipportnethash set types did
> not work with mixed "src" and "dst" direction parameters of the "set"
> and "SET" iptables match and target (reported by Dash Four)
> - Errorneous semaphore handling in error path fixed (reported by
> Jan Engelhardt, bugzilla id 668)
>
> Userspace changes:
> - Manpage fix to make it clear how ipset works on setlist type
> of sets (John Brendler, bugzilla id 640)
>
> Please note, if you use ipporthash, ipportiphash or ipportnethash type of
> sets with mixed direction parameters of the "set" and "SET" iptables match
> and target, then you must upgrade.
>
> You can download the new version from the usual places:
> http://ipset.netfilter.org
> git://git.netfilter.org/ipset.git
>
> Best regards,
> Jozsef
> -
> E-mail : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
> PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
> Address : KFKI Research Institute for Particle and Nuclear Physics
> H-1525 Budapest 114, POB. 49, Hungary
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH nf-next] ipt_LOG: add bufferisation to call printk() once
From: Eric Dumazet @ 2010-10-02 15:07 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, Netfilter Development Mailinglist
ipt_LOG & ip6t_LOG use lot of calls to printk() and use a lock in a hope
several cpus wont mix their output in syslog.
printk() being very expensive [1], its better to call it once, on a
prebuilt and complete line. Also, with mixed IPv4 and IPv6 trafic,
separate IPv4/IPv6 locks dont avoid garbage.
I used an allocation of a 1024 bytes structure, sort of seq_printf() but
with a fixed size limit.
Use a static buffer if dynamic allocation failed.
Emit a once time alert if buffer size happens to be too short.
[1]: printk() has various features like printk_delay()...
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/netfilter/xt_log.h | 54 ++++++++++
net/ipv4/netfilter/ipt_LOG.c | 145 ++++++++++++++--------------
net/ipv6/netfilter/ip6t_LOG.c | 157 +++++++++++++++----------------
3 files changed, 206 insertions(+), 150 deletions(-)
diff --git a/include/net/netfilter/xt_log.h b/include/net/netfilter/xt_log.h
index e69de29..0dfb34a 100644
--- a/include/net/netfilter/xt_log.h
+++ b/include/net/netfilter/xt_log.h
@@ -0,0 +1,54 @@
+#define S_SIZE (1024 - (sizeof(unsigned int) + 1))
+
+struct sbuff {
+ unsigned int count;
+ char buf[S_SIZE + 1];
+};
+static struct sbuff emergency, *emergency_ptr = &emergency;
+
+static int sb_add(struct sbuff *m, const char *f, ...)
+{
+ va_list args;
+ int len;
+
+ if (likely(m->count < S_SIZE)) {
+ va_start(args, f);
+ len = vsnprintf(m->buf + m->count, S_SIZE - m->count, f, args);
+ va_end(args);
+ if (likely(m->count + len < S_SIZE)) {
+ m->count += len;
+ return 0;
+ }
+ }
+ m->count = S_SIZE;
+ printk_once(KERN_ERR KBUILD_MODNAME " please increase S_SIZE\n");
+ return -1;
+}
+
+static struct sbuff *sb_open(void)
+{
+ struct sbuff *m = kmalloc(sizeof(*m), GFP_ATOMIC);
+
+ if (unlikely(!m)) {
+ local_bh_disable();
+ do {
+ m = xchg(&emergency_ptr, NULL);
+ } while (!m);
+ }
+ m->count = 0;
+ return m;
+}
+
+static void sb_close(struct sbuff *m)
+{
+ m->buf[m->count] = 0;
+ printk("%s\n", m->buf);
+
+ if (likely(m != &emergency))
+ kfree(m);
+ else {
+ xchg(&emergency_ptr, m);
+ local_bh_enable();
+ }
+}
+
diff --git a/net/ipv4/netfilter/ipt_LOG.c b/net/ipv4/netfilter/ipt_LOG.c
index 915fc17..72ffc8f 100644
--- a/net/ipv4/netfilter/ipt_LOG.c
+++ b/net/ipv4/netfilter/ipt_LOG.c
@@ -24,16 +24,15 @@
#include <linux/netfilter/x_tables.h>
#include <linux/netfilter_ipv4/ipt_LOG.h>
#include <net/netfilter/nf_log.h>
+#include <net/netfilter/xt_log.h>
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Netfilter Core Team <coreteam@netfilter.org>");
MODULE_DESCRIPTION("Xtables: IPv4 packet logging to syslog");
-/* Use lock to serialize, so printks don't overlap */
-static DEFINE_SPINLOCK(log_lock);
-
/* One level of recursion won't kill us */
-static void dump_packet(const struct nf_loginfo *info,
+static void dump_packet(struct sbuff *m,
+ const struct nf_loginfo *info,
const struct sk_buff *skb,
unsigned int iphoff)
{
@@ -48,32 +47,32 @@ static void dump_packet(const struct nf_loginfo *info,
ih = skb_header_pointer(skb, iphoff, sizeof(_iph), &_iph);
if (ih == NULL) {
- printk("TRUNCATED");
+ sb_add(m, "TRUNCATED");
return;
}
/* Important fields:
* TOS, len, DF/MF, fragment offset, TTL, src, dst, options. */
/* Max length: 40 "SRC=255.255.255.255 DST=255.255.255.255 " */
- printk("SRC=%pI4 DST=%pI4 ",
+ sb_add(m, "SRC=%pI4 DST=%pI4 ",
&ih->saddr, &ih->daddr);
/* Max length: 46 "LEN=65535 TOS=0xFF PREC=0xFF TTL=255 ID=65535 " */
- printk("LEN=%u TOS=0x%02X PREC=0x%02X TTL=%u ID=%u ",
+ sb_add(m, "LEN=%u TOS=0x%02X PREC=0x%02X TTL=%u ID=%u ",
ntohs(ih->tot_len), ih->tos & IPTOS_TOS_MASK,
ih->tos & IPTOS_PREC_MASK, ih->ttl, ntohs(ih->id));
/* Max length: 6 "CE DF MF " */
if (ntohs(ih->frag_off) & IP_CE)
- printk("CE ");
+ sb_add(m, "CE ");
if (ntohs(ih->frag_off) & IP_DF)
- printk("DF ");
+ sb_add(m, "DF ");
if (ntohs(ih->frag_off) & IP_MF)
- printk("MF ");
+ sb_add(m, "MF ");
/* Max length: 11 "FRAG:65535 " */
if (ntohs(ih->frag_off) & IP_OFFSET)
- printk("FRAG:%u ", ntohs(ih->frag_off) & IP_OFFSET);
+ sb_add(m, "FRAG:%u ", ntohs(ih->frag_off) & IP_OFFSET);
if ((logflags & IPT_LOG_IPOPT) &&
ih->ihl * 4 > sizeof(struct iphdr)) {
@@ -85,15 +84,15 @@ static void dump_packet(const struct nf_loginfo *info,
op = skb_header_pointer(skb, iphoff+sizeof(_iph),
optsize, _opt);
if (op == NULL) {
- printk("TRUNCATED");
+ sb_add(m, "TRUNCATED");
return;
}
/* Max length: 127 "OPT (" 15*4*2chars ") " */
- printk("OPT (");
+ sb_add(m, "OPT (");
for (i = 0; i < optsize; i++)
- printk("%02X", op[i]);
- printk(") ");
+ sb_add(m, "%02X", op[i]);
+ sb_add(m, ") ");
}
switch (ih->protocol) {
@@ -102,7 +101,7 @@ static void dump_packet(const struct nf_loginfo *info,
const struct tcphdr *th;
/* Max length: 10 "PROTO=TCP " */
- printk("PROTO=TCP ");
+ sb_add(m, "PROTO=TCP ");
if (ntohs(ih->frag_off) & IP_OFFSET)
break;
@@ -111,41 +110,41 @@ static void dump_packet(const struct nf_loginfo *info,
th = skb_header_pointer(skb, iphoff + ih->ihl * 4,
sizeof(_tcph), &_tcph);
if (th == NULL) {
- printk("INCOMPLETE [%u bytes] ",
+ sb_add(m, "INCOMPLETE [%u bytes] ",
skb->len - iphoff - ih->ihl*4);
break;
}
/* Max length: 20 "SPT=65535 DPT=65535 " */
- printk("SPT=%u DPT=%u ",
+ sb_add(m, "SPT=%u DPT=%u ",
ntohs(th->source), ntohs(th->dest));
/* Max length: 30 "SEQ=4294967295 ACK=4294967295 " */
if (logflags & IPT_LOG_TCPSEQ)
- printk("SEQ=%u ACK=%u ",
+ sb_add(m, "SEQ=%u ACK=%u ",
ntohl(th->seq), ntohl(th->ack_seq));
/* Max length: 13 "WINDOW=65535 " */
- printk("WINDOW=%u ", ntohs(th->window));
+ sb_add(m, "WINDOW=%u ", ntohs(th->window));
/* Max length: 9 "RES=0x3F " */
- printk("RES=0x%02x ", (u8)(ntohl(tcp_flag_word(th) & TCP_RESERVED_BITS) >> 22));
+ sb_add(m, "RES=0x%02x ", (u8)(ntohl(tcp_flag_word(th) & TCP_RESERVED_BITS) >> 22));
/* Max length: 32 "CWR ECE URG ACK PSH RST SYN FIN " */
if (th->cwr)
- printk("CWR ");
+ sb_add(m, "CWR ");
if (th->ece)
- printk("ECE ");
+ sb_add(m, "ECE ");
if (th->urg)
- printk("URG ");
+ sb_add(m, "URG ");
if (th->ack)
- printk("ACK ");
+ sb_add(m, "ACK ");
if (th->psh)
- printk("PSH ");
+ sb_add(m, "PSH ");
if (th->rst)
- printk("RST ");
+ sb_add(m, "RST ");
if (th->syn)
- printk("SYN ");
+ sb_add(m, "SYN ");
if (th->fin)
- printk("FIN ");
+ sb_add(m, "FIN ");
/* Max length: 11 "URGP=65535 " */
- printk("URGP=%u ", ntohs(th->urg_ptr));
+ sb_add(m, "URGP=%u ", ntohs(th->urg_ptr));
if ((logflags & IPT_LOG_TCPOPT) &&
th->doff * 4 > sizeof(struct tcphdr)) {
@@ -158,15 +157,15 @@ static void dump_packet(const struct nf_loginfo *info,
iphoff+ih->ihl*4+sizeof(_tcph),
optsize, _opt);
if (op == NULL) {
- printk("TRUNCATED");
+ sb_add(m, "TRUNCATED");
return;
}
/* Max length: 127 "OPT (" 15*4*2chars ") " */
- printk("OPT (");
+ sb_add(m, "OPT (");
for (i = 0; i < optsize; i++)
- printk("%02X", op[i]);
- printk(") ");
+ sb_add(m, "%02X", op[i]);
+ sb_add(m, ") ");
}
break;
}
@@ -177,9 +176,9 @@ static void dump_packet(const struct nf_loginfo *info,
if (ih->protocol == IPPROTO_UDP)
/* Max length: 10 "PROTO=UDP " */
- printk("PROTO=UDP " );
+ sb_add(m, "PROTO=UDP " );
else /* Max length: 14 "PROTO=UDPLITE " */
- printk("PROTO=UDPLITE ");
+ sb_add(m, "PROTO=UDPLITE ");
if (ntohs(ih->frag_off) & IP_OFFSET)
break;
@@ -188,13 +187,13 @@ static void dump_packet(const struct nf_loginfo *info,
uh = skb_header_pointer(skb, iphoff+ih->ihl*4,
sizeof(_udph), &_udph);
if (uh == NULL) {
- printk("INCOMPLETE [%u bytes] ",
+ sb_add(m, "INCOMPLETE [%u bytes] ",
skb->len - iphoff - ih->ihl*4);
break;
}
/* Max length: 20 "SPT=65535 DPT=65535 " */
- printk("SPT=%u DPT=%u LEN=%u ",
+ sb_add(m, "SPT=%u DPT=%u LEN=%u ",
ntohs(uh->source), ntohs(uh->dest),
ntohs(uh->len));
break;
@@ -221,7 +220,7 @@ static void dump_packet(const struct nf_loginfo *info,
[ICMP_ADDRESSREPLY] = 12 };
/* Max length: 11 "PROTO=ICMP " */
- printk("PROTO=ICMP ");
+ sb_add(m, "PROTO=ICMP ");
if (ntohs(ih->frag_off) & IP_OFFSET)
break;
@@ -230,19 +229,19 @@ static void dump_packet(const struct nf_loginfo *info,
ich = skb_header_pointer(skb, iphoff + ih->ihl * 4,
sizeof(_icmph), &_icmph);
if (ich == NULL) {
- printk("INCOMPLETE [%u bytes] ",
+ sb_add(m, "INCOMPLETE [%u bytes] ",
skb->len - iphoff - ih->ihl*4);
break;
}
/* Max length: 18 "TYPE=255 CODE=255 " */
- printk("TYPE=%u CODE=%u ", ich->type, ich->code);
+ sb_add(m, "TYPE=%u CODE=%u ", ich->type, ich->code);
/* Max length: 25 "INCOMPLETE [65535 bytes] " */
if (ich->type <= NR_ICMP_TYPES &&
required_len[ich->type] &&
skb->len-iphoff-ih->ihl*4 < required_len[ich->type]) {
- printk("INCOMPLETE [%u bytes] ",
+ sb_add(m, "INCOMPLETE [%u bytes] ",
skb->len - iphoff - ih->ihl*4);
break;
}
@@ -251,35 +250,35 @@ static void dump_packet(const struct nf_loginfo *info,
case ICMP_ECHOREPLY:
case ICMP_ECHO:
/* Max length: 19 "ID=65535 SEQ=65535 " */
- printk("ID=%u SEQ=%u ",
+ sb_add(m, "ID=%u SEQ=%u ",
ntohs(ich->un.echo.id),
ntohs(ich->un.echo.sequence));
break;
case ICMP_PARAMETERPROB:
/* Max length: 14 "PARAMETER=255 " */
- printk("PARAMETER=%u ",
+ sb_add(m, "PARAMETER=%u ",
ntohl(ich->un.gateway) >> 24);
break;
case ICMP_REDIRECT:
/* Max length: 24 "GATEWAY=255.255.255.255 " */
- printk("GATEWAY=%pI4 ", &ich->un.gateway);
+ sb_add(m, "GATEWAY=%pI4 ", &ich->un.gateway);
/* Fall through */
case ICMP_DEST_UNREACH:
case ICMP_SOURCE_QUENCH:
case ICMP_TIME_EXCEEDED:
/* Max length: 3+maxlen */
if (!iphoff) { /* Only recurse once. */
- printk("[");
- dump_packet(info, skb,
+ sb_add(m, "[");
+ dump_packet(m, info, skb,
iphoff + ih->ihl*4+sizeof(_icmph));
- printk("] ");
+ sb_add(m, "] ");
}
/* Max length: 10 "MTU=65535 " */
if (ich->type == ICMP_DEST_UNREACH &&
ich->code == ICMP_FRAG_NEEDED)
- printk("MTU=%u ", ntohs(ich->un.frag.mtu));
+ sb_add(m, "MTU=%u ", ntohs(ich->un.frag.mtu));
}
break;
}
@@ -292,19 +291,19 @@ static void dump_packet(const struct nf_loginfo *info,
break;
/* Max length: 9 "PROTO=AH " */
- printk("PROTO=AH ");
+ sb_add(m, "PROTO=AH ");
/* Max length: 25 "INCOMPLETE [65535 bytes] " */
ah = skb_header_pointer(skb, iphoff+ih->ihl*4,
sizeof(_ahdr), &_ahdr);
if (ah == NULL) {
- printk("INCOMPLETE [%u bytes] ",
+ sb_add(m, "INCOMPLETE [%u bytes] ",
skb->len - iphoff - ih->ihl*4);
break;
}
/* Length: 15 "SPI=0xF1234567 " */
- printk("SPI=0x%x ", ntohl(ah->spi));
+ sb_add(m, "SPI=0x%x ", ntohl(ah->spi));
break;
}
case IPPROTO_ESP: {
@@ -312,7 +311,7 @@ static void dump_packet(const struct nf_loginfo *info,
const struct ip_esp_hdr *eh;
/* Max length: 10 "PROTO=ESP " */
- printk("PROTO=ESP ");
+ sb_add(m, "PROTO=ESP ");
if (ntohs(ih->frag_off) & IP_OFFSET)
break;
@@ -321,25 +320,25 @@ static void dump_packet(const struct nf_loginfo *info,
eh = skb_header_pointer(skb, iphoff+ih->ihl*4,
sizeof(_esph), &_esph);
if (eh == NULL) {
- printk("INCOMPLETE [%u bytes] ",
+ sb_add(m, "INCOMPLETE [%u bytes] ",
skb->len - iphoff - ih->ihl*4);
break;
}
/* Length: 15 "SPI=0xF1234567 " */
- printk("SPI=0x%x ", ntohl(eh->spi));
+ sb_add(m, "SPI=0x%x ", ntohl(eh->spi));
break;
}
/* Max length: 10 "PROTO 255 " */
default:
- printk("PROTO=%u ", ih->protocol);
+ sb_add(m, "PROTO=%u ", ih->protocol);
}
/* Max length: 15 "UID=4294967295 " */
if ((logflags & IPT_LOG_UID) && !iphoff && skb->sk) {
read_lock_bh(&skb->sk->sk_callback_lock);
if (skb->sk->sk_socket && skb->sk->sk_socket->file)
- printk("UID=%u GID=%u ",
+ sb_add(m, "UID=%u GID=%u ",
skb->sk->sk_socket->file->f_cred->fsuid,
skb->sk->sk_socket->file->f_cred->fsgid);
read_unlock_bh(&skb->sk->sk_callback_lock);
@@ -347,7 +346,7 @@ static void dump_packet(const struct nf_loginfo *info,
/* Max length: 16 "MARK=0xFFFFFFFF " */
if (!iphoff && skb->mark)
- printk("MARK=0x%x ", skb->mark);
+ sb_add(m, "MARK=0x%x ", skb->mark);
/* Proto Max log string length */
/* IP: 40+46+6+11+127 = 230 */
@@ -364,7 +363,8 @@ static void dump_packet(const struct nf_loginfo *info,
/* maxlen = 230+ 91 + 230 + 252 = 803 */
}
-static void dump_mac_header(const struct nf_loginfo *info,
+static void dump_mac_header(struct sbuff *m,
+ const struct nf_loginfo *info,
const struct sk_buff *skb)
{
struct net_device *dev = skb->dev;
@@ -378,7 +378,7 @@ static void dump_mac_header(const struct nf_loginfo *info,
switch (dev->type) {
case ARPHRD_ETHER:
- printk("MACSRC=%pM MACDST=%pM MACPROTO=%04x ",
+ sb_add(m, "MACSRC=%pM MACDST=%pM MACPROTO=%04x ",
eth_hdr(skb)->h_source, eth_hdr(skb)->h_dest,
ntohs(eth_hdr(skb)->h_proto));
return;
@@ -387,17 +387,17 @@ static void dump_mac_header(const struct nf_loginfo *info,
}
fallback:
- printk("MAC=");
+ sb_add(m, "MAC=");
if (dev->hard_header_len &&
skb->mac_header != skb->network_header) {
const unsigned char *p = skb_mac_header(skb);
unsigned int i;
- printk("%02x", *p++);
+ sb_add(m, "%02x", *p++);
for (i = 1; i < dev->hard_header_len; i++, p++)
- printk(":%02x", *p);
+ sb_add(m, ":%02x", *p);
}
- printk(" ");
+ sb_add(m, " ");
}
static struct nf_loginfo default_loginfo = {
@@ -419,11 +419,12 @@ ipt_log_packet(u_int8_t pf,
const struct nf_loginfo *loginfo,
const char *prefix)
{
+ struct sbuff *m = sb_open();
+
if (!loginfo)
loginfo = &default_loginfo;
- spin_lock_bh(&log_lock);
- printk("<%d>%sIN=%s OUT=%s ", loginfo->u.log.level,
+ sb_add(m, "<%d>%sIN=%s OUT=%s ", loginfo->u.log.level,
prefix,
in ? in->name : "",
out ? out->name : "");
@@ -434,20 +435,20 @@ ipt_log_packet(u_int8_t pf,
physindev = skb->nf_bridge->physindev;
if (physindev && in != physindev)
- printk("PHYSIN=%s ", physindev->name);
+ sb_add(m, "PHYSIN=%s ", physindev->name);
physoutdev = skb->nf_bridge->physoutdev;
if (physoutdev && out != physoutdev)
- printk("PHYSOUT=%s ", physoutdev->name);
+ sb_add(m, "PHYSOUT=%s ", physoutdev->name);
}
#endif
/* MAC logging for input path only. */
if (in && !out)
- dump_mac_header(loginfo, skb);
+ dump_mac_header(m, loginfo, skb);
+
+ dump_packet(m, loginfo, skb, 0);
- dump_packet(loginfo, skb, 0);
- printk("\n");
- spin_unlock_bh(&log_lock);
+ sb_close(m);
}
static unsigned int
diff --git a/net/ipv6/netfilter/ip6t_LOG.c b/net/ipv6/netfilter/ip6t_LOG.c
index 0a07ae7..09c8889 100644
--- a/net/ipv6/netfilter/ip6t_LOG.c
+++ b/net/ipv6/netfilter/ip6t_LOG.c
@@ -23,6 +23,7 @@
#include <linux/netfilter/x_tables.h>
#include <linux/netfilter_ipv6/ip6_tables.h>
#include <net/netfilter/nf_log.h>
+#include <net/netfilter/xt_log.h>
MODULE_AUTHOR("Jan Rekorajski <baggins@pld.org.pl>");
MODULE_DESCRIPTION("Xtables: IPv6 packet logging to syslog");
@@ -32,11 +33,9 @@ struct in_device;
#include <net/route.h>
#include <linux/netfilter_ipv6/ip6t_LOG.h>
-/* Use lock to serialize, so printks don't overlap */
-static DEFINE_SPINLOCK(log_lock);
-
/* One level of recursion won't kill us */
-static void dump_packet(const struct nf_loginfo *info,
+static void dump_packet(struct sbuff *m,
+ const struct nf_loginfo *info,
const struct sk_buff *skb, unsigned int ip6hoff,
int recurse)
{
@@ -55,15 +54,15 @@ static void dump_packet(const struct nf_loginfo *info,
ih = skb_header_pointer(skb, ip6hoff, sizeof(_ip6h), &_ip6h);
if (ih == NULL) {
- printk("TRUNCATED");
+ sb_add(m, "TRUNCATED");
return;
}
/* Max length: 88 "SRC=0000.0000.0000.0000.0000.0000.0000.0000 DST=0000.0000.0000.0000.0000.0000.0000.0000 " */
- printk("SRC=%pI6 DST=%pI6 ", &ih->saddr, &ih->daddr);
+ sb_add(m, "SRC=%pI6 DST=%pI6 ", &ih->saddr, &ih->daddr);
/* Max length: 44 "LEN=65535 TC=255 HOPLIMIT=255 FLOWLBL=FFFFF " */
- printk("LEN=%Zu TC=%u HOPLIMIT=%u FLOWLBL=%u ",
+ sb_add(m, "LEN=%Zu TC=%u HOPLIMIT=%u FLOWLBL=%u ",
ntohs(ih->payload_len) + sizeof(struct ipv6hdr),
(ntohl(*(__be32 *)ih) & 0x0ff00000) >> 20,
ih->hop_limit,
@@ -78,35 +77,35 @@ static void dump_packet(const struct nf_loginfo *info,
hp = skb_header_pointer(skb, ptr, sizeof(_hdr), &_hdr);
if (hp == NULL) {
- printk("TRUNCATED");
+ sb_add(m, "TRUNCATED");
return;
}
/* Max length: 48 "OPT (...) " */
if (logflags & IP6T_LOG_IPOPT)
- printk("OPT ( ");
+ sb_add(m, "OPT ( ");
switch (currenthdr) {
case IPPROTO_FRAGMENT: {
struct frag_hdr _fhdr;
const struct frag_hdr *fh;
- printk("FRAG:");
+ sb_add(m, "FRAG:");
fh = skb_header_pointer(skb, ptr, sizeof(_fhdr),
&_fhdr);
if (fh == NULL) {
- printk("TRUNCATED ");
+ sb_add(m, "TRUNCATED ");
return;
}
/* Max length: 6 "65535 " */
- printk("%u ", ntohs(fh->frag_off) & 0xFFF8);
+ sb_add(m, "%u ", ntohs(fh->frag_off) & 0xFFF8);
/* Max length: 11 "INCOMPLETE " */
if (fh->frag_off & htons(0x0001))
- printk("INCOMPLETE ");
+ sb_add(m, "INCOMPLETE ");
- printk("ID:%08x ", ntohl(fh->identification));
+ sb_add(m, "ID:%08x ", ntohl(fh->identification));
if (ntohs(fh->frag_off) & 0xFFF8)
fragment = 1;
@@ -120,7 +119,7 @@ static void dump_packet(const struct nf_loginfo *info,
case IPPROTO_HOPOPTS:
if (fragment) {
if (logflags & IP6T_LOG_IPOPT)
- printk(")");
+ sb_add(m, ")");
return;
}
hdrlen = ipv6_optlen(hp);
@@ -132,10 +131,10 @@ static void dump_packet(const struct nf_loginfo *info,
const struct ip_auth_hdr *ah;
/* Max length: 3 "AH " */
- printk("AH ");
+ sb_add(m, "AH ");
if (fragment) {
- printk(")");
+ sb_add(m, ")");
return;
}
@@ -146,13 +145,13 @@ static void dump_packet(const struct nf_loginfo *info,
* Max length: 26 "INCOMPLETE [65535
* bytes] )"
*/
- printk("INCOMPLETE [%u bytes] )",
+ sb_add(m, "INCOMPLETE [%u bytes] )",
skb->len - ptr);
return;
}
/* Length: 15 "SPI=0xF1234567 */
- printk("SPI=0x%x ", ntohl(ah->spi));
+ sb_add(m, "SPI=0x%x ", ntohl(ah->spi));
}
@@ -164,10 +163,10 @@ static void dump_packet(const struct nf_loginfo *info,
const struct ip_esp_hdr *eh;
/* Max length: 4 "ESP " */
- printk("ESP ");
+ sb_add(m, "ESP ");
if (fragment) {
- printk(")");
+ sb_add(m, ")");
return;
}
@@ -177,23 +176,23 @@ static void dump_packet(const struct nf_loginfo *info,
eh = skb_header_pointer(skb, ptr, sizeof(_esph),
&_esph);
if (eh == NULL) {
- printk("INCOMPLETE [%u bytes] )",
+ sb_add(m, "INCOMPLETE [%u bytes] )",
skb->len - ptr);
return;
}
/* Length: 16 "SPI=0xF1234567 )" */
- printk("SPI=0x%x )", ntohl(eh->spi) );
+ sb_add(m, "SPI=0x%x )", ntohl(eh->spi) );
}
return;
default:
/* Max length: 20 "Unknown Ext Hdr 255" */
- printk("Unknown Ext Hdr %u", currenthdr);
+ sb_add(m, "Unknown Ext Hdr %u", currenthdr);
return;
}
if (logflags & IP6T_LOG_IPOPT)
- printk(") ");
+ sb_add(m, ") ");
currenthdr = hp->nexthdr;
ptr += hdrlen;
@@ -205,7 +204,7 @@ static void dump_packet(const struct nf_loginfo *info,
const struct tcphdr *th;
/* Max length: 10 "PROTO=TCP " */
- printk("PROTO=TCP ");
+ sb_add(m, "PROTO=TCP ");
if (fragment)
break;
@@ -213,40 +212,40 @@ static void dump_packet(const struct nf_loginfo *info,
/* Max length: 25 "INCOMPLETE [65535 bytes] " */
th = skb_header_pointer(skb, ptr, sizeof(_tcph), &_tcph);
if (th == NULL) {
- printk("INCOMPLETE [%u bytes] ", skb->len - ptr);
+ sb_add(m, "INCOMPLETE [%u bytes] ", skb->len - ptr);
return;
}
/* Max length: 20 "SPT=65535 DPT=65535 " */
- printk("SPT=%u DPT=%u ",
+ sb_add(m, "SPT=%u DPT=%u ",
ntohs(th->source), ntohs(th->dest));
/* Max length: 30 "SEQ=4294967295 ACK=4294967295 " */
if (logflags & IP6T_LOG_TCPSEQ)
- printk("SEQ=%u ACK=%u ",
+ sb_add(m, "SEQ=%u ACK=%u ",
ntohl(th->seq), ntohl(th->ack_seq));
/* Max length: 13 "WINDOW=65535 " */
- printk("WINDOW=%u ", ntohs(th->window));
+ sb_add(m, "WINDOW=%u ", ntohs(th->window));
/* Max length: 9 "RES=0x3C " */
- printk("RES=0x%02x ", (u_int8_t)(ntohl(tcp_flag_word(th) & TCP_RESERVED_BITS) >> 22));
+ sb_add(m, "RES=0x%02x ", (u_int8_t)(ntohl(tcp_flag_word(th) & TCP_RESERVED_BITS) >> 22));
/* Max length: 32 "CWR ECE URG ACK PSH RST SYN FIN " */
if (th->cwr)
- printk("CWR ");
+ sb_add(m, "CWR ");
if (th->ece)
- printk("ECE ");
+ sb_add(m, "ECE ");
if (th->urg)
- printk("URG ");
+ sb_add(m, "URG ");
if (th->ack)
- printk("ACK ");
+ sb_add(m, "ACK ");
if (th->psh)
- printk("PSH ");
+ sb_add(m, "PSH ");
if (th->rst)
- printk("RST ");
+ sb_add(m, "RST ");
if (th->syn)
- printk("SYN ");
+ sb_add(m, "SYN ");
if (th->fin)
- printk("FIN ");
+ sb_add(m, "FIN ");
/* Max length: 11 "URGP=65535 " */
- printk("URGP=%u ", ntohs(th->urg_ptr));
+ sb_add(m, "URGP=%u ", ntohs(th->urg_ptr));
if ((logflags & IP6T_LOG_TCPOPT) &&
th->doff * 4 > sizeof(struct tcphdr)) {
@@ -260,15 +259,15 @@ static void dump_packet(const struct nf_loginfo *info,
ptr + sizeof(struct tcphdr),
optsize, _opt);
if (op == NULL) {
- printk("OPT (TRUNCATED)");
+ sb_add(m, "OPT (TRUNCATED)");
return;
}
/* Max length: 127 "OPT (" 15*4*2chars ") " */
- printk("OPT (");
+ sb_add(m, "OPT (");
for (i =0; i < optsize; i++)
- printk("%02X", op[i]);
- printk(") ");
+ sb_add(m, "%02X", op[i]);
+ sb_add(m, ") ");
}
break;
}
@@ -279,9 +278,9 @@ static void dump_packet(const struct nf_loginfo *info,
if (currenthdr == IPPROTO_UDP)
/* Max length: 10 "PROTO=UDP " */
- printk("PROTO=UDP " );
+ sb_add(m, "PROTO=UDP " );
else /* Max length: 14 "PROTO=UDPLITE " */
- printk("PROTO=UDPLITE ");
+ sb_add(m, "PROTO=UDPLITE ");
if (fragment)
break;
@@ -289,12 +288,12 @@ static void dump_packet(const struct nf_loginfo *info,
/* Max length: 25 "INCOMPLETE [65535 bytes] " */
uh = skb_header_pointer(skb, ptr, sizeof(_udph), &_udph);
if (uh == NULL) {
- printk("INCOMPLETE [%u bytes] ", skb->len - ptr);
+ sb_add(m, "INCOMPLETE [%u bytes] ", skb->len - ptr);
return;
}
/* Max length: 20 "SPT=65535 DPT=65535 " */
- printk("SPT=%u DPT=%u LEN=%u ",
+ sb_add(m, "SPT=%u DPT=%u LEN=%u ",
ntohs(uh->source), ntohs(uh->dest),
ntohs(uh->len));
break;
@@ -304,7 +303,7 @@ static void dump_packet(const struct nf_loginfo *info,
const struct icmp6hdr *ic;
/* Max length: 13 "PROTO=ICMPv6 " */
- printk("PROTO=ICMPv6 ");
+ sb_add(m, "PROTO=ICMPv6 ");
if (fragment)
break;
@@ -312,18 +311,18 @@ static void dump_packet(const struct nf_loginfo *info,
/* Max length: 25 "INCOMPLETE [65535 bytes] " */
ic = skb_header_pointer(skb, ptr, sizeof(_icmp6h), &_icmp6h);
if (ic == NULL) {
- printk("INCOMPLETE [%u bytes] ", skb->len - ptr);
+ sb_add(m, "INCOMPLETE [%u bytes] ", skb->len - ptr);
return;
}
/* Max length: 18 "TYPE=255 CODE=255 " */
- printk("TYPE=%u CODE=%u ", ic->icmp6_type, ic->icmp6_code);
+ sb_add(m, "TYPE=%u CODE=%u ", ic->icmp6_type, ic->icmp6_code);
switch (ic->icmp6_type) {
case ICMPV6_ECHO_REQUEST:
case ICMPV6_ECHO_REPLY:
/* Max length: 19 "ID=65535 SEQ=65535 " */
- printk("ID=%u SEQ=%u ",
+ sb_add(m, "ID=%u SEQ=%u ",
ntohs(ic->icmp6_identifier),
ntohs(ic->icmp6_sequence));
break;
@@ -334,35 +333,35 @@ static void dump_packet(const struct nf_loginfo *info,
case ICMPV6_PARAMPROB:
/* Max length: 17 "POINTER=ffffffff " */
- printk("POINTER=%08x ", ntohl(ic->icmp6_pointer));
+ sb_add(m, "POINTER=%08x ", ntohl(ic->icmp6_pointer));
/* Fall through */
case ICMPV6_DEST_UNREACH:
case ICMPV6_PKT_TOOBIG:
case ICMPV6_TIME_EXCEED:
/* Max length: 3+maxlen */
if (recurse) {
- printk("[");
- dump_packet(info, skb, ptr + sizeof(_icmp6h),
- 0);
- printk("] ");
+ sb_add(m, "[");
+ dump_packet(m, info, skb,
+ ptr + sizeof(_icmp6h), 0);
+ sb_add(m, "] ");
}
/* Max length: 10 "MTU=65535 " */
if (ic->icmp6_type == ICMPV6_PKT_TOOBIG)
- printk("MTU=%u ", ntohl(ic->icmp6_mtu));
+ sb_add(m, "MTU=%u ", ntohl(ic->icmp6_mtu));
}
break;
}
/* Max length: 10 "PROTO=255 " */
default:
- printk("PROTO=%u ", currenthdr);
+ sb_add(m, "PROTO=%u ", currenthdr);
}
/* Max length: 15 "UID=4294967295 " */
if ((logflags & IP6T_LOG_UID) && recurse && skb->sk) {
read_lock_bh(&skb->sk->sk_callback_lock);
if (skb->sk->sk_socket && skb->sk->sk_socket->file)
- printk("UID=%u GID=%u ",
+ sb_add(m, "UID=%u GID=%u ",
skb->sk->sk_socket->file->f_cred->fsuid,
skb->sk->sk_socket->file->f_cred->fsgid);
read_unlock_bh(&skb->sk->sk_callback_lock);
@@ -370,10 +369,11 @@ static void dump_packet(const struct nf_loginfo *info,
/* Max length: 16 "MARK=0xFFFFFFFF " */
if (!recurse && skb->mark)
- printk("MARK=0x%x ", skb->mark);
+ sb_add(m, "MARK=0x%x ", skb->mark);
}
-static void dump_mac_header(const struct nf_loginfo *info,
+static void dump_mac_header(struct sbuff *m,
+ const struct nf_loginfo *info,
const struct sk_buff *skb)
{
struct net_device *dev = skb->dev;
@@ -387,7 +387,7 @@ static void dump_mac_header(const struct nf_loginfo *info,
switch (dev->type) {
case ARPHRD_ETHER:
- printk("MACSRC=%pM MACDST=%pM MACPROTO=%04x ",
+ sb_add(m, "MACSRC=%pM MACDST=%pM MACPROTO=%04x ",
eth_hdr(skb)->h_source, eth_hdr(skb)->h_dest,
ntohs(eth_hdr(skb)->h_proto));
return;
@@ -396,7 +396,7 @@ static void dump_mac_header(const struct nf_loginfo *info,
}
fallback:
- printk("MAC=");
+ sb_add(m, "MAC=");
if (dev->hard_header_len &&
skb->mac_header != skb->network_header) {
const unsigned char *p = skb_mac_header(skb);
@@ -408,19 +408,19 @@ fallback:
p = NULL;
if (p != NULL) {
- printk("%02x", *p++);
+ sb_add(m, "%02x", *p++);
for (i = 1; i < len; i++)
- printk(":%02x", p[i]);
+ sb_add(m, ":%02x", p[i]);
}
- printk(" ");
+ sb_add(m, " ");
if (dev->type == ARPHRD_SIT) {
const struct iphdr *iph =
(struct iphdr *)skb_mac_header(skb);
- printk("TUNNEL=%pI4->%pI4 ", &iph->saddr, &iph->daddr);
+ sb_add(m, "TUNNEL=%pI4->%pI4 ", &iph->saddr, &iph->daddr);
}
} else
- printk(" ");
+ sb_add(m, " ");
}
static struct nf_loginfo default_loginfo = {
@@ -442,22 +442,23 @@ ip6t_log_packet(u_int8_t pf,
const struct nf_loginfo *loginfo,
const char *prefix)
{
+ struct sbuff *m = sb_open();
+
if (!loginfo)
loginfo = &default_loginfo;
- spin_lock_bh(&log_lock);
- printk("<%d>%sIN=%s OUT=%s ", loginfo->u.log.level,
- prefix,
- in ? in->name : "",
- out ? out->name : "");
+ sb_add(m, "<%d>%sIN=%s OUT=%s ", loginfo->u.log.level,
+ prefix,
+ in ? in->name : "",
+ out ? out->name : "");
/* MAC logging for input path only. */
if (in && !out)
- dump_mac_header(loginfo, skb);
+ dump_mac_header(m, loginfo, skb);
+
+ dump_packet(m, loginfo, skb, skb_network_offset(skb), 1);
- dump_packet(loginfo, skb, skb_network_offset(skb), 1);
- printk("\n");
- spin_unlock_bh(&log_lock);
+ sb_close(m);
}
static unsigned int
^ permalink raw reply related
* Re: [PATCH 4/4] drivers/atm/idt77252.c: Remove unnecessary error check
From: Julia Lawall @ 2010-10-02 14:37 UTC (permalink / raw)
To: walter harms
Cc: Chas Williams, kernel-janitors, linux-atm-general, netdev,
linux-kernel
In-Reply-To: <4CA73D26.7090109@bfs.de>
This code does not call deinit_card(card); in an error case, as done in
other error-handling code in the same function. But actually, the called
function init_sram can only return 0, so there is no need for the error
check at all.
init_sram is also given a void return type, and its single return statement
at the end of the function is dropped.
A simplified version of the sematic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
@r@
statement S1,S2,S3;
constant C1,C2,C3;
@@
*if (...)
{... S1 return -C1;}
...
*if (...)
{... when != S1
return -C2;}
...
*if (...)
{... S1 return -C3;}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/atm/idt77252.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
index 1679cbf..bce5732 100644
--- a/drivers/atm/idt77252.c
+++ b/drivers/atm/idt77252.c
@@ -3152,7 +3152,7 @@ deinit_card(struct idt77252_dev *card)
}
-static int __devinit
+static void __devinit
init_sram(struct idt77252_dev *card)
{
int i;
@@ -3298,7 +3298,6 @@ init_sram(struct idt77252_dev *card)
SAR_REG_RXFD);
IPRINTK("%s: SRAM initialization complete.\n", card->name);
- return 0;
}
static int __devinit
@@ -3410,8 +3409,7 @@ init_card(struct atm_dev *dev)
writel(readl(SAR_REG_CFG) | conf, SAR_REG_CFG);
- if (init_sram(card) < 0)
- return -1;
+ init_sram(card);
/********************************************************************/
/* A L L O C R A M A N D S E T V A R I O U S T H I N G S */
^ permalink raw reply related
* Attention Beneficiary,
From: Barrister.Brandon Moore @ 2010-10-02 9:38 UTC (permalink / raw)
Attention Beneficiary,
Are you ready to pick up this $5,000.00 sent to you today. We have
concluded to effect your Compensation payment of $850.000.00 through
Western Union Money Transfer for $5,000.00 daily until your $850.000.00 is
completely transfered. Meanwhile, Western Union Office has $5,000.00 in
your name today. So contact Western Union Agent below to pick up this
$5,000 now:
Contact person: Mr.Kings Igwe ,
Western Union Office
E-mail:western.union.ofice@ufo.tc
Tele phone Number: +229-97844712.
Ask him to give you the MTCN, Sender's name, question and answer to pick
the $5,000.00. Also you should send to him your following information.
1. Your Full Name...............................
2. Your Address.................................
3. Your Marital Status..........................
4. Occupation and Age...........................
5. Your Mobile Numbers/Fax......................
6. Your International Passport/Driving License..
7. Country .....................................
Regards,
Barrister.Brandon Moore
- ILMCI Mail -
Fast and Clean Email by www.ILMCI.com
^ permalink raw reply
* Re: [PATCH 4/4] drivers/atm/idt77252.c: Remove unnecessary error check
From: Julia Lawall @ 2010-10-02 14:21 UTC (permalink / raw)
To: walter harms
Cc: Chas Williams, kernel-janitors, linux-atm-general, netdev,
linux-kernel
In-Reply-To: <4CA73D26.7090109@bfs.de>
On Sat, 2 Oct 2010, walter harms wrote:
>
>
> Julia Lawall schrieb:
> > This code does not call deinit_card(card); in an error case, as done in
> > other error-handling code in the same function. But actually, the called
> > function init_sram can only return 0, so there is no need for the error
> > check at all.
> >
>
>
> did you set init_sram() to void ?
No, that indeed seems like a reasonable change. Patch shortly.
julia
^ permalink raw reply
* Re: [PATCH 4/4] drivers/atm/idt77252.c: Remove unnecessary error check
From: walter harms @ 2010-10-02 14:09 UTC (permalink / raw)
To: Julia Lawall
Cc: Chas Williams, kernel-janitors, linux-atm-general, netdev,
linux-kernel
In-Reply-To: <1286027958-7333-4-git-send-email-julia@diku.dk>
Julia Lawall schrieb:
> This code does not call deinit_card(card); in an error case, as done in
> other error-handling code in the same function. But actually, the called
> function init_sram can only return 0, so there is no need for the error
> check at all.
>
did you set init_sram() to void ?
re,
wh
> A simplified version of the sematic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r exists@
> @r@
> statement S1,S2,S3;
> constant C1,C2,C3;
> @@
>
> *if (...)
> {... S1 return -C1;}
> ...
> *if (...)
> {... when != S1
> return -C2;}
> ...
> *if (...)
> {... S1 return -C3;}
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> drivers/atm/idt77252.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
> index 1679cbf..08ccde6 100644
> --- a/drivers/atm/idt77252.c
> +++ b/drivers/atm/idt77252.c
> @@ -3410,8 +3410,7 @@ init_card(struct atm_dev *dev)
>
> writel(readl(SAR_REG_CFG) | conf, SAR_REG_CFG);
>
> - if (init_sram(card) < 0)
> - return -1;
> + init_sram(card);
>
> /********************************************************************/
> /* A L L O C R A M A N D S E T V A R I O U S T H I N G S */
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* [PATCH 4/4] drivers/atm/idt77252.c: Remove unnecessary error check
From: Julia Lawall @ 2010-10-02 13:59 UTC (permalink / raw)
To: Chas Williams; +Cc: kernel-janitors, linux-atm-general, netdev, linux-kernel
This code does not call deinit_card(card); in an error case, as done in
other error-handling code in the same function. But actually, the called
function init_sram can only return 0, so there is no need for the error
check at all.
A simplified version of the sematic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
@r@
statement S1,S2,S3;
constant C1,C2,C3;
@@
*if (...)
{... S1 return -C1;}
...
*if (...)
{... when != S1
return -C2;}
...
*if (...)
{... S1 return -C3;}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/atm/idt77252.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
index 1679cbf..08ccde6 100644
--- a/drivers/atm/idt77252.c
+++ b/drivers/atm/idt77252.c
@@ -3410,8 +3410,7 @@ init_card(struct atm_dev *dev)
writel(readl(SAR_REG_CFG) | conf, SAR_REG_CFG);
- if (init_sram(card) < 0)
- return -1;
+ init_sram(card);
/********************************************************************/
/* A L L O C R A M A N D S E T V A R I O U S T H I N G S */
^ permalink raw reply related
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