[patch 0/6] ipv6 - ipv6 routing initialization

[patch 0/6] ipv6 - ipv6 routing initialization

[Daniel Lezcano]

This patchset provides modifications around the routes initialization
for ipv6. Actually the init functions does not return an error code
so the protocol can not be notified that there were an error while 
initializing the routing subsystems.

The patchset make the init functions to return an error code, so the ipv6
can safely handle the error and fail gracefully.

The error code can also let to catch the kmem_cache_creation failure without
doing a radical panic. That's allow just to fail to load the ipv6 module without 
crashing down the machine.

-- 

[patch 1/6] ipv6 - make fib6_init to return an error code

[Daniel Lezcano]

[-- Attachment #1: ip6_fib-handle-error-at-init.patch --]
[-- Type: text/plain, Size: 1652 bytes --]

If there is an error in the initialization function, nothing is followed up
to the caller. So I add a return value to be set for the init function.

Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-by: Benjamin Thery <benjamin.thery@bull.net>
---
 include/net/ip6_fib.h |    2 +-
 net/ipv6/ip6_fib.c    |   14 +++++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

Index: net-2.6.25/include/net/ip6_fib.h
===================================================================
--- net-2.6.25.orig/include/net/ip6_fib.h
+++ net-2.6.25/include/net/ip6_fib.h
@@ -224,7 +224,7 @@ extern void			fib6_run_gc(unsigned long 
 
 extern void			fib6_gc_cleanup(void);
 
-extern void			fib6_init(void);
+extern int			fib6_init(void);
 
 extern void			fib6_rules_init(void);
 extern void			fib6_rules_cleanup(void);
Index: net-2.6.25/net/ipv6/ip6_fib.c
===================================================================
--- net-2.6.25.orig/net/ipv6/ip6_fib.c
+++ net-2.6.25/net/ipv6/ip6_fib.c
@@ -1473,16 +1473,24 @@ void fib6_run_gc(unsigned long dummy)
 	spin_unlock_bh(&fib6_gc_lock);
 }
 
-void __init fib6_init(void)
+int __init fib6_init(void)
 {
+	int ret;
 	fib6_node_kmem = kmem_cache_create("fib6_nodes",
 					   sizeof(struct fib6_node),
 					   0, SLAB_HWCACHE_ALIGN|SLAB_PANIC,
 					   NULL);
-
 	fib6_tables_init();
 
-	__rtnl_register(PF_INET6, RTM_GETROUTE, NULL, inet6_dump_fib);
+	ret = __rtnl_register(PF_INET6, RTM_GETROUTE, NULL, inet6_dump_fib);
+	if (ret)
+		goto out_kmem_cache_create;
+out:
+	return ret;
+
+out_kmem_cache_create:
+	kmem_cache_destroy(fib6_node_kmem);
+	goto out;
 }
 
 void fib6_gc_cleanup(void)

-- 

[patch 2/6] ipv6 - make xfrm6_init to return an error code

[Daniel Lezcano]

[-- Attachment #1: xfrm-handle-error-at-init.patch --]
[-- Type: text/plain, Size: 2709 bytes --]

The xfrm initialization function does not return any error code, so
if there is an error, the caller can not be advise of that.
This patch checks the return code of the different called functions
in order to return a successful or failed initialization.

Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-by: Benjamin Thery <benjamin.thery@bull.net>
---
 include/net/xfrm.h      |    4 ++--
 net/ipv6/xfrm6_policy.c |   22 +++++++++++++++++-----
 net/ipv6/xfrm6_state.c  |    4 ++--
 3 files changed, 21 insertions(+), 9 deletions(-)

Index: net-2.6.25/include/net/xfrm.h
===================================================================
--- net-2.6.25.orig/include/net/xfrm.h
+++ net-2.6.25/include/net/xfrm.h
@@ -1066,11 +1066,11 @@ struct xfrm6_tunnel {
 
 extern void xfrm_init(void);
 extern void xfrm4_init(void);
-extern void xfrm6_init(void);
+extern int xfrm6_init(void);
 extern void xfrm6_fini(void);
 extern void xfrm_state_init(void);
 extern void xfrm4_state_init(void);
-extern void xfrm6_state_init(void);
+extern int xfrm6_state_init(void);
 extern void xfrm6_state_fini(void);
 
 extern int xfrm_state_walk(u8 proto, int (*func)(struct xfrm_state *, int, void*), void *);
Index: net-2.6.25/net/ipv6/xfrm6_policy.c
===================================================================
--- net-2.6.25.orig/net/ipv6/xfrm6_policy.c
+++ net-2.6.25/net/ipv6/xfrm6_policy.c
@@ -269,9 +269,9 @@ static struct xfrm_policy_afinfo xfrm6_p
 	.fill_dst =		xfrm6_fill_dst,
 };
 
-static void __init xfrm6_policy_init(void)
+static int __init xfrm6_policy_init(void)
 {
-	xfrm_policy_register_afinfo(&xfrm6_policy_afinfo);
+	return xfrm_policy_register_afinfo(&xfrm6_policy_afinfo);
 }
 
 static void xfrm6_policy_fini(void)
@@ -279,10 +279,22 @@ static void xfrm6_policy_fini(void)
 	xfrm_policy_unregister_afinfo(&xfrm6_policy_afinfo);
 }
 
-void __init xfrm6_init(void)
+int __init xfrm6_init(void)
 {
-	xfrm6_policy_init();
-	xfrm6_state_init();
+	int ret;
+
+	ret = xfrm6_policy_init();
+	if (ret)
+		goto out;
+
+	ret = xfrm6_state_init();
+	if (ret)
+		goto out_policy;
+out:
+	return ret;
+out_policy:
+	xfrm6_policy_fini();
+	goto out;
 }
 
 void xfrm6_fini(void)
Index: net-2.6.25/net/ipv6/xfrm6_state.c
===================================================================
--- net-2.6.25.orig/net/ipv6/xfrm6_state.c
+++ net-2.6.25/net/ipv6/xfrm6_state.c
@@ -198,9 +198,9 @@ static struct xfrm_state_afinfo xfrm6_st
 	.transport_finish	= xfrm6_transport_finish,
 };
 
-void __init xfrm6_state_init(void)
+int __init xfrm6_state_init(void)
 {
-	xfrm_state_register_afinfo(&xfrm6_state_afinfo);
+	return xfrm_state_register_afinfo(&xfrm6_state_afinfo);
 }
 
 void xfrm6_state_fini(void)

-- 

[patch 3/6] ipv6 - make fib6_rules_init to return an error code

[Daniel Lezcano]

[-- Attachment #1: fib6_rules-handle-error-at-init.patch --]
[-- Type: text/plain, Size: 3003 bytes --]

When the fib_rules initialization finished, no return code is provided
so there is no way to know, for the caller, if the initialization has
been successful or has failed. This patch fix that.

Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-by: Benjamin Thery <benjamin.thery@bull.net>
---
 include/net/fib_rules.h |    1 +
 include/net/ip6_fib.h   |    2 +-
 net/core/fib_rules.c    |    5 +++--
 net/ipv6/fib6_rules.c   |   19 ++++++++++++++++---
 4 files changed, 21 insertions(+), 6 deletions(-)

Index: net-2.6.25/include/net/ip6_fib.h
===================================================================
--- net-2.6.25.orig/include/net/ip6_fib.h
+++ net-2.6.25/include/net/ip6_fib.h
@@ -226,7 +226,7 @@ extern void			fib6_gc_cleanup(void);
 
 extern int			fib6_init(void);
 
-extern void			fib6_rules_init(void);
+extern int			fib6_rules_init(void);
 extern void			fib6_rules_cleanup(void);
 
 #endif
Index: net-2.6.25/net/ipv6/fib6_rules.c
===================================================================
--- net-2.6.25.orig/net/ipv6/fib6_rules.c
+++ net-2.6.25/net/ipv6/fib6_rules.c
@@ -265,10 +265,23 @@ static int __init fib6_default_rules_ini
 	return 0;
 }
 
-void __init fib6_rules_init(void)
+int __init fib6_rules_init(void)
 {
-	BUG_ON(fib6_default_rules_init());
-	fib_rules_register(&fib6_rules_ops);
+	int ret;
+
+	ret = fib6_default_rules_init();
+	if (ret)
+		goto out;
+
+	ret = fib_rules_register(&fib6_rules_ops);
+	if (ret)
+		goto out_default_rules_init;
+out:
+	return ret;
+
+out_default_rules_init:
+	fib_rules_cleanup_ops(&fib6_rules_ops);
+	goto out;
 }
 
 void fib6_rules_cleanup(void)
