* [PATCH 0/2] iproute2: add GENL helpers
@ 2012-09-08 9:48 Julian Anastasov
2012-09-08 9:48 ` [PATCH 1/2] iproute2: add libgenl files Julian Anastasov
2012-09-08 9:48 ` [PATCH 2/2] iproute2: use libgenl in ipl2tp Julian Anastasov
0 siblings, 2 replies; 7+ messages in thread
From: Julian Anastasov @ 2012-09-08 9:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Put common code for GENL users in new files.
I'll resend the tcp_metrics support for iproute2 when
the kernel part is in mainline.
Julian Anastasov (2):
iproute2: add libgenl files
iproute2: use libgenl in ipl2tp
include/libgenl.h | 25 ++++++++++++++++++++
ip/ipl2tp.c | 65 ++--------------------------------------------------
lib/Makefile | 2 +-
lib/libgenl.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 94 insertions(+), 63 deletions(-)
create mode 100644 include/libgenl.h
create mode 100644 lib/libgenl.c
--
1.7.3.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] iproute2: add libgenl files
2012-09-08 9:48 [PATCH 0/2] iproute2: add GENL helpers Julian Anastasov
@ 2012-09-08 9:48 ` Julian Anastasov
2012-09-10 16:55 ` Stephen Hemminger
2012-09-10 16:56 ` Stephen Hemminger
2012-09-08 9:48 ` [PATCH 2/2] iproute2: use libgenl in ipl2tp Julian Anastasov
1 sibling, 2 replies; 7+ messages in thread
From: Julian Anastasov @ 2012-09-08 9:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Create libgenl.h and libgenl.c. They will contain
common code for GENL users such as ipl2tp, tcp_metrics, etc.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
include/libgenl.h | 25 ++++++++++++++++++++
lib/Makefile | 2 +-
lib/libgenl.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 91 insertions(+), 1 deletions(-)
create mode 100644 include/libgenl.h
create mode 100644 lib/libgenl.c
diff --git a/include/libgenl.h b/include/libgenl.h
new file mode 100644
index 0000000..ef455c1
--- /dev/null
+++ b/include/libgenl.h
@@ -0,0 +1,25 @@
+#ifndef __LIBGENL_H__
+#define __LIBGENL_H__
+
+#include "libnetlink.h"
+
+#define GENL_DEFINE_REQUEST(req, hdrsize, bufsiz) \
+struct { \
+ struct nlmsghdr n; \
+ struct genlmsghdr g; \
+ char buf[NLMSG_ALIGN(hdrsize) + bufsiz]; \
+} req
+
+#define GENL_INIT_REQUEST(req, family, hdrsize, ver, cmd_, flags) \
+ do { \
+ memset(&req, 0, sizeof(req)); \
+ req.n.nlmsg_type = family; \
+ req.n.nlmsg_flags = flags; \
+ req.n.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN + hdrsize); \
+ req.g.cmd = cmd_; \
+ req.g.version = ver; \
+ } while (0)
+
+int libgenl_resolve_family(struct rtnl_handle *grth, const char *family);
+
+#endif /* __LIBGENL_H__ */
diff --git a/lib/Makefile b/lib/Makefile
index da2f0fc..bfbe672 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -2,7 +2,7 @@ CFLAGS += -fPIC
UTILOBJ=utils.o rt_names.o ll_types.o ll_proto.o ll_addr.o inet_proto.o
-NLOBJ=ll_map.o libnetlink.o
+NLOBJ=libgenl.o ll_map.o libnetlink.o
all: libnetlink.a libutil.a
diff --git a/lib/libgenl.c b/lib/libgenl.c
new file mode 100644
index 0000000..c1bf399
--- /dev/null
+++ b/lib/libgenl.c
@@ -0,0 +1,65 @@
+/*
+ * libgenl.c GENL library
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <linux/genetlink.h>
+#include "libgenl.h"
+
+static int libgenl_parse_getfamily(struct nlmsghdr *nlh)
+{
+ struct rtattr *tb[CTRL_ATTR_MAX + 1];
+ struct genlmsghdr *ghdr = NLMSG_DATA(nlh);
+ int len = nlh->nlmsg_len;
+ struct rtattr *attrs;
+
+ if (nlh->nlmsg_type != GENL_ID_CTRL) {
+ fprintf(stderr, "Not a controller message, nlmsg_len=%d "
+ "nlmsg_type=0x%x\n", nlh->nlmsg_len, nlh->nlmsg_type);
+ return -1;
+ }
+
+ len -= NLMSG_LENGTH(GENL_HDRLEN);
+
+ if (len < 0) {
+ fprintf(stderr, "wrong controller message len %d\n", len);
+ return -1;
+ }
+
+ if (ghdr->cmd != CTRL_CMD_NEWFAMILY) {
+ fprintf(stderr, "Unknown controller command %d\n", ghdr->cmd);
+ return -1;
+ }
+
+ attrs = (struct rtattr *) ((char *) ghdr + GENL_HDRLEN);
+ parse_rtattr(tb, CTRL_ATTR_MAX, attrs, len);
+
+ if (tb[CTRL_ATTR_FAMILY_ID] == NULL) {
+ fprintf(stderr, "Missing family id TLV\n");
+ return -1;
+ }
+
+ return rta_getattr_u16(tb[CTRL_ATTR_FAMILY_ID]);
+}
+
+int libgenl_resolve_family(struct rtnl_handle *grth, const char *family)
+{
+ GENL_DEFINE_REQUEST(req, 0, 1024);
+
+ GENL_INIT_REQUEST(req, GENL_ID_CTRL, 0, 0, CTRL_CMD_GETFAMILY,
+ NLM_F_REQUEST);
+
+ addattr_l(&req.n, 1024, CTRL_ATTR_FAMILY_NAME,
+ family, strlen(family) + 1);
+
+ if (rtnl_talk(grth, &req.n, 0, 0, &req.n) < 0) {
+ fprintf(stderr, "Error talking to the kernel\n");
+ return -2;
+ }
+
+ return libgenl_parse_getfamily(&req.n);
+}
+
--
1.7.3.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] iproute2: use libgenl in ipl2tp
2012-09-08 9:48 [PATCH 0/2] iproute2: add GENL helpers Julian Anastasov
2012-09-08 9:48 ` [PATCH 1/2] iproute2: add libgenl files Julian Anastasov
@ 2012-09-08 9:48 ` Julian Anastasov
1 sibling, 0 replies; 7+ messages in thread
From: Julian Anastasov @ 2012-09-08 9:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Use the common code from libgenl.c to parse family.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
ip/ipl2tp.c | 65 ++--------------------------------------------------------
1 files changed, 3 insertions(+), 62 deletions(-)
diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index 1cbed8d..c69091d 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -25,6 +25,7 @@
#include <linux/genetlink.h>
#include <linux/l2tp.h>
+#include "libgenl.h"
#include "utils.h"
#include "ip_common.h"
@@ -747,67 +748,6 @@ static int do_show(int argc, char **argv)
return 0;
}
-static int genl_parse_getfamily(struct nlmsghdr *nlh)
-{
- struct rtattr *tb[CTRL_ATTR_MAX + 1];
- struct genlmsghdr *ghdr = NLMSG_DATA(nlh);
- int len = nlh->nlmsg_len;
- struct rtattr *attrs;
-
- if (nlh->nlmsg_type != GENL_ID_CTRL) {
- fprintf(stderr, "Not a controller message, nlmsg_len=%d "
- "nlmsg_type=0x%x\n", nlh->nlmsg_len, nlh->nlmsg_type);
- return -1;
- }
-
- if (ghdr->cmd != CTRL_CMD_NEWFAMILY) {
- fprintf(stderr, "Unknown controller command %d\n", ghdr->cmd);
- return -1;
- }
-
- len -= NLMSG_LENGTH(GENL_HDRLEN);
-
- if (len < 0) {
- fprintf(stderr, "wrong controller message len %d\n", len);
- return -1;
- }
-
- attrs = (struct rtattr *) ((char *) ghdr + GENL_HDRLEN);
- parse_rtattr(tb, CTRL_ATTR_MAX, attrs, len);
-
- if (tb[CTRL_ATTR_FAMILY_ID] == NULL) {
- fprintf(stderr, "Missing family id TLV\n");
- return -1;
- }
-
- return rta_getattr_u16(tb[CTRL_ATTR_FAMILY_ID]);
-}
-
-int genl_ctrl_resolve_family(const char *family)
-{
- struct {
- struct nlmsghdr n;
- struct genlmsghdr g;
- char buf[1024];
- } req;
-
- memset(&req, 0, sizeof(req));
- req.n.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN);
- req.n.nlmsg_flags = NLM_F_REQUEST;
- req.n.nlmsg_type = GENL_ID_CTRL;
- req.g.cmd = CTRL_CMD_GETFAMILY;
-
- addattr_l(&req.n, 1024, CTRL_ATTR_FAMILY_NAME,
- family, strlen(family) + 1);
-
- if (rtnl_talk(&genl_rth, &req.n, 0, 0, &req.n) < 0) {
- fprintf(stderr, "Error talking to the kernel\n");
- return -2;
- }
-
- return genl_parse_getfamily(&req.n);
-}
-
int do_ipl2tp(int argc, char **argv)
{
if (genl_family < 0) {
@@ -816,7 +756,8 @@ int do_ipl2tp(int argc, char **argv)
exit(1);
}
- genl_family = genl_ctrl_resolve_family(L2TP_GENL_NAME);
+ genl_family = libgenl_resolve_family(&genl_rth,
+ L2TP_GENL_NAME);
if (genl_family < 0)
exit(1);
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iproute2: add libgenl files
2012-09-08 9:48 ` [PATCH 1/2] iproute2: add libgenl files Julian Anastasov
@ 2012-09-10 16:55 ` Stephen Hemminger
2012-09-10 21:54 ` Julian Anastasov
2012-09-10 16:56 ` Stephen Hemminger
1 sibling, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2012-09-10 16:55 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev
On Sat, 8 Sep 2012 12:48:24 +0300
Julian Anastasov <ja@ssi.bg> wrote:
> +
> +#define GENL_INIT_REQUEST(req, family, hdrsize, ver, cmd_, flags) \
> + do { \
> + memset(&req, 0, sizeof(req)); \
> + req.n.nlmsg_type = family; \
> + req.n.nlmsg_flags = flags; \
> + req.n.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN + hdrsize); \
> + req.g.cmd = cmd_; \
> + req.g.version = ver; \
> + } while (0)
> +
Why not an inline function, macro code is error prone.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iproute2: add libgenl files
2012-09-08 9:48 ` [PATCH 1/2] iproute2: add libgenl files Julian Anastasov
2012-09-10 16:55 ` Stephen Hemminger
@ 2012-09-10 16:56 ` Stephen Hemminger
1 sibling, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2012-09-10 16:56 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev
On Sat, 8 Sep 2012 12:48:24 +0300
Julian Anastasov <ja@ssi.bg> wrote:
> +static int libgenl_parse_getfamily(struct nlmsghdr *nlh)
I prefer just using 'genl' rather than 'libgenl' here. Shorter variable
names are clearer.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iproute2: add libgenl files
2012-09-10 16:55 ` Stephen Hemminger
@ 2012-09-10 21:54 ` Julian Anastasov
2012-09-10 22:42 ` Stephen Hemminger
0 siblings, 1 reply; 7+ messages in thread
From: Julian Anastasov @ 2012-09-10 21:54 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Hello,
On Mon, 10 Sep 2012, Stephen Hemminger wrote:
> On Sat, 8 Sep 2012 12:48:24 +0300
> Julian Anastasov <ja@ssi.bg> wrote:
>
> > +
> > +#define GENL_INIT_REQUEST(req, family, hdrsize, ver, cmd_, flags) \
> > + do { \
> > + memset(&req, 0, sizeof(req)); \
> > + req.n.nlmsg_type = family; \
> > + req.n.nlmsg_flags = flags; \
> > + req.n.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN + hdrsize); \
> > + req.g.cmd = cmd_; \
> > + req.g.version = ver; \
> > + } while (0)
> > +
>
> Why not an inline function, macro code is error prone.
The problem is that we define every request with
its own structure in GENL_DEFINE_REQUEST. We can use init
func instead of macro only if we define some base structure,
for example:
struct genl_header_base {
struct nlmsghdr n;
struct genlmsghdr g;
};
But then we will have new prefix when accessing
request fields (b.): b.n... and b.g... It will go to all places
that use addattr_*, eg. addattr_l(&req.b.n, sizeof(req), ...)
and such places can be many if there are many attributes.
#define GENL_DEFINE_REQUEST(req, hdrsize, bufsiz) \
struct { \
struct genl_header_base b; \
char buf[NLMSG_ALIGN(hdrsize) + bufsiz]; \
} req
int genl_init_request(struct genl_header_base *b,
b_size, family, hdrsize, ver, cmd_, flags)
Do you prefer this new variant? There will be many
places like req.n that will be changed like req.b.n.
I'll rename libgenl_ to genl_ when we decide what
to do with the request structure.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iproute2: add libgenl files
2012-09-10 21:54 ` Julian Anastasov
@ 2012-09-10 22:42 ` Stephen Hemminger
0 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2012-09-10 22:42 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev
On Tue, 11 Sep 2012 00:54:00 +0300 (EEST)
Julian Anastasov <ja@ssi.bg> wrote:
> > Why not an inline function, macro code is error prone.
>
> The problem is that we define every request with
> its own structure in GENL_DEFINE_REQUEST. We can use init
> func instead of macro only if we define some base structure,
> for example:
>
> struct genl_header_base {
> struct nlmsghdr n;
> struct genlmsghdr g;
> };
Ok, that makes sense. I prefer C99 style initializers
in general. Maybe:
#define GENL_INIT_REQUEST(family, hdrsize, ver, cmd_, flags) { \
.req = { \
.n = { \
.nlmsg_type = (family), \
.nlmsg_flags = (flags), \
.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN + (hdrsize)), \
} \
.g = { \
.cmd = (cmd), \
.version = (ver), \
} \
}
Or merge GENL_DEFINE_REQUEST and GENL_INIT_REQUEST.
These are all nits, and not that important, just want to make it as simple
as possible. So if you like it okay as it was, we can just use that.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-09-10 22:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-08 9:48 [PATCH 0/2] iproute2: add GENL helpers Julian Anastasov
2012-09-08 9:48 ` [PATCH 1/2] iproute2: add libgenl files Julian Anastasov
2012-09-10 16:55 ` Stephen Hemminger
2012-09-10 21:54 ` Julian Anastasov
2012-09-10 22:42 ` Stephen Hemminger
2012-09-10 16:56 ` Stephen Hemminger
2012-09-08 9:48 ` [PATCH 2/2] iproute2: use libgenl in ipl2tp Julian Anastasov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).