Netdev List
 help / color / mirror / Atom feed
* Re: BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
From: Timo Teräs @ 2010-06-23 20:34 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: Eric Dumazet, John W.Linville, netdev, Linux Kernel Mailing List,
	davem
In-Reply-To: <4C22508A.8060002@gmail.com>

On 06/23/2010 09:20 PM, Justin P. Mattock wrote:
> On 06/23/10 11:10, Timo Teräs wrote:
>> On 06/23/2010 08:29 PM, Eric Dumazet wrote:
>>   
>>> Le mercredi 23 juin 2010 à 10:00 -0700, Justin P. Mattock a écrit :
>>>     
>>>> o.k. the bisect is pointing to the below results..
>>>> (I tried git revert xxx but this commit is too big
>>>> so I'll(hopefully)manually revert it on the latest HEAD to
>>>> see if this is the actual problem im experiencing)
>>>>
>>>> 80c802f3073e84c956846e921e8a0b02dfa3755f is the first bad commit
>>>> commit 80c802f3073e84c956846e921e8a0b02dfa3755f
>>>> Author: Timo Teräs<timo.teras@iki.fi>
>>>> Date:   Wed Apr 7 00:30:05 2010 +0000
>>>>
>>>>       xfrm: cache bundles instead of policies for outgoing flows
>>>>
>>>>       __xfrm_lookup() is called for each packet transmitted out of
>>>>       system. The xfrm_find_bundle() does a linear search which can
>>>>       kill system performance depending on how many bundles are
>>>>       required per policy.
>>>>
>>>>       This modifies __xfrm_lookup() to store bundles directly in
>>>>       the flow cache. If we did not get a hit, we just create a new
>>>>       bundle instead of doing slow search. This means that we can now
>>>>       get multiple xfrm_dst's for same flow (on per-cpu basis).
>>>>
>>>>       Signed-off-by: Timo Teras<timo.teras@iki.fi>
>>>>       Signed-off-by: David S. Miller<davem@davemloft.net>
>>>>
>>>> :040000 040000 d8e60f5fa4c1329f450d9c7cdf98b34e6a177f22
>>>> 9f576e68e5bf4ce357d7f0305aee5f410250dfe2 M      include
>>>> :040000 040000 f2876df688ee36907af7b4123eea96592faaed3e
>>>> a3f6f6f94f0309106856cd99b38ec90b024eb016 M      net
>>>>        
>>> Thanks a lot for bisecting Jutin, this is really appreciated.
>>>
>>> crash is in xfrm_bundle_ok()
>>>
>>> if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
>>>     return 0;
>>>
>>> xdst->pols[0] contains a NULL pointer
>>>      
>> That does not really make sense, if we get this far; there's a valid
>> xfrm_state with the bundle. This means that there existed a policy with
>> it too.
>>
>> I'll take a deeper look at this tomorrow. Would it be possible to see
>> your xfrm policies?
>>
>> - Timo
>>
>>    
> 
> @Eric sure no problem doing the bisect...
> 
> as for the xfrm policy here is the link that I used to setup ipsec:
> http://www.linuxfromscratch.org/hints/downloads/files/ipsec.txt
> (just change the keys if doing real world work..(but for me just testing).
> 
> below is a temporary fix for me to get this working, tcpdump reports
> everything is doing what it should be
> 11:16:32.496166 IP xxxxx > xxxxx: AH(spi=0x00000200,seq=0x1090):
> ESP(spi=0x00000201,seq=0x1090), length 56
> 11:16:32.496212 IP xxxxx > xxxxx: AH(spi=0x00000200,seq=0x1091):
> ESP(spi=0x00000201,seq=0x1091), length 56
> 11:16:32.496259 IP xxxxx > xxxxx: AH(spi=0x00000200,seq=0x1092):
> ESP(spi=0x00000201,seq=0x1092), length 56
> 
> (tested a few mins ago, but not the right fix..)

Yes, that would break some other obscure scenarios.

Looks like it's ah inside esp. So you get chain of bundles. And only the
first bundle gets a policy. Should have thought of that. Does the below
fix it for you?

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 4bf27d9..af1c173 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2300,7 +2300,8 @@ int xfrm_bundle_ok(struct xfrm_policy *pol, struct
xfrm_dst *first,
 			return 0;
 		if (xdst->xfrm_genid != dst->xfrm->genid)
 			return 0;
-		if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
+		if (xdst->num_pols > 0 &&
+		    xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
 			return 0;

 		if (strict && fl &&

^ permalink raw reply related

* Re: [PATCH 1/2] syncookies: do not store rcv_wscale in tcp timestamp
From: Hagen Paul Pfeifer @ 2010-06-23 20:28 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev, Ilpo Järvinen
In-Reply-To: <1277156925-7295-1-git-send-email-fw@strlen.de>

* Florian Westphal | 2010-06-21 23:48:44 [+0200]:

>As pointed out by Fernando Gont there is no need to encode rcv_wscale
>into the cookie.
>
>We did not use the restored rcv_wscale anyway; it is recomputed
>via tcp_select_initial_window().

I speculate that this behavior was and is not correct. I suppose that their is
a race between the SYN/ACK where we initial force a particular window scale
and the next time where we recalculate the window via tcp_select_initial_window().

If the user change net.core.rmem_max or net.ipv4.tcp_rmem in between this
time, the recalculated window scale (rcv_wscale) can be smaller. But the
receiver still operates with the initial window scale and can overshot the
granted window - and bang.

There are several solutions: encode rcv_wscale into the syn cookie and don't
recalculate or disable window scaling and don't transmit any scaling option
when SYN cookies are active.

HGN

^ permalink raw reply

* Re: [PATCH 1/2] net - IP_NODEFRAG option for IPv4 socket
From: David Miller @ 2010-06-23 19:31 UTC (permalink / raw)
  To: jolsa; +Cc: eric.dumazet, jengelh, kaber, netdev, netfilter-devel, linux-man
In-Reply-To: <1276600052-16499-2-git-send-email-jolsa@redhat.com>

From: Jiri Olsa <jolsa@redhat.com>
Date: Tue, 15 Jun 2010 13:07:31 +0200

> this patch is implementing IP_NODEFRAG option for IPv4 socket.
> The reason is, there's no other way to send out the packet with user
> customized header of the reassembly part.
> 
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>

Applied to net-next-2.6, thanks!

^ permalink raw reply

* Re: [PATCH net-2.6] cnic: Disable statistics initialization for eth clients that do not support statistics
From: David Miller @ 2010-06-23 19:22 UTC (permalink / raw)
  To: dmitry; +Cc: netdev, mchan, eilong
In-Reply-To: <1277320179.5984.13.camel@lb-tlvb-dmitry>

From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Wed, 23 Jun 2010 22:09:39 +0300

> Disable statistics initialization for eth clients that do not support
>  statistics. This prevents memory corruption on bnx2x hw.
> 
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>

Applied, thank you.

^ permalink raw reply

* [PATCH net-2.6] cnic: Disable statistics initialization for eth clients that do not support statistics
From: Dmitry Kravkov @ 2010-06-23 19:09 UTC (permalink / raw)
  To: davem, netdev; +Cc: mchan, eilong

Disable statistics initialization for eth clients that do not support
 statistics. This prevents memory corruption on bnx2x hw.

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/cnic.c |   55 ++++++++++++++++++++++++++++++++-------------------
 1 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index fe92566..b314407 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -3919,8 +3919,9 @@ static void cnic_init_bnx2x_tx_ring(struct cnic_dev *dev)
 		HC_INDEX_DEF_C_ETH_ISCSI_CQ_CONS;
 	context->cstorm_st_context.status_block_id = BNX2X_DEF_SB_ID;
 
-	context->xstorm_st_context.statistics_data = (cli |
-				XSTORM_ETH_ST_CONTEXT_STATISTICS_ENABLE);
+	if (cli < MAX_X_STAT_COUNTER_ID)
+		context->xstorm_st_context.statistics_data = cli |
+				XSTORM_ETH_ST_CONTEXT_STATISTICS_ENABLE;
 
 	context->xstorm_ag_context.cdu_reserved =
 		CDU_RSRVD_VALUE_TYPE_A(BNX2X_HW_CID(BNX2X_ISCSI_L2_CID, func),
@@ -3928,10 +3929,12 @@ static void cnic_init_bnx2x_tx_ring(struct cnic_dev *dev)
 					ETH_CONNECTION_TYPE);
 
 	/* reset xstorm per client statistics */
-	val = BAR_XSTRORM_INTMEM +
-	      XSTORM_PER_COUNTER_ID_STATS_OFFSET(port, cli);
-	for (i = 0; i < sizeof(struct xstorm_per_client_stats) / 4; i++)
-		CNIC_WR(dev, val + i * 4, 0);
+	if (cli < MAX_X_STAT_COUNTER_ID) {
+		val = BAR_XSTRORM_INTMEM +
+		      XSTORM_PER_COUNTER_ID_STATS_OFFSET(port, cli);
+		for (i = 0; i < sizeof(struct xstorm_per_client_stats) / 4; i++)
+			CNIC_WR(dev, val + i * 4, 0);
+	}
 
 	cp->tx_cons_ptr =
 		&cp->bnx2x_def_status_blk->c_def_status_block.index_values[
@@ -3978,9 +3981,11 @@ static void cnic_init_bnx2x_rx_ring(struct cnic_dev *dev)
 						BNX2X_ISCSI_RX_SB_INDEX_NUM;
 	context->ustorm_st_context.common.clientId = cli;
 	context->ustorm_st_context.common.status_block_id = BNX2X_DEF_SB_ID;
-	context->ustorm_st_context.common.flags =
-		USTORM_ETH_ST_CONTEXT_CONFIG_ENABLE_STATISTICS;
-	context->ustorm_st_context.common.statistics_counter_id = cli;
+	if (cli < MAX_U_STAT_COUNTER_ID) {
+		context->ustorm_st_context.common.flags =
+			USTORM_ETH_ST_CONTEXT_CONFIG_ENABLE_STATISTICS;
+		context->ustorm_st_context.common.statistics_counter_id = cli;
+	}
 	context->ustorm_st_context.common.mc_alignment_log_size = 0;
 	context->ustorm_st_context.common.bd_buff_size =
 						cp->l2_single_buf_size;
@@ -4011,10 +4016,13 @@ static void cnic_init_bnx2x_rx_ring(struct cnic_dev *dev)
 
 	/* client tstorm info */
 	tstorm_client.mtu = cp->l2_single_buf_size - 14;
-	tstorm_client.config_flags =
-			(TSTORM_ETH_CLIENT_CONFIG_E1HOV_REM_ENABLE |
-			TSTORM_ETH_CLIENT_CONFIG_STATSITICS_ENABLE);
-	tstorm_client.statistics_counter_id = cli;
+	tstorm_client.config_flags = TSTORM_ETH_CLIENT_CONFIG_E1HOV_REM_ENABLE;
+
+	if (cli < MAX_T_STAT_COUNTER_ID) {
+		tstorm_client.config_flags |=
+				TSTORM_ETH_CLIENT_CONFIG_STATSITICS_ENABLE;
+		tstorm_client.statistics_counter_id = cli;
+	}
 
 	CNIC_WR(dev, BAR_TSTRORM_INTMEM +
 		   TSTORM_CLIENT_CONFIG_OFFSET(port, cli),
@@ -4024,16 +4032,21 @@ static void cnic_init_bnx2x_rx_ring(struct cnic_dev *dev)
 		   ((u32 *)&tstorm_client)[1]);
 
 	/* reset tstorm per client statistics */
-	val = BAR_TSTRORM_INTMEM +
-	      TSTORM_PER_COUNTER_ID_STATS_OFFSET(port, cli);
-	for (i = 0; i < sizeof(struct tstorm_per_client_stats) / 4; i++)
-		CNIC_WR(dev, val + i * 4, 0);
+	if (cli < MAX_T_STAT_COUNTER_ID) {
+
+		val = BAR_TSTRORM_INTMEM +
+		      TSTORM_PER_COUNTER_ID_STATS_OFFSET(port, cli);
+		for (i = 0; i < sizeof(struct tstorm_per_client_stats) / 4; i++)
+			CNIC_WR(dev, val + i * 4, 0);
+	}
 
 	/* reset ustorm per client statistics */
-	val = BAR_USTRORM_INTMEM +
-	      USTORM_PER_COUNTER_ID_STATS_OFFSET(port, cli);
-	for (i = 0; i < sizeof(struct ustorm_per_client_stats) / 4; i++)
-		CNIC_WR(dev, val + i * 4, 0);
+	if (cli < MAX_U_STAT_COUNTER_ID) {
+		val = BAR_USTRORM_INTMEM +
+		      USTORM_PER_COUNTER_ID_STATS_OFFSET(port, cli);
+		for (i = 0; i < sizeof(struct ustorm_per_client_stats) / 4; i++)
+			CNIC_WR(dev, val + i * 4, 0);
+	}
 
 	cp->rx_cons_ptr =
 		&cp->bnx2x_def_status_blk->u_def_status_block.index_values[
-- 
1.7.1





^ permalink raw reply related

* Re: [patch 00/11] s390: qeth patches for 2.6.36
From: David Miller @ 2010-06-23 19:08 UTC (permalink / raw)
  To: frank.blaschka; +Cc: netdev, linux-s390
In-Reply-To: <20100622085701.772508000@de.ibm.com>

From: frank.blaschka@de.ibm.com
Date: Tue, 22 Jun 2010 10:57:01 +0200

> Hi Dave,
> 
> this patch set contains qeth + 1 smsgiucv patches for 2.6.36 (net-next).

All applied to net-next-2.6, thanks!

^ permalink raw reply

* Re: [PATCH NEXT 0/8]qlcnic: driver update
From: David Miller @ 2010-06-23 19:04 UTC (permalink / raw)
  To: amit.salecha; +Cc: netdev, ameen.rahman
In-Reply-To: <1277212745-26857-1-git-send-email-amit.salecha@qlogic.com>

From: amit.salecha@qlogic.com
Date: Tue, 22 Jun 2010 06:18:57 -0700

>   Major changes:
>   1) Previously device context were created in probe and released in remove.
>      Now create during interface up and release in interface down.
>   2) Dont deallocate and allocate host memory during device reset.
>   3) Offload tx timeout recovery to fw recovery functions. Later will
>      check fw health before doing tx timeout recovery.
> 
>   Series of 8 patches. Please apply them on net-next branch.

All applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next-2.6] netfilter: allow nf_tproxy_core module to be removed
From: David Miller @ 2010-06-23 18:55 UTC (permalink / raw)
  To: fw; +Cc: jpirko, netdev, kaber
In-Reply-To: <20100623184611.GG27132@Chamillionaire.breakpoint.cc>

From: Florian Westphal <fw@strlen.de>
Date: Wed, 23 Jun 2010 20:46:11 +0200

> tproxy assigns skb->destructor, what prevents module unload while such skbs may
> still be around?

The only reference to nf_tproxy_core.ko is for the symbol, "nf_tproxy_assign_sock".
xt_TPROXY.c, which references this symbol, thus creates a symbol dependency on this
module, so xt_TPROXY.o needs to unload before nf_tproxy_core.ko can unload, and
xt_TPROXY.o has it's own manner for handling module references properly.

^ permalink raw reply

* Re: [PATCH] net: add dependency on fw class module
From: David Miller @ 2010-06-23 18:52 UTC (permalink / raw)
  To: amit.salecha; +Cc: netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <1277276075-29322-1-git-send-email-amit.salecha@qlogic.com>

From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Tue, 22 Jun 2010 23:54:35 -0700

> From: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
> 
> netxen_nic and qlcnic driver depends on firmware_class module.
> 
> Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH] snmp: fix SNMP_ADD_STATS()
From: David Miller @ 2010-06-23 18:48 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, therbert
In-Reply-To: <1277289123.2469.278.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 23 Jun 2010 12:32:03 +0200

> commit aa2ea0586d9d (tcp: fix outsegs stat for TSO segments) incorrectly
> assumed SNMP_ADD_STATS() was used from BH context.
> 
> Fix this using mib[!in_softirq()] instead of mib[0]
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks Eric.

^ permalink raw reply

* Re: [PATCH net-next-2.6] netfilter: allow nf_tproxy_core module to be removed
From: Florian Westphal @ 2010-06-23 18:46 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, kaber
In-Reply-To: <20100623183503.GE2687@psychotron.bos.redhat.com>

Jiri Pirko <jpirko@redhat.com> wrote:
> diff --git a/net/netfilter/nf_tproxy_core.c b/net/netfilter/nf_tproxy_core.c
> index 5490fc3..ec6737c 100644
> --- a/net/netfilter/nf_tproxy_core.c
> +++ b/net/netfilter/nf_tproxy_core.c
> @@ -89,7 +89,12 @@ static int __init nf_tproxy_init(void)
>  	return 0;
>  }
>  
> +static void __exit nf_tproxy_exit(void)
> +{
> +}
> +
>  module_init(nf_tproxy_init);
> +module_exit(nf_tproxy_exit);

