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
@@ -0,0 +1,65 @@+/*+*libgenl.cGENLlibrary+*/++#include<stdio.h>+#include<stdlib.h>+#include<unistd.h>++#include<linux/genetlink.h>+#include"libgenl.h"++staticintlibgenl_parse_getfamily(structnlmsghdr*nlh)+{+structrtattr*tb[CTRL_ATTR_MAX+1];+structgenlmsghdr*ghdr=NLMSG_DATA(nlh);+intlen=nlh->nlmsg_len;+structrtattr*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=(structrtattr*)((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;+}++returnrta_getattr_u16(tb[CTRL_ATTR_FAMILY_ID]);+}++intlibgenl_resolve_family(structrtnl_handle*grth,constchar*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;+}++returnlibgenl_parse_getfamily(&req.n);+}+
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(-)
@@ -747,67 +748,6 @@ static int do_show(int argc, char **argv)return0;}-staticintgenl_parse_getfamily(structnlmsghdr*nlh)-{-structrtattr*tb[CTRL_ATTR_MAX+1];-structgenlmsghdr*ghdr=NLMSG_DATA(nlh);-intlen=nlh->nlmsg_len;-structrtattr*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=(structrtattr*)((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;-}--returnrta_getattr_u16(tb[CTRL_ATTR_FAMILY_ID]);-}--intgenl_ctrl_resolve_family(constchar*family)-{-struct{-structnlmsghdrn;-structgenlmsghdrg;-charbuf[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;-}--returngenl_parse_getfamily(&req.n);-}-intdo_ipl2tp(intargc,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);}
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 [off-list ref]
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.