Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 09 Feb 2010 10:34:11 -0700 (MST)
From:      "M. Warner Losh" <imp@bsdimp.com>
To:        net@freebsd.org
Subject:   struct sockaddr * and alignment
Message-ID:  <20100209.103411.506052712871889475.imp@bsdimp.com>

next in thread | raw e-mail | index | archive | help
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.

Warner



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100209.103411.506052712871889475.imp>