From owner-freebsd-net@FreeBSD.ORG Wed Feb 10 16:26:21 2010 Return-Path: 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 35E381065670 for ; Wed, 10 Feb 2010 16:26:21 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id DA2958FC1E for ; Wed, 10 Feb 2010 16:26:20 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id o1AGJ0se011294; Wed, 10 Feb 2010 09:19:01 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Wed, 10 Feb 2010 09:19:03 -0700 (MST) Message-Id: <20100210.091903.177810546894923209.imp@bsdimp.com> To: marius@nuenneri.ch From: "M. Warner Losh" In-Reply-To: References: <20100209.103411.506052712871889475.imp@bsdimp.com> X-Mailer: Mew version 6.3 on Emacs 22.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=iso-8859-1 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Feb 2010 16:26:21 -0000 In message: Marius N=FCnnerich writes: : On Tue, Feb 9, 2010 at 18:34, M. Warner Losh 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. =A0It 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 alig= nment of target type : > : > which comes from : > =A0 =A0 =A0 =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 u= s : > that there might be a problem here. : > : > However, further digging shows that there will never be a problem : > here with alignment. =A0struct sockaddr_storage has a int64 in it t= o : > force it to be __aligned(8). =A0So I thought to myself "why don't I= just : > add __aligned(8) to the struct sockaddr definition?" =A0After all, = the : > kernel goes to great lengths to return data so aligned, and user co= de : > also keeps things aligned. : > : > Sure enough, that fixes this warning. =A0Yea. =A0But, sadly, it cau= ses : > other problems. =A0If you look at sbin/atm/atmconfig/natm.c you'll = see : > code like: : > : > static void : > store_route(struct rt_msghdr *rtm) : > { : > ... : > =A0 =A0 =A0 =A0char *cp : > =A0 =A0 =A0 =A0struct sockaddr *sa; : > =A0 =A0 =A0 =A0... : > : > =A0 =A0 =A0 =A0cp =3D (char *)(rtm + 1); : > ... : > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sa =3D (struct socka= ddr *)cp; : > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =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 t= han : > int64 like sockaddr_storage suggests is the proper alignment. =A0Bu= t 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. =A0This would solve the compiler warning, but would hav= e 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). =A0 This cu= re : > would be worse than the disease. : > : > So the question here is "What is the right solution here?" =A0It ha= s me : > stumped. =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 som= e : 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. 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