Patchwork rtnl_wilddump_request: fix alignment issue for embedded platforms

login
register
mail settings
Submitter Lutz Jaenicke
Date Aug. 30, 2012, 3:01 p.m.
Message ID <1346338894-12600-1-git-send-email-ljaenicke@innominate.com>
Download mbox | patch
Permalink /patch/180823/
State Accepted
Delegated to: stephen hemminger
Headers show

Comments

Lutz Jaenicke - Aug. 30, 2012, 3:01 p.m.
Platforms have different alignment requirements which need to be
fulfilled by the compiler. If the structure elements are already
4 byte (NLMGS_ALIGNTO) aligned by the compiler adding an explicit
padding element (align_rta) is not allowed.
Use __attribute__ ((aligned (NLMSG_ALIGNTO))) in order to achieve
the required alignment.
Experienced on ARM (xscale) with symptom
  netlink: 12 bytes leftover after parsing attributes

Tested on:
  ARM      (32bit Big Endian)
  PowerPC  (32bit Big Endian)
  x86_64   (64bit Little Endian)
Each with different aligment requirments.

Signed-off-by: Lutz Jaenicke <ljaenicke@innominate.com>
---
 lib/libnetlink.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
David Laight - Aug. 30, 2012, 3:51 p.m.
> +		/* attribute has to be NLMSG aligned */
> +		struct rtattr ext_req
__attribute__((aligned(NLMSG_ALIGNTO)));

Would it be better to apply the attribute to the definition
of 'struct rtattr' itself ?

	David



--
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
Lutz Jaenicke - Aug. 30, 2012, 4:09 p.m.
On Thu, Aug 30, 2012 at 04:51:49PM +0100, David Laight wrote:
> > +		/* attribute has to be NLMSG aligned */
> > +		struct rtattr ext_req
> __attribute__((aligned(NLMSG_ALIGNTO)));
> 
> Would it be better to apply the attribute to the definition
> of 'struct rtattr' itself ?

'struct rtattr' is defined in a kernel header and I am not really sure
what kind of side effects it would have.

The problem I intend to solve with my page is rather caused by the use case:
The request is sent as a single structure, the processing is however
performed step by step: first the nlmsg is parsed, then the pointer
is moved on to the parsing of the content. This pointer is hoever not
oriented at the structure element but by using size information which
is then adjust by NLMSG_ALIGN() etc. Consequently one should actually
use the same method in sending (generate the header first, then addattr_()
which enforces protocol conform alignment). This is the way all other
instances in iproute2 generate their messages btw.

I hence consider my proposed patch to be the least intrusive solution.

Best regards,
	Lutz

Patch

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 8e8c8b9..09b4277 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -94,8 +94,8 @@  int rtnl_wilddump_request(struct rtnl_handle *rth, int family, int type)
 	struct {
 		struct nlmsghdr nlh;
 		struct rtgenmsg g;
-		__u16 align_rta;	/* attribute has to be 32bit aligned */
-		struct rtattr ext_req;
+		/* attribute has to be NLMSG aligned */
+		struct rtattr ext_req __attribute__ ((aligned(NLMSG_ALIGNTO)));
 		__u32 ext_filter_mask;
 	} req;