* Re: Reproducible data corruption with sendfile+vsftp - splice regression?
From: Holger Hoffstaette @ 2007-12-13 2:19 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
In-Reply-To: <20071206184426.GA32599@electric-eye.fr.zoreil.com>
On Thu, 06 Dec 2007 19:44:26 +0100, Francois Romieu wrote:
> Holger Hoffstaette <holger@wizards.de> : [...]
>> Maybe turning off sendfile or NAPI just lead to random success - so far
>> it really looks like tso on the r8169 is the common cause.
>
> TSO on the r8169 is the magic switch but the regression makes imvho more
> sense from a VM pov:
>
> - the corrupted file has the same size as the expected file - the
> corrupted file exhibits holes which come as a multiple of 4096 bytes
> (8*4k, 2 places, there may be more)
> - the r8169 driver does not know what a page is - the 8169 hardware has a
> small 8192 bytes Tx buffer
>
> It would be nice if someone could do a sendfile + vsftp test with TSO on a
> different hardware. While I could not reproduce the corruption when simply
> downloading a file that I had copied on the server with scp, it triggered
> almost immediately after I copied it locally and tried to download the
> copy.
Here's an update - sorry for the delay but I need that machine for everyday work.
I have now gone back to enable TSO since vsftp with sendfile really seems
to be the only app that causes this. I have simply set it to
use_sendfile=NO and no corruption occurs at all; the machine is stable and
fast.
FWIW the corruption can still be reproduced with 2.6.24-rc5. For kicks I
have also tried -rc5 with SLAB instead of SLUB, but that didn't help
either.
The directory with the tcpdump & test data now also contains a few more
corrupted files; maybe comparing the corruption offsets gives someone a
better idea.
thanks
Holger
^ permalink raw reply
* Re: [IPSEC]: Fix reversed ICMP6 policy check
From: David Miller @ 2007-12-13 2:48 UTC (permalink / raw)
To: herbert; +Cc: akpm, ilpo.jarvinen, netdev
In-Reply-To: <20071213015856.GA32668@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 13 Dec 2007 09:58:56 +0800
> [IPSEC]: Fix reversed ICMP6 policy check
>
> The policy check I added for ICMP on IPv6 is reversed. This
> patch fixes that.
>
> It also adds an skb->sp check so that unprotected packets that
> fail the policy check do not crash the machine.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied, thanks Herbert.
^ permalink raw reply
* Re: [IPSEC]: Make callers of xfrm_lookup to use XFRM_LOOKUP_WAIT
From: David Miller @ 2007-12-13 2:49 UTC (permalink / raw)
To: herbert; +Cc: netdev
In-Reply-To: <20071213014623.GB32308@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 13 Dec 2007 09:46:23 +0800
> [IPSEC]: Make callers of xfrm_lookup to use XFRM_LOOKUP_WAIT
>
> This patch converts all callers of xfrm_lookup that used an
> explicit value of 1 to indiciate blocking to use the new flag
> XFRM_LOOKUP_WAIT.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied, thanks Herbert.
^ permalink raw reply
* Re: [IPSEC]: Fix reversed ICMP6 policy check
From: Herbert Xu @ 2007-12-13 2:49 UTC (permalink / raw)
To: Andrew Morton; +Cc: David Miller, ilpo.jarvinen, netdev
In-Reply-To: <20071213015856.GA32668@gondor.apana.org.au>
On Thu, Dec 13, 2007 at 09:58:56AM +0800, Herbert Xu wrote:
>
> [IPSEC]: Fix reversed ICMP6 policy check
While that won't crash anymore, it's still logically wrong.
Here's a more complete fix.
[IPSEC]: Fix reversed ICMP6 policy check
The policy check I added for ICMP on IPv6 is reversed. We were also
letting packets through incorrectly if the ICMP flag isn't set. This
patch fixes that.
It also adds an skb->sp check so that unprotected packets that
fail the policy check do not crash the machine.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 4e3bfcd..ccdef9a 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -978,10 +978,13 @@ int icmp_rcv(struct sk_buff *skb)
struct icmphdr *icmph;
struct rtable *rt = (struct rtable *)skb->dst;
- if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb) &&
- skb->sp->xvec[skb->sp->len - 1]->props.flags & XFRM_STATE_ICMP) {
+ if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
int nh;
+ if (!(skb->sp && skb->sp->xvec[skb->sp->len - 1]->props.flags &
+ XFRM_STATE_ICMP))
+ goto drop;
+
if (!pskb_may_pull(skb, sizeof(*icmph) + sizeof(struct iphdr)))
goto drop;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 478ee77..bbf4162 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -646,10 +646,13 @@ static int icmpv6_rcv(struct sk_buff *skb)
struct icmp6hdr *hdr;
int type;
- if (xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb) &&
- skb->sp->xvec[skb->sp->len - 1]->props.flags & XFRM_STATE_ICMP) {
+ if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
int nh;
+ if (!(skb->sp && skb->sp->xvec[skb->sp->len - 1]->props.flags &
+ XFRM_STATE_ICMP))
+ goto drop_no_count;
+
if (!pskb_may_pull(skb, sizeof(*hdr) + sizeof(*orig_hdr)))
goto drop_no_count;
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [IPSEC]: Fix reversed ICMP6 policy check
From: David Miller @ 2007-12-13 2:51 UTC (permalink / raw)
To: herbert; +Cc: akpm, ilpo.jarvinen, netdev
In-Reply-To: <20071213024918.GA731@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 13 Dec 2007 10:49:18 +0800
> On Thu, Dec 13, 2007 at 09:58:56AM +0800, Herbert Xu wrote:
> >
> > [IPSEC]: Fix reversed ICMP6 policy check
>
> While that won't crash anymore, it's still logically wrong.
>
> Here's a more complete fix.
I already applied the first one, please send a relative
fixup which I'll combine as appropriate next rebase.
^ permalink raw reply
* Re: [IPSEC]: Fix reversed ICMP6 policy check
From: Herbert Xu @ 2007-12-13 2:51 UTC (permalink / raw)
To: David Miller; +Cc: akpm, ilpo.jarvinen, netdev
In-Reply-To: <20071212.184830.25856041.davem@davemloft.net>
On Wed, Dec 12, 2007 at 06:48:30PM -0800, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Thu, 13 Dec 2007 09:58:56 +0800
>
> > [IPSEC]: Fix reversed ICMP6 policy check
>
> Applied, thanks Herbert.
You're too quick :) Before you ask for an incremental patch, here's
a preemptive strike :)
[IPSEC]: Do not let packets pass when ICMP flag is off
This fixes a logical error in ICMP policy checks which lets
packets through if the state ICMP flag is off.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 132e879..ccdef9a 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -978,10 +978,13 @@ int icmp_rcv(struct sk_buff *skb)
struct icmphdr *icmph;
struct rtable *rt = (struct rtable *)skb->dst;
- if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb) && skb->sp &&
- skb->sp->xvec[skb->sp->len - 1]->props.flags & XFRM_STATE_ICMP) {
+ if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
int nh;
+ if (!(skb->sp && skb->sp->xvec[skb->sp->len - 1]->props.flags &
+ XFRM_STATE_ICMP))
+ goto drop;
+
if (!pskb_may_pull(skb, sizeof(*icmph) + sizeof(struct iphdr)))
goto drop;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 64d78c9..bbf4162 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -646,10 +646,13 @@ static int icmpv6_rcv(struct sk_buff *skb)
struct icmp6hdr *hdr;
int type;
- if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb) && skb->sp &&
- skb->sp->xvec[skb->sp->len - 1]->props.flags & XFRM_STATE_ICMP) {
+ if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
int nh;
+ if (!(skb->sp && skb->sp->xvec[skb->sp->len - 1]->props.flags &
+ XFRM_STATE_ICMP))
+ goto drop_no_count;
+
if (!pskb_may_pull(skb, sizeof(*hdr) + sizeof(*orig_hdr)))
goto drop_no_count;
^ permalink raw reply related
* Re: [IPSEC]: Fix reversed ICMP6 policy check
From: David Miller @ 2007-12-13 2:54 UTC (permalink / raw)
To: herbert; +Cc: akpm, ilpo.jarvinen, netdev
In-Reply-To: <20071213025155.GA784@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 13 Dec 2007 10:51:56 +0800
> You're too quick :) Before you ask for an incremental patch, here's
> a preemptive strike :)
Hehe :)
> [IPSEC]: Do not let packets pass when ICMP flag is off
>
> This fixes a logical error in ICMP policy checks which lets
> packets through if the state ICMP flag is off.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
From: Herbert Xu @ 2007-12-13 3:13 UTC (permalink / raw)
To: David Miller; +Cc: wangchen, jgarzik, khc, netdev
In-Reply-To: <20071212.104614.208083873.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote:
> From: Wang Chen <wangchen@cn.fujitsu.com>
> Date: Wed, 12 Dec 2007 10:35:56 +0800
>
>> [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
>
> Applied, thanks for finding and fixing this bug.
Let's see who else is doing this:
[NET]: Check RTNL status in unregister_netdevice
The caller must hold the RTNL so let's check it in unregister_netdevice.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/core/dev.c b/net/core/dev.c
index 06615df..b254e52 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3967,6 +3967,8 @@ void synchronize_net(void)
void unregister_netdevice(struct net_device *dev)
{
+ ASSERT_RTNL();
+
rollback_registered(dev);
/* Finish processing unregister after unlock */
net_set_todo(dev);
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
From: David Miller @ 2007-12-13 3:22 UTC (permalink / raw)
To: herbert; +Cc: wangchen, jgarzik, khc, netdev
In-Reply-To: <E1J2eW8-0000GQ-00@gondolin.me.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 13 Dec 2007 11:13:44 +0800
> Let's see who else is doing this:
>
> [NET]: Check RTNL status in unregister_netdevice
>
> The caller must hold the RTNL so let's check it in unregister_netdevice.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Good idea, applied to net-2.6.25, thanks.
^ permalink raw reply
* [DECNET]: Fix inverted wait flag in xfrm_lookup call
From: Herbert Xu @ 2007-12-13 5:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071212.184919.157572948.davem@davemloft.net>
On Wed, Dec 12, 2007 at 06:49:19PM -0800, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Thu, 13 Dec 2007 09:46:23 +0800
>
> > [IPSEC]: Make callers of xfrm_lookup to use XFRM_LOOKUP_WAIT
Sorry, I was too quick too :) I managed to invert the DECNet
xfrm_lookup call. Here's a fix.
[DECNET]: Fix inverted wait flag in xfrm_lookup call
My previous patch made the wait flag take the opposite value to what it
should be. This patch fixes that.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 62f3ae7..2d95cf1 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1197,7 +1197,7 @@ int dn_route_output_sock(struct dst_entry **pprt, struct flowi *fl, struct sock
err = __dn_route_output_key(pprt, fl, flags & MSG_TRYHARD);
if (err == 0 && fl->proto) {
err = xfrm_lookup(pprt, fl, sk, (flags & MSG_DONTWAIT) ?
- XFRM_LOOKUP_WAIT : 0);
+ 0 : XFRM_LOOKUP_WAIT);
}
return err;
}
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* [PATCH] IPv6: Fix the return value of ipv6_getsockopt
From: Wei Yongjun @ 2007-12-13 5:44 UTC (permalink / raw)
To: netdev
If CONFIG_NETFILTER if not selected when compile the kernel source code,
ipv6_getsockopt will returen an EINVAL error if optname is not supported by
the kernel. But if CONFIG_NETFILTER is selected, ENOPROTOOPT error will
be return.
This patch fix to always return ENOPROTOOPT error if optname argument of
ipv6_getsockopt is not supported by the kernel.
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
--- a/net/ipv6/ipv6_sockglue.c 2007-12-11 04:33:00.000000000 -0500
+++ b/net/ipv6/ipv6_sockglue.c 2007-12-11 06:40:18.000000000 -0500
@@ -1046,7 +1046,7 @@ static int do_ipv6_getsockopt(struct soc
break;
default:
- return -EINVAL;
+ return -ENOPROTOOPT;
}
len = min_t(unsigned int, sizeof(int), len);
if(put_user(len, optlen))
@@ -1069,9 +1069,8 @@ int ipv6_getsockopt(struct sock *sk, int
err = do_ipv6_getsockopt(sk, level, optname, optval, optlen);
#ifdef CONFIG_NETFILTER
- /* we need to exclude all possible EINVALs except default case */
- if (err == -EINVAL && optname != IPV6_ADDRFORM &&
- optname != MCAST_MSFILTER) {
+ /* we need to exclude all possible ENOPROTOOPTs except default case */
+ if (err == -ENOPROTOOPT && optname != IPV6_2292PKTOPTIONS) {
int len;
if (get_user(len, optlen))
@@ -1108,9 +1107,8 @@ int compat_ipv6_getsockopt(struct sock *
err = do_ipv6_getsockopt(sk, level, optname, optval, optlen);
#ifdef CONFIG_NETFILTER
- /* we need to exclude all possible EINVALs except default case */
- if (err == -EINVAL && optname != IPV6_ADDRFORM &&
- optname != MCAST_MSFILTER) {
+ /* we need to exclude all possible ENOPROTOOPTs except default case */
+ if (err == -ENOPROTOOPT && optname != IPV6_2292PKTOPTIONS) {
int len;
if (get_user(len, optlen))
^ permalink raw reply
* net-2.6.235 warning
From: Andrew Morton @ 2007-12-13 5:55 UTC (permalink / raw)
To: netdev; +Cc: Ilpo Järvinen, David S. Miller
This is net-2.6.25 pulled ten hours ago. Top of tree was
dc2d76df54c8211df766819749beb802f69733fc. Herbert's two ipsec patches are
applied.
Mac g5 acting as a distcc server had been running an hour or so.
------------[ cut here ]------------
Badness at net/ipv4/tcp_input.c:1280
NIP: c0000000003d1fb0 LR: c0000000003d27d8 CTR: c0000000003d68dc
REGS: c00000012d506ee0 TRAP: 0700 Not tainted (2.6.24-rc5-mm1)
MSR: 9000000000029032 <EE,ME,IR,DR> CR: 28028428 XER: 20000000
TASK = c0000001783f1280[10480] 'as' THREAD: c00000012d504000 CPU: 3
GPR00: 0000000000000028 c00000012d507160 c0000000006d4518 0000000000000001
GPR04: c00000012d1e3870 0000000007be5279 0000000007be57fe c0000001772f4d88
GPR08: 000000000000002a 0000000000000000 0000000007be5279 0000000000000025
GPR12: 0000000088048428 c00000000061b280 0000000000000000 0000000000000000
GPR16: 0000000000000003 0000000007bd75e1 00000000e8510e30 c00000012d5072d0
GPR20: c00000012d1e3d30 0000000000000000 c00000012d5072d0 c00000012d5072d4
GPR24: c00000012d1e3938 0000000007be5279 0000000007be57fe c00000012d1e3938
GPR28: c00000012d1e3870 0000000000000000 c000000000699b58 c000000177d3a818
NIP [c0000000003d1fb0] .tcp_sacktag_walk+0x3e0/0x7e8
LR [c0000000003d27d8] .tcp_sacktag_write_queue+0x388/0xc84
Call Trace:
[c00000012d507160] [c00000012d5071f0] 0xc00000012d5071f0 (unreliable)
[c00000012d507240] [c0000000003d27d8] .tcp_sacktag_write_queue+0x388/0xc84
[c00000012d507350] [c0000000003d5308] .tcp_ack+0x950/0x1d98
[c00000012d507520] [c0000000003d99dc] .tcp_rcv_state_process+0x138/0xe90
[c00000012d5075e0] [c0000000003e3818] .tcp_v4_do_rcv+0xb0/0x580
[c00000012d5076d0] [c0000000003e492c] .tcp_v4_rcv+0xc44/0xd28
[c00000012d5077e0] [c0000000003be3e4] .ip_local_deliver+0xb4/0x2cc
[c00000012d507870] [c0000000003be8c8] .ip_rcv+0x2cc/0x6f0
[c00000012d507930] [c0000000003974d0] .netif_receive_skb+0x440/0x4bc
[c00000012d5079f0] [c0000000002830a8] .tg3_poll+0x71c/0xb78
[c00000012d507b30] [c000000000397914] .net_rx_action+0x1e4/0x26c
[c00000012d507be0] [c00000000006a33c] .__do_softirq+0xd8/0x190
[c00000012d507ca0] [c00000000000b95c] .do_softirq+0x94/0x9c
[c00000012d507d30] [c00000000006a4a8] .irq_exit+0x70/0x88
[c00000012d507db0] [c00000000000b83c] .do_IRQ+0xa0/0x12c
[c00000012d507e30] [c000000000004790] hardware_interrupt_entry+0x18/0x1c
Instruction dump:
7c0807b4 409efee4 801c03f8 7c005050 78090fe3 4082fef0 801c0588 7f804040
409c0010 801c046c 7f804040 409c0008 <0fe00000> 881f0079 540007be 2f800001
Here:
WARN_ON((fack_count <= tp->fackets_out) ||
(fack_count > tp->packets_out));
It isn't a good idea to put two conditions in a single assert like
this, btw. When it fires you don't know which one caused it..
^ permalink raw reply
* Re: [PATCH 4/4] sky2: rtnl_lock out of loop will be faster
From: Wang Chen @ 2007-12-13 6:26 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Jeff Garzik, netdev
In-Reply-To: <20071212083011.731773b9@freepuppy.rosehill>
Stephen Hemminger said the following on 2007-12-13 0:30:
> Umm, okay but it doesn't matter really, there can only be 2 ports and
> 90+% of the boards only have one port. So why bother??
>
Thanks Stephen.
David, please ignore all of the four patches.
^ permalink raw reply
* Re: [RFC] ehea: kdump support using new shutdown hook
From: Michael Ellerman @ 2007-12-13 6:30 UTC (permalink / raw)
To: Dave Jones
Cc: Thomas Klein, Paul Mackerras, Christoph Raisch, Jan-Bernd Themann,
linux-kernel, linux-ppc, Marcus Eder, netdev, Stefan Roscher,
Michael Neuling
In-Reply-To: <20071212170440.GD2359@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 668 bytes --]
On Wed, 2007-12-12 at 12:04 -0500, Dave Jones wrote:
> On Wed, Dec 12, 2007 at 05:53:43PM +0100, Thomas Klein wrote:
>
> > +static void ehea_update_adapter_handles(struct ehea_adapter *adapter)
> > +{
> > + int i, k;
> > + int j = 0;
> > +
> > + memset(adapter->res_handles, sizeof(adapter->res_handles), 0);
>
> arguments wrong way around.
Remind me why bzero is deprecated again? :)
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [RFC] ehea: kdump support using new shutdown hook
From: Michael Ellerman @ 2007-12-13 6:41 UTC (permalink / raw)
To: Thomas Klein
Cc: Paul Mackerras, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, netdev, Stefan Roscher, Michael Neuling
In-Reply-To: <200712121753.43543.osstklei@de.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 5607 bytes --]
On Wed, 2007-12-12 at 17:53 +0100, Thomas Klein wrote:
> This patch adds kdump support using the new PPC crash shutdown hook to the
> ehea driver. The driver now keeps a list of firmware handles which have to
> be freed in case of a crash. The crash handler does the minimum required: it
> frees the firmware resource handles plus broadcast/multicast registrations.
>
> Please comment.
Hi Thomas,
Here's a few ..
> ---
> diff -Nurp -X dontdiff linux-2.6.24-rc5/drivers/net/ehea/ehea.h patched_kernel/drivers/net/ehea/ehea.h
> --- linux-2.6.24-rc5/drivers/net/ehea/ehea.h 2007-12-11 04:48:43.000000000 +0100
> +++ patched_kernel/drivers/net/ehea/ehea.h 2007-12-12 17:30:53.000000000 +0100
> @@ -386,6 +386,7 @@ struct ehea_port_res {
>
>
> #define EHEA_MAX_PORTS 16
> +#define EHEA_MAX_RES_HANDLES (100 * EHEA_MAX_PORTS + 10)
> struct ehea_adapter {
> u64 handle;
> struct of_device *ofdev;
> @@ -397,6 +398,7 @@ struct ehea_adapter {
> u64 max_mc_mac; /* max number of multicast mac addresses */
> int active_ports;
> struct list_head list;
> + u64 res_handles[EHEA_MAX_RES_HANDLES];
> };
I don't like this ..
> diff -Nurp -X dontdiff linux-2.6.24-rc5/drivers/net/ehea/ehea_main.c patched_kernel/drivers/net/ehea/ehea_main.c
> --- linux-2.6.24-rc5/drivers/net/ehea/ehea_main.c 2007-12-11 04:48:43.000000000 +0100
> +++ patched_kernel/drivers/net/ehea/ehea_main.c 2007-12-12 17:30:53.000000000 +0100
> @@ -35,6 +35,7 @@
> #include <linux/if_ether.h>
> #include <linux/notifier.h>
> #include <linux/reboot.h>
> +#include <asm-powerpc/kexec.h>
Just <asm/kexec.h>
> @@ -3302,6 +3333,71 @@ static int __devexit ehea_remove(struct
> return 0;
> }
>
> +void ehea_crash_deregister(void)
> +{
> + struct ehea_adapter *adapter;
> + int i;
> + u64 hret;
> + u8 reg_type;
> +
> + list_for_each_entry(adapter, &adapter_list, list) {
> + for (i = 0; i < EHEA_MAX_PORTS; i++) {
> + struct ehea_port *port = adapter->port[i];
> + if (port->state == EHEA_PORT_UP) {
> + struct ehea_mc_list *mc_entry = port->mc_list;
> + struct list_head *pos;
> + struct list_head *temp;
> +
> + /* Undo multicast registrations */
> + list_for_each_safe(pos, temp,
> + &(port->mc_list->list)) {
> + mc_entry = list_entry(pos,
> + struct ehea_mc_list,
> + list);
> + ehea_multicast_reg_helper(port,
> + mc_entry->macaddr,
> + H_DEREG_BCMC);
> + }
> +
> + /* Undo broad registration */
> + reg_type = EHEA_BCMC_BROADCAST |
> + EHEA_BCMC_UNTAGGED;
> + ehea_h_reg_dereg_bcmc(port->adapter->handle,
> + port->logical_port_id,
> + reg_type, port->mac_addr,
> + 0, H_DEREG_BCMC);
> +
> + reg_type = EHEA_BCMC_BROADCAST |
> + EHEA_BCMC_VLANID_ALL;
> + ehea_h_reg_dereg_bcmc(port->adapter->handle,
> + port->logical_port_id,
> + reg_type, port->mac_addr,
> + 0, H_DEREG_BCMC);
> + }
> + }
> + for (i = 0; i < EHEA_MAX_RES_HANDLES; i++) {
> + u64 handle = adapter->res_handles[i];
> + if (handle) {
> + hret = ehea_h_free_resource(adapter->handle,
> + handle,
> + FORCE_FREE);
> + }
> + }
> +
> + if (adapter->neq) {
> + hret = ehea_h_free_resource(adapter->handle,
> + adapter->neq->fw_handle,
> + FORCE_FREE);
> + }
> +
> + if (adapter->mr.handle) {
> + hret = ehea_h_free_resource(adapter->handle,
> + adapter->mr.handle,
> + FORCE_FREE);
> + }
> + }
> +}
This is sort of on the right track, I like that the ehea_h_.. routines
are basically just hypercalls, but there's a few things wrong. You're
walking a linked list, which is asking for trouble, if a single pointer
is corrupted you'll walk off into lala land. Then you're pulling fields
out of structs via pointers, again hoping that they're all valid. Then
you walk another linked list .. And then you're pulling bits out of the
adapter again.
Ideally it would look like this:
static u64 things_to_free[];
void ehea_crash_deregister(void)
{
for (i = 0; i < num_things_to_free; i++)
h_call_free_a_thing(things_to_free[i]);
}
> static int ehea_reboot_notifier(struct notifier_block *nb,
> unsigned long action, void *unused)
> {
> @@ -3373,6 +3469,9 @@ int __init ehea_module_init(void)
> goto out;
>
> register_reboot_notifier(&ehea_reboot_nb);
> + ret = crash_shutdown_register(&ehea_crash_deregister);
> + if (ret)
> + ehea_info("failed registering crash handler");
Your naming is a little confusing here, you're registering the
deregister function. I think I get it, you're unregistering the fw
handles, but perhaps ehea_crash_shutdown() would be clearer.
> @@ -3396,10 +3496,15 @@ out:
>
> static void __exit ehea_module_exit(void)
> {
> + int ret;
> +
> flush_scheduled_work();
> driver_remove_file(&ehea_driver.driver, &driver_attr_capabilities);
> ibmebus_unregister_driver(&ehea_driver);
> unregister_reboot_notifier(&ehea_reboot_nb);
> + ret = crash_shutdown_unregister(&ehea_crash_deregister);
> + if (ret)
> + ehea_info("failed unregistering crash handler");
You don't need ret if that's all you're going to do with it.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 6/7] : tehuti Fix possible causing oops of net_rx_action
From: Joonwoo Park @ 2007-12-13 7:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel, jgarzik, baum, andy
In-Reply-To: <20071212.071832.07747779.davem@davemloft.net>
2007/12/13, David Miller <davem@davemloft.net>:
> From: "Joonwoo Park" <joonwpark81@gmail.com>
> Date: Wed, 12 Dec 2007 13:01:27 +0900
>
>
> Any time your trying to make a caller "happy" by adjusting
> a return value forcefully, it's a hack.
>
> And I stated this in another reply about this issue.
>
> Please do not fix the problem this way.
>
> The correct way to fix this is, if we did process a full
> "weight" or work, we should not netif_rx_complete() and
> we should not re-enable chip interrupts.
>
> Instead we should return the true "work_done" value and
> allow the caller to thus poll us one more time.
>
Thanks so much for your advice.
I agree, returning work_done itself exactly.
I will rework for these drivers.
Thanks.
Joonwoo
^ permalink raw reply
* Re: [PATCH 2.6.25] IPv4: thresholds in fib_trie.c are not modified, make them const
From: Denis V. Lunev @ 2007-12-13 7:32 UTC (permalink / raw)
To: David Miller; +Cc: den, containers, devel, netdev
In-Reply-To: <20071212.105833.237798467.davem@davemloft.net>
David Miller wrote:
> From: "Denis V. Lunev" <den@openvz.org>
> Date: Wed, 12 Dec 2007 14:05:55 +0300
>
>> -static int halve_threshold = 25;
>> -static int inflate_threshold = 50;
>> -static int halve_threshold_root = 15;
>> -static int inflate_threshold_root = 25;
>
> halve_threshold_root and inflate_threshold_root have different
> values in the net-2.6.25 tree.
oops. this was accidentally changed during porting of the code from the
one old tree and committed by mistake. I'll resend correct patch in a minute
>
> What tree are you patching against? These were changed way
> back in:
^ permalink raw reply
* Re: [RFC] net: napi fix
From: Joonwoo Park @ 2007-12-13 7:41 UTC (permalink / raw)
To: Kok, Auke
Cc: David Miller, gallatin, netdev, linux-kernel, jgarzik, shemminger,
Jesse Brandeburg
In-Reply-To: <47602B77.2090202@intel.com>
2007/12/13, Kok, Auke <auke-jan.h.kok@intel.com>:
> David Miller wrote:
> > From: Andrew Gallatin <gallatin@myri.com>
> > Date: Wed, 12 Dec 2007 12:29:23 -0500
> >
> >> Is the netif_running() check even required?
> >
> > No, it is not.
> >
> > When a device is brought down, one of the first things
> > that happens is that we wait for all pending NAPI polls
> > to complete, then block any new polls from starting.
>
> I think this was previously (pre-2.6.24) not the case, which is why e1000 et al
> has this check as well and that's exactly what is causing most of the
> net_rx_action oopses in the first place. Without the netif_running() check
> previously the drivers were just unusable with NAPI and prone to many races with
> down (i.e. touching some ethtool ioctl which wants to do a reset while routing
> small packets at high numbers). that's why we added the netif_running() check in
> the first place :)
>
> There might be more drivers lurking that need this change...
>
> Auke
>
Also in my case, without netif_running() check, I cannot do ifconfig down.
It stucked if packet generator was sending packets.
Thanks
Joonwoo
^ permalink raw reply
* [PATCH] MACB: clear transmit buffers properly on TX Underrun
From: Gregory CLEMENT @ 2007-12-13 7:51 UTC (permalink / raw)
To: hskinnemoen, linux-arm-kernel, netdev
Hi,
I generated this patch for linux 2.6.24-rc5 and test it on AT91SAM9263
with iperf.
From: Gregory CLEMENT <gclement00@gmail.com>
Date: Wed, 12 Dec 2007 18:10:14 +0100
Subject: [PATCH] MACB: clear transmit buffers properly on TX Underrun
Initially transmit buffer pointers were only reset. But buffer descriptors
were possibly still set as ready, and buffer in upper layer was not
freed. This caused driver hang under big load.
Now reset clean properly the buffer descriptor and freed upper layer.
Signed-off-by: Gregory CLEMENT <gclement00@gmail.com>
---
drivers/net/macb.c | 26 +++++++++++++++++++++++++-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 047ea7b..2ee1dab 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -307,9 +307,33 @@ static void macb_tx(struct macb *bp)
(unsigned long)status);
if (status & MACB_BIT(UND)) {
+ int i;
printk(KERN_ERR "%s: TX underrun, resetting buffers\n",
- bp->dev->name);
+ bp->dev->name);
+
+ head = bp->tx_head;
+
+ /* free transmit buffer in upper layer*/
+ for (tail = bp->tx_tail; tail != head; tail = NEXT_TX(tail)) {
+ struct ring_info *rp = &bp->tx_skb[tail];
+ struct sk_buff *skb = rp->skb;
+
+ BUG_ON(skb == NULL);
+
+ rmb();
+
+ dma_unmap_single(&bp->pdev->dev, rp->mapping, skb->len,
+ DMA_TO_DEVICE);
+ rp->skb = NULL;
+ dev_kfree_skb_irq(skb);
+ }
+
+ /*Mark all the buffer as used to avoid sending a lost buffer*/
+ for (i = 0; i < RX_RING_SIZE; i++)
+ bp->tx_ring[i].ctrl = MACB_BIT(TX_USED);
+
bp->tx_head = bp->tx_tail = 0;
+
}
if (!(status & MACB_BIT(COMP)))
--
1.5.3.7
--
Gregory CLEMENT
Adeneo
Adetel Group
2, chemin du Ruisseau
69134 ECULLY - FRANCE
Tél. : +33 (0)4 72 18 08 40 - Fax : +33 (0)4 72 18 08 41
www.adetelgroup.com
^ permalink raw reply related
* [PATCH 2.6.25] [IPV4] Thresholds in fib_trie.c are used as consts, so make them const.
From: Denis V. Lunev @ 2007-12-13 8:06 UTC (permalink / raw)
To: davem; +Cc: containers, devel, netdev
[IPV4] Thresholds in fib_trie.c are used as consts, so make them const.
There are several thresholds for trie fib hash management. They are used
in the code as a constants. Make them constants from the compiler point of
view.
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -299,10 +299,10 @@ static inline void check_tnode(const struct tnode *tn)
WARN_ON(tn && tn->pos+tn->bits > 32);
}
-static int halve_threshold = 25;
-static int inflate_threshold = 50;
-static int halve_threshold_root = 8;
-static int inflate_threshold_root = 15;
+static const int halve_threshold = 25;
+static const int inflate_threshold = 50;
+static const int halve_threshold_root = 8;
+static const int inflate_threshold_root = 15;
static void __alias_free_mem(struct rcu_head *head)
^ permalink raw reply
* Re: [PATCH] r6040 various cleanups
From: Florian Fainelli @ 2007-12-13 8:33 UTC (permalink / raw)
To: Francois Romieu; +Cc: jeff, Andrew Morton, Sten Wang, netdev
In-Reply-To: <20071212223541.GA32305@electric-eye.fr.zoreil.com>
Hi Francois,
Francois Romieu a écrit :
> Thanks, I have split it in parts. The serie should be available shortly at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/romieu/netdev-2.6.git r6040
You are welcome, thank you for taking care of this driver !
>
> Please note that:
> 1. TIMER_WUT has been removed as it was not used any more
> 2. I have kept the difference below. Was the patch really supposed to update
> the same error counter twice ?
You are right, I did not pay attention to this, thanks for fixing !
^ permalink raw reply
* Re: [virtio-net][PATCH] Don't arm tx hrtimer with a constant 500us each transmit
From: Dor Laor @ 2007-12-13 8:44 UTC (permalink / raw)
To: Christian Borntraeger
Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
netdev-u79uwXL29TY76Z2rM5mHXA, virtualization
In-Reply-To: <200712121733.07804.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
Christian Borntraeger wrote:
>
> Am Mittwoch, 12. Dezember 2007 schrieb Dor Laor:
> > Christian Borntraeger wrote:
> > >
> > > Am Mittwoch, 12. Dezember 2007 schrieb Dor Laor:
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -406,10 +405,10 @@ again:
> > >
> > > Hmm, while I agree in general with the patch, I fail to find the
> proper
> > > version of virtio_net where this patch applies. I tried kvm.git and
> > > linux-2.6.git from kernel.org. Can you give me a pointer to the
> repository
> > > where you work on virtio?
> > >
> > Sorry for that, I added some debug prints of my one.
> > Here it is: *git clone
> > git*://kvm.*qumranet*.com/home/*dor*/src/linux-2.6-nv use branch
> 'virtio'.
>
> Ah, ok. I will look into that branch.
>
> > BTW: what git repository do you use?
>
> I use Avis git from kernel.org:
> git://git.kernel.org/pub/scm/linux/kernel/git/avi/kvm
>
I patch it with Anthony's http://hg.codemonkey.ws/linux-virtio/
Over that I send the patches. One can use my repository directly.
>
> Christian
>
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
^ permalink raw reply
* net-2.6.25 section warnings
From: Andrew Morton @ 2007-12-13 10:07 UTC (permalink / raw)
To: netdev
WARNING: vmlinux.o(.init.text+0x1cfc4): Section mismatch: reference to .exit.text:tcpv6_exit (between 'inet6_init' and 'ac6_proc_init')
WARNING: vmlinux.o(.init.text+0x1cfc9): Section mismatch: reference to .exit.text:udplitev6_exit (between 'inet6_init' and 'ac6_proc_init')
WARNING: vmlinux.o(.init.text+0x1cfce): Section mismatch: reference to .exit.text:udpv6_exit (between 'inet6_init' and 'ac6_proc_init')
WARNING: vmlinux.o(.init.text+0x1cfdd): Section mismatch: reference to .exit.text:addrconf_cleanup (between 'inet6_init' and 'ac6_proc_init')
WARNING: vmlinux.o(.init.text+0x1d03c): Section mismatch: reference to .exit.text:rawv6_exit (between 'inet6_init' and 'ac6_proc_init')
http://userweb.kernel.org/~akpm/config-x.txt (x86_64)
^ permalink raw reply
* RE: [PATCH 1/7] : e1000 Fix possible causing oops of net_rx_action
From: Joonwoo Park @ 2007-12-13 10:18 UTC (permalink / raw)
To: 'Kok, Auke'
Cc: netdev, linux-kernel, jesse.brandeburg, jgarzik, shemminger,
'David Miller'
2007/12/12, Joonwoo Park <joonwpark81@gmail.com>:
> [NETDEV]: e1000 Fix possible causing oops of net_rx_action
> returning work_done == weight as true after calling netif_rx_complete will cause oops in net_rx_action.
>
I tried two types of patches for oops and ifconfig down hang for e1000 first.
Just blowing netif_running up is not best solution I think, it makes ifconfig down hang at least for e1000.
I would like to listen to the others suggestions courteously, please enlighten me :-)
The first:
- if !netif_running, stop receiving process, up to 64 (e1000) packets in the queue would be dropped.
---
drivers/net/e1000/e1000_main.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 4f37506..664312b 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3938,12 +3938,12 @@ e1000_clean(struct napi_struct *napi, int budget)
spin_unlock(&adapter->tx_queue_lock);
}
- adapter->clean_rx(adapter, &adapter->rx_ring[0],
- &work_done, budget);
+ if (likely(netif_running(poll_dev)))
+ adapter->clean_rx(adapter, &adapter->rx_ring[0],
+ &work_done, budget);
/* If no Tx and not enough Rx work done, exit the polling mode */
- if ((!tx_cleaned && (work_done == 0)) ||
- !netif_running(poll_dev)) {
+ if ((!tx_cleaned && (work_done == 0))) {
quit_polling:
if (likely(adapter->itr_setting & 3))
e1000_set_itr(adapter);
---
The second:
- if !netif_running, receive up to weight - 1 packets, one packets in the queue can be dropped.
---
drivers/net/e1000/e1000_main.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 4f37506..8e53c5b 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3919,7 +3919,7 @@ e1000_clean(struct napi_struct *napi, int budget)
{
struct e1000_adapter *adapter = container_of(napi, struct e1000_adapter, napi);
struct net_device *poll_dev = adapter->netdev;
- int tx_cleaned = 0, work_done = 0;
+ int tx_cleaned = 0, work_done = 0, running;
/* Must NOT use netdev_priv macro here. */
adapter = poll_dev->priv;
@@ -3938,12 +3938,13 @@ e1000_clean(struct napi_struct *napi, int budget)
spin_unlock(&adapter->tx_queue_lock);
}
+ running = netif_running(poll_dev);
+
adapter->clean_rx(adapter, &adapter->rx_ring[0],
- &work_done, budget);
+ &work_done, budget - !running);
/* If no Tx and not enough Rx work done, exit the polling mode */
- if ((!tx_cleaned && (work_done == 0)) ||
- !netif_running(poll_dev)) {
+ if ((!tx_cleaned && (work_done == 0)) || !running) {
quit_polling:
if (likely(adapter->itr_setting & 3))
e1000_set_itr(adapter);
---
Thanks,
Joonwoo
^ permalink raw reply related
* Re: net-2.6.25 section warnings
From: Daniel Lezcano @ 2007-12-13 10:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: netdev
In-Reply-To: <20071213020722.2790e023.akpm@linux-foundation.org>
Andrew Morton wrote:
> WARNING: vmlinux.o(.init.text+0x1cfc4): Section mismatch: reference to .exit.text:tcpv6_exit (between 'inet6_init' and 'ac6_proc_init')
> WARNING: vmlinux.o(.init.text+0x1cfc9): Section mismatch: reference to .exit.text:udplitev6_exit (between 'inet6_init' and 'ac6_proc_init')
> WARNING: vmlinux.o(.init.text+0x1cfce): Section mismatch: reference to .exit.text:udpv6_exit (between 'inet6_init' and 'ac6_proc_init')
> WARNING: vmlinux.o(.init.text+0x1cfdd): Section mismatch: reference to .exit.text:addrconf_cleanup (between 'inet6_init' and 'ac6_proc_init')
> WARNING: vmlinux.o(.init.text+0x1d03c): Section mismatch: reference to .exit.text:rawv6_exit (between 'inet6_init' and 'ac6_proc_init')
>
> http://userweb.kernel.org/~akpm/config-x.txt (x86_64)
> --
> 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
>
My fault. I will fix that.
Sorry.
-- Daniel
--
Sauf indication contraire ci-dessus:
Compagnie IBM France
Siège Social : Tour Descartes, 2, avenue Gambetta, La Défense 5, 92400
Courbevoie
RCS Nanterre 552 118 465
Forme Sociale : S.A.S.
Capital Social : 542.737.118 ?
SIREN/SIRET : 552 118 465 02430
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox