From e59d82fd33f7670cf67fd69cf684aa589ec8340a Mon Sep 17 00:00:00 2001 From: Mathias Krause Date: Fri, 9 May 2014 23:43:41 +0200 Subject: [PATCH 1/3] vti6: Simplify error handling in module init and exit The error handling in the module init and exit functions can be shortened to safe us some code. 1/ Remove the code duplications in the init function, jump straight to the existing cleanup code by adding some labels. Also give the error message some more value by telling the reason why loading the module has failed. 2/ Remove the error handling in the exit function as the only legitimate reason xfrm6_protocol_deregister() might fail is inet6_del_protocol() returning -1. That, in turn, means some other protocol handler had been registered for this very protocol in the meantime. But that essentially means we haven't been handling that protocol any more, anyway. What it definitely means not is that we "can't deregister protocol". Therefore just get rid of that bogus warning. It's plain wrong. Signed-off-by: Mathias Krause Signed-off-by: Steffen Klassert --- net/ipv6/ip6_vti.c | 51 ++++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c index 9aaa6bb229e4..b61b0b1bb7ce 100644 --- a/net/ipv6/ip6_vti.c +++ b/net/ipv6/ip6_vti.c @@ -1089,36 +1089,26 @@ static struct xfrm6_protocol vti_ipcomp6_protocol __read_mostly = { **/ static int __init vti6_tunnel_init(void) { - int err; + const char *msg; + int err; + msg = "tunnel device"; err = register_pernet_device(&vti6_net_ops); if (err < 0) - goto out_pernet; + goto pernet_dev_failed; + msg = "tunnel protocols"; err = xfrm6_protocol_register(&vti_esp6_protocol, IPPROTO_ESP); - if (err < 0) { - pr_err("%s: can't register vti6 protocol\n", __func__); - - goto out; - } - + if (err < 0) + goto xfrm_proto_esp_failed; err = xfrm6_protocol_register(&vti_ah6_protocol, IPPROTO_AH); - if (err < 0) { - xfrm6_protocol_deregister(&vti_esp6_protocol, IPPROTO_ESP); - pr_err("%s: can't register vti6 protocol\n", __func__); - - goto out; - } - + if (err < 0) + goto xfrm_proto_ah_failed; err = xfrm6_protocol_register(&vti_ipcomp6_protocol, IPPROTO_COMP); - if (err < 0) { - xfrm6_protocol_deregister(&vti_ah6_protocol, IPPROTO_AH); - xfrm6_protocol_deregister(&vti_esp6_protocol, IPPROTO_ESP); - pr_err("%s: can't register vti6 protocol\n", __func__); - - goto out; - } + if (err < 0) + goto xfrm_proto_comp_failed; + msg = "netlink interface"; err = rtnl_link_register(&vti6_link_ops); if (err < 0) goto rtnl_link_failed; @@ -1127,11 +1117,14 @@ static int __init vti6_tunnel_init(void) rtnl_link_failed: xfrm6_protocol_deregister(&vti_ipcomp6_protocol, IPPROTO_COMP); +xfrm_proto_comp_failed: xfrm6_protocol_deregister(&vti_ah6_protocol, IPPROTO_AH); +xfrm_proto_ah_failed: xfrm6_protocol_deregister(&vti_esp6_protocol, IPPROTO_ESP); -out: +xfrm_proto_esp_failed: unregister_pernet_device(&vti6_net_ops); -out_pernet: +pernet_dev_failed: + pr_err("vti6 init: failed to register %s\n", msg); return err; } @@ -1141,13 +1134,9 @@ out_pernet: static void __exit vti6_tunnel_cleanup(void) { rtnl_link_unregister(&vti6_link_ops); - if (xfrm6_protocol_deregister(&vti_ipcomp6_protocol, IPPROTO_COMP)) - pr_info("%s: can't deregister protocol\n", __func__); - if (xfrm6_protocol_deregister(&vti_ah6_protocol, IPPROTO_AH)) - pr_info("%s: can't deregister protocol\n", __func__); - if (xfrm6_protocol_deregister(&vti_esp6_protocol, IPPROTO_ESP)) - pr_info("%s: can't deregister protocol\n", __func__); - + xfrm6_protocol_deregister(&vti_ipcomp6_protocol, IPPROTO_COMP); + xfrm6_protocol_deregister(&vti_ah6_protocol, IPPROTO_AH); + xfrm6_protocol_deregister(&vti_esp6_protocol, IPPROTO_ESP); unregister_pernet_device(&vti6_net_ops); } From 1990e4f883a6383b04ef37f32bdaa02dc0dbae8d Mon Sep 17 00:00:00 2001 From: Mathias Krause Date: Fri, 9 May 2014 23:43:42 +0200 Subject: [PATCH 2/3] vti: Simplify error handling in module init and exit The error handling in the module init and exit functions can be shortened to safe us some code. 1/ Remove the code duplications in the init function, jump straight to the existing cleanup code by adding some labels. Also give the error message some more value by telling the reason why loading the module has failed. Furthermore fix the "IPSec" typo -- it should be "IPsec" instead. 2/ Remove the error handling in the exit function as the only legitimate reason xfrm4_protocol_deregister() might fail is inet_del_protocol() returning -1. That, in turn, means some other protocol handler had been registered for this very protocol in the meantime. But that essentially means we haven't been handling that protocol any more, anyway. What it definitely means not is that we "can't deregister tunnel". Therefore just get rid of that bogus warning. It's plain wrong. Signed-off-by: Mathias Krause Signed-off-by: Steffen Klassert --- net/ipv4/ip_vti.c | 54 ++++++++++++++++++----------------------------- 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c index b8960f3527f3..e453cb724a95 100644 --- a/net/ipv4/ip_vti.c +++ b/net/ipv4/ip_vti.c @@ -534,40 +534,28 @@ static struct rtnl_link_ops vti_link_ops __read_mostly = { static int __init vti_init(void) { + const char *msg; int err; - pr_info("IPv4 over IPSec tunneling driver\n"); + pr_info("IPv4 over IPsec tunneling driver\n"); + msg = "tunnel device"; err = register_pernet_device(&vti_net_ops); if (err < 0) - return err; + goto pernet_dev_failed; + + msg = "tunnel protocols"; err = xfrm4_protocol_register(&vti_esp4_protocol, IPPROTO_ESP); - if (err < 0) { - unregister_pernet_device(&vti_net_ops); - pr_info("vti init: can't register tunnel\n"); - - return err; - } - + if (err < 0) + goto xfrm_proto_esp_failed; err = xfrm4_protocol_register(&vti_ah4_protocol, IPPROTO_AH); - if (err < 0) { - xfrm4_protocol_deregister(&vti_esp4_protocol, IPPROTO_ESP); - unregister_pernet_device(&vti_net_ops); - pr_info("vti init: can't register tunnel\n"); - - return err; - } - + if (err < 0) + goto xfrm_proto_ah_failed; err = xfrm4_protocol_register(&vti_ipcomp4_protocol, IPPROTO_COMP); - if (err < 0) { - xfrm4_protocol_deregister(&vti_ah4_protocol, IPPROTO_AH); - xfrm4_protocol_deregister(&vti_esp4_protocol, IPPROTO_ESP); - unregister_pernet_device(&vti_net_ops); - pr_info("vti init: can't register tunnel\n"); - - return err; - } + if (err < 0) + goto xfrm_proto_comp_failed; + msg = "netlink interface"; err = rtnl_link_register(&vti_link_ops); if (err < 0) goto rtnl_link_failed; @@ -576,23 +564,23 @@ static int __init vti_init(void) rtnl_link_failed: xfrm4_protocol_deregister(&vti_ipcomp4_protocol, IPPROTO_COMP); +xfrm_proto_comp_failed: xfrm4_protocol_deregister(&vti_ah4_protocol, IPPROTO_AH); +xfrm_proto_ah_failed: xfrm4_protocol_deregister(&vti_esp4_protocol, IPPROTO_ESP); +xfrm_proto_esp_failed: unregister_pernet_device(&vti_net_ops); +pernet_dev_failed: + pr_err("vti init: failed to register %s\n", msg); return err; } static void __exit vti_fini(void) { rtnl_link_unregister(&vti_link_ops); - if (xfrm4_protocol_deregister(&vti_ipcomp4_protocol, IPPROTO_COMP)) - pr_info("vti close: can't deregister tunnel\n"); - if (xfrm4_protocol_deregister(&vti_ah4_protocol, IPPROTO_AH)) - pr_info("vti close: can't deregister tunnel\n"); - if (xfrm4_protocol_deregister(&vti_esp4_protocol, IPPROTO_ESP)) - pr_info("vti close: can't deregister tunnel\n"); - - + xfrm4_protocol_deregister(&vti_ipcomp4_protocol, IPPROTO_COMP); + xfrm4_protocol_deregister(&vti_ah4_protocol, IPPROTO_AH); + xfrm4_protocol_deregister(&vti_esp4_protocol, IPPROTO_ESP); unregister_pernet_device(&vti_net_ops); } From 1759389e8af46d724220785bf710b7bdbebdfa48 Mon Sep 17 00:00:00 2001 From: Christoph Paasch Date: Sat, 28 Jun 2014 14:12:44 +0200 Subject: [PATCH 3/3] xfrm4: Remove duplicate semicolon 3328715e6c1fc (xfrm4: Add IPsec protocol multiplexer) adds a duplicate semicolon after the return-statement. Although it has no negative impact, the second semicolon should be removed. Cc: Steffen Klassert Cc: Herbert Xu Signed-off-by: Christoph Paasch Signed-off-by: Steffen Klassert --- net/ipv4/xfrm4_protocol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv4/xfrm4_protocol.c b/net/ipv4/xfrm4_protocol.c index a2ce0101eaac..dccefa9d84cf 100644 --- a/net/ipv4/xfrm4_protocol.c +++ b/net/ipv4/xfrm4_protocol.c @@ -124,7 +124,7 @@ static int xfrm4_ah_rcv(struct sk_buff *skb) for_each_protocol_rcu(ah4_handlers, handler) if ((ret = handler->handler(skb)) != -EINVAL) - return ret;; + return ret; icmp_send(skb, ICMP_DEST_UNREACH, ICMP_PORT_UNREACH, 0);