On Fri, Jan 29, 2010 at 8:59 PM, Mike Christie <micha...@cs.wisc.edu> wrote: > On 01/28/2010 09:25 AM, Erez Zilber wrote: >>> >>> >>> +struct iscsi_noop_info { >>> + struct timeval tv; >>> >>> >>> Can you pass this between userspace and the kernel safely? The fields in >>> there are longs, and I thought those could be different sizes if you had >>> something like 32bit userspace and 64 kernels. >> >> What do you suggest to do here? > > I do not know. Am I even right? After I sent that, I was thinking syscalls > must pass it around ok, so it must be ok or there must be some way to do it. > I did not look into it any more though.
I must say that I don't know how to handle the scenario of 32bit userspace and 64 kernels. > > >> I'm not very familiar with debugfs. What do you want to do with it? >> > > Nothing. I was just saying it is an option. If you were also fine with the > netlink interface you used, so am I, so no changes needed. > The following patch (built over the original patch) fixes the problems that we discussed (except for the 32-64 problem). If you're okay with it, I will send a single updated patch. diff --git a/doc/iscsiadm.8 b/doc/iscsiadm.8 index 6f85cb5..9f60beb 100644 --- a/doc/iscsiadm.8 +++ b/doc/iscsiadm.8 @@ -126,14 +126,14 @@ operator. .TP \fB\-i\fR, \fB\-\-debuginfo=\fIdebug_info_op\fR -Specifies a debug infomration operation \fIdebug_info_op\fR. \fIdebug_info_op\fR must be \fIdump\fR or \fIclear\fR. +Specifies a debug infomration operation \fIdebug_info_op\fR. \fIdebug_info_op\fR must be \fIdump_noop\fR or \fIclear_noop\fR. .IP This option is valid only for the session mode. .IP -\fIdump\fR prints a list of recent Nop-out PDUs that almost timed out with information about the time that the Nop-out PDU was sent and its delay. +\fIdump_noop\fR prints a list of recent Nop-out PDUs that almost timed out with information about the time that the Nop-out PDU was sent and its delay. A Nop-out PDU (that did not time out) is defined as 'almost timed out' if its delay is greater than node.session.noop_threshold * node.conn[0].timeo.noop_out_timeout. .IP -\fIclear\fR clears the list of recent Nop-out PDUs that almost timed out. +\fIclear_noop\fR clears the list of recent Nop-out PDUs that almost timed out. .TP \fB\-o\fR, \fB\-\-op=\fIop\fR diff --git a/include/iscsi_if.h b/include/iscsi_if.h index 7b96692..0b7dfe8 100644 --- a/include/iscsi_if.h +++ b/include/iscsi_if.h @@ -328,9 +328,10 @@ enum iscsi_param { ISCSI_PARAM_ISID, ISCSI_PARAM_INITIATOR_NAME, + ISCSI_PARAM_TGT_RESET_TMO, + ISCSI_PARAM_NOOP_THRESHOLD, - ISCSI_PARAM_TGT_RESET_TMO, /* must always be last */ ISCSI_PARAM_MAX, }; diff --git a/kernel/libiscsi.c b/kernel/libiscsi.c index 5011139..3f10940 100644 --- a/kernel/libiscsi.c +++ b/kernel/libiscsi.c @@ -980,6 +980,15 @@ static void iscsi_send_nopout(struct iscsi_conn *conn, struct iscsi_nopin *rhdr) } } +static unsigned long iscsi_get_ping_delay(unsigned long last_ping) +{ + /* Check if there was a jiffies rollover */ + if (jiffies < last_ping) + return ULONG_MAX - last_ping + jiffies; + else + return jiffies - last_ping; +} + static int iscsi_nop_out_rsp(struct iscsi_task *task, struct iscsi_nopin *nop, char *data, int datalen) { @@ -998,8 +1007,7 @@ static int iscsi_nop_out_rsp(struct iscsi_task *task, data, datalen)) rc = ISCSI_ERR_CONN_FAILED; } else { - ping_delay = jiffies - conn->last_ping; - + ping_delay = iscsi_get_ping_delay(conn->last_ping); /* Check if the ping has almost timed out */ if (ping_delay >= (session->noop_threshold * (conn->ping_timeout * HZ)) / diff --git a/usr/config.h b/usr/config.h index 90063fe..0ac6ff5 100644 --- a/usr/config.h +++ b/usr/config.h @@ -164,11 +164,11 @@ typedef enum discovery_type { DISCOVERY_TYPE_FW, } discovery_type_e; -typedef enum noop_op_type { - NOOP_OP_NONE, - NOOP_OP_DUMP, - NOOP_OP_CLEAR, -} noop_op_type_e; +typedef enum debug_info_op_type { + DBG_INFO_OP_NONE, + DBG_INFO_OP_DUMP_NOOP, + DBG_INFO_OP_CLEAR_NOOP, +} debug_info_op_type_e; typedef struct conn_rec { iscsi_startup_e startup; diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c index bcbaf27..4670118 100644 --- a/usr/iscsiadm.c +++ b/usr/iscsiadm.c @@ -95,7 +95,7 @@ static struct option const long_options[] = {"help", no_argument, NULL, 'h'}, {NULL, 0, NULL, 0}, }; -static char *short_options = "RlVhm:p:P:T:H:I:U:k:L:d:r:n:v:o:sSt:u:i:"; +static char *short_options = "RlVhm:p:P:T:H:I:U:k:L:d:r:n:v:o:sSt:ui:"; static void usage(int status) { @@ -179,17 +179,17 @@ str_to_type(char *str) return type; } -static noop_op_type_e -str_to_noop_op(char *str) +static debug_info_op_type_e +str_to_debug_info_op(char *str) { - noop_op_type_e op; + debug_info_op_type_e op; - if (!strcmp("dump", str)) - op = NOOP_OP_DUMP; - else if (!strcmp("clear", str)) - op = NOOP_OP_CLEAR; + if (!strcmp("dump_noop", str)) + op = DBG_INFO_OP_DUMP_NOOP; + else if (!strcmp("clear_noop", str)) + op = DBG_INFO_OP_CLEAR_NOOP; else - op = NOOP_OP_NONE; + op = DBG_INFO_OP_NONE; return op; } @@ -1545,7 +1545,8 @@ update_fail: static int exec_node_op(int op, int do_login, int do_logout, int do_show, int do_rescan, int do_stats, int info_level, struct node_rec *rec, - char *name, char *value, noop_op_type_e noop_op) + char *name, char *value, + debug_info_op_type_e debug_info_op) { int rc = 0; struct db_set_param set_param; @@ -1589,20 +1590,20 @@ static int exec_node_op(int op, int do_login, int do_logout, if ((!do_login && !do_logout && op == OP_NOOP && - noop_op == NOOP_OP_NONE) && + debug_info_op == DBG_INFO_OP_NONE) && (!strlen(rec->name) && !strlen(rec->conn[0].address) && !strlen(rec->iface.name))) { rc = print_nodes(info_level, rec); goto out; } - switch (noop_op) { - case NOOP_OP_DUMP: + switch (debug_info_op) { + case DBG_INFO_OP_DUMP_NOOP: if (for_each_session(rec, session_noop_dump)) rc = -1; goto out; - case NOOP_OP_CLEAR: + case DBG_INFO_OP_CLEAR_NOOP: if (for_each_session(rec, session_noop_clear)) rc = -1; goto out; @@ -1832,7 +1833,7 @@ main(int argc, char **argv) struct iface_rec *iface = NULL, *tmp; struct node_rec *rec = NULL; uint32_t host_no = -1; - noop_op_type_e noop_op = NOOP_OP_NONE; + debug_info_op_type_e debug_info_op = DBG_INFO_OP_NONE; memset(&drec, 0, sizeof(discovery_rec_t)); INIT_LIST_HEAD(&ifaces); @@ -1960,11 +1961,12 @@ main(int argc, char **argv) break; case 'i': /* Currently, we have only noop info here */ - noop_op = str_to_noop_op(optarg); - if (noop_op == NOOP_OP_NONE) { - log_error("Invalid noop operation '%s'. Noop " - "operation must be 'dump' or " - "'clear'.", optarg); + debug_info_op = str_to_debug_info_op(optarg); + if (debug_info_op == DBG_INFO_OP_NONE) { + log_error("Invalid debug info operation '%s'. " + "Debug info operation must be " + "'dump_noop' or 'clear_noop'.", + optarg); rc = -1; goto free_ifaces; } @@ -2201,7 +2203,7 @@ main(int argc, char **argv) rc = exec_node_op(op, do_login, do_logout, do_show, do_rescan, do_stats, info_level, rec, - name, value, noop_op); + name, value, debug_info_op); break; case MODE_SESSION: if ((rc = verify_mode_params(argc, argv, @@ -2260,17 +2262,17 @@ main(int argc, char **argv) /* drop down to node ops */ rc = exec_node_op(op, do_login, do_logout, do_show, do_rescan, do_stats, info_level, - rec, name, value, noop_op); + rec, name, value, debug_info_op); free_info: free(info); goto out; } else { if (do_logout || do_rescan || do_stats || - noop_op != NOOP_OP_NONE) { + debug_info_op != DBG_INFO_OP_NONE) { rc = exec_node_op(op, do_login, do_logout, do_show, do_rescan, do_stats, info_level, NULL, name, value, - noop_op); + debug_info_op); goto out; } -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-is...@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.