Date: Wed, 10 Feb 2010 09:19:03 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: marius@nuenneri.ch Cc: net@FreeBSD.org Subject: Re: struct sockaddr * and alignment Message-ID: <20100210.091903.177810546894923209.imp@bsdimp.com> In-Reply-To: <b649e5e1002100148r759f3aacr3d5fcdfb5efd9001@mail.gmail.com> References: <20100209.103411.506052712871889475.imp@bsdimp.com> <b649e5e1002100148r759f3aacr3d5fcdfb5efd9001@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <b649e5e1002100148r759f3aacr3d5fcdfb5efd9001@mail.gmail.com>
Marius Nünnerich <marius@nuenneri.ch> writes:
: On Tue, Feb 9, 2010 at 18:34, M. Warner Losh <imp@bsdimp.com> wrote:
: > Greetings,
: >
: > I've found a few inconsistencies in the sockaddr stuff in the tree.
: > I'm not sure where to go to get a definitive answer, so I thought I'd
: > start here.
: >
: > I got here looking at the recent wake breakage on mips. It turns out
: > that the warning was:
: >
: > src/usr.sbin/wake/wake.c: In function 'find_ether':
: > src/usr.sbin/wake/wake.c:123: warning: cast increases required alignment of target type
: >
: > which comes from
: > sdl = (struct sockaddr_dl *)ifa->ifa_addr;
: >
: > The problem is that on MIPS struct sockaddr * is byte aligned and
: > sockaddr_dl * is word aligned, so the compiler is rightly telling us
: > that there might be a problem here.
: >
: > However, further digging shows that there will never be a problem
: > here with alignment. struct sockaddr_storage has a int64 in it to
: > force it to be __aligned(8). So I thought to myself "why don't I just
: > add __aligned(8) to the struct sockaddr definition?" After all, the
: > kernel goes to great lengths to return data so aligned, and user code
: > also keeps things aligned.
: >
: > Sure enough, that fixes this warning. Yea. But, sadly, it causes
: > other problems. If you look at sbin/atm/atmconfig/natm.c you'll see
: > code like:
: >
: > static void
: > store_route(struct rt_msghdr *rtm)
: > {
: > ...
: > char *cp
: > struct sockaddr *sa;
: > ...
: >
: > cp = (char *)(rtm + 1);
: > ...
: > sa = (struct sockaddr *)cp;
: > cp += roundup(sa->sa_len, sizeof(long));
: > ...
: >
: > which breaks because we're now casting from an __aligned(1) char * to
: > an __aligned(8) sockaddr *.
: >
: > And it is only rounding the size of the structure to long, rather than
: > int64 like sockaddr_storage suggests is the proper alignment. But I
: > haven't looked in the kernel to see if there's an issue there with
: > routing sockets or not.
: >
: > The other extreme is to put __aligned(1) on all the sockaddr_foo
: > structures. This would solve the compiler warning, but would have a
: > negative effect on performance in accessing these elements (because
: > the compiler would have to generate calls to bcopy or equivalent to
: > access the scalar members that are larger than a byte). This cure
: > would be worse than the disease.
: >
: > So the question here is "What is the right solution here?" It has me
: > stumped. So I dropped WARNS level down from 6 to 3 for wake.c.
:
: Hi Warner,
:
: I got into the same kind of trouble when I tried to raise the WARNS
: level above 3 for inetd and others. I guess everything which uses some
: sockaddr casting or (in the case of inetd) some of these macros:
: http://fxr.googlebit.com/source/sys/netinet6/in6.h?v=8-CURRENT#L233
:
: It's a pity that only this keeps some programs from going to a WARNS level of 6.
Well, if there were some way to tell the compiler "Yes, I know this is
right" then we'd be set. But I've not found what that way might be.
Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100210.091903.177810546894923209.imp>