diff mbox

BUG: unable to handle kernel NULL pointer dereference at 000000000000002c

Message ID 4F2B963F.4020504@profihost.ag
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Stefan Priebe - Profihost AG Feb. 3, 2012, 8:09 a.m. UTC
Hi,

attached you find the patch files applying cleanly to 3.0.X.

>> I've made my own backport of the patch and removed at least 1-2
>> dependencies. Anybody interested?
>>
> 
> If you did the work, post it for review.

Thanks!

Stefan

Comments

Eric Dumazet Feb. 3, 2012, 11:04 a.m. UTC | #1
Le vendredi 03 février 2012 à 09:09 +0100, Stefan Priebe - Profihost AG
a écrit :
> Hi,
> 
> attached you find the patch files applying cleanly to 3.0.X.

First, thanks a lot for doing this work.

OK, lets try to provide a cumulative patch with a good Changelog ?

And please try a "allyesconfig" build, because it doesnt build right
now :

  CC [M]  net/decnet/dn_neigh.o
net/decnet/dn_route.c: In function 'dn_forward':
net/decnet/dn_route.c:757:31: error: 'struct dst_entry' has no member
named 'neighbour'
make[2]: *** [net/decnet/dn_route.o] Error 1
make[2]: *** Waiting for unfinished jobs....
net/decnet/dn_neigh.c: In function 'dn_long_output':
net/decnet/dn_neigh.c:230:31: error: 'struct dst_entry' has no member
named 'neighbour'
net/decnet/dn_neigh.c: In function 'dn_short_output':
net/decnet/dn_neigh.c:277:31: error: 'struct dst_entry' has no member
named 'neighbour'
net/decnet/dn_neigh.c: In function 'dn_phase3_output':
net/decnet/dn_neigh.c:321:31: error: 'struct dst_entry' has no member
named 'neighbour'
make[2]: *** [net/decnet/dn_neigh.o] Error 1
make[1]: *** [net/decnet] Error 2


So there is at least one patch missing ?

Once you believe its ok, please send a proper patch with changelog like
this :

[PATCH] net: hot fix for stable kernels

This cumulative patch fixes IP redirect NULL dereferences
added in commit f39925dbde77
(ipv4: Cache learned redirect information in inetpeer.)

Backport of [xx] upstream commits

9cbb7ecbcff85077bb12301aaf4c9b5a56c5993d
(ipv6: Get rid of rt6i_nexthop macro.)

69cce1d1404968f78b177a0314f5822d5afdbbfb
(net: Abstract dst->neighbour accesses behind helpers.) 
...

[Full list of xx commits with their ID in linux tree and their title,
to ease future maintenance]

Signed-off-by: [your email]
---
[diffstat -p1 -w70 of your patch]

[content of the patch]


--
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
Greg Kroah-Hartman Feb. 3, 2012, 3:53 p.m. UTC | #2
On Fri, Feb 03, 2012 at 12:04:57PM +0100, Eric Dumazet wrote:
> Le vendredi 03 février 2012 à 09:09 +0100, Stefan Priebe - Profihost AG
> a écrit :
> > Hi,
> > 
> > attached you find the patch files applying cleanly to 3.0.X.
> 
> First, thanks a lot for doing this work.
> 
> OK, lets try to provide a cumulative patch with a good Changelog ?

Individual patches are also ok, if they build properly :)

thanks,

greg k-h
--
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
Stefan Priebe - Profihost AG Feb. 6, 2012, 9:02 a.m. UTC | #3
Hi Eric,

