Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-05 Thread Patrick McHardy
Herbert Xu wrote: On Thu, Aug 03, 2006 at 11:40:01AM +0200, Patrick McHardy wrote: Yes, the 32-bit thing is a bug, I meant it works fine without inverting the checksum. Right, I forgot about the checksum invariance under negation. In that case we can get rid of the tildes two lines down

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-05 Thread Herbert Xu
Hi Patrick: On Sat, Aug 05, 2006 at 09:13:51AM +0200, Patrick McHardy wrote: I've made the other changes you suggested, but removing the tildes here didn't work, so I left them in. I'll send the new patches in a few minutes. You're right of course. I was simply confused. The partial

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Patrick McHardy
Herbert Xu wrote: On Tue, Aug 01, 2006 at 09:19:34AM +0200, Patrick McHardy wrote: - nfct/nfctinfo/nfct_reasm: the xfrm output path does an immediate nf_reset, so they were not necessary until now. Queueing can happen on any hook, so we need to preserve them. This looks OK to me.

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Patrick McHardy
Herbert Xu wrote: +u_int16_t nf_proto_csum_update(struct sk_buff *skb, +u_int32_t oldval, u_int32_t newval, +u_int16_t csum, int pseudohdr) +{ + if (skb-ip_summed != CHECKSUM_PARTIAL) { + csum = nf_csum_update(oldval, newval,

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Herbert Xu
On Thu, Aug 03, 2006 at 11:21:45AM +0200, Patrick McHardy wrote: This looks OK. But we also need this for the redirect to loopback case, do you want me to put it in dev_gso_segment as well? nfmark and secmark should go in skb_segment I guess? OK, you've convinced me. Let's put them all into

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Patrick McHardy
Herbert Xu wrote: On Mon, Jul 31, 2006 at 08:36:58PM +0200, Patrick McHardy wrote: diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c index 662a869..df1f4e5 100644 --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c I presume we need similar patches for the old

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Patrick McHardy
Herbert Xu wrote: On Mon, Jul 31, 2006 at 09:30:50PM +1000, herbert wrote: diff --git a/net/ipv4/netfilter/ip_nat_core.c b/net/ipv4/netfilter/ip_nat_core.c index 1741d55..731efbb 100644 --- a/net/ipv4/netfilter/ip_nat_core.c +++ b/net/ipv4/netfilter/ip_nat_core.c @@ -443,7 +443,9 @@ int

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Herbert Xu
On Thu, Aug 03, 2006 at 11:29:41AM +0200, Patrick McHardy wrote: +u_int16_t nf_proto_csum_update(struct sk_buff *skb, + u_int32_t oldval, u_int32_t newval, + u_int16_t csum, int pseudohdr) +{ + if (skb-ip_summed != CHECKSUM_PARTIAL) { +

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Patrick McHardy
Herbert Xu wrote: On Thu, Aug 03, 2006 at 11:21:45AM +0200, Patrick McHardy wrote: This looks OK. But we also need this for the redirect to loopback case, do you want me to put it in dev_gso_segment as well? nfmark and secmark should go in skb_segment I guess? OK, you've convinced me.

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Herbert Xu
On Thu, Aug 03, 2006 at 11:32:32AM +0200, Patrick McHardy wrote: The checksum is verified here because a full checksum update is done later in that function and we don't want to accidentally fix up broken checksums. Sorry, I missed that. BTW, we should make sure that we clear

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Patrick McHardy
Herbert Xu wrote: On Thu, Aug 03, 2006 at 11:29:41AM +0200, Patrick McHardy wrote: +u_int16_t nf_proto_csum_update(struct sk_buff *skb, + u_int32_t oldval, u_int32_t newval, + u_int16_t csum, int pseudohdr) +{ + if (skb-ip_summed !=

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Patrick McHardy
Herbert Xu wrote: On Thu, Aug 03, 2006 at 11:32:32AM +0200, Patrick McHardy wrote: The checksum is verified here because a full checksum update is done later in that function and we don't want to accidentally fix up broken checksums. Sorry, I missed that. BTW, we should make sure that

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-03 Thread Herbert Xu
On Thu, Aug 03, 2006 at 11:40:01AM +0200, Patrick McHardy wrote: Are you sure? If ip_summed is CHECKSUM_COMPLETE then skb-csum is the checksum of the payload *without* the pseudo header. The pseudohdr is included indirectly through the tcp/udp checksum. Yes of course, the payload gets

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread David Miller
From: Patrick McHardy [EMAIL PROTECTED] Date: Mon, 31 Jul 2006 20:36:58 +0200 Herbert Xu wrote: So I'd rather see a patch to disable the warnings for 2.6.18 so that the proper fix can be tested more thoroughly. We should remember that the 2.6.18 minus the warning is still going to be

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
On Tue, Aug 01, 2006 at 12:00:59AM -0700, David Miller wrote: I'm going to kill the warning for 2.6.18, using the patch below. We can queue up Patrick's changes for 2.6.19, just give me the word and I'll apply it to net-2.6.19 Thanks Dave. This should give us plenty of time to produce a

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Patrick McHardy
Patrick McHardy wrote:--- [NETFILTER]: nf_queue: handle GSO packets Handle GSO packets in nf_queue by segmenting them before queueing to avoid breaking GSO in case they get mangled. While testing this patch I noticed that some meta-data is lost when segmenting packets. With the

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Patrick McHardy
David Miller wrote: I'm going to kill the warning for 2.6.18, using the patch below. We can queue up Patrick's changes for 2.6.19, just give me the word and I'll apply it to net-2.6.19 Besides the problem I mentioned in the mail I just wrote everything looks good so far. I'll probably send

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
Hi Patrick: On Tue, Aug 01, 2006 at 09:19:34AM +0200, Patrick McHardy wrote: - nfct/nfctinfo/nfct_reasm: the xfrm output path does an immediate nf_reset, so they were not necessary until now. Queueing can happen on any hook, so we need to preserve them. - nf_bridge: needed for GSO on

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread David Miller
From: Herbert Xu [EMAIL PROTECTED] Date: Tue, 1 Aug 2006 17:23:58 +1000 - tc_verd/tc_index/input_dev: when directing a packet from a device supporting GSO to a device not supporting GSO using tc actions, these fields may be set. This doesn't look right though. GSO should occur just

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Patrick McHardy
Herbert Xu wrote: - tc_verd/tc_index/input_dev: when directing a packet from a device supporting GSO to a device not supporting GSO using tc actions, these fields may be set. This doesn't look right though. GSO should occur just before hard_start_xmit (after all tc actions have taken

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
On Tue, Aug 01, 2006 at 12:36:14AM -0700, David Miller wrote: - tc_verd/tc_index/input_dev: when directing a packet from a device supporting GSO to a device not supporting GSO using tc actions, these fields may be set. This doesn't look right though. GSO should occur just

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
On Tue, Aug 01, 2006 at 09:19:34AM +0200, Patrick McHardy wrote: - nfct/nfctinfo/nfct_reasm: the xfrm output path does an immediate nf_reset, so they were not necessary until now. Queueing can happen on any hook, so we need to preserve them. This looks OK to me. However, we should do

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Jamal Hadi Salim
On Tue, 2006-01-08 at 17:45 +1000, Herbert Xu wrote: On Tue, Aug 01, 2006 at 12:36:14AM -0700, David Miller wrote: - tc_verd/tc_index/input_dev: when directing a packet from a device supporting GSO to a device not supporting GSO using tc actions, these fields may be set.

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
On Mon, Jul 31, 2006 at 08:36:58PM +0200, Patrick McHardy wrote: [NETFILTER]: Get rid of HW checksum invalidation Signed-off-by: Patrick McHardy [EMAIL PROTECTED] It all looks great except for the csum update function. diff --git a/net/netfilter/core.c b/net/netfilter/core.c index

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
On Tue, Aug 01, 2006 at 08:00:48AM -0400, Jamal Hadi Salim wrote: - tc_verd/tc_index/input_dev: when directing a packet from a device supporting GSO to a device not supporting GSO using tc actions, these fields may be set. This doesn't look right though. GSO should

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Jamal Hadi Salim
On Tue, 2006-01-08 at 22:34 +1000, Herbert Xu wrote: What I'd like to know is do we really need to preserve tc_verd/tc_index/input_dev for a packet crossing loopback's xmit function? My instinctive reaction is to say no. Heres a (slightly complex) example: -- eth0(GSO ON) --- lo

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Phil Oester
On Tue, Aug 01, 2006 at 12:00:59AM -0700, David Miller wrote: What we have now is infinitely better than the past, wherein all TSO packets were dropped due to corrupt checksums as soon at the NAT module was loaded. At what point did this problem begin? 2.6.18-rc or prior? Phil -

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
On Tue, Aug 01, 2006 at 08:34:05AM -0700, Phil Oester wrote: On Tue, Aug 01, 2006 at 12:00:59AM -0700, David Miller wrote: What we have now is infinitely better than the past, wherein all TSO packets were dropped due to corrupt checksums as soon at the NAT module was loaded.

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
On Mon, Jul 31, 2006 at 08:36:58PM +0200, Patrick McHardy wrote: diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c index 662a869..df1f4e5 100644 --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c I presume we need similar patches for the old ipv4/ipv6 versions as

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-08-01 Thread Herbert Xu
On Mon, Jul 31, 2006 at 09:30:50PM +1000, herbert wrote: diff --git a/net/ipv4/netfilter/ip_nat_core.c b/net/ipv4/netfilter/ip_nat_core.c index 1741d55..731efbb 100644 --- a/net/ipv4/netfilter/ip_nat_core.c +++ b/net/ipv4/netfilter/ip_nat_core.c @@ -443,7 +443,9 @@ int

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-31 Thread Patrick McHardy
Patrick McHardy wrote: David Miller wrote: I would like to see this fixed for 2.6.18, no later. Either that or disable the bug trap, but taking this route is severely discouraged. :) I'm actually updateing my patch for this on top of Herbert's CHECKSUM_PARTIAL patch right now.

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-31 Thread Herbert Xu
On Mon, Jul 31, 2006 at 12:39:51PM +0200, Patrick McHardy wrote: These are the patches (some variantions tested, but not all) on top of Herbert's CHECKSUM_PARTIAL patch. The first one fixes up the CHECKSUM_PARTIAL patch for 2.6.18-rc3, the second one fixes checksumming in all of netfilter

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-31 Thread David Miller
From: Patrick McHardy [EMAIL PROTECTED] Date: Mon, 31 Jul 2006 20:36:58 +0200 I'm going to do some more testing now .. Thanks for all of this work Patrick. I noticed a subtle semantic change for nf_queue(). Previously, if we can't grab the module reference for the matching entry, we'd not

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-31 Thread Patrick McHardy
David Miller wrote: I noticed a subtle semantic change for nf_queue(). Previously, if we can't grab the module reference for the matching entry, we'd not free the skb, return 0, and the caller tries to iterate to the next hook. That behavior is preserved for singleton frames, but that's not

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-31 Thread David Miller
From: Patrick McHardy [EMAIL PROTECTED] Date: Mon, 31 Jul 2006 23:36:29 +0200 David Miller wrote: Does this matter? I don't think it does. Its a huge corner case (unloading of the module which issued the QUEUE verdict while queueing the packet), and worst case is that we drop some

BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-30 Thread David Coulson
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 This machine has four NICs running the e1000 kernel module. Other than the BUG() messages, it seems to be running fine. I was running 2.6.15.4 without any issues on the same hardware, although I noticed the e1000 has been updated (and I went for rc3

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-30 Thread Patrick McHardy
David Coulson wrote: This machine has four NICs running the e1000 kernel module. Other than the BUG() messages, it seems to be running fine. I was running 2.6.15.4 without any issues on the same hardware, although I noticed the e1000 has been updated (and I went for rc3 since I was hitting the

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-30 Thread Andrew Morton
On Mon, 31 Jul 2006 00:16:21 -0400 David Coulson [EMAIL PROTECTED] wrote: This machine has four NICs running the e1000 kernel module. Other than the BUG() messages, it seems to be running fine. I was running 2.6.15.4 without any issues on the same hardware, although I noticed the e1000 has

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-30 Thread David Miller
From: Patrick McHardy [EMAIL PROTECTED] Date: Mon, 31 Jul 2006 06:24:31 +0200 This is a known problem with NAT and HW checksum and will probably get fixed in 2.6.19. I would like to see this fixed for 2.6.18, no later. Either that or disable the bug trap, but taking this route is severely

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-30 Thread Patrick McHardy
David Miller wrote: From: Patrick McHardy [EMAIL PROTECTED] Date: Mon, 31 Jul 2006 06:24:31 +0200 This is a known problem with NAT and HW checksum and will probably get fixed in 2.6.19. I would like to see this fixed for 2.6.18, no later. Either that or disable the bug trap, but

Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

2006-07-30 Thread David Coulson
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Now, I started the heartbeat process on the same machine, and it blew up trying to do something with IPaddr. I can't even sysrq the box back into life at this point :-/ BUG: unable to handle kernel paging request at virtual address 9e045900 printing