* Re: [PATCH 1/1] NET: netpoll, fix potential NULL ptr dereference
From: Matt Mackall @ 2010-03-16 17:56 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Jiri Slaby, davem, netdev, linux-kernel, Daniel Borkmann
In-Reply-To: <4B9FBE49.6010709@gmail.com>
On Tue, 2010-03-16 at 18:22 +0100, Jiri Slaby wrote:
> On 03/16/2010 06:12 PM, Matt Mackall wrote:
> > I don't get it. The source of the branch tests for !ndev->npinfo and the
> > original destination of the branch also tests for !ndev->npinfo. I don't
> > see how it gets dereferenced.
>
> Let's look at more of the context:
> if (!ndev->npinfo) {
> npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
> if (!npinfo) { // npinfo is NULL
> err = -ENOMEM;
> goto release;
> }
> ...
> release: // npinfo is still NULL
> if (!ndev->npinfo) { // condition is the same (holds)
> // dereference below: vvvvvvvvvvvvvvv
> spin_lock_irqsave(&npinfo->rx_lock, flags);
> list_for_each_entry_safe(npe, tmp, &npinfo->rx_np, rx) {
> npe->dev = NULL;
> }
> spin_unlock_irqrestore(&npinfo->rx_lock, flags);
>
> kfree(npinfo);
> }
Ok, you're correct, I read the second test backwards.
Acked-by: Matt Mackall <mpm@selenic.com>
--
http://selenic.com : development and support for Mercurial and Linux
^ permalink raw reply
* GET BACK TO ME,
From: Mr Shun Yin @ 2010-03-16 13:41 UTC (permalink / raw)
--
Hello Friend,
I am Mr. Shun Yin, I have a business proposal of $17,300.000.00 for you
to handle with me from my bank. If you are interested, please send me
your full contact details and after that I shall provide you with more
details of the business. Email: yinshun66887@yahoo.com.hk
Regards,
Shun.
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
^ permalink raw reply
* Re: [PATCH] route: Fix caught BUG_ON during rt_secret_rebuild_oneshot()
From: Neil Horman @ 2010-03-16 17:35 UTC (permalink / raw)
To: Vitaliy Gusev; +Cc: David S. Miller, Eric Dumazet, Herbert Xu, netdev
In-Reply-To: <201003161827.08595.vgusev@openvz.org>
On Tue, Mar 16, 2010 at 06:27:08PM +0300, Vitaliy Gusev wrote:
> On Tuesday 16 March 2010 05:34:37 pm Neil Horman wrote:
> ...
> > Yeah, this seems reasonable. Although this might be a good time to bring up the
> > subject of deprecating or removing the secret interval. We've had our
> > statistical analysis bits in place to drive the rebuilding of the cache in the
> > event that any one chain gets too long, and in the year or so that we've had it,
> > it seems we've shaken out a few bugs with it (which suggests that people are
> > using it routinely). As such, might it be time to just drop the rebuild timer
> > in its entirety?
>
> Rebuild timer was introduced by secure reason when there was no analysis for
> chain lenght. Now it seems that rebuild timer can be removed.
>
Dave, Eric, Herbert, what are your opinions on this? Is the statistical analysis code
mature enough at this point to warrant the removal of the rebuild timer, or
should we still keep it as a fallback mechanism?
Neil
>
> >
> > Until that decision is made of course, the above seems like a sane fix for it.
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> >
> >
>
>
>
> Thanks,
> Vitaliy Gusev
>
^ permalink raw reply
* Re: [PATCH 1/1] NET: netpoll, fix potential NULL ptr dereference
From: Jiri Slaby @ 2010-03-16 17:22 UTC (permalink / raw)
To: Matt Mackall; +Cc: Jiri Slaby, davem, netdev, linux-kernel, Daniel Borkmann
In-Reply-To: <1268759527.25503.2980.camel@calx>
On 03/16/2010 06:12 PM, Matt Mackall wrote:
> I don't get it. The source of the branch tests for !ndev->npinfo and the
> original destination of the branch also tests for !ndev->npinfo. I don't
> see how it gets dereferenced.
Let's look at more of the context:
if (!ndev->npinfo) {
npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
if (!npinfo) { // npinfo is NULL
err = -ENOMEM;
goto release;
}
...
release: // npinfo is still NULL
if (!ndev->npinfo) { // condition is the same (holds)
// dereference below: vvvvvvvvvvvvvvv
spin_lock_irqsave(&npinfo->rx_lock, flags);
list_for_each_entry_safe(npe, tmp, &npinfo->rx_np, rx) {
npe->dev = NULL;
}
spin_unlock_irqrestore(&npinfo->rx_lock, flags);
kfree(npinfo);
}
thanks,
--
js
^ permalink raw reply
* nexthdr bug
From: Marco @ 2010-03-16 17:20 UTC (permalink / raw)
To: netdev
Hi
Option nexthdr in u32 don't work.
My questions are?
lartc project is actually supported?
layer7 project (I ask for it because there is not other big project for traffic shaping) is actually supported?
Thank for your response.....
Bye
^ permalink raw reply
* Re: [PATCH 1/1] NET: netpoll, fix potential NULL ptr dereference
From: Matt Mackall @ 2010-03-16 17:12 UTC (permalink / raw)
To: Jiri Slaby; +Cc: davem, netdev, linux-kernel, jirislaby, Daniel Borkmann
In-Reply-To: <1268753394-17765-1-git-send-email-jslaby@suse.cz>
On Tue, 2010-03-16 at 16:29 +0100, Jiri Slaby wrote:
> Stanse found that one error path in netpoll_setup dereferences npinfo
> even though it is NULL. Avoid that by adding new label and go to that
> instead.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Daniel Borkmann <danborkmann@googlemail.com>
> Cc: David S. Miller <davem@davemloft.net>
> ---
> net/core/netpoll.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 7aa6972..d4ec38f 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -735,7 +735,7 @@ int netpoll_setup(struct netpoll *np)
> npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
> if (!npinfo) {
> err = -ENOMEM;
> - goto release;
> + goto put;
> }
>
> npinfo->rx_flags = 0;
> @@ -845,7 +845,7 @@ int netpoll_setup(struct netpoll *np)
>
> kfree(npinfo);
> }
> -
> +put:
> dev_put(ndev);
> return err;
> }
I don't get it. The source of the branch tests for !ndev->npinfo and the
original destination of the branch also tests for !ndev->npinfo. I don't
see how it gets dereferenced.
This looks like it just patches over a false positive in your tool
(which isn't correlating the validity of npinfo with ndev->npinfo)
without actually improving the code. However, it seems that we can drop
the second check at release if we add your new exit point.
--
http://selenic.com : development and support for Mercurial and Linux
^ permalink raw reply
* [PATCH] gigaset: correct range checking off by one error
From: Tilman Schmidt @ 2010-03-16 17:04 UTC (permalink / raw)
To: Karsten Keil, David Miller
Cc: Dan Carpenter, Hansjoerg Lipp, isdn4linux, i4ldeveloper, netdev,
linux-kernel, stable
Correct a potential array overrun due to an off by one error in the
range check on the CAPI CONNECT_REQ CIPValue parameter.
Found and reported by Dan Carpenter using smatch.
Impact: bugfix
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/gigaset/capi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 4a31962..0220c19 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -1301,7 +1301,7 @@ static void do_connect_req(struct gigaset_capi_ctr *iif,
}
/* check parameter: CIP Value */
- if (cmsg->CIPValue > ARRAY_SIZE(cip2bchlc) ||
+ if (cmsg->CIPValue >= ARRAY_SIZE(cip2bchlc) ||
(cmsg->CIPValue > 0 && cip2bchlc[cmsg->CIPValue].bc == NULL)) {
dev_notice(cs->dev, "%s: unknown CIP value %d\n",
"CONNECT_REQ", cmsg->CIPValue);
--
1.6.5.3.298.g39add
^ permalink raw reply related
* Re: [PATCH 1/1] NET: netpoll, fix potential NULL ptr dereference
From: Laurent Chavey @ 2010-03-16 16:57 UTC (permalink / raw)
To: Jiri Slaby; +Cc: davem, netdev, linux-kernel, jirislaby, Daniel Borkmann
In-Reply-To: <1268753394-17765-1-git-send-email-jslaby@suse.cz>
Acked-by: chavey@google.com
On Tue, Mar 16, 2010 at 8:29 AM, Jiri Slaby <jslaby@suse.cz> wrote:
> Stanse found that one error path in netpoll_setup dereferences npinfo
> even though it is NULL. Avoid that by adding new label and go to that
> instead.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Daniel Borkmann <danborkmann@googlemail.com>
> Cc: David S. Miller <davem@davemloft.net>
> ---
> net/core/netpoll.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 7aa6972..d4ec38f 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -735,7 +735,7 @@ int netpoll_setup(struct netpoll *np)
> npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
> if (!npinfo) {
> err = -ENOMEM;
> - goto release;
> + goto put;
> }
>
> npinfo->rx_flags = 0;
> @@ -845,7 +845,7 @@ int netpoll_setup(struct netpoll *np)
>
> kfree(npinfo);
> }
> -
> +put:
> dev_put(ndev);
> return err;
> }
> --
> 1.7.0.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" 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] l2tp: Fix oops in pppol2tp_xmit
From: James Chapman @ 2010-03-16 16:46 UTC (permalink / raw)
To: netdev
When transmitting L2TP frames, we derive the outgoing interface's UDP
checksum hardware assist capabilities from the tunnel dst dev. This
can sometimes be NULL, especially when routing protocols are used and
routing changes occur. This patch just checks for NULL dst or dev
pointers when checking for netdev hardware assist features.
BUG: unable to handle kernel NULL pointer dereference at 0000000c
IP: [<f89d074c>] pppol2tp_xmit+0x341/0x4da [pppol2tp]
*pde = 00000000
Oops: 0000 [#1] SMP
last sysfs file: /sys/class/net/lo/operstate
Modules linked in: pppol2tp pppox ppp_generic slhc ipv6 dummy loop snd_hda_codec_atihdmi snd_hda_intel snd_hda_codec snd_pcm snd_timer snd soundcore snd_page_alloc evdev psmouse serio_raw processor button i2c_piix4 i2c_core ati_agp agpgart pcspkr ext3 jbd mbcache sd_mod ide_pci_generic atiixp ide_core ahci ata_generic floppy ehci_hcd ohci_hcd libata e1000e scsi_mod usbcore nls_base thermal fan thermal_sys [last unloaded: scsi_wait_scan]
Pid: 0, comm: swapper Not tainted (2.6.32.8 #1)
EIP: 0060:[<f89d074c>] EFLAGS: 00010297 CPU: 3
EIP is at pppol2tp_xmit+0x341/0x4da [pppol2tp]
EAX: 00000000 EBX: f64d1680 ECX: 000005b9 EDX: 00000000
ESI: f6b91850 EDI: f64d16ac EBP: f6a0c4c0 ESP: f70a9cac
DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
Process swapper (pid: 0, ti=f70a8000 task=f70a31c0 task.ti=f70a8000)
Stack:
000005a9 000005b9 f734c400 f66652c0 f7352e00 f67dc800 00000000 f6b91800
<0> 000005a3 f70ef6c4 f67dcda9 000005a3 f89b192e 00000246 000005a3 f64d1680
<0> f63633e0 f6363320 f64d1680 f65a7320 f65a7364 f65856c0 f64d1680 f679f02f
Call Trace:
[<f89b192e>] ? ppp_push+0x459/0x50e [ppp_generic]
[<f89b217f>] ? ppp_xmit_process+0x3b6/0x430 [ppp_generic]
[<f89b2306>] ? ppp_start_xmit+0x10d/0x120 [ppp_generic]
[<c11c15cb>] ? dev_hard_start_xmit+0x21f/0x2b2
[<c11d0947>] ? sch_direct_xmit+0x48/0x10e
[<c11c19a0>] ? dev_queue_xmit+0x263/0x3a6
[<c11e2a9f>] ? ip_finish_output+0x1f7/0x221
[<c11df682>] ? ip_forward_finish+0x2e/0x30
[<c11de645>] ? ip_rcv_finish+0x295/0x2a9
[<c11c0b19>] ? netif_receive_skb+0x3e9/0x404
[<f814b791>] ? e1000_clean_rx_irq+0x253/0x2fc [e1000e]
[<f814cb7a>] ? e1000_clean+0x63/0x1fc [e1000e]
[<c1047eff>] ? sched_clock_local+0x15/0x11b
[<c11c1095>] ? net_rx_action+0x96/0x195
[<c1035750>] ? __do_softirq+0xaa/0x151
[<c1035828>] ? do_softirq+0x31/0x3c
[<c10358fe>] ? irq_exit+0x26/0x58
[<c1004b21>] ? do_IRQ+0x78/0x89
[<c1003729>] ? common_interrupt+0x29/0x30
[<c101ac28>] ? native_safe_halt+0x2/0x3
[<c1008c54>] ? default_idle+0x55/0x75
[<c1009045>] ? c1e_idle+0xd2/0xd5
[<c100233c>] ? cpu_idle+0x46/0x62
Code: 8d 45 08 f0 ff 45 08 89 6b 08 c7 43 68 7e fb 9c f8 8a 45 24 83 e0 0c 3c 04 75 09 80 63 64 f3 e9 b4 00 00 00 8b 43 18 8b 4c 24 04 <8b> 40 0c 8d 79 11 f6 40 44 0e 8a 43 64 75 51 6a 00 8b 4c 24 08
EIP: [<f89d074c>] pppol2tp_xmit+0x341/0x4da [pppol2tp] SS:ESP 0068:f70a9cac
CR2: 000000000000000c
Signed-off-by: James Chapman <jchapman@katalix.com>
---
This patch has been verified in several ISP setups (heavy L2TP users).
This patch may be a candidate for -stable.
---
drivers/net/pppol2tp.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
index 9fbb2eb..5861ee9 100644
--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -1180,7 +1180,8 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
/* Calculate UDP checksum if configured to do so */
if (sk_tun->sk_no_check == UDP_CSUM_NOXMIT)
skb->ip_summed = CHECKSUM_NONE;
- else if (!(skb_dst(skb)->dev->features & NETIF_F_V4_CSUM)) {
+ else if ((skb_dst(skb) && skb_dst(skb)->dev) &&
+ (!(skb_dst(skb)->dev->features & NETIF_F_V4_CSUM))) {
skb->ip_summed = CHECKSUM_COMPLETE;
csum = skb_checksum(skb, 0, udp_len, 0);
uh->check = csum_tcpudp_magic(inet->inet_saddr,
^ permalink raw reply related
* [RESEND: PATCH] l2tp: Fix UDP socket reference count bugs in the pppol2tp driver
From: James Chapman @ 2010-03-16 16:29 UTC (permalink / raw)
To: netdev
This patch fixes UDP socket refcnt bugs in the pppol2tp driver.
A bug can cause a kernel stack trace when a tunnel socket is closed.
A way to reproduce the issue is to prepare the UDP socket for L2TP (by
opening a tunnel pppol2tp socket) and then close it before any L2TP
sessions are added to it. The sequence is
Create UDP socket
Create tunnel pppol2tp socket to prepare UDP socket for L2TP
pppol2tp_connect: session_id=0, peer_session_id=0
L2TP SCCRP control frame received (tunnel_id==0)
pppol2tp_recv_core: sock_hold()
pppol2tp_recv_core: sock_put
L2TP ZLB control frame received (tunnel_id=nnn)
pppol2tp_recv_core: sock_hold()
pppol2tp_recv_core: sock_put
Close tunnel management socket
pppol2tp_release: session_id=0, peer_session_id=0
Close UDP socket
udp_lib_close: BUG
The addition of sock_hold() in pppol2tp_connect() solves the problem.
For data frames, two sock_put() calls were added to plug a refcnt leak
per received data frame. The ref that is grabbed at the top of
pppol2tp_recv_core() must always be released, but this wasn't done for
accepted data frames or data frames discarded because of bad UDP
checksums. This leak meant that any UDP socket that had passed L2TP
data traffic (i.e. L2TP data frames, not just L2TP control frames)
using pppol2tp would not be released by the kernel.
WARNING: at include/net/sock.h:435 udp_lib_unhash+0x117/0x120()
Pid: 1086, comm: openl2tpd Not tainted 2.6.33-rc1 #8
Call Trace:
[<c119e9b7>] ? udp_lib_unhash+0x117/0x120
[<c101b871>] ? warn_slowpath_common+0x71/0xd0
[<c119e9b7>] ? udp_lib_unhash+0x117/0x120
[<c101b8e3>] ? warn_slowpath_null+0x13/0x20
[<c119e9b7>] ? udp_lib_unhash+0x117/0x120
[<c11598a7>] ? sk_common_release+0x17/0x90
[<c11a5e33>] ? inet_release+0x33/0x60
[<c11577b0>] ? sock_release+0x10/0x60
[<c115780f>] ? sock_close+0xf/0x30
[<c106e542>] ? __fput+0x52/0x150
[<c106b68e>] ? filp_close+0x3e/0x70
[<c101d2e2>] ? put_files_struct+0x62/0xb0
[<c101eaf7>] ? do_exit+0x5e7/0x650
[<c1081623>] ? mntput_no_expire+0x13/0x70
[<c106b68e>] ? filp_close+0x3e/0x70
[<c101eb8a>] ? do_group_exit+0x2a/0x70
[<c101ebe1>] ? sys_exit_group+0x11/0x20
[<c10029b0>] ? sysenter_do_call+0x12/0x26
Signed-off-by: James Chapman <jchapman@katalix.com>
---
This patch was originally submitted 21-jan-2010,
http://marc.info/?l=linux-netdev&m=126409025900561&w=2. Its
description has since been updated and it has been refreshed against
latest netdev git.
This patch may be a candidate for -stable.
---
drivers/net/pppol2tp.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
index 5861ee9..449a982 100644
--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -756,6 +756,7 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb)
/* Try to dequeue as many skbs from reorder_q as we can. */
pppol2tp_recv_dequeue(session);
+ sock_put(sock);
return 0;
@@ -772,6 +773,7 @@ discard_bad_csum:
UDP_INC_STATS_USER(&init_net, UDP_MIB_INERRORS, 0);
tunnel->stats.rx_errors++;
kfree_skb(skb);
+ sock_put(sock);
return 0;
@@ -1662,6 +1664,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
if (tunnel_sock == NULL)
goto end;
+ sock_hold(tunnel_sock);
tunnel = tunnel_sock->sk_user_data;
} else {
tunnel = pppol2tp_tunnel_find(sock_net(sk), sp->pppol2tp.s_tunnel);
^ permalink raw reply related
* Re: [Regression] 2.6.34-rc1 sky2 mtu on second port defaults to 576
From: Michael Breuer @ 2010-03-16 16:17 UTC (permalink / raw)
To: Linux Kernel Mailing List, netdev
In-Reply-To: <4B9F9DAD.7060308@majjas.com>
On 3/16/2010 11:03 AM, Michael Breuer wrote:
> Haven't bisected - problem seemed to have started around
> 2.6.34-rc1-0933 (not seen before the rc1 tag).
>
> On boot, eth1 ends up with MTU of 576 (eth0 is 1500). My f12 startup
> scripts are not setting MTU (never have).
>
>
> Nothing of note in dmesg
> No change to .config
> No mtu change reported in log.
>
> Basically, sky2 reports eth1 up... and within the same second begins
> reporting receive length errors. Most network functions via eth1 fail.
>
> ifconfig eth1 reveals mtu of 576, setting mtu to 1500 restores network
> functionality.
>
> Booting back to 2.6.33-06233 (git from March 8) the problem does not
> manifest.
>
> I'll bisect if this isn't obvious to someone.
> --
> 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/
Never mind... Cablevision changed my cable modem... dhcp is setting the
mtu option to 576 (never did before). grrrr. My one fallback test seems
to have been a fluke.
^ permalink raw reply
* Re: locking issue in vhost_net_set_backend
From: Jeff Dike @ 2010-03-16 16:11 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Michael S. Tsirkin, netdev, LKML
In-Reply-To: <4B9F8E7F.7070709@gmail.com>
On Tue, Mar 16, 2010 at 02:58:23PM +0100, Jiri Slaby wrote:
> I don't see how the lock is unlocked on the error paths and as it is not
> on none of the them maybe I'm missing something?
I sent Michael a patch for this a couple of weeks ago.
Jeff
--
Work email - jdike at linux dot intel dot com
^ permalink raw reply
* Re: [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
From: Eric Dumazet @ 2010-03-16 15:52 UTC (permalink / raw)
To: Yegor Yefremov; +Cc: netdev, davem
In-Reply-To: <f69abfc31003160842r59c25bbdl2029d3f20e668e53@mail.gmail.com>
Le mardi 16 mars 2010 à 16:42 +0100, Yegor Yefremov a écrit :
> KS8695: update ksp->next_rx_desc_read at the end of rx loop
>
> There is no need to adjust the next rx descriptor after each packet,
> so do it only once at the end of the routine.
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>
> Index: linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
> ===================================================================
> --- linux-2.6.34-rc1.orig/drivers/net/arm/ks8695net.c
> +++ linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
> @@ -538,12 +538,13 @@ rx_finished:
> */
> last_rx_processed = buff_n;
> buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
> - /*And note which RX descriptor we last did */
> - if (likely(last_rx_processed != -1))
> - ksp->next_rx_desc_read =
> - (last_rx_processed + 1) &
> - MAX_RX_DESC_MASK;
> }
> +
> + /*And note which RX descriptor we last did */
> + if (likely(last_rx_processed != -1))
> + ksp->next_rx_desc_read =
> + (last_rx_processed + 1) & MAX_RX_DESC_MASK;
> +
Very strange to see all these computations...
At this point, why not use
if (likely(last_rx_processed != -1))
ksp->next_rx_desc_read = buff_n;
or even get rid of last_rx_processed completely and do :
diff --git a/drivers/net/arm/ks8695net.c b/drivers/net/arm/ks8695net.c
index a1d4188..ac6ab04 100644
--- a/drivers/net/arm/ks8695net.c
+++ b/drivers/net/arm/ks8695net.c
@@ -461,7 +461,6 @@ static int ks8695_rx(struct ks8695_priv *ksp, int budget)
int buff_n;
u32 flags;
int pktlen;
- int last_rx_processed = -1;
int received = 0;
buff_n = ksp->next_rx_desc_read;
@@ -533,17 +532,9 @@ rx_failure:
ksp->rx_ring[buff_n].status = cpu_to_le32(RDES_OWN);
rx_finished:
received++;
- /* And note this as processed so we can start
- * from here next time
- */
- last_rx_processed = buff_n;
buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
- /*And note which RX descriptor we last did */
- if (likely(last_rx_processed != -1))
- ksp->next_rx_desc_read =
- (last_rx_processed + 1) &
- MAX_RX_DESC_MASK;
}
+ ksp->next_rx_desc_read = buff_n;
/* And refill the buffers */
ks8695_refill_rxbuffers(ksp);
^ permalink raw reply related
* [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
From: Yegor Yefremov @ 2010-03-16 15:42 UTC (permalink / raw)
To: netdev; +Cc: davem
KS8695: update ksp->next_rx_desc_read at the end of rx loop
There is no need to adjust the next rx descriptor after each packet,
so do it only once at the end of the routine.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Index: linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
===================================================================
--- linux-2.6.34-rc1.orig/drivers/net/arm/ks8695net.c
+++ linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
@@ -538,12 +538,13 @@ rx_finished:
*/
last_rx_processed = buff_n;
buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
- /*And note which RX descriptor we last did */
- if (likely(last_rx_processed != -1))
- ksp->next_rx_desc_read =
- (last_rx_processed + 1) &
- MAX_RX_DESC_MASK;
}
+
+ /*And note which RX descriptor we last did */
+ if (likely(last_rx_processed != -1))
+ ksp->next_rx_desc_read =
+ (last_rx_processed + 1) & MAX_RX_DESC_MASK;
+
/* And refill the buffers */
ks8695_refill_rxbuffers(ksp);
^ permalink raw reply
* [PATCH 1/1] NET: netpoll, fix potential NULL ptr dereference
From: Jiri Slaby @ 2010-03-16 15:29 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, jirislaby, Daniel Borkmann
Stanse found that one error path in netpoll_setup dereferences npinfo
even though it is NULL. Avoid that by adding new label and go to that
instead.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Daniel Borkmann <danborkmann@googlemail.com>
Cc: David S. Miller <davem@davemloft.net>
---
net/core/netpoll.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 7aa6972..d4ec38f 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -735,7 +735,7 @@ int netpoll_setup(struct netpoll *np)
npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
if (!npinfo) {
err = -ENOMEM;
- goto release;
+ goto put;
}
npinfo->rx_flags = 0;
@@ -845,7 +845,7 @@ int netpoll_setup(struct netpoll *np)
kfree(npinfo);
}
-
+put:
dev_put(ndev);
return err;
}
--
1.7.0.1
^ permalink raw reply related
* [PATCH] ne: Do not use slashes in irq name string
From: Atsushi Nemoto @ 2010-03-16 15:27 UTC (permalink / raw)
To: netdev; +Cc: David Miller
This patch fixes following warning introduced by commit
12bac0d9f4dbf3445a0319beee848d15fa32775e ("proc: warn on non-existing
proc entries"):
WARNING: at /work/mips-linux/make/linux/fs/proc/generic.c:316 __xlate_proc_name+0xe0/0xe8()
name 'RBHMA4X00/RTL8019'
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
drivers/net/ne.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ne.c b/drivers/net/ne.c
index 992dbff..f4347f8 100644
--- a/drivers/net/ne.c
+++ b/drivers/net/ne.c
@@ -142,7 +142,7 @@ bad_clone_list[] __initdata = {
{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */
{"REALTEK", "RTL8019", {0x00, 0x00, 0xe8}}, /* no-name with Realtek chip */
#ifdef CONFIG_MACH_TX49XX
- {"RBHMA4X00-RTL8019", "RBHMA4X00/RTL8019", {0x00, 0x60, 0x0a}}, /* Toshiba built-in */
+ {"RBHMA4X00-RTL8019", "RBHMA4X00-RTL8019", {0x00, 0x60, 0x0a}}, /* Toshiba built-in */
#endif
{"LCS-8834", "LCS-8836", {0x04, 0x04, 0x37}}, /* ShinyNet (SET) */
{NULL,}
--
1.5.6.5
^ permalink raw reply related
* [PATCH] NET_DMA: free skbs periodically
From: Steven J. Magnani @ 2010-03-16 15:22 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, linux-kernel,
Steven J. Magnani
Under NET_DMA, data transfer can grind to a halt when userland issues a
large read on a socket with a high RCVLOWAT (i.e., 512 KB for both).
This appears to be because the NET_DMA design queues up lots of memcpy
operations, but doesn't issue or wait for them (and thus free the
associated skbs) until it is time for tcp_recvmesg() to return.
The socket hangs when its TCP window goes to zero before enough data is
available to satisfy the read.
Periodically issue asynchronous memcpy operations, and free skbs for ones
that have completed, to prevent sockets from going into zero-window mode.
Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
diff -uprN a/net/ipv4/tcp.c b/net/ipv4/tcp.c
--- a/net/ipv4/tcp.c 2010-03-16 09:15:31.000000000 -0500
+++ b/net/ipv4/tcp.c 2010-03-16 09:59:06.000000000 -0500
@@ -1254,6 +1254,39 @@ static void tcp_prequeue_process(struct
tp->ucopy.memory = 0;
}
+#ifdef CONFIG_NET_DMA
+static void tcp_service_net_dma(struct sock *sk, bool wait)
+{
+ dma_cookie_t done, used;
+ dma_cookie_t last_issued;
+ struct tcp_sock *tp = tcp_sk(sk);
+
+ if (!tp->ucopy.dma_chan)
+ return;
+
+ last_issued = tp->ucopy.dma_cookie;
+ dma_async_memcpy_issue_pending(tp->ucopy.dma_chan);
+
+ do {
+ if (dma_async_memcpy_complete(tp->ucopy.dma_chan,
+ last_issued, &done,
+ &used) == DMA_SUCCESS) {
+ /* Safe to free early-copied skbs now */
+ __skb_queue_purge(&sk->sk_async_wait_queue);
+ break;
+ } else {
+ struct sk_buff *skb;
+ while ((skb = skb_peek(&sk->sk_async_wait_queue)) &&
+ (dma_async_is_complete(skb->dma_cookie, done,
+ used) == DMA_SUCCESS)) {
+ __skb_dequeue(&sk->sk_async_wait_queue);
+ kfree_skb(skb);
+ }
+ }
+ } while (wait);
+}
+#endif
+
static inline struct sk_buff *tcp_recv_skb(struct sock *sk, u32 seq, u32 *off)
{
struct sk_buff *skb;
@@ -1546,6 +1579,10 @@ int tcp_recvmsg(struct kiocb *iocb, stru
/* __ Set realtime policy in scheduler __ */
}
+#ifdef CONFIG_NET_DMA
+ if (tp->ucopy.dma_chan)
+ dma_async_memcpy_issue_pending(tp->ucopy.dma_chan);
+#endif
if (copied >= target) {
/* Do not sleep, just process backlog. */
release_sock(sk);
@@ -1554,6 +1591,7 @@ int tcp_recvmsg(struct kiocb *iocb, stru
sk_wait_data(sk, &timeo);
#ifdef CONFIG_NET_DMA
+ tcp_service_net_dma(sk, false); /* Don't block */
tp->ucopy.wakeup = 0;
#endif
@@ -1633,6 +1671,9 @@ do_prequeue:
copied = -EFAULT;
break;
}
+
+ dma_async_memcpy_issue_pending(tp->ucopy.dma_chan);
+
if ((offset + used) == skb->len)
copied_early = 1;
@@ -1702,27 +1743,9 @@ skip_copy:
}
#ifdef CONFIG_NET_DMA
- if (tp->ucopy.dma_chan) {
- dma_cookie_t done, used;
-
- dma_async_memcpy_issue_pending(tp->ucopy.dma_chan);
-
- while (dma_async_memcpy_complete(tp->ucopy.dma_chan,
- tp->ucopy.dma_cookie, &done,
- &used) == DMA_IN_PROGRESS) {
- /* do partial cleanup of sk_async_wait_queue */
- while ((skb = skb_peek(&sk->sk_async_wait_queue)) &&
- (dma_async_is_complete(skb->dma_cookie, done,
- used) == DMA_SUCCESS)) {
- __skb_dequeue(&sk->sk_async_wait_queue);
- kfree_skb(skb);
- }
- }
+ tcp_service_net_dma(sk, true); /* Wait for queue to drain */
+ tp->ucopy.dma_chan = NULL;
- /* Safe to free early-copied skbs now */
- __skb_queue_purge(&sk->sk_async_wait_queue);
- tp->ucopy.dma_chan = NULL;
- }
if (tp->ucopy.pinned_list) {
dma_unpin_iovec_pages(tp->ucopy.pinned_list);
tp->ucopy.pinned_list = NULL;
^ permalink raw reply
* Re: [PATCH] route: Fix caught BUG_ON during rt_secret_rebuild_oneshot()
From: Vitaliy Gusev @ 2010-03-16 15:27 UTC (permalink / raw)
To: Neil Horman; +Cc: David S. Miller, Eric Dumazet, Herbert Xu, netdev
In-Reply-To: <20100316143437.GA8225@hmsreliant.think-freely.org>
On Tuesday 16 March 2010 05:34:37 pm Neil Horman wrote:
...
> Yeah, this seems reasonable. Although this might be a good time to bring up the
> subject of deprecating or removing the secret interval. We've had our
> statistical analysis bits in place to drive the rebuilding of the cache in the
> event that any one chain gets too long, and in the year or so that we've had it,
> it seems we've shaken out a few bugs with it (which suggests that people are
> using it routinely). As such, might it be time to just drop the rebuild timer
> in its entirety?
Rebuild timer was introduced by secure reason when there was no analysis for
chain lenght. Now it seems that rebuild timer can be removed.
>
> Until that decision is made of course, the above seems like a sane fix for it.
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
>
>
Thanks,
Vitaliy Gusev
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Eric Dumazet @ 2010-03-16 15:20 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Oleg Nesterov, David Miller, Ingo Molnar, Francois Romieu,
Peter Zijlstra, netdev, linux-kernel
In-Reply-To: <20100316151023.GC3332@swordfish.minsk.epam.com>
Le mardi 16 mars 2010 à 17:10 +0200, Sergey Senozhatsky a écrit :
> [...]
> >> NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
> Isn't it r8169 related?
>
This is tx side which seems blocked for more than 6 seconds.
To really test the patch, you need following setup :
machine A with r8169 NIC, patch applied, receiver of pktgen flood.
machine B with any NIC, preferably a not buggy one, doing the pktgen
flood to machine A
If machine A survives, my patch is tested and ok.
If machine B crashes, we have another problem to investigate
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Sergey Senozhatsky @ 2010-03-16 15:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: Sergey Senozhatsky, Oleg Nesterov, David Miller, Ingo Molnar,
Francois Romieu, Peter Zijlstra, netdev, linux-kernel
In-Reply-To: <1268751933.3094.45.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 2843 bytes --]
On (03/16/10 16:05), Eric Dumazet wrote:
> > Nope!
> > Here it is:
> >
> >
> > [17250.998293] ------------[ cut here ]------------
> > [17250.998305] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
> > [17250.998308] Hardware name: F3JC
> > [17250.998312] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
> > [17250.998315] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci asus_laptop sparse_keymap
> > mmc_core led_class snd_hda_intel snd_hda_codec snd_pcm snd_timer snd_page_alloc rng_core sg evdev i2c_i801 snd soundcore psmouse r8169 serio_raw mii uhci_hcd ehci_hcd sr_mod
> > usbcore cdrom sd_mod ata_piix
> > [17250.998371] Pid: 3985, comm: kpktgend_0 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #46
> > [17250.998375] Call Trace:
> > [17250.998383] [<c102e353>] warn_slowpath_common+0x65/0x7c
> > [17250.998388] [<c126b654>] ? dev_watchdog+0xc1/0x125
> > [17250.998393] [<c102e39e>] warn_slowpath_fmt+0x24/0x27
> > [17250.998398] [<c126b654>] dev_watchdog+0xc1/0x125
> > [17250.998405] [<c1036bbb>] ? run_timer_softirq+0x120/0x1eb
> > [17250.998411] [<c1036c11>] run_timer_softirq+0x176/0x1eb
> > [17250.998416] [<c1036bbb>] ? run_timer_softirq+0x120/0x1eb
> > [17250.998421] [<c126b593>] ? dev_watchdog+0x0/0x125
> > [17250.998426] [<c1032df9>] __do_softirq+0x8d/0x117
> > [17250.998431] [<c1032eae>] do_softirq+0x2b/0x43
> > [17250.998436] [<c1032fd3>] irq_exit+0x38/0x75
> > [17250.998442] [<c1014e75>] smp_apic_timer_interrupt+0x66/0x74
> > [17250.998448] [<c12c812a>] apic_timer_interrupt+0x36/0x3c
> > [17250.998457] [<c1185d18>] ? do_raw_spin_trylock+0x28/0x37
> > [17250.998464] [<c12c7101>] _raw_spin_lock+0x2f/0x58
> > [17250.998472] [<f80855ca>] ? spin_lock+0x8/0xa [pktgen]
> > [17250.998478] [<f80855ca>] spin_lock+0x8/0xa [pktgen]
> > [17250.998484] [<f80873b6>] pktgen_thread_worker+0x9b/0x631 [pktgen]
> > [17250.998491] [<c103f9f1>] ? autoremove_wake_function+0x0/0x2f
> > [17250.998497] [<c103f9f1>] ? autoremove_wake_function+0x0/0x2f
> > [17250.998503] [<f808731b>] ? pktgen_thread_worker+0x0/0x631 [pktgen]
> > [17250.998508] [<c103f6ce>] kthread+0x6a/0x6f
> > [17250.998514] [<c103f664>] ? kthread+0x0/0x6f
> > [17250.998520] [<c1002e42>] kernel_thread_helper+0x6/0x1a
> > [17250.998523] ---[ end trace a22d306b065d4a68 ]---
> > [17251.011663] r8169 0000:02:00.0: eth0: link up
> >
> > [17419.011748] NOHZ: local_softirq_pending 08
> >
> >
>
> But this stack trace is on pktgen side (the sender), and my patch was
> about the receiver ?
>
[...]
>> NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
Isn't it r8169 related?
> I wonder if you dont hit another problem :)
:)
Sergey
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Eric Dumazet @ 2010-03-16 15:05 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Oleg Nesterov, David Miller, Ingo Molnar, Francois Romieu,
Peter Zijlstra, netdev, linux-kernel
In-Reply-To: <20100316145914.GB3332@swordfish.minsk.epam.com>
Le mardi 16 mars 2010 à 17:00 +0200, Sergey Senozhatsky a écrit :
> Nope!
> Here it is:
>
>
> [17250.998293] ------------[ cut here ]------------
> [17250.998305] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
> [17250.998308] Hardware name: F3JC
> [17250.998312] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
> [17250.998315] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci asus_laptop sparse_keymap
> mmc_core led_class snd_hda_intel snd_hda_codec snd_pcm snd_timer snd_page_alloc rng_core sg evdev i2c_i801 snd soundcore psmouse r8169 serio_raw mii uhci_hcd ehci_hcd sr_mod
> usbcore cdrom sd_mod ata_piix
> [17250.998371] Pid: 3985, comm: kpktgend_0 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #46
> [17250.998375] Call Trace:
> [17250.998383] [<c102e353>] warn_slowpath_common+0x65/0x7c
> [17250.998388] [<c126b654>] ? dev_watchdog+0xc1/0x125
> [17250.998393] [<c102e39e>] warn_slowpath_fmt+0x24/0x27
> [17250.998398] [<c126b654>] dev_watchdog+0xc1/0x125
> [17250.998405] [<c1036bbb>] ? run_timer_softirq+0x120/0x1eb
> [17250.998411] [<c1036c11>] run_timer_softirq+0x176/0x1eb
> [17250.998416] [<c1036bbb>] ? run_timer_softirq+0x120/0x1eb
> [17250.998421] [<c126b593>] ? dev_watchdog+0x0/0x125
> [17250.998426] [<c1032df9>] __do_softirq+0x8d/0x117
> [17250.998431] [<c1032eae>] do_softirq+0x2b/0x43
> [17250.998436] [<c1032fd3>] irq_exit+0x38/0x75
> [17250.998442] [<c1014e75>] smp_apic_timer_interrupt+0x66/0x74
> [17250.998448] [<c12c812a>] apic_timer_interrupt+0x36/0x3c
> [17250.998457] [<c1185d18>] ? do_raw_spin_trylock+0x28/0x37
> [17250.998464] [<c12c7101>] _raw_spin_lock+0x2f/0x58
> [17250.998472] [<f80855ca>] ? spin_lock+0x8/0xa [pktgen]
> [17250.998478] [<f80855ca>] spin_lock+0x8/0xa [pktgen]
> [17250.998484] [<f80873b6>] pktgen_thread_worker+0x9b/0x631 [pktgen]
> [17250.998491] [<c103f9f1>] ? autoremove_wake_function+0x0/0x2f
> [17250.998497] [<c103f9f1>] ? autoremove_wake_function+0x0/0x2f
> [17250.998503] [<f808731b>] ? pktgen_thread_worker+0x0/0x631 [pktgen]
> [17250.998508] [<c103f6ce>] kthread+0x6a/0x6f
> [17250.998514] [<c103f664>] ? kthread+0x0/0x6f
> [17250.998520] [<c1002e42>] kernel_thread_helper+0x6/0x1a
> [17250.998523] ---[ end trace a22d306b065d4a68 ]---
> [17251.011663] r8169 0000:02:00.0: eth0: link up
>
> [17419.011748] NOHZ: local_softirq_pending 08
>
>
But this stack trace is on pktgen side (the sender), and my patch was
about the receiver ?
I wonder if you dont hit another problem :)
^ permalink raw reply
* [Regression] 2.6.34-rc1 sky2 mtu on second port defaults to 576
From: Michael Breuer @ 2010-03-16 15:03 UTC (permalink / raw)
To: Linux Kernel Mailing List, netdev
Haven't bisected - problem seemed to have started around 2.6.34-rc1-0933
(not seen before the rc1 tag).
On boot, eth1 ends up with MTU of 576 (eth0 is 1500). My f12 startup
scripts are not setting MTU (never have).
Nothing of note in dmesg
No change to .config
No mtu change reported in log.
Basically, sky2 reports eth1 up... and within the same second begins
reporting receive length errors. Most network functions via eth1 fail.
ifconfig eth1 reveals mtu of 576, setting mtu to 1500 restores network
functionality.
Booting back to 2.6.33-06233 (git from March 8) the problem does not
manifest.
I'll bisect if this isn't obvious to someone.
^ permalink raw reply
* Re: [PATCH] netlink: convert DIY reader/writer to mutex and RCU (v2)
From: Eric Dumazet @ 2010-03-16 15:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20100308140021.234a120a@nehalam>
Le lundi 08 mars 2010 à 14:00 -0800, Stephen Hemminger a écrit :
> The netlink table locking was open coded version of reader/writer
> sleeping lock. Change to using mutex and RCU which makes
> code clearer, shorter, and simpler.
>
> Could use sk_list nulls but then would have to have kmem_cache
> for netlink handles and that seems like unnecessary bloat.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> ---
> v1 -> v2 do RCU correctly...
> * use spinlock not mutex (not safe to sleep in normal RCU)
> * use _rcu variants of add/delete
> kfree(nlk->groups);
> nlk->groups = NULL;
> @@ -533,6 +477,8 @@ static int netlink_release(struct socket
> local_bh_disable();
> sock_prot_inuse_add(sock_net(sk), &netlink_proto, -1);
> local_bh_enable();
> +
> + synchronize_rcu();
> sock_put(sk);
> return 0;
I am a bit scared by synchronize_rcu() proliferation. This can slow down
some workloads (some scripts invoking netlink commands, not using batch
mode)
We could change sk_prot_free() behavior and optionaly call a callback to
free a socket (and the module_put()) after rcu grace period. As the
rcu_head is not inside struct sock, I could not find a generic way to
code this.
For TCP/UDP sockets this was considered not a viable alternative, but
for other sockets its certainly better than synchronize_rcu() ?
diff --git a/include/net/sock.h b/include/net/sock.h
index 092b055..3a2d598 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -693,6 +693,8 @@ struct proto {
int (*backlog_rcv) (struct sock *sk,
struct sk_buff *skb);
+ void (*free_socket)(struct sock *sk);
+
/* Keeping track of sk's, looking them up, and port selection methods. */
void (*hash)(struct sock *sk);
void (*unhash)(struct sock *sk);
diff --git a/net/core/sock.c b/net/core/sock.c
index c5812bb..3060c9b 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1027,18 +1027,20 @@ out_free:
static void sk_prot_free(struct proto *prot, struct sock *sk)
{
- struct kmem_cache *slab;
- struct module *owner;
+ security_sk_free(sk);
+ if (prot->free_socket)
+ prot->free_socket(sk);
+ else {
+ struct kmem_cache *slab;
- owner = prot->owner;
- slab = prot->slab;
+ slab = prot->slab;
- security_sk_free(sk);
- if (slab != NULL)
- kmem_cache_free(slab, sk);
- else
- kfree(sk);
- module_put(owner);
+ if (slab != NULL)
+ kmem_cache_free(slab, sk);
+ else
+ kfree(sk);
+ module_put(prot->owner);
+ }
}
/**
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 320d042..fae2344 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -81,6 +81,7 @@ struct netlink_sock {
struct mutex cb_def_mutex;
void (*netlink_rcv)(struct sk_buff *skb);
struct module *module;
+ struct rcu_head rcu;
};
struct listeners_rcu_head {
@@ -394,10 +395,22 @@ static void netlink_remove(struct sock *sk)
netlink_table_ungrab();
}
+static void rcu_free_socket(struct rcu_head *rcu)
+{
+ kfree(container_of(rcu, struct netlink_sock, rcu));
+ module_put(THIS_MODULE);
+}
+
+static void free_socket(struct sock *sk)
+{
+ call_rcu(&nlk_sk(sk)->rcu, rcu_free_socket);
+}
+
static struct proto netlink_proto = {
.name = "NETLINK",
.owner = THIS_MODULE,
.obj_size = sizeof(struct netlink_sock),
+ .free_socket = free_socket,
};
static int __netlink_create(struct net *net, struct socket *sock,
^ permalink raw reply related
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Sergey Senozhatsky @ 2010-03-16 15:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: Oleg Nesterov, David Miller, Ingo Molnar, Francois Romieu,
Peter Zijlstra, netdev, linux-kernel
In-Reply-To: <1268699602.2824.14.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 5442 bytes --]
Nope!
Here it is:
[17250.998293] ------------[ cut here ]------------
[17250.998305] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
[17250.998308] Hardware name: F3JC
[17250.998312] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
[17250.998315] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci asus_laptop sparse_keymap
mmc_core led_class snd_hda_intel snd_hda_codec snd_pcm snd_timer snd_page_alloc rng_core sg evdev i2c_i801 snd soundcore psmouse r8169 serio_raw mii uhci_hcd ehci_hcd sr_mod
usbcore cdrom sd_mod ata_piix
[17250.998371] Pid: 3985, comm: kpktgend_0 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #46
[17250.998375] Call Trace:
[17250.998383] [<c102e353>] warn_slowpath_common+0x65/0x7c
[17250.998388] [<c126b654>] ? dev_watchdog+0xc1/0x125
[17250.998393] [<c102e39e>] warn_slowpath_fmt+0x24/0x27
[17250.998398] [<c126b654>] dev_watchdog+0xc1/0x125
[17250.998405] [<c1036bbb>] ? run_timer_softirq+0x120/0x1eb
[17250.998411] [<c1036c11>] run_timer_softirq+0x176/0x1eb
[17250.998416] [<c1036bbb>] ? run_timer_softirq+0x120/0x1eb
[17250.998421] [<c126b593>] ? dev_watchdog+0x0/0x125
[17250.998426] [<c1032df9>] __do_softirq+0x8d/0x117
[17250.998431] [<c1032eae>] do_softirq+0x2b/0x43
[17250.998436] [<c1032fd3>] irq_exit+0x38/0x75
[17250.998442] [<c1014e75>] smp_apic_timer_interrupt+0x66/0x74
[17250.998448] [<c12c812a>] apic_timer_interrupt+0x36/0x3c
[17250.998457] [<c1185d18>] ? do_raw_spin_trylock+0x28/0x37
[17250.998464] [<c12c7101>] _raw_spin_lock+0x2f/0x58
[17250.998472] [<f80855ca>] ? spin_lock+0x8/0xa [pktgen]
[17250.998478] [<f80855ca>] spin_lock+0x8/0xa [pktgen]
[17250.998484] [<f80873b6>] pktgen_thread_worker+0x9b/0x631 [pktgen]
[17250.998491] [<c103f9f1>] ? autoremove_wake_function+0x0/0x2f
[17250.998497] [<c103f9f1>] ? autoremove_wake_function+0x0/0x2f
[17250.998503] [<f808731b>] ? pktgen_thread_worker+0x0/0x631 [pktgen]
[17250.998508] [<c103f6ce>] kthread+0x6a/0x6f
[17250.998514] [<c103f664>] ? kthread+0x0/0x6f
[17250.998520] [<c1002e42>] kernel_thread_helper+0x6/0x1a
[17250.998523] ---[ end trace a22d306b065d4a68 ]---
[17251.011663] r8169 0000:02:00.0: eth0: link up
[17419.011748] NOHZ: local_softirq_pending 08
Sergey
On (03/16/10 01:33), Eric Dumazet wrote:
> > Yes, this is wrong. In this context (process context, not softirq), we
> > should use netif_rx() or just drop frames if we are in reset phase.
> >
>
> Sergey,
>
> Here is a compiled but untested patch (I dont have the hardware), could
> you please test it ?
>
> Thanks
>
> [PATCH] r8169: Fix rtl8169_rx_interrupt()
>
> In case a reset is performed, rtl8169_rx_interrupt() is called from
> process context instead of softirq context. Special care must be taken
> to call appropriate network core services (netif_rx() instead of
> netif_receive_skb()). VLAN handling also corrected.
>
> Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Diagnosed-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 9d3ebf3..d873639 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -1038,14 +1038,14 @@ static void rtl8169_vlan_rx_register(struct net_device *dev,
> }
>
> static int rtl8169_rx_vlan_skb(struct rtl8169_private *tp, struct RxDesc *desc,
> - struct sk_buff *skb)
> + struct sk_buff *skb, int polling)
> {
> u32 opts2 = le32_to_cpu(desc->opts2);
> struct vlan_group *vlgrp = tp->vlgrp;
> int ret;
>
> if (vlgrp && (opts2 & RxVlanTag)) {
> - vlan_hwaccel_receive_skb(skb, vlgrp, swab16(opts2 & 0xffff));
> + __vlan_hwaccel_rx(skb, vlgrp, swab16(opts2 & 0xffff), polling);
> ret = 0;
> } else
> ret = -1;
> @@ -1062,7 +1062,7 @@ static inline u32 rtl8169_tx_vlan_tag(struct rtl8169_private *tp,
> }
>
> static int rtl8169_rx_vlan_skb(struct rtl8169_private *tp, struct RxDesc *desc,
> - struct sk_buff *skb)
> + struct sk_buff *skb, int polling)
> {
> return -1;
> }
> @@ -4429,12 +4429,20 @@ out:
> return done;
> }
>
> +/*
> + * Warning : rtl8169_rx_interrupt() might be called :
> + * 1) from NAPI (softirq) context
> + * (polling = 1 : we should call netif_receive_skb())
> + * 2) from process context (rtl8169_reset_task())
> + * (polling = 0 : we must call netif_rx() instead)
> + */
> static int rtl8169_rx_interrupt(struct net_device *dev,
> struct rtl8169_private *tp,
> void __iomem *ioaddr, u32 budget)
> {
> unsigned int cur_rx, rx_left;
> unsigned int delta, count;
> + int polling = (budget != ~(u32)0) ? 1 : 0;
>
> cur_rx = tp->cur_rx;
> rx_left = NUM_RX_DESC + tp->dirty_rx - cur_rx;
> @@ -4496,8 +4504,12 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
> skb_put(skb, pkt_size);
> skb->protocol = eth_type_trans(skb, dev);
>
> - if (rtl8169_rx_vlan_skb(tp, desc, skb) < 0)
> - netif_receive_skb(skb);
> + if (rtl8169_rx_vlan_skb(tp, desc, skb, polling) < 0) {
> + if (likely(polling))
> + netif_receive_skb(skb);
> + else
> + netif_rx(skb);
> + }
>
> dev->stats.rx_bytes += pkt_size;
> dev->stats.rx_packets++;
>
>
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* [PATCH 1/1] NET: ksz884x, fix lock imbalance
From: Jiri Slaby @ 2010-03-16 14:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, jirislaby, Tristram Ha
Stanse found that one error path (when alloc_skb fails) in netdev_tx
omits to unlock hw_priv->hwlock. Fix that.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Tristram Ha <Tristram.Ha@micrel.com>
Cc: David S. Miller <davem@davemloft.net>
---
drivers/net/ksz884x.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ksz884x.c b/drivers/net/ksz884x.c
index 7264a3e..0f59099 100644
--- a/drivers/net/ksz884x.c
+++ b/drivers/net/ksz884x.c
@@ -4899,8 +4899,10 @@ static int netdev_tx(struct sk_buff *skb, struct net_device *dev)
struct sk_buff *org_skb = skb;
skb = dev_alloc_skb(org_skb->len);
- if (!skb)
- return NETDEV_TX_BUSY;
+ if (!skb) {
+ rc = NETDEV_TX_BUSY;
+ goto unlock;
+ }
skb_copy_and_csum_dev(org_skb, skb->data);
org_skb->ip_summed = 0;
skb->len = org_skb->len;
@@ -4914,7 +4916,7 @@ static int netdev_tx(struct sk_buff *skb, struct net_device *dev)
netif_stop_queue(dev);
rc = NETDEV_TX_BUSY;
}
-
+unlock:
spin_unlock_irq(&hw_priv->hwlock);
return rc;
--
1.7.0.1
^ 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