> And please try a "allyesconfig" build, because it doesnt build right
> now :
> 
>   CC [M]  net/decnet/dn_neigh.o
> net/decnet/dn_route.c: In function 'dn_forward':
> net/decnet/dn_route.c:757:31: error: 'struct dst_entry' has no member
> named 'neighbour'
> make[2]: *** [net/decnet/dn_route.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> net/decnet/dn_neigh.c: In function 'dn_long_output':
> net/decnet/dn_neigh.c:230:31: error: 'struct dst_entry' has no member
> named 'neighbour'
> net/decnet/dn_neigh.c: In function 'dn_short_output':
> net/decnet/dn_neigh.c:277:31: error: 'struct dst_entry' has no member
> named 'neighbour'
> net/decnet/dn_neigh.c: In function 'dn_phase3_output':
> net/decnet/dn_neigh.c:321:31: error: 'struct dst_entry' has no member
> named 'neighbour'
> make[2]: *** [net/decnet/dn_neigh.o] Error 1
> make[1]: *** [net/decnet] Error 2
> 
> 
> So there is at least one patch missing ?

how to get an allyesconfig? Or just doing something like:
rm -f .config; /usr/bin/yes | make oldconfig

Stefan
--
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
Stefan Priebe - Profihost AG Feb. 6, 2012, 9:04 a.m. UTC | #4
Hi Eric, Hi David

today i've seen this:
[1048676.660457] ------------[ cut here ]------------
[1048676.688131] WARNING: at net/ipv4/tcp_input.c:2964
tcp_ack+0xfe1/0x2420()
[1048676.716291] Hardware name: X8SIL
[1048676.744292] Modules linked in: xt_tcpudp ipt_REJECT iptable_filter
ip_tables x_tables coretemp k8temp ipv6 dm_snapshot dm_mod
[1048676.802468] Pid: 0, comm: kworker/0:1 Not tainted 2.6.40.17.1intel #1
[1048676.831737] Call Trace:
[1048676.860455]  <IRQ>  [<ffffffff81565921>] ? tcp_ack+0xfe1/0x2420
[1048676.860765]  [<ffffffff81045e10>] warn_slowpath_common+0x80/0xc0
[1048676.860771]  [<ffffffff81045e65>] warn_slowpath_null+0x15/0x20
[1048676.860777]  [<ffffffff81565921>] tcp_ack+0xfe1/0x2420
[1048676.860784]  [<ffffffff81567060>] tcp_rcv_established+0x300/0x630
[1048676.860791]  [<ffffffff815708a4>] tcp_v4_do_rcv+0x154/0x2d0
[1048676.860796]  [<ffffffff8157111b>] tcp_v4_rcv+0x6fb/0x880
[1048676.860804]  [<ffffffff8154e4e7>] ip_local_deliver_finish+0x127/0x250
[1048676.860810]  [<ffffffff8154e69d>] ip_local_deliver+0x8d/0xa0
[1048676.860815]  [<ffffffff8154dda2>] ip_rcv_finish+0x172/0x340
[1048676.860820]  [<ffffffff8154e1e5>] ip_rcv+0x275/0x2f0
[1048676.860827]  [<ffffffff81523387>] __netif_receive_skb+0x427/0x4a0
[1048676.860832]  [<ffffffff81529148>] netif_receive_skb+0x78/0x80
[1048676.860837]  [<ffffffff81529280>] napi_skb_finish+0x50/0x70
[1048676.860842]  [<ffffffff81529735>] napi_gro_receive+0xc5/0xd0
[1048676.860851]  [<ffffffff81462786>] e1000_receive_skb+0x56/0x70
[1048676.860856]  [<ffffffff814646eb>] e1000_clean_rx_irq+0x22b/0x3d0
[1048676.860862]  [<ffffffff814630f2>] e1000_clean+0xb2/0x2f0
[1048676.860868]  [<ffffffff81054efc>] ? run_timer_softirq+0x3c/0x320
[1048676.860873]  [<ffffffff815298fa>] net_rx_action+0x10a/0x2b0
[1048676.860879]  [<ffffffff8104c300>] __do_softirq+0xd0/0x1c0
[1048676.860887]  [<ffffffff815eb20c>] call_softirq+0x1c/0x30
[1048676.860895]  [<ffffffff810047b5>] do_softirq+0x55/0x90
[1048676.860900]  [<ffffffff8104c0dd>] irq_exit+0xad/0xe0
[1048676.860905]  [<ffffffff81003f94>] do_IRQ+0x64/0xe0
[1048676.860910]  [<ffffffff815e9a93>] common_interrupt+0x13/0x13
[1048676.860913]  <EOI>  [<ffffffff8106bdff>] ?
notifier_call_chain+0x3f/0x80
[1048676.860926]  [<ffffffff813117b3>] ? intel_idle+0xb3/0x120
[1048676.860931]  [<ffffffff81311795>] ? intel_idle+0x95/0x120
[1048676.860937]  [<ffffffff814fc27c>] cpuidle_idle_call+0xdc/0x1a0
[1048676.860942]  [<ffffffff81002091>] cpu_idle+0xb1/0x110
[1048676.860948]  [<ffffffff81b0d7aa>] start_secondary+0x201/0x297
[1048676.860953] ---[ end trace 4d27234ace919a1b ]---

Any idea about that? Is it due to my custom patch being buggy or is it
anything you know which is missing in 3.0.X too?

Thanks!

Stefan
--
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
Eric Dumazet Feb. 6, 2012, 9:16 a.m. UTC | #5
Le lundi 06 février 2012 à 10:02 +0100, Stefan Priebe - Profihost AG a
écrit :

> how to get an allyesconfig? Or just doing something like:
> rm -f .config; /usr/bin/yes | make oldconfig

Save your .config, since next step will destroy it :

make allyesconfig

Then build :

make   $your_favorite_build_options



--
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
Eric Dumazet Feb. 6, 2012, 9:19 a.m. UTC | #6
Le lundi 06 février 2012 à 10:04 +0100, Stefan Priebe - Profihost AG a
écrit :
> Hi Eric, Hi David
> 
> today i've seen this:
> [1048676.660457] ------------[ cut here ]------------
> [1048676.688131] WARNING: at net/ipv4/tcp_input.c:2964
> tcp_ack+0xfe1/0x2420()
> [1048676.716291] Hardware name: X8SIL
> [1048676.744292] Modules linked in: xt_tcpudp ipt_REJECT iptable_filter
> ip_tables x_tables coretemp k8temp ipv6 dm_snapshot dm_mod
> [1048676.802468] Pid: 0, comm: kworker/0:1 Not tainted 2.6.40.17.1intel #1
> [1048676.831737] Call Trace:
> [1048676.860455]  <IRQ>  [<ffffffff81565921>] ? tcp_ack+0xfe1/0x2420
> [1048676.860765]  [<ffffffff81045e10>] warn_slowpath_common+0x80/0xc0
> [1048676.860771]  [<ffffffff81045e65>] warn_slowpath_null+0x15/0x20
> [1048676.860777]  [<ffffffff81565921>] tcp_ack+0xfe1/0x2420
> [1048676.860784]  [<ffffffff81567060>] tcp_rcv_established+0x300/0x630
> [1048676.860791]  [<ffffffff815708a4>] tcp_v4_do_rcv+0x154/0x2d0
> [1048676.860796]  [<ffffffff8157111b>] tcp_v4_rcv+0x6fb/0x880
> [1048676.860804]  [<ffffffff8154e4e7>] ip_local_deliver_finish+0x127/0x250
> [1048676.860810]  [<ffffffff8154e69d>] ip_local_deliver+0x8d/0xa0
> [1048676.860815]  [<ffffffff8154dda2>] ip_rcv_finish+0x172/0x340
> [1048676.860820]  [<ffffffff8154e1e5>] ip_rcv+0x275/0x2f0
> [1048676.860827]  [<ffffffff81523387>] __netif_receive_skb+0x427/0x4a0
> [1048676.860832]  [<ffffffff81529148>] netif_receive_skb+0x78/0x80
> [1048676.860837]  [<ffffffff81529280>] napi_skb_finish+0x50/0x70
> [1048676.860842]  [<ffffffff81529735>] napi_gro_receive+0xc5/0xd0
> [1048676.860851]  [<ffffffff81462786>] e1000_receive_skb+0x56/0x70
> [1048676.860856]  [<ffffffff814646eb>] e1000_clean_rx_irq+0x22b/0x3d0
> [1048676.860862]  [<ffffffff814630f2>] e1000_clean+0xb2/0x2f0
> [1048676.860868]  [<ffffffff81054efc>] ? run_timer_softirq+0x3c/0x320
> [1048676.860873]  [<ffffffff815298fa>] net_rx_action+0x10a/0x2b0
> [1048676.860879]  [<ffffffff8104c300>] __do_softirq+0xd0/0x1c0
> [1048676.860887]  [<ffffffff815eb20c>] call_softirq+0x1c/0x30
> [1048676.860895]  [<ffffffff810047b5>] do_softirq+0x55/0x90
> [1048676.860900]  [<ffffffff8104c0dd>] irq_exit+0xad/0xe0
> [1048676.860905]  [<ffffffff81003f94>] do_IRQ+0x64/0xe0
> [1048676.860910]  [<ffffffff815e9a93>] common_interrupt+0x13/0x13
> [1048676.860913]  <EOI>  [<ffffffff8106bdff>] ?
> notifier_call_chain+0x3f/0x80
> [1048676.860926]  [<ffffffff813117b3>] ? intel_idle+0xb3/0x120
> [1048676.860931]  [<ffffffff81311795>] ? intel_idle+0x95/0x120
> [1048676.860937]  [<ffffffff814fc27c>] cpuidle_idle_call+0xdc/0x1a0
> [1048676.860942]  [<ffffffff81002091>] cpu_idle+0xb1/0x110
> [1048676.860948]  [<ffffffff81b0d7aa>] start_secondary+0x201/0x297
> [1048676.860953] ---[ end trace 4d27234ace919a1b ]---
> 
> Any idea about that? Is it due to my custom patch being buggy or is it
> anything you know which is missing in 3.0.X too?

Thats the tcp_fastretrans_alert()

	if (WARN_ON(!tp->sacked_out && tp->fackets_out))
		tp->fackets_out = 0;

I dont know if some recent patch addressed this issue.

CC Ilpo on this one.


--
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
Ilpo Järvinen Feb. 6, 2012, 12:47 p.m. UTC | #7
On Mon, 6 Feb 2012, Eric Dumazet wrote:

> Le lundi 06 février 2012 à 10:04 +0100, Stefan Priebe - Profihost AG a
> écrit :
> > today i've seen this:
> > [1048676.660457] ------------[ cut here ]------------
> > [1048676.688131] WARNING: at net/ipv4/tcp_input.c:2964
> > tcp_ack+0xfe1/0x2420()
> > [1048676.716291] Hardware name: X8SIL
> > [1048676.744292] Modules linked in: xt_tcpudp ipt_REJECT iptable_filter
> > ip_tables x_tables coretemp k8temp ipv6 dm_snapshot dm_mod
> > [1048676.802468] Pid: 0, comm: kworker/0:1 Not tainted 2.6.40.17.1intel #1
> > [1048676.831737] Call Trace:
> > [1048676.860455]  <IRQ>  [<ffffffff81565921>] ? tcp_ack+0xfe1/0x2420
> > [1048676.860765]  [<ffffffff81045e10>] warn_slowpath_common+0x80/0xc0
> > [1048676.860771]  [<ffffffff81045e65>] warn_slowpath_null+0x15/0x20
> > [1048676.860777]  [<ffffffff81565921>] tcp_ack+0xfe1/0x2420
> > [1048676.860784]  [<ffffffff81567060>] tcp_rcv_established+0x300/0x630
> > [1048676.860791]  [<ffffffff815708a4>] tcp_v4_do_rcv+0x154/0x2d0
> > [1048676.860796]  [<ffffffff8157111b>] tcp_v4_rcv+0x6fb/0x880
> > [1048676.860804]  [<ffffffff8154e4e7>] ip_local_deliver_finish+0x127/0x250
> > [1048676.860810]  [<ffffffff8154e69d>] ip_local_deliver+0x8d/0xa0
> > [1048676.860815]  [<ffffffff8154dda2>] ip_rcv_finish+0x172/0x340
> > [1048676.860820]  [<ffffffff8154e1e5>] ip_rcv+0x275/0x2f0
> > [1048676.860827]  [<ffffffff81523387>] __netif_receive_skb+0x427/0x4a0
> > [1048676.860832]  [<ffffffff81529148>] netif_receive_skb+0x78/0x80
> > [1048676.860837]  [<ffffffff81529280>] napi_skb_finish+0x50/0x70
> > [1048676.860842]  [<ffffffff81529735>] napi_gro_receive+0xc5/0xd0
> > [1048676.860851]  [<ffffffff81462786>] e1000_receive_skb+0x56/0x70
> > [1048676.860856]  [<ffffffff814646eb>] e1000_clean_rx_irq+0x22b/0x3d0
> > [1048676.860862]  [<ffffffff814630f2>] e1000_clean+0xb2/0x2f0
> > [1048676.860868]  [<ffffffff81054efc>] ? run_timer_softirq+0x3c/0x320
> > [1048676.860873]  [<ffffffff815298fa>] net_rx_action+0x10a/0x2b0
> > [1048676.860879]  [<ffffffff8104c300>] __do_softirq+0xd0/0x1c0
> > [1048676.860887]  [<ffffffff815eb20c>] call_softirq+0x1c/0x30
> > [1048676.860895]  [<ffffffff810047b5>] do_softirq+0x55/0x90
> > [1048676.860900]  [<ffffffff8104c0dd>] irq_exit+0xad/0xe0
> > [1048676.860905]  [<ffffffff81003f94>] do_IRQ+0x64/0xe0
> > [1048676.860910]  [<ffffffff815e9a93>] common_interrupt+0x13/0x13
> > [1048676.860913]  <EOI>  [<ffffffff8106bdff>] ?
> > notifier_call_chain+0x3f/0x80
> > [1048676.860926]  [<ffffffff813117b3>] ? intel_idle+0xb3/0x120
> > [1048676.860931]  [<ffffffff81311795>] ? intel_idle+0x95/0x120
> > [1048676.860937]  [<ffffffff814fc27c>] cpuidle_idle_call+0xdc/0x1a0
> > [1048676.860942]  [<ffffffff81002091>] cpu_idle+0xb1/0x110
> > [1048676.860948]  [<ffffffff81b0d7aa>] start_secondary+0x201/0x297
> > [1048676.860953] ---[ end trace 4d27234ace919a1b ]---
> > 
> > Any idea about that? Is it due to my custom patch being buggy or is it
> > anything you know which is missing in 3.0.X too?

This warning is known to trigger every now and then...

> Thats the tcp_fastretrans_alert()
> 
> 	if (WARN_ON(!tp->sacked_out && tp->fackets_out))
> 		tp->fackets_out = 0;
> 
> I dont know if some recent patch addressed this issue.

...the recent fix from Neal to pick correct MSS might fix this but it 
is of course hard to confirm for sure (we'll see it indirectly eventually 
if there won't be anymore these rare splats). If one has infinite time it 
would be quite simple to see if changing mss setup triggers this and if 
the Neal's fix helped or not, however, I don't consider this particular 
inconsistency worth the effort.

...What I can say for sure is at least tp->fackets_out -= min(pkts_acked, 
tp->fackets_out); seems to fail when pkts_acked (u32) underflows due to 
the mss badness we used to have. So it could actually solve this for real.

The effects of this counter inconsistency are not that devastating. 
Fackets_out mainly affect when recovery is triggered/which segments to 
mark lost in the recovery itself. Two extremes I can think of: recovery 
not triggered => RTO triggers and everyone is happy except some researcher 
who finds that odd and unwanted and needs to fix it :-); recovery in 
progress but works too much ahead, as if dupthresh (tp->reordering) would 
be slightly smaller (if in-order behavior in the network is assumed this 
is still fully safe, dupthresh is there to help in cases of minor 
reordering).
Stefan Priebe - Profihost AG Feb. 8, 2012, 8:26 a.m. UTC | #8
Hi Eric,

Am 06.02.2012 13:47, schrieb Ilpo Järvinen:
>>> Any idea about that? Is it due to my custom patch being buggy or is it
>>> anything you know which is missing in 3.0.X too?
> 
> This warning is known to trigger every now and then...
> 
>> Thats the tcp_fastretrans_alert()
>>
>> 	if (WARN_ON(!tp->sacked_out && tp->fackets_out))
>> 		tp->fackets_out = 0;
>>
>> I dont know if some recent patch addressed this issue.
> 
> ...the recent fix from Neal to pick correct MSS might fix this but it 
> is of course hard to confirm for sure (we'll see it indirectly eventually 
> if there won't be anymore these rare splats). If one has infinite time it 
> would be quite simple to see if changing mss setup triggers this and if 
> the Neal's fix helped or not, however, I don't consider this particular 
> inconsistency worth the effort.
> 
> ...What I can say for sure is at least tp->fackets_out -= min(pkts_acked, 
> tp->fackets_out); seems to fail when pkts_acked (u32) underflows due to 
> the mss badness we used to have. So it could actually solve this for real.
> 
> The effects of this counter inconsistency are not that devastating. 
> Fackets_out mainly affect when recovery is triggered/which segments to 
> mark lost in the recovery itself. Two extremes I can think of: recovery 
> not triggered => RTO triggers and everyone is happy except some researcher 
> who finds that odd and unwanted and needs to fix it :-); recovery in 
> progress but works too much ahead, as if dupthresh (tp->reordering) would 
> be slightly smaller (if in-order behavior in the network is assumed this 
> is still fully safe, dupthresh is there to help in cases of minor 
> reordering).

What do you think about this? Can anybody give me the commit id?

Stefan
--
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
Eric Dumazet Feb. 8, 2012, 9:15 a.m. UTC | #9
Le mercredi 08 février 2012 à 09:26 +0100, Stefan Priebe - Profihost AG
a écrit :
> Hi Eric,

> What do you think about this? Can anybody give me the commit id?
> 
> Stefan

commit 5b35e1e6e9ca651e6b291c96d1106043c9af314a
Author: Neal Cardwell <ncardwell@google.com>
Date:   Sat Jan 28 17:29:46 2012 +0000

    tcp: fix tcp_trim_head() to adjust segment count with skb MSS



Its quite recent and David will push it to stable, dont worry.



--
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
Eric Dumazet Feb. 8, 2012, 9:28 a.m. UTC | #10
Le mercredi 08 février 2012 à 10:15 +0100, Eric Dumazet a écrit :
> Le mercredi 08 février 2012 à 09:26 +0100, Stefan Priebe - Profihost AG
> a écrit :
> > Hi Eric,
> 
> > What do you think about this? Can anybody give me the commit id?
> > 
> > Stefan
> 
> commit 5b35e1e6e9ca651e6b291c96d1106043c9af314a
> Author: Neal Cardwell <ncardwell@google.com>
> Date:   Sat Jan 28 17:29:46 2012 +0000
> 
>     tcp: fix tcp_trim_head() to adjust segment count with skb MSS
> 
> 
> 
> Its quite recent and David will push it to stable, dont worry.
> 
> 

Actually already taken by stable teams :

For 3.0 kernel, search archives with :

[56/65] tcp: fix tcp_trim_head() to adjust segment count with skb MSS


--
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
David Miller Feb. 9, 2012, 1:26 a.m. UTC | #11
From: Stefan Priebe - Profihost AG <s.priebe@profihost.ag>
Date: Fri, 03 Feb 2012 09:09:35 +0100

> Hi,
> 
> attached you find the patch files applying cleanly to 3.0.X.
> 
>>> I've made my own backport of the patch and removed at least 1-2
>>> dependencies. Anybody interested?
>>>
>> 
>> If you did the work, post it for review.
> 
> Thanks!

I think you backported way too much, the ->lookup_neigh() dst ops
abstraction is unnecessary, all of the neighbour hash function
changes are also completely unnecessary.

All you needed to backport is that bit that abstracts dst->neighbour()
behind helper routines.

The point of this exercise is not to keep backporting changes until
the subsequent patch applies cleanly, it's to backport the minimal
amount necessary to get the effect of the patch you're ultimately
interested in adding.

I'll work on coming up with something much more sensible tonight.
--
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
Stefan Priebe - Profihost AG Feb. 9, 2012, 6:44 a.m. UTC | #12
Am 09.02.2012 02:26, schrieb David Miller:
> From: Stefan Priebe - Profihost AG <s.priebe@profihost.ag>
> Date: Fri, 03 Feb 2012 09:09:35 +0100
> 
>> Hi,
>>
>> attached you find the patch files applying cleanly to 3.0.X.
>>
>>>> I've made my own backport of the patch and removed at least 1-2
>>>> dependencies. Anybody interested?
>>>>
>>>
>>> If you did the work, post it for review.
>>
>> Thanks!
> 
> I think you backported way too much, the ->lookup_neigh() dst ops
> abstraction is unnecessary, all of the neighbour hash function
> changes are also completely unnecessary.
> 
> All you needed to backport is that bit that abstracts dst->neighbour()
> behind helper routines.
> 
> The point of this exercise is not to keep backporting changes until
> the subsequent patch applies cleanly, it's to backport the minimal
> amount necessary to get the effect of the patch you're ultimately
> interested in adding.
> 
> I'll work on coming up with something much more sensible tonight.

Thank you very much. As im pinted out i'm not a C dev nor a kernel
hacker. So i was pretty happy that it was working.

Stefan
--
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
diff mbox

Patch

From 103f10c1235673d1d0dc7fec8b5254cb104d4a0a Mon Sep 17 00:00:00 2001
From: David S. Miller <davem@davemloft.net>
Date: Sun, 17 Jul 2011 20:06:13 -0700
Subject: [PATCH 01/13] ipv6: Get rid of rt6i_nexthop macro.

It just makes it harder to see 1) what the code is doing
and 2) grep for all users of dst{->,.}neighbour

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/ip6_fib.h |    1 -
 net/ipv6/addrconf.c   |    2 +-
 net/ipv6/ip6_fib.c    |    2 +-
 net/ipv6/ndisc.c      |    4 ++--
 net/ipv6/route.c      |   30 +++++++++++++++---------------
 5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 477ef75..5735a0f 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -87,7 +87,6 @@  struct rt6_info {
 	struct dst_entry		dst;
 
 #define rt6i_dev			dst.dev
-#define rt6i_nexthop			dst.neighbour
 #define rt6i_expires			dst.expires
 
 	/*
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 498b927..bcd7aed 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -656,7 +656,7 @@  ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr, int pfxlen,
 	 * layer address of our nexhop router
 	 */
 
-	if (rt->rt6i_nexthop == NULL)
+	if (rt->dst.neighbour == NULL)
 		ifa->flags &= ~IFA_F_OPTIMISTIC;
 
 	ifa->idev = idev;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 4076a0b..098dc76 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1455,7 +1455,7 @@  static int fib6_age(struct rt6_info *rt, void *arg)
 			RT6_TRACE("aging clone %p\n", rt);
 			return -1;
 		} else if ((rt->rt6i_flags & RTF_GATEWAY) &&
-			   (!(rt->rt6i_nexthop->flags & NTF_ROUTER))) {
+			   (!(rt->dst.neighbour->flags & NTF_ROUTER))) {
 			RT6_TRACE("purging route %p via non-router but gateway\n",
 				  rt);
 			return -1;
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 7596f07..dee1a19 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1244,7 +1244,7 @@  static void ndisc_router_discovery(struct sk_buff *skb)
 	rt = rt6_get_dflt_router(&ipv6_hdr(skb)->saddr, skb->dev);
 
 	if (rt)
-		neigh = rt->rt6i_nexthop;
+		neigh = rt->dst.neighbour;
 
 	if (rt && lifetime == 0) {
 		neigh_clone(neigh);
@@ -1265,7 +1265,7 @@  static void ndisc_router_discovery(struct sk_buff *skb)
 			return;
 		}
 
-		neigh = rt->rt6i_nexthop;
+		neigh = rt->dst.neighbour;
 		if (neigh == NULL) {
 			ND_PRINTK0(KERN_ERR
 				   "ICMPv6 RA: %s() got default router without neighbour.\n",
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0ef1f08..6ee9307 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -356,7 +356,7 @@  out:
 #ifdef CONFIG_IPV6_ROUTER_PREF
 static void rt6_probe(struct rt6_info *rt)
 {
-	struct neighbour *neigh = rt ? rt->rt6i_nexthop : NULL;
+	struct neighbour *neigh = rt ? rt->dst.neighbour : NULL;
 	/*
 	 * Okay, this does not seem to be appropriate
 	 * for now, however, we need to check if it
@@ -404,7 +404,7 @@  static inline int rt6_check_dev(struct rt6_info *rt, int oif)
 
 static inline int rt6_check_neigh(struct rt6_info *rt)
 {
-	struct neighbour *neigh = rt->rt6i_nexthop;
+	struct neighbour *neigh = rt->dst.neighbour;
 	int m;
 	if (rt->rt6i_flags & RTF_NONEXTHOP ||
 	    !(rt->rt6i_flags & RTF_GATEWAY))
@@ -745,7 +745,7 @@  static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort, const struct in6_add
 			dst_free(&rt->dst);
 			return NULL;
 		}
-		rt->rt6i_nexthop = neigh;
+		rt->dst.neighbour = neigh;
 
 	}
 
@@ -760,7 +760,7 @@  static struct rt6_info *rt6_alloc_clone(struct rt6_info *ort, const struct in6_a
 		rt->rt6i_dst.plen = 128;
 		rt->rt6i_flags |= RTF_CACHE;
 		rt->dst.flags |= DST_HOST;
-		rt->rt6i_nexthop = neigh_clone(ort->rt6i_nexthop);
+		rt->dst.neighbour = neigh_clone(ort->dst.neighbour);
 	}
 	return rt;
 }
@@ -794,7 +794,7 @@  restart:
 	dst_hold(&rt->dst);
 	read_unlock_bh(&table->tb6_lock);
 
-	if (!rt->rt6i_nexthop && !(rt->rt6i_flags & RTF_NONEXTHOP))
+	if (!rt->dst.neighbour && !(rt->rt6i_flags & RTF_NONEXTHOP))
 		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
 	else if (!(rt->dst.flags & DST_HOST))
 		nrt = rt6_alloc_clone(rt, &fl6->daddr);
@@ -1058,7 +1058,7 @@  struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
 	}
 
 	rt->rt6i_idev     = idev;
-	rt->rt6i_nexthop  = neigh;
+	rt->dst.neighbour  = neigh;
 	atomic_set(&rt->dst.__refcnt, 1);
 	dst_metric_set(&rt->dst, RTAX_HOPLIMIT, 255);
 	rt->dst.output  = ip6_output;
@@ -1338,10 +1338,10 @@  int ip6_route_add(struct fib6_config *cfg)
 		rt->rt6i_prefsrc.plen = 0;
 
 	if (cfg->fc_flags & (RTF_GATEWAY | RTF_NONEXTHOP)) {
-		rt->rt6i_nexthop = __neigh_lookup_errno(&nd_tbl, &rt->rt6i_gateway, dev);
-		if (IS_ERR(rt->rt6i_nexthop)) {
-			err = PTR_ERR(rt->rt6i_nexthop);
-			rt->rt6i_nexthop = NULL;
+		rt->dst.neighbour = __neigh_lookup_errno(&nd_tbl, &rt->rt6i_gateway, dev);
+		if (IS_ERR(rt->dst.neighbour)) {
+			err = PTR_ERR(rt->dst.neighbour);
+			rt->dst.neighbour = NULL;
 			goto out;
 		}
 	}
@@ -1590,7 +1590,7 @@  void rt6_redirect(const struct in6_addr *dest, const struct in6_addr *src,
 	nrt->dst.flags |= DST_HOST;
 
 	ipv6_addr_copy(&nrt->rt6i_gateway, (struct in6_addr*)neigh->primary_key);
-	nrt->rt6i_nexthop = neigh_clone(neigh);
+	nrt->dst.neighbour = neigh_clone(neigh);
 
 	if (ip6_ins_rt(nrt))
 		goto out;
@@ -1670,7 +1670,7 @@  again:
 	   1. It is connected route. Action: COW
 	   2. It is gatewayed route or NONEXTHOP route. Action: clone it.
 	 */
-	if (!rt->rt6i_nexthop && !(rt->rt6i_flags & RTF_NONEXTHOP))
+	if (!rt->dst.neighbour && !(rt->rt6i_flags & RTF_NONEXTHOP))
 		nrt = rt6_alloc_cow(rt, daddr, saddr);
 	else
 		nrt = rt6_alloc_clone(rt, daddr);
@@ -2035,7 +2035,7 @@  struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 
 		return ERR_CAST(neigh);
 	}
-	rt->rt6i_nexthop = neigh;
+	rt->dst.neighbour = neigh;
 
 	ipv6_addr_copy(&rt->rt6i_dst.addr, addr);
 	rt->rt6i_dst.plen = 128;
@@ -2594,8 +2594,8 @@  static int rt6_info_route(struct rt6_info *rt, void *p_arg)
 	seq_puts(m, "00000000000000000000000000000000 00 ");
 #endif
 
-	if (rt->rt6i_nexthop) {
-		seq_printf(m, "%pi6", rt->rt6i_nexthop->primary_key);
+	if (rt->dst.neighbour) {
+		seq_printf(m, "%pi6", rt->dst.neighbour->primary_key);
 	} else {
 		seq_puts(m, "00000000000000000000000000000000");
 	}
-- 
1.7.1