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.
