From owner-freebsd-net@FreeBSD.ORG Wed Feb 10 10:13:04 2010 Return-Path: <owner-freebsd-net@FreeBSD.ORG> Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E56341065692 for <net@freebsd.org>; Wed, 10 Feb 2010 10:13:03 +0000 (UTC) (envelope-from marius@nuenneri.ch) Received: from ey-out-2122.google.com (ey-out-2122.google.com [74.125.78.24]) by mx1.freebsd.org (Postfix) with ESMTP id 896498FC25 for <net@freebsd.org>; Wed, 10 Feb 2010 10:13:03 +0000 (UTC) Received: by ey-out-2122.google.com with SMTP id 25so2595eya.9 for <net@freebsd.org>; Wed, 10 Feb 2010 02:13:02 -0800 (PST) MIME-Version: 1.0 Received: by 10.216.88.8 with SMTP id z8mr2759882wee.212.1265795339245; Wed, 10 Feb 2010 01:48:59 -0800 (PST) In-Reply-To: <20100209.103411.506052712871889475.imp@bsdimp.com> References: <20100209.103411.506052712871889475.imp@bsdimp.com> From: =?UTF-8?Q?Marius_N=C3=BCnnerich?= <marius@nuenneri.ch> Date: Wed, 10 Feb 2010 10:48:09 +0100 Message-ID: <b649e5e1002100148r759f3aacr3d5fcdfb5efd9001@mail.gmail.com> To: "M. Warner Losh" <imp@bsdimp.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: net@freebsd.org Subject: Re: struct sockaddr * and alignment X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD <freebsd-net.freebsd.org> List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>, <mailto:freebsd-net-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/freebsd-net> List-Post: <mailto:freebsd-net@freebsd.org> List-Help: <mailto:freebsd-net-request@freebsd.org?subject=help> List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>, <mailto:freebsd-net-request@freebsd.org?subject=subscribe> X-List-Received-Date: Wed, 10 Feb 2010 10:13:04 -0000 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. =C2=A0It turns ou= t > 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 > =C2=A0 =C2=A0 =C2=A0 =C2=A0sdl =3D (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. =C2=A0struct sockaddr_storage has a int64 in it to > force it to be __aligned(8). =C2=A0So I thought to myself "why don't I ju= st > add __aligned(8) to the struct sockaddr definition?" =C2=A0After 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. =C2=A0Yea. =C2=A0But, sadly, it cau= ses > other problems. =C2=A0If you look at sbin/atm/atmconfig/natm.c you'll see > code like: > > static void > store_route(struct rt_msghdr *rtm) > { > ... > =C2=A0 =C2=A0 =C2=A0 =C2=A0char *cp > =C2=A0 =C2=A0 =C2=A0 =C2=A0struct sockaddr *sa; > =C2=A0 =C2=A0 =C2=A0 =C2=A0... > > =C2=A0 =C2=A0 =C2=A0 =C2=A0cp =3D (char *)(rtm + 1); > ... > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0sa =3D (struct sockaddr *)cp; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0cp +=3D 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. =C2=A0But 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. =C2=A0This 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). =C2=A0 This cure > would be worse than the disease. > > So the question here is "What is the right solution here?" =C2=A0It has m= e > stumped. =C2=A0So 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=3D8-CURRENT#L233 It's a pity that only this keeps some programs from going to a WARNS level = of 6.