Index: net-2.6.25/include/net/fib_rules.h
===================================================================
--- net-2.6.25.orig/include/net/fib_rules.h
+++ net-2.6.25/include/net/fib_rules.h
@@ -103,6 +103,7 @@ static inline u32 frh_get_table(struct f
 
 extern int			fib_rules_register(struct fib_rules_ops *);
 extern int			fib_rules_unregister(struct fib_rules_ops *);
+extern void                     fib_rules_cleanup_ops(struct fib_rules_ops *);
 
 extern int			fib_rules_lookup(struct fib_rules_ops *,
 						 struct flowi *, int flags,
Index: net-2.6.25/net/core/fib_rules.c
===================================================================
--- net-2.6.25.orig/net/core/fib_rules.c
+++ net-2.6.25/net/core/fib_rules.c
@@ -102,7 +102,7 @@ errout:
 
 EXPORT_SYMBOL_GPL(fib_rules_register);
 
-static void cleanup_ops(struct fib_rules_ops *ops)
+void fib_rules_cleanup_ops(struct fib_rules_ops *ops)
 {
 	struct fib_rule *rule, *tmp;
 
@@ -111,6 +111,7 @@ static void cleanup_ops(struct fib_rules
 		fib_rule_put(rule);
 	}
 }
+EXPORT_SYMBOL_GPL(fib_rules_cleanup_ops);
 
 int fib_rules_unregister(struct fib_rules_ops *ops)
 {
@@ -121,7 +122,7 @@ int fib_rules_unregister(struct fib_rule
 	list_for_each_entry(o, &rules_ops, list) {
 		if (o == ops) {
 			list_del_rcu(&o->list);
-			cleanup_ops(ops);
+			fib_rules_cleanup_ops(ops);
 			goto out;
 		}
 	}

-- 

[patch 4/6] ipv6 - make ip6_route_init to return an error code

[Daniel Lezcano]

[-- Attachment #1: route6-handle-error-at-init.patch --]
[-- Type: text/plain, Size: 3747 bytes --]

The route initialization function does not return any value to notify if
the initialization is successful or not. This patch checks all calls made
for the initilization in order to return a value for the caller.

Unfortunatly, proc_net_fops_create will return a NULL pointer if CONFIG_PROC_FS
is off, so we can not check the return code without an ifdef CONFIG_PROC_FS 
block in the ip6_route_init function.

Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-by: Benjamin Thery <benjamin.thery@bull.net>
---
 include/net/ip6_route.h |    2 -
 net/ipv6/route.c        |   66 +++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 55 insertions(+), 13 deletions(-)

Index: net-2.6.25/include/net/ip6_route.h
===================================================================
--- net-2.6.25.orig/include/net/ip6_route.h
+++ net-2.6.25/include/net/ip6_route.h
@@ -50,7 +50,7 @@ extern void			ip6_route_input(struct sk_
 extern struct dst_entry *	ip6_route_output(struct sock *sk,
 						 struct flowi *fl);
 
-extern void			ip6_route_init(void);
+extern int			ip6_route_init(void);
 extern void			ip6_route_cleanup(void);
 
 extern int			ipv6_route_ioctl(unsigned int cmd, void __user *arg);
Index: net-2.6.25/net/ipv6/route.c
===================================================================
--- net-2.6.25.orig/net/ipv6/route.c
+++ net-2.6.25/net/ipv6/route.c
@@ -2460,26 +2460,70 @@ ctl_table ipv6_route_table[] = {
 
 #endif
 
-void __init ip6_route_init(void)
+int __init ip6_route_init(void)
 {
+	int ret;
+
 	ip6_dst_ops.kmem_cachep =
 		kmem_cache_create("ip6_dst_cache", sizeof(struct rt6_info), 0,
 				  SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
 	ip6_dst_blackhole_ops.kmem_cachep = ip6_dst_ops.kmem_cachep;
 
-	fib6_init();
-	proc_net_fops_create(&init_net, "ipv6_route", 0, &ipv6_route_proc_fops);
-	proc_net_fops_create(&init_net, "rt6_stats", S_IRUGO, &rt6_stats_seq_fops);
+	ret = fib6_init();
+	if (ret)
+		goto out_kmem_cache;
+
+#ifdef CONFIG_PROC_FS
+	ret = -ENOMEM;
+	if (!proc_net_fops_create(&init_net, "ipv6_route",
+				  0, &ipv6_route_proc_fops))
+		goto out_fib6_init;
+
+	if (!proc_net_fops_create(&init_net, "rt6_stats",
+				  S_IRUGO, &rt6_stats_seq_fops))
+		goto out_proc_ipv6_route;
+#endif
+
 #ifdef CONFIG_XFRM
-	xfrm6_init();
+	ret = xfrm6_init();
+	if (ret)
+		goto out_proc_rt6_stats;
 #endif
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
-	fib6_rules_init();
-#endif
+	ret = fib6_rules_init();
+	if (ret)
+		goto xfrm6_init;
+#endif
+	ret = -ENOBUFS;
+	if (__rtnl_register(PF_INET6, RTM_NEWROUTE, inet6_rtm_newroute, NULL) ||
+	    __rtnl_register(PF_INET6, RTM_DELROUTE, inet6_rtm_delroute, NULL) ||
+	    __rtnl_register(PF_INET6, RTM_GETROUTE, inet6_rtm_getroute, NULL))
+		goto fib6_rules_init;
+
+	ret = 0;
+out:
+	return ret;
 
-	__rtnl_register(PF_INET6, RTM_NEWROUTE, inet6_rtm_newroute, NULL);
-	__rtnl_register(PF_INET6, RTM_DELROUTE, inet6_rtm_delroute, NULL);
-	__rtnl_register(PF_INET6, RTM_GETROUTE, inet6_rtm_getroute, NULL);
+fib6_rules_init:
+#ifdef CONFIG_IPV6_MULTIPLE_TABLES
+	fib6_rules_cleanup();
+xfrm6_init:
+#endif
+#ifdef CONFIG_XFRM
+	xfrm6_fini();
+out_proc_rt6_stats:
+#endif
+#ifdef CONFIG_PROC_FS
+	proc_net_remove(&init_net, "rt6_stats");
+out_proc_ipv6_route:
+	proc_net_remove(&init_net, "ipv6_route");
+out_fib6_init:
+#endif
+	rt6_ifdown(NULL);
+	fib6_gc_cleanup();
+out_kmem_cache:
+	kmem_cache_destroy(ip6_dst_ops.kmem_cachep);
+	goto out;
 }
 
 void ip6_route_cleanup(void)
@@ -2487,10 +2531,8 @@ void ip6_route_cleanup(void)
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
 	fib6_rules_cleanup();
 #endif
-#ifdef CONFIG_PROC_FS
 	proc_net_remove(&init_net, "ipv6_route");
 	proc_net_remove(&init_net, "rt6_stats");
-#endif
 #ifdef CONFIG_XFRM
 	xfrm6_fini();
 #endif

-- 

[patch 5/6] ipv6 - make af_inet6 to check ip6_route_init return value

[Daniel Lezcano]

[-- Attachment #1: af_inet6-handle-subsystem-errors-at-init.patch --]
[-- Type: text/plain, Size: 1377 bytes --]

The af_inet6 initialization function does not check the return code
of the route initilization, so if something goes wrong, the protocol
initialization will continue anyway.
This patch takes into account the modification made in the different
route's initialization subroutines to check the return value and to 
make the protocol initialization to fail.

Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-by: Benjamin Thery <benjamin.thery@bull.net>
---
 net/ipv6/af_inet6.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: net-2.6.25/net/ipv6/af_inet6.c
===================================================================
--- net-2.6.25.orig/net/ipv6/af_inet6.c
+++ net-2.6.25/net/ipv6/af_inet6.c
@@ -849,7 +849,9 @@ static int __init inet6_init(void)
 	if (if6_proc_init())
 		goto proc_if6_fail;
 #endif
-	ip6_route_init();
+	err = ip6_route_init();
+	if (err)
+		goto ip6_route_fail;
 	ip6_flowlabel_init();
 	err = addrconf_init();
 	if (err)
@@ -874,6 +876,7 @@ out:
 addrconf_fail:
 	ip6_flowlabel_cleanup();
 	ip6_route_cleanup();
+ip6_route_fail:
 #ifdef CONFIG_PROC_FS
 	if6_proc_exit();
 proc_if6_fail:
@@ -904,6 +907,7 @@ icmp_fail:
 	cleanup_ipv6_mibs();
 out_unregister_sock:
 	sock_unregister(PF_INET6);
+	rtnl_unregister_all(PF_INET6);
 out_unregister_raw_proto:
 	proto_unregister(&rawv6_prot);
 out_unregister_udplite_proto:

-- 

[patch 6/6] ipv6 - route6/fib6 : dont panic a kmem_cache_create

[Daniel Lezcano]

[-- Attachment #1: ip6_fib-dont-panic-on-kmem_cache-creation-error.patch --]
[-- Type: text/plain, Size: 1732 bytes --]

If the kmem_cache_creation fails, the kernel will panic. It is acceptable
if the system is booting, but if the ipv6 protocol is compiled as a module
and it is loaded after the system has booted, do we want to panic instead
of just failing to initialize the protocol ?

The init function is now returning an error and this one is checked for
protocol initialization. So the ipv6 protocol will safely fails.

Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-by: Benjamin Thery <benjamin.thery@bull.net>
---
 net/ipv6/ip6_fib.c |    5 ++++-
 net/ipv6/route.c   |    5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

Index: net-2.6.25/net/ipv6/ip6_fib.c
===================================================================
--- net-2.6.25.orig/net/ipv6/ip6_fib.c
+++ net-2.6.25/net/ipv6/ip6_fib.c
@@ -1478,8 +1478,11 @@ int __init fib6_init(void)
 	int ret;
 	fib6_node_kmem = kmem_cache_create("fib6_nodes",
 					   sizeof(struct fib6_node),
-					   0, SLAB_HWCACHE_ALIGN|SLAB_PANIC,
+					   0, SLAB_HWCACHE_ALIGN,
 					   NULL);
+	if (!fib6_node_kmem)
+		return -ENOMEM;
+
 	fib6_tables_init();
 
 	ret = __rtnl_register(PF_INET6, RTM_GETROUTE, NULL, inet6_dump_fib);
Index: net-2.6.25/net/ipv6/route.c
===================================================================
--- net-2.6.25.orig/net/ipv6/route.c
+++ net-2.6.25/net/ipv6/route.c
@@ -2466,7 +2466,10 @@ int __init ip6_route_init(void)
 
 	ip6_dst_ops.kmem_cachep =
 		kmem_cache_create("ip6_dst_cache", sizeof(struct rt6_info), 0,
-				  SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+				  SLAB_HWCACHE_ALIGN, NULL);
+	if (!ip6_dst_ops.kmem_cachep)
+		return -ENOMEM;
+
 	ip6_dst_blackhole_ops.kmem_cachep = ip6_dst_ops.kmem_cachep;
 
 	ret = fib6_init();

-- 

Re: [patch 1/6] ipv6 - make fib6_init to return an error code

[David Miller]

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:30 +0100

> If there is an error in the initialization function, nothing is followed up
> to the caller. So I add a return value to be set for the init function.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied.

Please format your header subject lines as:

	[patch N/M] [IPV6]: Blah blah blah.

Since this is what I edit them into anyways.

Thanks.

Re: [patch 2/6] ipv6 - make xfrm6_init to return an error code

[David Miller]

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:31 +0100

> The xfrm initialization function does not return any error code, so
> if there is an error, the caller can not be advise of that.
> This patch checks the return code of the different called functions
> in order to return a successful or failed initialization.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied, thanks.

Re: [patch 3/6] ipv6 - make fib6_rules_init to return an error code

[David Miller]

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:32 +0100

> When the fib_rules initialization finished, no return code is provided
> so there is no way to know, for the caller, if the initialization has
> been successful or has failed. This patch fix that.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied, thanks.

Re: [patch 4/6] ipv6 - make ip6_route_init to return an error code

[David Miller]

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:33 +0100

> The route initialization function does not return any value to notify if
> the initialization is successful or not. This patch checks all calls made
> for the initilization in order to return a value for the caller.
> 
> Unfortunatly, proc_net_fops_create will return a NULL pointer if CONFIG_PROC_FS
> is off, so we can not check the return code without an ifdef CONFIG_PROC_FS 
> block in the ip6_route_init function.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied, thanks.

Re: [patch 5/6] ipv6 - make af_inet6 to check ip6_route_init return value

[David Miller]

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:34 +0100

> The af_inet6 initialization function does not check the return code
> of the route initilization, so if something goes wrong, the protocol
> initialization will continue anyway.
> This patch takes into account the modification made in the different
> route's initialization subroutines to check the return value and to 
> make the protocol initialization to fail.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied, thanks!

Re: [patch 6/6] ipv6 - route6/fib6 : dont panic a kmem_cache_create

[David Miller]

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:35 +0100

> If the kmem_cache_creation fails, the kernel will panic. It is acceptable
> if the system is booting, but if the ipv6 protocol is compiled as a module
> and it is loaded after the system has booted, do we want to panic instead
> of just failing to initialize the protocol ?
> 
> The init function is now returning an error and this one is checked for
> protocol initialization. So the ipv6 protocol will safely fails.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Also applied.