| Message ID | 1448544365-23153-1-git-send-email-phil@nwl.cc |
|---|---|
| State | Accepted, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
Phil Sutter <phil@nwl.cc> writes: > This macro aims to simplify most netlink users' pattern to prepare a > request, which is to create an unnamed struct and initialize it: > > | struct { > | struct nlmsghdr n; > | struct whatever foo; > | char buf[arbitrary number]; > | } req; > | > | memset(&req, 0, sizeof(req)); > | req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct whatever)); > | req.n.nlmsg_flags = NLM_F_REQUEST; > > Having this patch applied, the above can be replaced by a static > initializer like so: > > | DECLARE_NLREQ(req, n, struct whatever foo, arbitrary number); > > There is an added benefit, as well: Due to explicit alignment, the > requested tailroom is really as big as requested no matter what size > struct whatever really is. > > Signed-off-by: Phil Sutter <phil@nwl.cc> > --- > This patch is RFC because I want to wait for peer review and upstream > acceptance before sending in the big refactoring patch itself. > --- > include/libnetlink.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/libnetlink.h b/include/libnetlink.h > index 2280c39c670a4..7e12a7b12b55d 100644 > --- a/include/libnetlink.h > +++ b/include/libnetlink.h > @@ -196,5 +196,16 @@ int rtnl_from_file(FILE *, rtnl_listen_filter_t handler, > * messages from dump file */ > #define NLMSG_TSTAMP 15 > > +/* declare a netlink request with initial payload */ > +#define DECLARE_NLREQ(name, hdrname, payload, tailroom) \ > + struct { \ > + struct nlmsghdr hdrname; \ > + payload; \ > + char __b[tailroom] __attribute__((aligned(NLMSG_ALIGNTO))); \ > + } name = { .hdrname = { \ > + .nlmsg_len = (unsigned long)&name.__b - (unsigned long)&name, \ offsetof(typeof(name), __b) ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 26, 2015 at 02:56:30PM +0100, Hannes Frederic Sowa wrote: > > +#define DECLARE_NLREQ(name, hdrname, payload, tailroom) \ > > + struct { \ > > + struct nlmsghdr hdrname; \ > > + payload; \ > > + char __b[tailroom] __attribute__((aligned(NLMSG_ALIGNTO))); \ > > + } name = { .hdrname = { \ > > + .nlmsg_len = (unsigned long)&name.__b - (unsigned long)&name, \ > > offsetof(typeof(name), __b) ? Ah, thanks! I already considered offsetof(), but the fact it needs a type name and the declared struct is anonymous appeared unsolvable to me. Good to know typeof() can be used this way! Thanks, Phil -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 26 Nov 2015 14:26:05 +0100 Phil Sutter <phil@nwl.cc> wrote: > This macro aims to simplify most netlink users' pattern to prepare a > request, which is to create an unnamed struct and initialize it: > > | struct { > | struct nlmsghdr n; > | struct whatever foo; > | char buf[arbitrary number]; > | } req; > | > | memset(&req, 0, sizeof(req)); > | req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct whatever)); > | req.n.nlmsg_flags = NLM_F_REQUEST; > > Having this patch applied, the above can be replaced by a static > initializer like so: > > | DECLARE_NLREQ(req, n, struct whatever foo, arbitrary number); > > There is an added benefit, as well: Due to explicit alignment, the > requested tailroom is really as big as requested no matter what size > struct whatever really is. > > Signed-off-by: Phil Sutter <phil@nwl.cc> > --- > This patch is RFC because I want to wait for peer review and upstream > acceptance before sending in the big refactoring patch itself. > --- > include/libnetlink.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) I am not a fan of complex macros. But netlink seems to get lots of them. You need to add more parens round arguments (like name). Really longterm would rather iproute2 switched to a cleaner library like libmnl -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Sun, Nov 29, 2015 at 12:07:52PM -0800, Stephen Hemminger wrote: > On Thu, 26 Nov 2015 14:26:05 +0100 > Phil Sutter <phil@nwl.cc> wrote: > > > This macro aims to simplify most netlink users' pattern to prepare a > > request, which is to create an unnamed struct and initialize it: > > > > | struct { > > | struct nlmsghdr n; > > | struct whatever foo; > > | char buf[arbitrary number]; > > | } req; > > | > > | memset(&req, 0, sizeof(req)); > > | req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct whatever)); > > | req.n.nlmsg_flags = NLM_F_REQUEST; > > > > Having this patch applied, the above can be replaced by a static > > initializer like so: > > > > | DECLARE_NLREQ(req, n, struct whatever foo, arbitrary number); > > > > There is an added benefit, as well: Due to explicit alignment, the > > requested tailroom is really as big as requested no matter what size > > struct whatever really is. > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > --- > > This patch is RFC because I want to wait for peer review and upstream > > acceptance before sending in the big refactoring patch itself. > > --- > > include/libnetlink.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > I am not a fan of complex macros. But netlink seems to get lots of them. > You need to add more parens round arguments (like name). > > Really longterm would rather iproute2 switched to a cleaner library like libmnl Ah, indeed! Someone else suggested that once, as well. I'll have a look into that before continuing this path. Thanks, Phil -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlZbcNwACgkQnMPprxY1hCeZNwCfazSwuC8W6DU3vnY12TEwX6yp d6YAn2JjJYPObXxGBOH/8e+GgY2zIeEq =wbf7 -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Nov 29, 2015 at 12:07:52PM -0800, Stephen Hemminger wrote: > On Thu, 26 Nov 2015 14:26:05 +0100 > Phil Sutter <phil@nwl.cc> wrote: > > > This macro aims to simplify most netlink users' pattern to prepare a > > request, which is to create an unnamed struct and initialize it: > > > > | struct { > > | struct nlmsghdr n; > > | struct whatever foo; > > | char buf[arbitrary number]; > > | } req; > > | > > | memset(&req, 0, sizeof(req)); > > | req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct whatever)); > > | req.n.nlmsg_flags = NLM_F_REQUEST; > > > > Having this patch applied, the above can be replaced by a static > > initializer like so: > > > > | DECLARE_NLREQ(req, n, struct whatever foo, arbitrary number); > > > > There is an added benefit, as well: Due to explicit alignment, the > > requested tailroom is really as big as requested no matter what size > > struct whatever really is. > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > --- > > This patch is RFC because I want to wait for peer review and upstream > > acceptance before sending in the big refactoring patch itself. > > --- > > include/libnetlink.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > I am not a fan of complex macros. But netlink seems to get lots of them. > You need to add more parens round arguments (like name). > > Really longterm would rather iproute2 switched to a cleaner library like libmnl libmnl looks nice and simple (unlike libnl I was initially looking at by accident). Now how to pull this off: I don't think mandatorily depending on libmnl will be acceptable, do you? So I can imagine two ways to do this: A) Have a libmnl version of lib/libnetlink.c which is used instead of the old one if libmnl is present. B) Pull a copy of libmnl into iproute2 sources so it's always available (as fallback) and make it replace lib/libnetlink.c. This sounds worse than it is, using git-subtree allows to do this without imposing user knowledge about it (like git-submodule does). What do you think? Cheers, Phil -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 30 Nov 2015 16:47:25 +0100 Phil Sutter <phil@nwl.cc> wrote: > libmnl looks nice and simple (unlike libnl I was initially looking at by > accident). Now how to pull this off: > > I don't think mandatorily depending on libmnl will be acceptable, do > you? So I can imagine two ways to do this: Having libmnl be mandatory is fine, but please put in net-next. Every distro has libmnl and as long as it is documented not a big deal. > A) Have a libmnl version of lib/libnetlink.c which is used instead of > the old one if libmnl is present. > > B) Pull a copy of libmnl into iproute2 sources so it's always available > (as fallback) and make it replace lib/libnetlink.c. This sounds worse > than it is, using git-subtree allows to do this without imposing user > knowledge about it (like git-submodule does). Just incrementally change code to use libmnl instead of libnetlink. Start with simple stuff. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/libnetlink.h b/include/libnetlink.h index 2280c39c670a4..7e12a7b12b55d 100644 --- a/include/libnetlink.h +++ b/include/libnetlink.h @@ -196,5 +196,16 @@ int rtnl_from_file(FILE *, rtnl_listen_filter_t handler, * messages from dump file */ #define NLMSG_TSTAMP 15 +/* declare a netlink request with initial payload */ +#define DECLARE_NLREQ(name, hdrname, payload, tailroom) \ + struct { \ + struct nlmsghdr hdrname; \ + payload; \ + char __b[tailroom] __attribute__((aligned(NLMSG_ALIGNTO))); \ + } name = { .hdrname = { \ + .nlmsg_len = (unsigned long)&name.__b - (unsigned long)&name, \ + .nlmsg_flags = NLM_F_REQUEST, \ + }} + #endif /* __LIBNETLINK_H__ */
This macro aims to simplify most netlink users' pattern to prepare a request, which is to create an unnamed struct and initialize it: | struct { | struct nlmsghdr n; | struct whatever foo; | char buf[arbitrary number]; | } req; | | memset(&req, 0, sizeof(req)); | req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct whatever)); | req.n.nlmsg_flags = NLM_F_REQUEST; Having this patch applied, the above can be replaced by a static initializer like so: | DECLARE_NLREQ(req, n, struct whatever foo, arbitrary number); There is an added benefit, as well: Due to explicit alignment, the requested tailroom is really as big as requested no matter what size struct whatever really is. Signed-off-by: Phil Sutter <phil@nwl.cc> --- This patch is RFC because I want to wait for peer review and upstream acceptance before sending in the big refactoring patch itself. --- include/libnetlink.h | 11 +++++++++++ 1 file changed, 11 insertions(+)