The problem with these type of patches has not been that we cannot find
a solution that works on new setups with the patches. The problem is
that we are not allowed to merge patches in the upstream kernel that
break existing setups. For example if the user has only upgraded the
kernel or only upgraded userspace tools, then we cannot break their
setups. This patch does not work on 32 bit kernels/userpsace if the
users only updates the tools.

Per your request I've written a patch to allow for 32bit machines to work with un-patched 32bit and 64bit kernels. It should be trivial to sunset this patch with a kernel version number check when the kernel has the previous patch I submitted applied.

I've tested this patch on 32/32 and 32/64 with an unpatched kernel.

Thanks,
~Lisa M

diff --git a/include/iscsi_if.h b/include/iscsi_if.h index dad9fd8..db97332 100644 --- a/include/iscsi_if.h +++ b/include/iscsi_if.h @@ -3,6 +3,7 @@
  *
  * Copyright (C) 2005 Dmitry Yusupov
  * Copyright (C) 2005 Alex Aizman
+ * Copyright (C) 2012 Lisa Maginnis
  * maintained by open-iscsi@googlegroups.com
  *
  * This program is free software; you can redistribute it and/or modify
@@ -106,6 +107,14 @@ struct iscsi_uevent {
        uint32_t iferror; /* carries interface or resource errors */
        uint64_t transport_handle;

+       /* Structs in this union can differ on size between 32bit and 64bit
+        * systems. This will break systems running a 32bit userspace with a
+ * 64bit kernel. It is important to specify an alignment for the + * struct(s) so that their sizes match on both in both 32 and 64bit
+        * systems. Currently the two largest structs are:
+        * msg_bind_conn: 20 bytes, 24 aligned to the sizeof(uint64_t)
+        * stop_conn:     20 bytes, 24 aligned to the sizeof(uint64_t)
+        */
        union {
                /* messages u -> k */
                struct msg_create_session {
@@ -131,7 +140,7 @@ struct iscsi_uevent {
                        uint32_t        cid;
                        uint64_t        transport_eph;
                        uint32_t        is_leading;
-               } b_conn;
+               } b_conn __attribute__((aligned (sizeof(uint64_t))));
                struct msg_destroy_conn {
                        uint32_t        sid;
                        uint32_t        cid;
@@ -157,7 +166,7 @@ struct iscsi_uevent {
                        uint32_t        cid;
                        uint64_t        conn_handle;
                        uint32_t        flag;
-               } stop_conn;
+               } stop_conn __attribute__((aligned (sizeof(uint64_t))));
                struct msg_get_stats {
                        uint32_t        sid;
                        uint32_t        cid;
diff --git a/usr/iscsi_netlink.h b/usr/iscsi_netlink.h
index 25b41db..6b482ad 100644
--- a/usr/iscsi_netlink.h
+++ b/usr/iscsi_netlink.h
@@ -2,6 +2,7 @@
  * iSCSI Netlink attr helpers
  *
  * Copyright (C) 2011 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2012 Lisa Maginnis
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published
@@ -30,4 +31,10 @@ struct iovec;

 extern struct nlattr *iscsi_nla_alloc(uint16_t type, uint16_t len);

+#ifdef __i386__
+int fix_32bit_kernel_structs; /* If set to 1, will cause reads from the kernel 
to be
+                              * padded with an extra 4 bytes between 
iscsi_uevent.u and
+                              * iscsi_uevent.r */
+#endif
+
 #endif
diff --git a/usr/iscsid.c b/usr/iscsid.c
index b4bb65b..53381bf 100644
--- a/usr/iscsid.c
+++ b/usr/iscsid.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2004 Dmitry Yusupov, Alex Aizman
  * Copyright (C) 2006 Mike Christie
  * Copyright (C) 2006 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2012 Lisa Maginnis

  * maintained by open-iscsi@googlegroups.com
  *
@@ -52,6 +53,10 @@
 #include "iscsid_req.h"
 #include "iscsi_err.h"

+#ifdef __i386__
+extern int fix_32bit_kernel_structs; /* For i386 compatibility. */
+#endif
+
 /* global config info */
 struct iscsi_daemon_config daemon_config;
 struct iscsi_daemon_config *dconfig = &daemon_config;
@@ -336,6 +341,28 @@ static void missing_iname_warn(char *initiatorname_file)
                  "ignored.\n", initiatorname_file, initiatorname_file);
 }

+#ifdef __i386__
+/* check_kernel_compatibility()
+ * This function detects when we are built in i386 and running a x86_64 (64bit) kernel and + * sets a flag. This flag is used to to signal the nlpayload_read() and nl_read() functions
+ * in netlink.c
+ */
+void check_kernel_compatibility()
+{
+  struct utsname uname_data;
+  fix_32bit_kernel_structs = 0;
+
+  /* Get our kernel and machine type for compatibility */
+  uname(&uname_data);
+
+  /* If we are not running a x86_64 kernel, enable the kernel struct fix.
+   * TODO: When/if kernel patches are made, a kernel version check should be 
done here. */
+  if(strcmp(uname_data.machine, "x86_64") != 0) {
+ fix_32bit_kernel_structs = 1; + }
+}
+#endif
+
 int main(int argc, char *argv[])
 {
        struct utsname host_info; /* will use to compound initiator alias */
@@ -409,6 +436,11 @@ int main(int argc, char *argv[])
                exit(ISCSI_ERR);
        }

+#ifdef __i386__
+       /* Check and set kernel compatibility options (x86/x86_64 user/kernel). 
*/
+       check_kernel_compatibility();
+#endif
+
        umask(0177);

        mgmt_ipc_fd = -1;
diff --git a/usr/netlink.c b/usr/netlink.c
index d65cd7c..b363439 100644
--- a/usr/netlink.c
+++ b/usr/netlink.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2004 Dmitry Yusupov, Alex Aizman
  * Copyright (C) 2006 Mike Christie
  * Copyright (C) 2006 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2012 Lisa Maginnis
  * maintained by open-iscsi@googlegroups.com
  *
  * This program is free software; you can redistribute it and/or modify
@@ -90,6 +91,38 @@ struct nlattr *iscsi_nla_alloc(uint16_t type, uint16_t len)
        return attr;
 }

