[openib-general] Re: [openib-commits] r2196 - in gen2/trunk/src/linux-kernel/infiniband: core include]

[Hal Rosenstock]

Hi Sean,

I may have missed this but how is the need for the non natural
alignment accomodated now ?

I am unable to IPoIB ping from one node to another as the SA query for
PathRecord is not answered as something is now wrong in the query. When
I add back the packing (patch below), it works again.

I do think the packing is for more than just 64 bit architectures as I
am running this on an Intel 386.

-- Hal

Index: ib_mad.h
===================================================================
--- ib_mad.h	(revision 2209)
+++ ib_mad.h	(working copy)
@@ -134,12 +134,18 @@
 
 #define IB_SA_COMP_MASK(n) ((__force ib_sa_comp_mask) cpu_to_be64(1ull
<< n))
 
+/*
+ * ib_sa_hdr and ib_sa_mad structures must be packed because they have 
+ * 64-bit fields that are only 32-bit aligned. 64-bit architectures
will
+ * lay them out wrong otherwise.  (And unfortunately they are sent on 
+ * the wire so we can't change the layout)
+ */
 struct ib_sa_hdr {
 	u64			sm_key;
 	u16			attr_offset;
 	u16			reserved;
 	ib_sa_comp_mask		comp_mask;
-};
+} __attribute__ ((packed));
 
 struct ib_mad {
 	struct ib_mad_hdr	mad_hdr;
@@ -157,7 +163,7 @@
 	struct ib_rmpp_hdr	rmpp_hdr;
 	struct ib_sa_hdr	sa_hdr;
 	u8			data[200];
-};
+} __attribute__ ((packed));
 
 struct ib_vendor_mad {
 	struct ib_mad_hdr	mad_hdr;



--Forwarded Message--


From: sean.hefty at openib.org
To: openib-commits at openib.org
Subject: [openib-commits] r2196 - in gen2/trunk/src/linux-kernel/infiniband: core include
Date: 20 Apr 2005 10:58:56 -0700

Author: sean.hefty
Date: 2005-04-20 10:58:55 -0700 (Wed, 20 Apr 2005)
New Revision: 2196

Modified:
   gen2/trunk/src/linux-kernel/infiniband/core/sa_query.c
   gen2/trunk/src/linux-kernel/infiniband/include/ib_mad.h
   gen2/trunk/src/linux-kernel/infiniband/include/ib_sa.h
Log:
Move SA MAD definitions to ib_mad.h.  Removed unneeded packed attribute
from MAD structure definitions.

Signed-off-by: Sean Hefty <sean.hefty at intel.com>



Modified: gen2/trunk/src/linux-kernel/infiniband/core/sa_query.c
===================================================================
-- gen2/trunk/src/linux-kernel/infiniband/core/sa_query.c	2005-04-20 17:35:52 UTC (rev 2195)
+++ gen2/trunk/src/linux-kernel/infiniband/core/sa_query.c	2005-04-20 17:58:55 UTC (rev 2196)
@@ -50,26 +50,6 @@
 MODULE_DESCRIPTION("InfiniBand subnet administration query support");
 MODULE_LICENSE("Dual BSD/GPL");
 
-/*
- * These two structures must be packed because they have 64-bit fields
- * that are only 32-bit aligned.  64-bit architectures will lay them
- * out wrong otherwise.  (And unfortunately they are sent on the wire
- * so we can't change the layout)
- */
-struct ib_sa_hdr {
-	u64			sm_key;
-	u16			attr_offset;
-	u16			reserved;
-	ib_sa_comp_mask		comp_mask;
-} __attribute__ ((packed));
-
-struct ib_sa_mad {
-	struct ib_mad_hdr	mad_hdr;
-	struct ib_rmpp_hdr	rmpp_hdr;
-	struct ib_sa_hdr	sa_hdr;
-	u8			data[200];
-} __attribute__ ((packed));
-
 struct ib_sa_sm_ah {
 	struct ib_ah        *ah;
 	struct kref          ref;

Modified: gen2/trunk/src/linux-kernel/infiniband/include/ib_mad.h
===================================================================
-- gen2/trunk/src/linux-kernel/infiniband/include/ib_mad.h	2005-04-20 17:35:52 UTC (rev 2195)
+++ gen2/trunk/src/linux-kernel/infiniband/include/ib_mad.h	2005-04-20 17:58:55 UTC (rev 2196)
@@ -117,7 +117,7 @@
 	u16	attr_id;
 	u16	resv;
 	u32	attr_mod;
-} __attribute__ ((packed));
+};
 
 struct ib_rmpp_hdr {
 	u8	rmpp_version;
@@ -126,26 +126,44 @@
 	u8	rmpp_status;
 	u32	seg_num;
 	u32	paylen_newwin;
-} __attribute__ ((packed));
+};
 
+typedef u64 __bitwise ib_sa_comp_mask;
+
+#define IB_SA_COMP_MASK(n) ((__force ib_sa_comp_mask) cpu_to_be64(1ull << n))
+
+struct ib_sa_hdr {
+	u64			sm_key;
+	u16			attr_offset;
+	u16			reserved;
+	ib_sa_comp_mask		comp_mask;
+};
+
 struct ib_mad {
 	struct ib_mad_hdr	mad_hdr;
 	u8			data[232];
-} __attribute__ ((packed));
+};
 
 struct ib_rmpp_mad {
 	struct ib_mad_hdr	mad_hdr;
 	struct ib_rmpp_hdr	rmpp_hdr;
 	u8			data[220];
-} __attribute__ ((packed));
+};
 
+struct ib_sa_mad {
+	struct ib_mad_hdr	mad_hdr;
+	struct ib_rmpp_hdr	rmpp_hdr;
+	struct ib_sa_hdr	sa_hdr;
+	u8			data[200];
+};
+
 struct ib_vendor_mad {
 	struct ib_mad_hdr	mad_hdr;
 	struct ib_rmpp_hdr	rmpp_hdr;
 	u8			reserved;
 	u8			oui[3];
 	u8			data[216];
-} __attribute__ ((packed));
+};