Subject: ancillary data alignment and binary backward compatibility: please pick one
To: None <tech-net@netbsd.org>
From: None <itojun@iijlab.net>
List: tech-net
Date: 02/28/2000 21:32:23
------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <27048.951741050.1@coconut.itojun.org>
Content-Transfer-Encoding: 7bit

	Here are two fixes to ancillary data alignment issue.  I would like
	to pick one of them, however, I'm still wondering which is more
	correct.

	I personally vote for the latter, since the former will break if we
	try to run 32bit sparc binary on 64bit sparc kernel.
	However, I'm still not sure if it is okay to make CMSG_xx non-constant.

	Both fix will pass "make build" just fine.

itojun


first multipart attachment:
	- use ALIGNBYTES in CMSG_ALIGN().
	- adds dependency from sys/socket.h to machine/param.h.
	- CMSG_xx are all constant, so you can do:
		char foo[CMSG_SPACE(int)];
	- old compiled binary may choke if we upgrade kernel with different
	  ALIGNBYTES (typical exapmle: 32bit sparc binary on sparc64 does
	  not work).

second multipart attachment:
	- use sysctl to get alignment constraint for CMSG_ALIGN().
	- no dependency from sys/socket.h to machine/param.h.
	- CMSG_xx are non-constant.  you can't do the following:
		char foo[CMSG_SPACE(int)];
	- old compiled binary is okay on new kernel, even if they sees
	  different defined in machine/param.h.
	- extra function calls on CMSG_ALIGN().  (we can decrease them by
	  making static variable visible, I thought it would be safer to
	  keep it inside function)


------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <27048.951741050.2@coconut.itojun.org>
Content-Transfer-Encoding: 7bit

Index: socket.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/socket.h,v
retrieving revision 1.50
diff -u -r1.50 socket.h
--- socket.h	2000/02/18 05:19:25	1.50
+++ socket.h	2000/02/28 12:24:48
@@ -68,6 +68,11 @@
 #define	_SYS_SOCKET_H_
 
 /*
+ * needed for CMSG_ALIGN
+ */
+#include <machine/param.h>
+
+/*
  * Definitions related to sockets: types, address families, options.
  */
 
@@ -395,9 +400,17 @@
 /* followed by	u_char  cmsg_data[]; */
 };
 
-/* given pointer to struct cmsghdr, return pointer to data */
-#define	CMSG_DATA(cmsg) \
-	((u_char *)(cmsg) + CMSG_ALIGN(sizeof(struct cmsghdr)))
+/*
+ * Alignment requirement for CMSG struct manipulation.
+ *
+ * XXX
+ * This is still a little bit questionable from two points:
+ * (1) It is not future adaptable.  If old binaries and new kernel uses
+ * different definition for ALIGNBYTES, old binaries will choke.
+ * (2) Also, it may not be correct to add dependency from sys/socket.h to
+ * machine/param.h.
+ */
+#define CMSG_ALIGN(n)	(((n) + ALIGNBYTES) & ~ALIGNBYTES)
 
 /*
  * Alignment requirement for CMSG struct manipulation.

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <27048.951741050.3@coconut.itojun.org>
Content-Transfer-Encoding: 7bit

Index: lib/libc/net/Makefile.inc
===================================================================
RCS file: /cvsroot/basesrc/lib/libc/net/Makefile.inc,v
retrieving revision 1.51
diff -u -r1.51 Makefile.inc
--- Makefile.inc	2000/02/23 15:44:00	1.51
+++ Makefile.inc	2000/02/28 12:23:20
@@ -4,8 +4,8 @@
 # net sources
 .PATH: ${ARCHDIR}/net ${.CURDIR}/net
 
-SRCS+=	base64.c ethers.c gethnamaddr.c getifaddrs.c getnetnamadr.c \
-	getnetent.c getproto.c \
+SRCS+=	__cmsg_alignbytes.c base64.c ethers.c gethnamaddr.c getifaddrs.c \
+	getnetnamadr.c getnetent.c getproto.c \
 	getprotoent.c getprotoname.c getservbyname.c getservbyport.c \
 	getservent.c herror.c hesiod.c inet_lnaof.c inet_makeaddr.c \
 	inet_net_ntop.c inet_net_pton.c inet_neta.c inet_ntop.c inet_pton.c \
Index: lib/libc/net/__cmsg_alignbytes.c
===================================================================
RCS file: __cmsg_alignbytes.c
diff -N __cmsg_alignbytes.c
--- /dev/null	Mon Feb 28 03:56:32 2000
+++ __cmsg_alignbytes.c	Mon Feb 28 04:23:20 2000
@@ -0,0 +1,65 @@
+/*	$NetBSD$	*/
+
+/*-
+ * Copyright (c) 1996 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * This code is derived from software contributed to The NetBSD Foundation
+ * by Jun-ichiro Hagino.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of The NetBSD Foundation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/types.h>
+#include <sys/param.h>
+#include <sys/sysctl.h>
+#include <sys/socket.h>
+
+const int
+__cmsg_alignbytes()
+{
+	static int alignbytes = -1;
+#ifdef HW_ALIGNBYTES
+	int mib[2];
+	size_t len;
+	int ret;
+#endif
+
+	if (alignbytes > 0)
+		return alignbytes;
+
+#ifdef HW_ALIGNBYTES
+	mib[0] = CTL_HW;
+	mib[1] = HW_ALIGNBYTES;
+	len = sizeof(alignbytes);
+	ret = sysctl(mib, sizeof(mib)/sizeof(mib[0]), (void *)&alignbytes,
+		    &len, NULL, 0);
+	if (ret >= 0)
+		return alignbytes;
+#endif
+	/* last resort */
+	alignbytes = ALIGNBYTES;
+	return alignbytes;
+}
Index: sys/sys/socket.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/socket.h,v
retrieving revision 1.50
diff -u -r1.50 socket.h
--- socket.h	2000/02/18 05:19:25	1.50
+++ socket.h	2000/02/28 12:23:21
@@ -401,10 +401,14 @@
 
 /*
  * Alignment requirement for CMSG struct manipulation.
- * This is different from ALIGN() defined in ARCH/include/param.h.
- * XXX think again carefully about architecture dependencies.
+ * This basically behaves the same as ALIGN() ARCH/include/param.h.
+ * We declare it separately for two reasons:
+ * (1) avoid dependency between machine/param.h, and (2) to sync with kernel's
+ * idea of ALIGNBYTES at runtime.
+ * without (2), we can't guarantee binary compatibility in case of future
+ * changes in ALIGNBYTES.
  */
-#define CMSG_ALIGN(n)	(((n) + (sizeof(long) - 1)) & ~(sizeof(long) - 1))
+#define CMSG_ALIGN(n)	(((n) + __cmsg_alignbytes()) & ~__cmsg_alignbytes())
 
 /* given pointer to struct cmsghdr, return pointer to next cmsghdr */
 #define	CMSG_NXTHDR(mhdr, cmsg)	\
@@ -454,6 +458,8 @@
 	int	msg_accrightslen;
 };
 #endif
+
+const int	__cmsg_alignbytes __P((void));
 
 #ifndef	_KERNEL
 
Index: sys/lib/libkern/Makefile
===================================================================
RCS file: /cvsroot/syssrc/sys/lib/libkern/Makefile,v
retrieving revision 1.53
diff -u -r1.53 Makefile
--- Makefile	1999/05/07 14:49:52	1.53
+++ Makefile	2000/02/28 12:23:21
@@ -23,7 +23,7 @@
 .endif
 
 # Other stuff
-SRCS+=	inet_addr.c intoa.c md5c.c sha1.c pmatch.c
+SRCS+=	__cmsg_alignbytes.c inet_addr.c intoa.c md5c.c sha1.c pmatch.c
 
 # Files to clean up
 CLEANFILES+= lib${LIB}.o lib${LIB}.po
Index: sys/lib/libkern/__cmsg_alignbytes.c
===================================================================
RCS file: __cmsg_alignbytes.c
diff -N __cmsg_alignbytes.c
--- /dev/null	Mon Feb 28 03:56:32 2000
+++ __cmsg_alignbytes.c	Mon Feb 28 04:23:21 2000
@@ -0,0 +1,44 @@
+/*	$NetBSD$	*/
+
+/*-
+ * Copyright (c) 1996 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * This code is derived from software contributed to The NetBSD Foundation
+ * by Jun-ichiro Hagino.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of The NetBSD Foundation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/param.h>
+
+int
+__cmsg_alignbytes()
+{
+
+	return ALIGNBYTES;
+}

------- =_aaaaaaaaaa0--