+#ifdef __i386__
+#define OLD_32BIT_UEVENT_U_END_BYTE 35 /* Location of the iscsi_uevent struct 
*/
+/* fix_32bit_uevent_struct(char*)
+ * Aligns the iscsi_uevent struct sent from an i386 (32bit) kernel to match our
+ * alignment in iscsi_if.h by adding a 4 byte (sizeof(uint32)) block of padding
+ * between two unions (u/r) in the struct.
+ * data  Pointer to the uevent_iscsi struct.
+ * count Total number of bytes to copy.
+ * peek  Set to 1 if uevent_iscsi was pulled from nl_read()/MSG_PEEK, else
+ *       set to 0.
+*/
+void fix_32bit_uevent_struct(char *data, size_t count, int peek) {
+
+  log_debug(7, "in %s", __FUNCTION__);
+
+  /* Skip creating the offset if we are a PDU, CHAP, or STATS unless it is a 
peek
+   * (which is called from nl_read()) */
+ if(((uint8_t)data[0] != ISCSI_KEVENT_RECV_PDU + && (uint8_t)data[0] != ISCSI_UEVENT_GET_STATS
+     && (uint8_t)data[0] != ISCSI_UEVENT_GET_CHAP) || peek) {
+
+    /* Move iscsi_uevent.r four bytes (sizeof(uint32_t)) higher  */
+    memmove(&data[OLD_32BIT_UEVENT_U_END_BYTE+sizeof(uint32_t)],
+           &data[OLD_32BIT_UEVENT_U_END_BYTE],
+           count-OLD_32BIT_UEVENT_U_END_BYTE-sizeof(uint32_t));
+
+    /* Clear the `uint32_t' padding between iscsi_uevent.u and iscsi_uevent.r 
*/
+    memset(&data[OLD_32BIT_UEVENT_U_END_BYTE], 0, sizeof(uint32_t));
+ } +}
+#endif
+
 static int
 kread(char *data, int count)
 {
@@ -176,8 +209,17 @@ nlpayload_read(int ctrl_fd, char *data, int count, int 
flags)
         */
        rc = recvmsg(ctrl_fd, &msg, flags);

-       if (data)
-               memcpy(data, NLMSG_DATA(iov.iov_base), count);
+       if (data) {
+               memcpy(data, NLMSG_DATA(iov.iov_base), count);
+
+#ifdef __i386__
+              /* Check if we need to clean this data for compatibility. */
+              if(fix_32bit_kernel_structs == 1) {
+                fix_32bit_uevent_struct(data, count, 0);
+         }
+#endif
+
+       }

        return rc;
 }
@@ -1281,6 +1323,13 @@ static int ctldev_handle(void)
        nlh = (struct nlmsghdr *)nlm_ev;
        ev = (struct iscsi_uevent *)NLMSG_DATA(nlm_ev);

+#ifdef __i386__
+       /* Check if we need to clean this data for compatibility. */
+       if(fix_32bit_kernel_structs == 1) {
+         fix_32bit_uevent_struct((char*)ev, sizeof(struct iscsi_uevent), 1);
+       }
+#endif
+
        log_debug(7, "%s got event type %u\n", __FUNCTION__, ev->type);
        /* drivers like qla4xxx can be inserted after iscsid is started */
        switch (ev->type) {


On Mon, 29 Oct 2012, Mike Christie wrote:

Date: Mon, 29 Oct 2012 14:48:52 -0500
From: Mike Christie <micha...@cs.wisc.edu>
To: open-iscsi@googlegroups.com
Cc: Lisa Marie <nullo...@sdf.org>
Subject: Re: iSCSI timeouts on login with 32/64bit user/kernel

On 10/28/2012 11:42 PM, Lisa Marie wrote:

I've attached the below patches which can be applied to both the kernel
and the open-iscsi client. They have been tested on
x86/x86 user/kernel, and a x86/x86_64 user/kernel. This still needs to
be tested on an x86_64/x86_64 user/kernel as well as any
other system that has a 32bit userspace and a 64bit kernel.

Thanks a lot for the patch.

The problem with these type of patches has not been that we cannot find
a solution that works on new setups with the patches. The problem is
that we are not allowed to merge patches in the upstream kernel that
break existing setups. For example if the user has only upgraded the
kernel or only upgraded userspace tools, then we cannot break their
setups. This patch does not work on 32 bit kernels/userpsace if the
users only updates the tools.


nullo...@sdf.org
SDF Public Access UNIX System - http://sdf.org

--
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com.
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/open-iscsi?hl=en.

Reply via email to