tproxy assigns skb->destructor, what prevents module unload while such skbs may
still be around?

^ permalink raw reply

* [PATCH net-next-2.6] netfilter: allow nf_tproxy_core module to be removed
From: Jiri Pirko @ 2010-06-23 18:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, kaber


Signed-off-by: Jiri Pirko <jpirko@redhat.com>

diff --git a/net/netfilter/nf_tproxy_core.c b/net/netfilter/nf_tproxy_core.c
index 5490fc3..ec6737c 100644
--- a/net/netfilter/nf_tproxy_core.c
+++ b/net/netfilter/nf_tproxy_core.c
@@ -89,7 +89,12 @@ static int __init nf_tproxy_init(void)
 	return 0;
 }
 
+static void __exit nf_tproxy_exit(void)
+{
+}
+
 module_init(nf_tproxy_init);
+module_exit(nf_tproxy_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Krisztian Kovacs");

^ permalink raw reply related

* Re: [PATCH] gainfar.c : skb_over_panic
From: David Miller @ 2010-06-23 18:27 UTC (permalink / raw)
  To: liberty; +Cc: galak, netdev
In-Reply-To: <4C2249E6.4030202@extricom.com>

From: Eran Liberty <liberty@extricom.com>
Date: Wed, 23 Jun 2010 20:52:38 +0300

> David Miller wrote:
>> At this stage in the code we know exactly what modifications, if any,
>> we've made to the SKB state.  Therefore it makes sense to only fix
>> up the tiny amount of changes we've made instead of doing a complete
>> skb_recycle_call() which seems entirely excessive in this situation.
>>
>>   
> Agreed.
> 
> Attached is the corrected patch for the gianfar.c

Can you formally submit this with a proper commit message and
"Signed-off-by: " tag as per linux/Documentation/SubmittingPatches?

THanks.

^ permalink raw reply

* Re: BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
From: Justin P. Mattock @ 2010-06-23 18:20 UTC (permalink / raw)
  To: Timo Teräs
  Cc: Eric Dumazet, John W.Linville, netdev, Linux Kernel Mailing List,
	davem
In-Reply-To: <4C224E06.40806@iki.fi>

On 06/23/10 11:10, Timo Teräs wrote:
> On 06/23/2010 08:29 PM, Eric Dumazet wrote:
>    
>> Le mercredi 23 juin 2010 à 10:00 -0700, Justin P. Mattock a écrit :
>>      
>>> o.k. the bisect is pointing to the below results..
>>> (I tried git revert xxx but this commit is too big
>>> so I'll(hopefully)manually revert it on the latest HEAD to
>>> see if this is the actual problem im experiencing)
>>>
>>> 80c802f3073e84c956846e921e8a0b02dfa3755f is the first bad commit
>>> commit 80c802f3073e84c956846e921e8a0b02dfa3755f
>>> Author: Timo Teräs<timo.teras@iki.fi>
>>> Date:   Wed Apr 7 00:30:05 2010 +0000
>>>
>>>       xfrm: cache bundles instead of policies for outgoing flows
>>>
>>>       __xfrm_lookup() is called for each packet transmitted out of
>>>       system. The xfrm_find_bundle() does a linear search which can
>>>       kill system performance depending on how many bundles are
>>>       required per policy.
>>>
>>>       This modifies __xfrm_lookup() to store bundles directly in
>>>       the flow cache. If we did not get a hit, we just create a new
>>>       bundle instead of doing slow search. This means that we can now
>>>       get multiple xfrm_dst's for same flow (on per-cpu basis).
>>>
>>>       Signed-off-by: Timo Teras<timo.teras@iki.fi>
>>>       Signed-off-by: David S. Miller<davem@davemloft.net>
>>>
>>> :040000 040000 d8e60f5fa4c1329f450d9c7cdf98b34e6a177f22
>>> 9f576e68e5bf4ce357d7f0305aee5f410250dfe2 M      include
>>> :040000 040000 f2876df688ee36907af7b4123eea96592faaed3e
>>> a3f6f6f94f0309106856cd99b38ec90b024eb016 M      net
>>>        
>> Thanks a lot for bisecting Jutin, this is really appreciated.
>>
>> crash is in xfrm_bundle_ok()
>>
>> if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
>> 	return 0;
>>
>> xdst->pols[0] contains a NULL pointer
>>      
> That does not really make sense, if we get this far; there's a valid
> xfrm_state with the bundle. This means that there existed a policy with
> it too.
>
> I'll take a deeper look at this tomorrow. Would it be possible to see
> your xfrm policies?
>
> - Timo
>
>    

@Eric sure no problem doing the bisect...

as for the xfrm policy here is the link that I used to setup ipsec:
http://www.linuxfromscratch.org/hints/downloads/files/ipsec.txt
(just change the keys if doing real world work..(but for me just testing).

below is a temporary fix for me to get this working, tcpdump reports 
everything is doing what it should be
11:16:32.496166 IP xxxxx > xxxxx: AH(spi=0x00000200,seq=0x1090): 
ESP(spi=0x00000201,seq=0x1090), length 56
11:16:32.496212 IP xxxxx > xxxxx: AH(spi=0x00000200,seq=0x1091): 
ESP(spi=0x00000201,seq=0x1091), length 56
11:16:32.496259 IP xxxxx > xxxxx: AH(spi=0x00000200,seq=0x1092): 
ESP(spi=0x00000201,seq=0x1092), length 56

(tested a few mins ago, but not the right fix..)

 From a280d9048c8f8f5756f9f0dcc608b4499645593e Mon Sep 17 00:00:00 2001
From: Justin P. Mattock <justinmattock@gmail.com>
Date: Wed, 23 Jun 2010 11:10:28 -0700
Subject: [PATCH] fix ipsec
  Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
  net/xfrm/xfrm_policy.c |    2 --
  1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 4bf27d9..701d69a 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2300,8 +2300,6 @@ int xfrm_bundle_ok(struct xfrm_policy *pol, struct 
xfrm_dst *first,
              return 0;
          if (xdst->xfrm_genid != dst->xfrm->genid)
              return 0;
-        if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
-            return 0;

          if (strict && fl &&
              !(dst->xfrm->outer_mode->flags & XFRM_MODE_FLAG_TUNNEL) &&
-- 
1.7.1.rc1.21.gf3bd6


I've got the machine ready for any patches.. let me know  o.k...

Justin P. Mattock

^ permalink raw reply related

* Re: BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
From: Timo Teräs @ 2010-06-23 18:10 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Justin P.Mattock, John W.Linville, netdev,
	Linux Kernel Mailing List, davem
In-Reply-To: <1277314167.2469.1144.camel@edumazet-laptop>

On 06/23/2010 08:29 PM, Eric Dumazet wrote:
> Le mercredi 23 juin 2010 à 10:00 -0700, Justin P. Mattock a écrit :
>> o.k. the bisect is pointing to the below results..
>> (I tried git revert xxx but this commit is too big
>> so I'll(hopefully)manually revert it on the latest HEAD to
>> see if this is the actual problem im experiencing)
>>
>> 80c802f3073e84c956846e921e8a0b02dfa3755f is the first bad commit
>> commit 80c802f3073e84c956846e921e8a0b02dfa3755f
>> Author: Timo Teräs <timo.teras@iki.fi>
>> Date:   Wed Apr 7 00:30:05 2010 +0000
>>
>>      xfrm: cache bundles instead of policies for outgoing flows
>>
>>      __xfrm_lookup() is called for each packet transmitted out of
>>      system. The xfrm_find_bundle() does a linear search which can
>>      kill system performance depending on how many bundles are
>>      required per policy.
>>
>>      This modifies __xfrm_lookup() to store bundles directly in
>>      the flow cache. If we did not get a hit, we just create a new
>>      bundle instead of doing slow search. This means that we can now
>>      get multiple xfrm_dst's for same flow (on per-cpu basis).
>>
>>      Signed-off-by: Timo Teras <timo.teras@iki.fi>
>>      Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>> :040000 040000 d8e60f5fa4c1329f450d9c7cdf98b34e6a177f22 
>> 9f576e68e5bf4ce357d7f0305aee5f410250dfe2 M      include
>> :040000 040000 f2876df688ee36907af7b4123eea96592faaed3e 
>> a3f6f6f94f0309106856cd99b38ec90b024eb016 M      net
> 
> Thanks a lot for bisecting Jutin, this is really appreciated.
> 
> crash is in xfrm_bundle_ok()
> 
> if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
> 	return 0;
> 
> xdst->pols[0] contains a NULL pointer

That does not really make sense, if we get this far; there's a valid
xfrm_state with the bundle. This means that there existed a policy with
it too.

I'll take a deeper look at this tomorrow. Would it be possible to see
your xfrm policies?

- Timo

^ permalink raw reply

* Re: [PATCH] gainfar.c : skb_over_panic
From: Eran Liberty @ 2010-06-23 17:52 UTC (permalink / raw)
  To: David Miller; +Cc: galak, netdev
In-Reply-To: <20100621.134733.115953029.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

David Miller wrote:
> At this stage in the code we know exactly what modifications, if any,
> we've made to the SKB state.  Therefore it makes sense to only fix
> up the tiny amount of changes we've made instead of doing a complete
> skb_recycle_call() which seems entirely excessive in this situation.
>
>   
Agreed.

Attached is the corrected patch for the gianfar.c

-- Liberty

[-- Attachment #2: gianfar_skb_over_panic.patch --]
[-- Type: text/x-diff, Size: 610 bytes --]

---
 drivers/net/gianfar.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1854,14 +1854,9 @@
 			if (unlikely(!newskb))
 				newskb = skb;
 			else if (skb) {
-				/*
-				 * We need to reset ->data to what it
-				 * was before gfar_new_skb() re-aligned
-				 * it to an RXBUF_ALIGNMENT boundary
-				 * before we put the skb back on the
-				 * recycle list.
-				 */
 				skb->data = skb->head + NET_SKB_PAD;
+				skb->len = 0;
+				skb_reset_tail_pointer(skb);
 				__skb_queue_head(&priv->rx_recycle, skb);
 			}
 		} else {

^ permalink raw reply

* Re: BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
From: Eric Dumazet @ 2010-06-23 17:29 UTC (permalink / raw)
  To: Justin P.Mattock
  Cc: John W.Linville, netdev, Linux Kernel Mailing List, davem,
	timo.teras
In-Reply-To: <4C223DCA.5090704@gmail.com>

Le mercredi 23 juin 2010 à 10:00 -0700, Justin P. Mattock a écrit :
> o.k. the bisect is pointing to the below results..
> (I tried git revert xxx but this commit is too big
> so I'll(hopefully)manually revert it on the latest HEAD to
> see if this is the actual problem im experiencing)
> 
> 
> 
> 80c802f3073e84c956846e921e8a0b02dfa3755f is the first bad commit
> commit 80c802f3073e84c956846e921e8a0b02dfa3755f
> Author: Timo Teräs <timo.teras@iki.fi>
> Date:   Wed Apr 7 00:30:05 2010 +0000
> 
>      xfrm: cache bundles instead of policies for outgoing flows
> 
>      __xfrm_lookup() is called for each packet transmitted out of
>      system. The xfrm_find_bundle() does a linear search which can
>      kill system performance depending on how many bundles are
>      required per policy.
> 
>      This modifies __xfrm_lookup() to store bundles directly in
>      the flow cache. If we did not get a hit, we just create a new
>      bundle instead of doing slow search. This means that we can now
>      get multiple xfrm_dst's for same flow (on per-cpu basis).
> 
>      Signed-off-by: Timo Teras <timo.teras@iki.fi>
>      Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> :040000 040000 d8e60f5fa4c1329f450d9c7cdf98b34e6a177f22 
> 9f576e68e5bf4ce357d7f0305aee5f410250dfe2 M      include
> :040000 040000 f2876df688ee36907af7b4123eea96592faaed3e 
> a3f6f6f94f0309106856cd99b38ec90b024eb016 M      net
> 
> 

Thanks a lot for bisecting Jutin, this is really appreciated.

crash is in xfrm_bundle_ok()

if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
	return 0;

xdst->pols[0] contains a NULL pointer

^ permalink raw reply

* Re: BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
From: Justin P. Mattock @ 2010-06-23 17:00 UTC (permalink / raw)
  To: John W. Linville; +Cc: netdev, Linux Kernel Mailing List, davem, timo.teras
In-Reply-To: <20100623141622.GC15205@tuxdriver.com>

o.k. the bisect is pointing to the below results..
(I tried git revert xxx but this commit is too big
so I'll(hopefully)manually revert it on the latest HEAD to
see if this is the actual problem im experiencing)



80c802f3073e84c956846e921e8a0b02dfa3755f is the first bad commit
commit 80c802f3073e84c956846e921e8a0b02dfa3755f
Author: Timo Teräs <timo.teras@iki.fi>
Date:   Wed Apr 7 00:30:05 2010 +0000

     xfrm: cache bundles instead of policies for outgoing flows

     __xfrm_lookup() is called for each packet transmitted out of
     system. The xfrm_find_bundle() does a linear search which can
     kill system performance depending on how many bundles are
     required per policy.

     This modifies __xfrm_lookup() to store bundles directly in
     the flow cache. If we did not get a hit, we just create a new
     bundle instead of doing slow search. This means that we can now
     get multiple xfrm_dst's for same flow (on per-cpu basis).

     Signed-off-by: Timo Teras <timo.teras@iki.fi>
     Signed-off-by: David S. Miller <davem@davemloft.net>

:040000 040000 d8e60f5fa4c1329f450d9c7cdf98b34e6a177f22 
9f576e68e5bf4ce357d7f0305aee5f410250dfe2 M      include
:040000 040000 f2876df688ee36907af7b4123eea96592faaed3e 
a3f6f6f94f0309106856cd99b38ec90b024eb016 M      net



Justin P. Mattock

^ permalink raw reply

* Re: [PATCH] sky2: enable rx/tx in sky2_phy_reinit()
From: Brandon Philips @ 2010-06-23 17:00 UTC (permalink / raw)
  To: Mike McCormack; +Cc: Stephen Hemminger, netdev, davem
In-Reply-To: <AANLkTilzHA_URoZONtMf6A9-IRV4wA2ncr0b8Jff1ge9@mail.gmail.com>

On 23:57 Tue 22 Jun 2010, Mike McCormack wrote:
> Tested, and verified that it fixes the bug reported.

Thanks for testing Mike. 

While testing this I found a new unrelated bug. If I turn off speed
autoneg the interface stops sending packets until I turn off pause
autoneg also.

To illustrate start pinging some machine and run this:

 $ ethtool --change eth0 speed 100 duplex full autoneg off
 # At this point the ping stops

 $ ethtool -A eth0 autoneg off
 # Ping starts up again

When I tried capturing packets nothing was coming from the device.

0ea065e52eb6a0f029b5fa5ed2f142be1b66a153 implemeneted the behaviour of
having seperate pause and speed ethtool controls for sky2. Reverting
this (see quick ugly revert below) obviously fixes the issue.

Any ideas on how to fix this in a proper way though?

Cheers,

	Brandon

>From 8de50faa1911933dc545f663a24f32d0caeea3b4 Mon Sep 17 00:00:00 2001
From: Brandon Philips <brandon@ifup.org>
Date: Wed, 23 Jun 2010 09:56:20 -0700
Subject: [PATCH] sky2: revert "fix pause negotiation"

Revert 0ea065e52eb6a0f029b5fa5ed2f142be1b66a153
---
 drivers/net/sky2.c |   31 ++++++++++++-------------------
 drivers/net/sky2.h |    2 ++
 2 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 7985165..a638171 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -333,7 +333,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 	struct sky2_port *sky2 = netdev_priv(hw->dev[port]);
 	u16 ctrl, ct1000, adv, pg, ledctrl, ledover, reg;
 
-	if ( (sky2->flags & SKY2_FLAG_AUTO_SPEED) &&
+	if (sky2->autoneg == AUTONEG_ENABLE &&
 	    !(hw->flags & SKY2_HW_NEWER_PHY)) {
 		u16 ectrl = gm_phy_read(hw, port, PHY_MARV_EXT_CTRL);
 
@@ -375,7 +375,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 			ctrl |= PHY_M_PC_MDI_XMODE(PHY_M_PC_ENA_AUTO);
 
 			/* downshift on PHY 88E1112 and 88E1149 is changed */
-			if ( (sky2->flags & SKY2_FLAG_AUTO_SPEED) &&
+			if (sky2->autoneg == AUTONEG_ENABLE &&
 			     (hw->flags & SKY2_HW_NEWER_PHY)) {
 				/* set downshift counter to 3x and enable downshift */
 				ctrl &= ~PHY_M_PC_DSC_MSK;
@@ -420,7 +420,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 	adv = PHY_AN_CSMA;
 	reg = 0;
 
-	if (sky2->flags & SKY2_FLAG_AUTO_SPEED) {
+	if (sky2->autoneg == AUTONEG_ENABLE) {
 		if (sky2_is_copper(hw)) {
 			if (sky2->advertising & ADVERTISED_1000baseT_Full)
 				ct1000 |= PHY_M_1000C_AFD;
@@ -435,11 +435,14 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 			if (sky2->advertising & ADVERTISED_10baseT_Half)
 				adv |= PHY_M_AN_10_HD;
 
+			adv |= copper_fc_adv[sky2->flow_mode];
 		} else {	/* special defines for FIBER (88E1040S only) */
 			if (sky2->advertising & ADVERTISED_1000baseT_Full)
 				adv |= PHY_M_AN_1000X_AFD;
 			if (sky2->advertising & ADVERTISED_1000baseT_Half)
 				adv |= PHY_M_AN_1000X_AHD;
+
+			adv |= fiber_fc_adv[sky2->flow_mode];
 		}
 
 		/* Restart Auto-negotiation */
@@ -448,8 +451,8 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 		/* forced speed/duplex settings */
 		ct1000 = PHY_M_1000C_MSE;
 
-		/* Disable auto update for duplex flow control and duplex */
-		reg |= GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_SPD_DIS;
+		/* Disable auto update for duplex flow control and speed */
+		reg |= GM_GPCR_AU_ALL_DIS;
 
 		switch (sky2->speed) {
 		case SPEED_1000:
@@ -467,15 +470,8 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 			ctrl |= PHY_CT_DUP_MD;
 		} else if (sky2->speed < SPEED_1000)
 			sky2->flow_mode = FC_NONE;
-	}
 
-	if (sky2->flags & SKY2_FLAG_AUTO_PAUSE) {
-		if (sky2_is_copper(hw))
-			adv |= copper_fc_adv[sky2->flow_mode];
-		else
-			adv |= fiber_fc_adv[sky2->flow_mode];
-	} else {
-		reg |= GM_GPCR_AU_FCT_DIS;
+
  		reg |= gm_fc_disable[sky2->flow_mode];
 
 		/* Forward pause packets to GMAC? */
@@ -620,8 +616,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 		/* no effect on Yukon-XL */
 		gm_phy_write(hw, port, PHY_MARV_LED_CTRL, ledctrl);
 
-		if (!(sky2->flags & SKY2_FLAG_AUTO_SPEED) ||
-		    sky2->speed == SPEED_100) {
+		if (sky2->autoneg == AUTONEG_DISABLE || sky2->speed == SPEED_100) {
 			/* turn on 100 Mbps LED (LED_LINK100) */
 			ledover |= PHY_M_LED_MO_100(MO_LED_ON);
 		}
@@ -632,7 +627,7 @@ static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 	}
 
 	/* Enable phy interrupt on auto-negotiation complete (or link up) */
-	if (sky2->flags & SKY2_FLAG_AUTO_SPEED)
+	if (sky2->autoneg == AUTONEG_ENABLE)
 		gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_IS_AN_COMPL);
 	else
 		gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_DEF_MSK);
@@ -688,9 +683,7 @@ static void sky2_phy_power_down(struct sky2_hw *hw, unsigned port)
 
 	/* setup General Purpose Control Register */
 	gma_write16(hw, port, GM_GP_CTRL,
-		    GM_GPCR_FL_PASS | GM_GPCR_SPEED_100 |
-		    GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_FCT_DIS |
-		    GM_GPCR_AU_SPD_DIS);
+		    GM_GPCR_FL_PASS | GM_GPCR_SPEED_100 | GM_GPCR_AU_ALL_DIS);
 
 	if (hw->chip_id != CHIP_ID_YUKON_EC) {
 		if (hw->chip_id == CHIP_ID_YUKON_EC_U) {
diff --git a/drivers/net/sky2.h b/drivers/net/sky2.h
index 084eff2..db0b2ad 100644
--- a/drivers/net/sky2.h
+++ b/drivers/net/sky2.h
@@ -1755,6 +1755,7 @@ enum {
 };
 
 #define GM_GPCR_SPEED_1000	(GM_GPCR_GIGS_ENA | GM_GPCR_SPEED_100)
+#define GM_GPCR_AU_ALL_DIS	(GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_FCT_DIS|GM_GPCR_AU_SPD_DIS)
 
 /*	GM_TX_CTRL			16 bit r/w	Transmit Control Register */
 enum {
@@ -2247,6 +2248,7 @@ struct sky2_port {
 	u16		     speed;		/* SPEED_1000, SPEED_100, ... */
 	u8		     wol;		/* WAKE_ bits */
 	u8		     duplex;		/* DUPLEX_HALF, DUPLEX_FULL */
+	u8		     autoneg;	/* AUTONEG_ENABLE, AUTONEG_DISABLE */
 	u16		     flags;
 #define SKY2_FLAG_RX_CHECKSUM		0x0001
 #define SKY2_FLAG_AUTO_SPEED		0x0002
-- 
1.7.1


^ permalink raw reply related

* Re: Question about xfrm by MARK feature
From: Patrick McHardy @ 2010-06-23 16:15 UTC (permalink / raw)
  To: Gerd v. Egidy; +Cc: jamal, timo.teras, herbert, netdev
In-Reply-To: <201006231803.17261.lists@egidy.de>

Gerd v. Egidy wrote:
> Hi Jamal,
>
> while looking through the 2.6.34 changelog I found the xfrm by MARK feature 
> you developed in february. I'm currently working on NAT for ipsec connections 
> and thought your feature might help me.
>
> For example I have 2 different remote networks with the same ip network each 
> and both of them have a tunnel to the same local network. I map their IPs to 
> something different so I can distinguish them in the local network. But after 
> the nat the xfrm code sees two tunnels with exactly the same values. So this 
> can't work.
>
> But if I understood your feature correctly, I can now mark the packets (e.g. 
> in iptables with ... -j MARK --set-mark 1) and have xfrm select the correct 
> ipsec tunnel via the mark. Correct?
>
> But does your feature also set the mark on packets decrypted by xfrm? I need 
> some way to find out from which tunnel the packet came to correctly treat it. 
>   

You should be able to use the policy match to distinguish the tunnels,
f.i. by matching on the tunnel endpoints.


^ permalink raw reply

* Re: SO_REUSEPORT
From: Brian Bloniarz @ 2010-06-23 16:09 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: linux-kernel, netdev
In-Reply-To: <06r8f7-jsn.ln1@chipmunk.wormnet.eu>

On 06/23/2010 03:54 AM, Alexander Clouter wrote:
> Hi,
> 
> Tim Prepscius <timprepscius@gmail.com> wrote:
>>
>> Is SO_REUSEPORT available 2.6.ish - (or any version)?
>> I've been searching for a conclusive answer to this question and can't
>> find it.
>>
> That will be a no then:
> ----
> alex@berk:~$ grep SO_REUSEPORT -r /usr/src/linux-2.6-stable/include/
> /usr/src/linux-2.6-stable/include/asm-generic/socket.h:/* To add :#define SO_REUSEPORT 15 */
> ----
> 
>> (yes I know of SO_REUSEADDR, and I know the difference between this
>> and *PORT, and yes I know that I *definitely* need SO_REUSEPORT, no,
>> I'm unconcerned this may/may not be part of a "standard," yes I know
>> it is implemented differently on different systems, yes I know there
>> may be security problems, but no, I don't care about this.)
>>
> This really sounds like the sort of thing (for TCP/SCTP) where the 
> 'master' process would maintain the listening socket and upon accept() 
> you would fork() or pass the file descriptor off to a thread.  This 
> would make SO_REUSEPORT un-necessary and also your code would be 
> portable.
> 
> If you are doing things with UDP (or another datagram-esque stream) then 
> your master listener could pass off the incoming packets to 
> threads/processes trivially.
> 
> Of course this depends on what you are doing, but my opinion is that the 
> functionality has been unneeded so far by people in the kernel, so *I* 
> must be doing something wrong ;)

Tom Herbert gave a pretty great description of why you
might want this functionality in his original patch submission:

http://kerneltrap.org/mailarchive/linux-netdev/2010/4/19/6274993

If you follow that thread though, there wasn't consensus about
the best architecture & API for it, and nothing has made it
yet.

I'm adding netdev to the cc, AFAIK it's the place for discussing
stuff like this.

^ permalink raw reply

* Question about xfrm by MARK feature
From: Gerd v. Egidy @ 2010-06-23 16:03 UTC (permalink / raw)
  To: jamal; +Cc: timo.teras, kaber, herbert, netdev

Hi Jamal,

while looking through the 2.6.34 changelog I found the xfrm by MARK feature 
you developed in february. I'm currently working on NAT for ipsec connections 
and thought your feature might help me.

For example I have 2 different remote networks with the same ip network each 
and both of them have a tunnel to the same local network. I map their IPs to 
something different so I can distinguish them in the local network. But after 
the nat the xfrm code sees two tunnels with exactly the same values. So this 
can't work.

But if I understood your feature correctly, I can now mark the packets (e.g. 
in iptables with ... -j MARK --set-mark 1) and have xfrm select the correct 
ipsec tunnel via the mark. Correct?

But does your feature also set the mark on packets decrypted by xfrm? I need 
some way to find out from which tunnel the packet came to correctly treat it. 

Do you know if any of the ipsec solutions for linux (e.g. strongswan, 
openswan, racoon) already have support for this feature or are developing on 
it?

Kind regards,

Gerd

-- 
Address (better: trap) for people I really don't want to get mail from:
jonas@cactusamerica.com

^ permalink raw reply

* Re: [PATCH 31/40] trace syscalls: Convert various generic compat syscalls
From: H. Peter Anvin @ 2010-06-23 15:51 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Andi Kleen, Ian Munsie, linux-kernel, linuxppc-dev, Jason Baron,
	Steven Rostedt, Ingo Molnar, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Alexander Viro, Andrew Morton,
	Jeff Moyer, David Howells, Oleg Nesterov, Arnd Bergmann,
	David S. Miller, Greg Kroah-Hartman, Dinakar Guniguntala,
	Thomas Gleixner, Ingo Molnar, Eric Biederman <ebie
In-Reply-To: <20100623113806.GD5242@nowhere>

On 06/23/2010 04:38 AM, Frederic Weisbecker wrote:
> 
> I haven't heard any complains about existing syscalls wrappers.
> 

Then you truly haven't been listening.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH] gainfar.c : skb_over_panic
From: Eran Liberty @ 2010-06-23 15:03 UTC (permalink / raw)
  To: David Miller; +Cc: galak, netdev
In-Reply-To: <20100617.122030.112600189.davem@davemloft.net>

David Miller wrote:
> From: Eran Liberty <liberty@extricom.com>
> Date: Thu, 17 Jun 2010 19:32:54 +0300
>
>   
>> I have demonstrated skb_over_panic with linux 2.6.32.15 on a mpc8548
>> based product.
>>     
>
> A fix for a similar bug was necessary for the ucc_geth driver,
> see below.
>
> The real problem is that skb->data assignment, the rest of the
> SKB state has to be reset, and not doing that is what results in
> the skb_over_panic calls.
>
> >From db176edc89abbf22e6db6853f8581f9475fe8ec1 Mon Sep 17 00:00:00 2001
> From: Sergey Matyukevich <geomatsi@gmail.com>
> Date: Mon, 14 Jun 2010 06:35:20 +0000
> Subject: [PATCH] ucc_geth: fix for RX skb buffers recycling
>
> This patch implements a proper modification of RX skb buffers before
> recycling. Adjusting only skb->data is not enough because after that
> skb->tail and skb->len become incorrect.
>
> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  drivers/net/ucc_geth.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index 4a34833..807470e 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -3215,6 +3215,8 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
>  					   __func__, __LINE__, (u32) skb);
>  			if (skb) {
>  				skb->data = skb->head + NET_SKB_PAD;
> +				skb->len = 0;
> +				skb_reset_tail_pointer(skb);
>  				__skb_queue_head(&ugeth->rx_recycle, skb);
>  			}
>  
>   
When I do go via this code this patch helps. But, I have managed to
reach the skb_over_panic without going first via __skb_queue_head()
which render this patch useless... So I am investigating this before
suggesting any patch.

doing something like this:
                if (unlikely(skb->tail + pkt_len > skb->end)) {
                    pr_err("gfar_clean_rx_ring():  skb_over_panic event avoided\n");
                    dev_kfree_skb_any(skb);
                } else {
                    skb_put(skb, pkt_len);
                    dev->stats.rx_bytes += pkt_len;

                    if (in_irq() || irqs_disabled())
                        printk("Interrupt problem!\n");
                    gfar_process_frame(dev, skb, amount_pull);
                }

successfully avoids the skb_over_panic(), But I rather find the offending skb creator then continuously defend against its arrival.

-- Liberty



^ permalink raw reply

* Re: [PATCH] ipv6: fix NULL reference in proxy neighbor discovery
From: YOSHIFUJI Hideaki @ 2010-06-23 14:44 UTC (permalink / raw)
  To: Stephen Hemminger, David Miller
  Cc: Andreas Klauer, Hagen Paul Pfeifer, netdev, Octavian Purdila,
	YOSHIFUJI Hideaki
In-Reply-To: <20100621140013.508741df@nehalam>

Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

(2010/06/22 6:00), Stephen Hemminger wrote:
> The addition of TLLAO option created a kernel OOPS regression
> for the case where neighbor advertisement is being sent via
> proxy path.  When using proxy, ipv6_get_ifaddr() returns NULL
> causing the NULL dereference.
> 
> Change causing the bug was:
> commit f7734fdf61ec6bb848e0bafc1fb8bad2c124bb50
> Author: Octavian Purdila<opurdila@ixiacom.com>
> Date:   Fri Oct 2 11:39:15 2009 +0000
> 
>      make TLLAO option for NA packets configurable
> 
> Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
> 
> ---
> Patch for -net and -stable.
> Applies to 2.6.33 and later.
> 
> --- a/net/ipv6/ndisc.c	2010-06-11 08:13:13.008657498 -0700
> +++ b/net/ipv6/ndisc.c	2010-06-21 13:52:57.961486303 -0700
> @@ -586,6 +586,7 @@ static void ndisc_send_na(struct net_dev
>   		src_addr = solicited_addr;
>   		if (ifp->flags&  IFA_F_OPTIMISTIC)
>   			override = 0;
> +		inc_opt |= ifp->idev->cnf.force_tllao;
>   		in6_ifa_put(ifp);
>   	} else {
>   		if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
> @@ -599,7 +600,6 @@ static void ndisc_send_na(struct net_dev
>   	icmp6h.icmp6_solicited = solicited;
>   	icmp6h.icmp6_override = override;
> 
> -	inc_opt |= ifp->idev->cnf.force_tllao;
>   	__ndisc_send(dev, neigh, daddr, src_addr,
>   		&icmp6h, solicited_addr,
>   		     inc_opt ? ND_OPT_TARGET_LL_ADDR : 0);
> --
> 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox