Skip to contentSkip to conversation

improve initialization of genl handle

PATCH In review Sabrina Dubroca 2 participants 6 messages Aug 16, 2016 – Aug 23, 2016

Subsystems (3)
Networking Network drivers Library code
lore
Share & tools
lore

Sabrina Dubroca

author
#0 lore raw

cover improve initialization of genl handle

This patchset fixes several issues for users of genetlink:

- Duplicated code for initialization of a genl handle. All users of genl must open a socket and resolve the family, and use the same code to do that.
Solution: add a library function.

- genl family resolution fails when the module that handles this family is not loaded yet.
This means that if we try to initialize the genl handle early in handling the command, we cannot display usage, because we fail hard when trying to resolve the genl family.
Solution: move the genl handle initialization after handling the ip * help case.

Sabrina Dubroca (4):
libgenl: introduce genl_init_handle
macsec: show usage even if the module is not available
fou: show usage even if the module is not available
ila: show usage even if the module is not available

 include/libgenl.h |  2 ++
 ip/ipfou.c        | 20 ++++++--------------
 ip/ipila.c        | 19 ++++++-------------
 ip/ipl2tp.c       | 12 ++----------
 ip/ipmacsec.c     | 20 +++-----------------
 ip/tcp_metrics.c  | 14 +++-----------
 lib/libgenl.c     | 17 +++++++++++++++++
 7 files changed, 39 insertions(+), 65 deletions(-)
Signature · 1 line
2.9.3
author
#1 lore raw

PATCH 1/4 libgenl: introduce genl_init_handle

All users of genl have the same code to open a genl socket and resolve the family for their specific protocol. Introduce a helper to intialize the handle, and use it in all the genl code.

Signed-off-by Sabrina Dubroca <[email protected]>
---
 include/libgenl.h |  2 ++
 ip/ipfou.c        | 12 ++----------
 ip/ipila.c        | 12 ++----------
 ip/ipl2tp.c       | 12 ++----------
 ip/ipmacsec.c     | 18 ++----------------
 ip/tcp_metrics.c  | 14 +++-----------
 lib/libgenl.c     | 17 +++++++++++++++++
 7 files changed, 30 insertions(+), 57 deletions(-)
include/libgenl.h +2 -0
index 9db4bafd511b..2dbb4b36adcb 100644
@@ -21,5 +21,7 @@ struct {								\
 }
 
 extern int genl_resolve_family(struct rtnl_handle *grth, const char *family);
+extern int genl_init_handle(struct rtnl_handle *grth, const char *family,
+			    int *genl_family);
 
 #endif /* __LIBGENL_H__ */
ip/ipfou.c +2 -10
index 0673d11ff6f9..9f0911215749 100644
@@ -136,16 +136,8 @@ static int do_del(int argc, char **argv)
 
 int do_ipfou(int argc, char **argv)
 {
-	if (genl_family < 0) {
-		if (rtnl_open_byproto(&genl_rth, 0, NETLINK_GENERIC) < 0) {
-			fprintf(stderr, "Cannot open generic netlink socket\n");
-			exit(1);
-		}
-
-		genl_family = genl_resolve_family(&genl_rth, FOU_GENL_NAME);
-		if (genl_family < 0)
-			exit(1);
-	}
+	if (genl_init_handle(&genl_rth, FOU_GENL_NAME, &genl_family))
+		exit(1);
 
 	if (argc < 1)
 		usage();
ip/ipila.c +2 -10
index c30bdbf166a6..244245ca3c40 100644
@@ -237,16 +237,8 @@ static int do_del(int argc, char **argv)
 
 int do_ipila(int argc, char **argv)
 {
-	if (genl_family < 0) {
-		if (rtnl_open_byproto(&genl_rth, 0, NETLINK_GENERIC) < 0) {
-			fprintf(stderr, "Cannot open generic netlink socket\n");
-			exit(1);
-		}
-
-		genl_family = genl_resolve_family(&genl_rth, ILA_GENL_NAME);
-		if (genl_family < 0)
-			exit(1);
-	}
+	if (genl_init_handle(&genl_rth, ILA_GENL_NAME, &genl_family))
+		exit(1);
 
 	if (argc < 1)
 		usage();
ip/ipl2tp.c +2 -10
index 9ebda135e5d6..d3338acecc9c 100644
@@ -767,16 +767,8 @@ int do_ipl2tp(int argc, char **argv)
 	if (argc < 1 || !matches(*argv, "help"))
 		usage();
 
-	if (genl_family < 0) {
-		if (rtnl_open_byproto(&genl_rth, 0, NETLINK_GENERIC) < 0) {
-			fprintf(stderr, "Cannot open generic netlink socket\n");
-			exit(1);
-		}
-
-		genl_family = genl_resolve_family(&genl_rth, L2TP_GENL_NAME);
-		if (genl_family < 0)
-			exit(1);
-	}
+	if (genl_init_handle(&genl_rth, L2TP_GENL_NAME, &genl_family))
+		exit(1);
 
 	if (matches(*argv, "add") == 0)
 		return do_add(argc-1, argv+1);
ip/ipmacsec.c +2 -16
index 329be00fb32b..9eabfe241980 100644
@@ -79,21 +79,6 @@ static int genl_family = -1;
 		     _cmd, _flags)
 
 
-static void init_genl(void)
-{
-	if (genl_family >= 0)
-		return;
-
-	if (rtnl_open_byproto(&genl_rth, 0, NETLINK_GENERIC) < 0) {
-		fprintf(stderr, "Cannot open generic netlink socket\n");
-		exit(1);
-	}
-
-	genl_family = genl_resolve_family(&genl_rth, MACSEC_GENL_NAME);
-	if (genl_family < 0)
-		exit(1);
-}
-
 static void ipmacsec_usage(void)
 {
 	fprintf(stderr, "Usage: ip macsec add DEV tx sa { 0..3 } [ OPTS ] key ID KEY\n");

@@ -1001,7 +986,8 @@ static int do_show(int argc, char **argv)
 
 int do_ipmacsec(int argc, char **argv)
 {
-	init_genl();
+	if (genl_init_handle(&genl_rth, MACSEC_GENL_NAME, &genl_family))
+		exit(1);
 
 	if (argc < 1)
 		ipmacsec_usage();
ip/tcp_metrics.c +3 -11
index ac2613a0a880..8972acd05fb2 100644
@@ -398,17 +398,9 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 		ack = 0;
 	}
 
-	if (genl_family < 0) {
-		if (rtnl_open_byproto(&grth, 0, NETLINK_GENERIC) < 0) {
-			fprintf(stderr, "Cannot open generic netlink socket\n");
-			exit(1);
-		}
-		genl_family = genl_resolve_family(&grth,
-						  TCP_METRICS_GENL_NAME);
-		if (genl_family < 0)
-			exit(1);
-		req.n.nlmsg_type = genl_family;
-	}
+	if (genl_init_handle(&grth, TCP_METRICS_GENL_NAME, &genl_family))
+		exit(1);
+	req.n.nlmsg_type = genl_family;
 
 	if (!(cmd & CMD_FLUSH) && (atype >= 0 || (cmd & CMD_DEL))) {
 		if (ack)
lib/libgenl.c +17 -0
index a1a37a440f64..5fba1d86c624 100644
@@ -60,3 +60,20 @@ int genl_resolve_family(struct rtnl_handle *grth, const char *family)
 
 	return genl_parse_getfamily(&req.n);
 }
+
+int genl_init_handle(struct rtnl_handle *grth, const char *family, int *genl_family)
+{
+	if (*genl_family >= 0)
+		return 0;
+
+	if (rtnl_open_byproto(grth, 0, NETLINK_GENERIC) < 0) {
+		fprintf(stderr, "Cannot open generic netlink socket\n");
+		return -1;
+	}
+
+	*genl_family = genl_resolve_family(grth, family);
+	if (*genl_family < 0)
+		return -1;
+
+	return 0;
+}
Signature · 1 line
2.9.3
author
#2 lore raw

PATCH 2/4 macsec: show usage even if the module is not available

In reply to Sabrina Dubroca

Currently, the ip macsec command tries to initialize a genl context even when we just want to see the help for the command, which doesn't require to talk to the kernel at all.

Delay genl initialization, which can fail if the module isn't loaded, until the point where we will actually need it.

Fixes b26fc590ce62 ("ip: add MACsec support")
Signed-off-by Sabrina Dubroca <[email protected]>
---
 ip/ipmacsec.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
ip/ipmacsec.c +3 -3
index 9eabfe241980..6bd1f54fbfd7 100644
@@ -986,15 +986,15 @@ static int do_show(int argc, char **argv)
 
 int do_ipmacsec(int argc, char **argv)
 {
-	if (genl_init_handle(&genl_rth, MACSEC_GENL_NAME, &genl_family))
-		exit(1);
-
 	if (argc < 1)
 		ipmacsec_usage();
 
 	if (matches(*argv, "help") == 0)
 		ipmacsec_usage();
 
+	if (genl_init_handle(&genl_rth, MACSEC_GENL_NAME, &genl_family))
+		exit(1);
+
 	if (matches(*argv, "show") == 0)
 		return do_show(argc-1, argv+1);
Signature · 1 line
2.9.3
author
#3 lore raw

PATCH 3/4 fou: show usage even if the module is not available

In reply to Sabrina Dubroca

Currently, the ip fou command tries to initialize a genl context even when we just want to see the help for the command, which doesn't require to talk to the kernel at all.

Delay genl initialization, which can fail if the module isn't loaded, until the point where we will actually need it.

Fixes 6928747b6e79 ("ip fou: Support to configure foo-over-udp RX")
Signed-off-by Sabrina Dubroca <[email protected]>
---
 ip/ipfou.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
ip/ipfou.c +6 -6
index 9f0911215749..00dbe150710d 100644
@@ -136,19 +136,19 @@ static int do_del(int argc, char **argv)
 
 int do_ipfou(int argc, char **argv)
 {
-	if (genl_init_handle(&genl_rth, FOU_GENL_NAME, &genl_family))
-		exit(1);
-
 	if (argc < 1)
 		usage();
 
+	if (matches(*argv, "help") == 0)
+		usage();
+
+	if (genl_init_handle(&genl_rth, FOU_GENL_NAME, &genl_family))
+		exit(1);
+
 	if (matches(*argv, "add") == 0)
 		return do_add(argc-1, argv+1);
 	if (matches(*argv, "delete") == 0)
 		return do_del(argc-1, argv+1);
-	if (matches(*argv, "help") == 0)
-		usage();
-
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip fou help\".\n", *argv);
 	exit(-1);
 }
Signature · 1 line
2.9.3
author
#4 lore raw

PATCH 4/4 ila: show usage even if the module is not available

In reply to Sabrina Dubroca

Currently, the ip ila command tries to initialize a genl context even when we just want to see the help for the command, which doesn't require to talk to the kernel at all.

Delay genl initialization, which can fail if the module isn't loaded, until the point where we will actually need it.

Fixes ec71cae0bb7b ("ila: Support for configuring ila to use netfilter hook")
Signed-off-by Sabrina Dubroca <[email protected]>
---
 ip/ipila.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
ip/ipila.c +6 -5
index 244245ca3c40..27f55e961a43 100644
@@ -237,20 +237,21 @@ static int do_del(int argc, char **argv)
 
 int do_ipila(int argc, char **argv)
 {
-	if (genl_init_handle(&genl_rth, ILA_GENL_NAME, &genl_family))
-		exit(1);
-
 	if (argc < 1)
 		usage();
 
+	if (matches(*argv, "help") == 0)
+		usage();
+
+	if (genl_init_handle(&genl_rth, ILA_GENL_NAME, &genl_family))
+		exit(1);
+
 	if (matches(*argv, "add") == 0)
 		return do_add(argc-1, argv+1);
 	if (matches(*argv, "delete") == 0)
 		return do_del(argc-1, argv+1);
 	if (matches(*argv, "list") == 0)
 		return do_list(argc-1, argv+1);
-	if (matches(*argv, "help") == 0)
-		usage();
 
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip ila help\".\n",
 		*argv);
Signature · 1 line
2.9.3
Acked-by on the v1 series
#5 lore raw
In reply to Sabrina Dubroca
Quoted 21 lines · Sabrina Dubroca

This patchset fixes several issues for users of genetlink:

- Duplicated code for initialization of a genl handle. All users of genl must open a socket and resolve the family, and use the same code to do that.
Solution: add a library function.

- genl family resolution fails when the module that handles this family is not loaded yet.
This means that if we try to initialize the genl handle early in handling the command, we cannot display usage, because we fail hard when trying to resolve the genl family.
Solution: move the genl handle initialization after handling the ip * help case.

Sabrina Dubroca (4):
libgenl: introduce genl_init_handle
macsec: show usage even if the module is not available
fou: show usage even if the module is not available
ila: show usage even if the module is not available

Quoted 8 lines
 include/libgenl.h |  2 ++
 ip/ipfou.c        | 20 ++++++--------------
 ip/ipila.c        | 19 ++++++-------------
 ip/ipl2tp.c       | 12 ++----------
 ip/ipmacsec.c     | 20 +++-----------------
 ip/tcp_metrics.c  | 14 +++-----------
 lib/libgenl.c     | 17 +++++++++++++++++
 7 files changed, 39 insertions(+), 65 deletions(-)

Series:

Acked-by Phil Sutter <[email protected]>