[PATCH] Maintain a list of nop-out PDUs that almost timed out

5 views
Skip to first unread message

Erez Zilber

unread,
Dec 1, 2009, 7:57:40 AM12/1/09
to open-...@googlegroups.com
On Sun, Nov 22, 2009 at 11:24 PM, Erez Zilber <erezz...@gmail.com> wrote:
> On Sun, Nov 22, 2009 at 10:44 PM, Mike Christie <mich...@cs.wisc.edu> wrote:
>> Erez Zilber wrote:
>>> On Thu, Nov 19, 2009 at 10:02 PM, Mike Christie <mich...@cs.wisc.edu> wrote:
>>>> Ulrich Windl wrote:
>>>>> On 19 Nov 2009 at 11:07, Erez Zilber wrote:
>>>>>
>>>>>> On Thu, Nov 19, 2009 at 9:38 AM, Ulrich Windl
>>>>>> <ulrich...@rz.uni-regensburg.de> wrote:
>>>>>>> Hi!
>>>>>>>
>>>>>>> Wouldn't it be more obvious to calculate the average delay to a ping request?
>>>>>>> (Possibly exponential average as for the system loads) (min and Max would be good
>>>>>>> as well, but standard deviation probably requires use of the FPU, so that's not
>>>>>>> possible in kernel modules (AFAIK)).
>>>>>> It's in userspace, so (almost) everything is possible. It's nice to
>>>>>> have counters, average delay etc, but I want to be able to know
>>>>>> exactly when bad things almost happened (i.e. timeout almost expired).
>>>>>> Counters/average delay will not help me.
>>>>> I thought you want to tune the timeouts. So if properly tuned, the kernel will log
>>>>> when when your measurements are unusual (i.e. timeout exceeded).
>>>>>
>>>> I think that is what I wanted. I think Erez wants something a little
>>>> different, right Erez?
>>>
>>> I think that it would be nice if we had both:
>>> 1. The average delay of a ping request.
>>> 2. A list of ping requests that almost timed out with some helpful
>>> info (when was the ping sent and how much time until we got a
>>> response). With this information, you can understand and debug the
>>> whole system: you can check your target and see what caused it to be
>>> so slow on that specific time, you can see if your network was very
>>> busy during that time etc.
>>>
>>
>> I think this sounds good to me.
>>
>
> Great. I will try to send a patch soon.
>
> Erez
>

Maintain a list of nop-out PDUs that almost timed out.
With this information, you can understand and debug the
whole system: you can check your target and see what caused
it to be so slow on that specific time, you can see if your
network was very busy during that time etc.

Signed-off-by: Erez Zilber <erezz...@gmail.com>
Maintain-a-list-of-nop-out-PDUs-that-almost-timed-ou.patch

Erez Zilber

unread,
Dec 1, 2009, 8:06:31 AM12/1/09
to open-...@googlegroups.com
One comment - I've created a compat patch only for 2.6.14-23. If
everybody is pleased with this patch, I will make sure that all other
compat patches work and resend the patch.

Erez

Ulrich Windl

unread,
Dec 1, 2009, 8:16:45 AM12/1/09
to open-...@googlegroups.com
On 1 Dec 2009 at 14:57, Erez Zilber wrote:

> Maintain a list of nop-out PDUs that almost timed out.
> With this information, you can understand and debug the
> whole system: you can check your target and see what caused
> it to be so slow on that specific time, you can see if your
> network was very busy during that time etc.
>

Hi!

Having studied TCP overload protection and flow control mechanisms recently, I
wondered if a look at the TCP window sizes could be a indicator equivalent to
timed-out nops. My idea is: Why implement something, if it's possibly already
there for free.

Regards,
Ulrich

Erez Zilber

unread,
Dec 1, 2009, 8:38:10 AM12/1/09
to open-...@googlegroups.com
We've discussed and agreed on the idea of a list of nop-out PDUs that
almost timed out
(http://groups.google.com/group/open-iscsi/msg/02b93de35e80697c) and
here's the patch. I'm not familiar enough with the mechanism that you
describe, but the patch that I sent does the job with an easy to use
iscsiadm interface.

Erez

Mike Christie

unread,
Dec 7, 2009, 12:05:55 PM12/7/09
to open-...@googlegroups.com
Erez Zilber wrote:
>
> Maintain a list of nop-out PDUs that almost timed out.
> With this information, you can understand and debug the
> whole system: you can check your target and see what caused
> it to be so slow on that specific time, you can see if your
> network was very busy during that time etc.
>

Thanks Erez. I was moving last week, and am looking it over now.

Mike Christie

unread,
Dec 7, 2009, 12:20:05 PM12/7/09
to open-...@googlegroups.com
The problem with the nop timeout code is that it detects:

1 If the target is not reachable because something wrong is in the network.
2 If the target is dead.
3 If the network layer is not sending/receiving data fast enough (within
the nop timeout).

#3 is a problem because we do not know if it is not sending/receiving
data quickly because of #1 or #2 or just because we are trying to
process more data than the network can handle within the nop timeout value.

Do you thing we should we be trying to send iscsi pdus with data
segments that are smaller than the window size or some other value or
something like that? Or is there a way to get the time it is taking for
tcp packets, and could we use that to automatically determine the nop
value? Should we just send a network ping and forget doing the iscsi
nop/ping?

Ulrich Windl

unread,
Dec 8, 2009, 3:24:47 AM12/8/09
to open-...@googlegroups.com
On 7 Dec 2009 at 11:20, Mike Christie wrote:

> Ulrich Windl wrote:
> > On 1 Dec 2009 at 14:57, Erez Zilber wrote:
> >
> >> Maintain a list of nop-out PDUs that almost timed out.
> >> With this information, you can understand and debug the
> >> whole system: you can check your target and see what caused
> >> it to be so slow on that specific time, you can see if your
> >> network was very busy during that time etc.
> >>
> >
> > Hi!
> >
> > Having studied TCP overload protection and flow control mechanisms recently, I
> > wondered if a look at the TCP window sizes could be a indicator equivalent to
> > timed-out nops. My idea is: Why implement something, if it's possibly already
> > there for free.
> >
>
> The problem with the nop timeout code is that it detects:
>
> 1 If the target is not reachable because something wrong is in the network.
> 2 If the target is dead.
> 3 If the network layer is not sending/receiving data fast enough (within
> the nop timeout).
>
> #3 is a problem because we do not know if it is not sending/receiving
> data quickly because of #1 or #2 or just because we are trying to
> process more data than the network can handle within the nop timeout value.
>
> Do you thing we should we be trying to send iscsi pdus with data
> segments that are smaller than the window size or some other value or
> something like that? Or is there a way to get the time it is taking for

No, I mean if the network is dead (#1), sending nop's doesn't help.
If the target is dead (#2), TCP will time out anyway.
I'm unsure about #3: You want to check a guaranteed round-trip time (which is the
nop timeout). But what can you really do if the nop times out? You can notice that
the network doesn't guarante your expectations. Re-establishing a connection won't
make the network faster, I'm afraid. But can't you get that (timestamps) piggy-
backed on TCP anyway?

> tcp packets, and could we use that to automatically determine the nop
> value? Should we just send a network ping and forget doing the iscsi
> nop/ping?

I basically meant this: If the network "fills", the TCP sending window will
shrink, and if the network is doing well, the window will widen (that's the
overload control). If the receiver is not ready to accept data, the window size
will be zero (that's flow control). Now if the network is "tight", sending a NOP
over the same TCP connection may not have the desired effect.

So NOPs just answer the question "are you still alive over there?", but it does
not answer the question "how fast are you" in all the cases.

Maybe on a switched LAN things are all different from what I describe...

Regards,
Ulrich

Mike Christie

unread,
Dec 15, 2009, 3:34:55 AM12/15/09
to open-...@googlegroups.com
Erez Zilber wrote:
> Maintain a list of nop-out PDUs that almost timed out.
> With this information, you can understand and debug the
> whole system: you can check your target and see what caused
> it to be so slow on that specific time, you can see if your
> network was very busy during that time etc.
>
> Signed-off-by: Erez Zilber <erezz...@gmail.com>
>

Sorry for the late reply. Thanks for doing this work!


@@ -88,11 +89,12 @@ static struct option const long_options[] =
{"killiscsid", required_argument, NULL, 'k'},
{"debug", required_argument, NULL, 'd'},
{"show", no_argument, NULL, 'S'},
+ {"noopinfo", required_argument, NULL, 'N'},
{"version", no_argument, NULL, 'V'},
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0},
};


Do you think you want something more generic like a get-debug-info type
of feature? Maybe in the future we could also show how many times a scsi
command has timed out or almost timed out (user can adjust scsi timer
then) or how many times the scsi eh has fired and how many times a
abort, lu or target reset has timed out?

So maybe it would be get like get stats where then the noop info is the
first stat and in the future we will have more?




@@ -994,8 +997,25 @@ static int iscsi_nop_out_rsp(struct iscsi_task *task,
if (iscsi_recv_pdu(conn->cls_conn, (struct iscsi_hdr *)nop,
data, datalen))
rc = ISCSI_ERR_CONN_FAILED;
- } else
+ } else {
+ ping_delay = jiffies - conn->last_ping;
+
+ /* Check if the ping has almost timed out */
+ if (ping_delay >=
+ (session->noop_threshold * (conn->ping_timeout * HZ)) /
+ 100) {
+ mutex_lock(&conn->noop_info_mutex);



We annot use a mutex here, because we run from a bottom half (softirq
for iscsi and a tasklet for iser), and we cannot sleep in a bh since
there is no process context.


+ idx = conn->noop_info_arr_idx;
+ conn->noop_info_arr[idx].tv = conn->tmp_noop_tv;
+ conn->noop_info_arr[idx].elapsed_time_msec =
ping_delay;

I think ping_delay is just in jiffies, so you have to do some sort of
jiffies_to_msec call (I cannot rememeber the name of the helper but
there is one).


+ conn->noop_info_arr[idx].init = 1;
+ conn->noop_info_arr_idx =
+ (conn->noop_info_arr_idx + 1) %
NOOP_INFO_ARR_LEN;


I am not getting the reason for the division?


+ mutex_unlock(&conn->noop_info_mutex);
+ }
+
mod_timer(&conn->transport_timer, jiffies +
conn->recv_timeout);
+ }
iscsi_complete_task(task, ISCSI_TASK_COMPLETED);
return rc;
}



@@ -2026,6 +2046,12 @@ static void
iscsi_check_transport_timeouts(unsigned long data)
if (time_before_eq(last_recv + recv_timeout, jiffies)) {
/* send a ping to try to provoke some traffic */
ISCSI_DBG_CONN(conn, "Sending nopout as ping\n");
+
+ /* First, save the time of the ping for later use */
+ mutex_lock(&conn->noop_info_mutex);

Cannot use a mutex here either, because this is run from a timer. Timers
are run from bhs too.


+ do_gettimeofday(&conn->tmp_noop_tv);
+ mutex_unlock(&conn->noop_info_mutex);
+

Erez Zilber

unread,
Dec 15, 2009, 6:23:15 AM12/15/09
to open-...@googlegroups.com
On Tue, Dec 15, 2009 at 10:34 AM, Mike Christie <mich...@cs.wisc.edu> wrote:
> Erez Zilber wrote:
>> Maintain a list of nop-out PDUs that almost timed out.
>> With this information, you can understand and debug the
>> whole system: you can check your target and see what caused
>> it to be so slow on that specific time, you can see if your
>> network was very busy during that time etc.
>>
>> Signed-off-by: Erez Zilber <erezz...@gmail.com>
>>
>
> Sorry for the late reply. Thanks for doing this work!
>
>
> @@ -88,11 +89,12 @@ static struct option const long_options[] =
>         {"killiscsid", required_argument, NULL, 'k'},
>         {"debug", required_argument, NULL, 'd'},
>         {"show", no_argument, NULL, 'S'},
> +       {"noopinfo", required_argument, NULL, 'N'},
>         {"version", no_argument, NULL, 'V'},
>         {"help", no_argument, NULL, 'h'},
>         {NULL, 0, NULL, 0},
>  };
>
>
> Do you think you want something more generic like a get-debug-info type
> of feature? Maybe in the future we could also show how many times a scsi
> command has timed out or almost timed out (user can adjust scsi timer
> then) or how many times the scsi eh has fired and how many times a
> abort, lu or target reset has timed out?
>
> So maybe it would be get like get stats where then the noop info is the
> first stat and in the future we will have more?

Sounds good. Let's change it to {"debuginfo", required_argument, NULL, 'i'}. OK?
>
>
>
>
> @@ -994,8 +997,25 @@ static int iscsi_nop_out_rsp(struct iscsi_task *task,
>                 if (iscsi_recv_pdu(conn->cls_conn, (struct iscsi_hdr *)nop,
>                                    data, datalen))
>                         rc = ISCSI_ERR_CONN_FAILED;
> -       } else
> +       } else {
> +               ping_delay = jiffies - conn->last_ping;
> +
> +               /* Check if the ping has almost timed out */
> +               if (ping_delay >=
> +                   (session->noop_threshold * (conn->ping_timeout * HZ)) /
> +                   100) {
> +                       mutex_lock(&conn->noop_info_mutex);
>
>
>
> We annot use a mutex here, because we run from a bottom half (softirq
> for iscsi and a tasklet for iser), and we cannot sleep in a bh since
> there is no process context.

You're right. I'll replace the mutex with a spinlock.

>
>
> +                       idx = conn->noop_info_arr_idx;
> +                       conn->noop_info_arr[idx].tv = conn->tmp_noop_tv;
> +                       conn->noop_info_arr[idx].elapsed_time_msec =
> ping_delay;
>
> I think ping_delay is just in jiffies, so you have to do some sort of
> jiffies_to_msec call (I cannot rememeber the name of the helper but
> there is one).

I think it should be: conn->noop_info_arr[idx].elapsed_time_msec =
ping_delay * HZ / 1000

>
>
> +                       conn->noop_info_arr[idx].init = 1;
> +                       conn->noop_info_arr_idx =
> +                               (conn->noop_info_arr_idx + 1) %
> NOOP_INFO_ARR_LEN;
>
>
> I am not getting the reason for the division?

Are you talking about "conn->noop_info_arr_idx =
(conn->noop_info_arr_idx + 1) % NOOP_INFO_ARR_LEN"? It's a cyclic
array.

>
>
> +                       mutex_unlock(&conn->noop_info_mutex);
> +               }
> +
>                 mod_timer(&conn->transport_timer, jiffies +
> conn->recv_timeout);
> +       }
>         iscsi_complete_task(task, ISCSI_TASK_COMPLETED);
>         return rc;
>  }
>
>
>
> @@ -2026,6 +2046,12 @@ static void
> iscsi_check_transport_timeouts(unsigned long data)
>         if (time_before_eq(last_recv + recv_timeout, jiffies)) {
>                 /* send a ping to try to provoke some traffic */
>                 ISCSI_DBG_CONN(conn, "Sending nopout as ping\n");
> +
> +               /* First, save the time of the ping for later use */
> +               mutex_lock(&conn->noop_info_mutex);
>
> Cannot use a mutex here either, because this is run from a timer. Timers
> are run from bhs too.

mutex -> spinlock.

Let me know if you're OK with my suggestions and I'll resend the patch.

Erez

Mike Christie

unread,
Dec 16, 2009, 10:01:35 PM12/16/09
to open-...@googlegroups.com

Ok with me.

I think the function is jiffies_to_msecs()

>
>>
>> + conn->noop_info_arr[idx].init = 1;
>> + conn->noop_info_arr_idx =
>> + (conn->noop_info_arr_idx + 1) %
>> NOOP_INFO_ARR_LEN;
>>
>>
>> I am not getting the reason for the division?
>
> Are you talking about "conn->noop_info_arr_idx =
> (conn->noop_info_arr_idx + 1) % NOOP_INFO_ARR_LEN"? It's a cyclic
> array.
>

Ah, nevermind :)

Erez Zilber

unread,
Dec 20, 2009, 8:57:23 AM12/20/09
to open-...@googlegroups.com
> --
>

Attached with the fixes that we discussed. I tested it with a 2.6.18
kernel. I was able to apply the following patches:

* 2.6.14-23_compat.patch
* 2.6.24_compat.patch
* 2.6.26_compat.patch
* 2.6.27_compat.patch

What about the following patches?
* 2.6.14-19_compat.patch
* 2.6.16-suse.patch
* 2.6.20-21_compat.patch

Are they still maintained? If not, can we throw (at least some of) them away?

Erez

Maintain-a-list-of-nop-out-PDUs-that-almost-timed-ou-v2.patch

Erez Zilber

unread,
Dec 28, 2009, 6:10:11 AM12/28/09
to open-...@googlegroups.com

Found a small (and stupid) bug in my patch:

diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c
index 6188f43..ee4cfc4 100644
--- a/usr/iscsiadm.c
+++ b/usr/iscsiadm.c
@@ -94,7 +94,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)
{

Mike - I will send an updated patch after you review the rest of the patch.

Erez

Mike Christie

unread,
Jan 6, 2010, 1:31:15 PM1/6/10
to open-...@googlegroups.com, Erez Zilber

Thanks for doing this and sorry for the late reply.

> * 2.6.14-23_compat.patch
> * 2.6.24_compat.patch
> * 2.6.26_compat.patch
> * 2.6.27_compat.patch
>
> What about the following patches?
> * 2.6.14-19_compat.patch

not used

> * 2.6.16-suse.patch


I think Chelsio fixed up the 2.6.14-19_compat.patch patch to work with
the older SLES, so I do not think this is needed. Do not worry about it
though. It is broken.

> * 2.6.20-21_compat.patch


not needed.

>
> Are they still maintained? If not, can we throw (at least some of) them away?

Yeah. Will clean up the repository.

Testing out patch now.

Erez Zilber

unread,
Jan 28, 2010, 10:25:51 AM1/28/10
to Mike Christie, open-...@googlegroups.com
On Tue, Jan 19, 2010 at 2:27 PM, Mike Christie <mich...@cs.wisc.edu> wrote:

> On 12/28/2009 05:10 AM, Erez Zilber wrote:
>>
>> diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c
>> index 6188f43..ee4cfc4 100644
>> --- a/usr/iscsiadm.c
>> +++ b/usr/iscsiadm.c
>> @@ -94,7 +94,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)
>>  {
>>
>
> Sorry the late reply, and thanks again for working on this.
>
>
> @@ -317,6 +328,8 @@ enum iscsi_param {
>        ISCSI_PARAM_ISID,
>        ISCSI_PARAM_INITIATOR_NAME,
>
> +       ISCSI_PARAM_NOOP_THRESHOLD,
> +
>        ISCSI_PARAM_TGT_RESET_TMO,
>
>
> ISCSI_PARAM_NOOP_THRESHOLD should be after ISCSI_PARAM_TGT_RESET_TMO. I
> think it got messed up when you updated a patch.

Done.

>
>
> +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?

>
>
>
> +       unsigned int elapsed_time_msec;
> +       short init;


> +};
>
>
>               ping_delay = jiffies - conn->last_ping;
> +
> +               /* Check if the ping has almost timed out */
> +               if (ping_delay >=
> +                   (session->noop_threshold * (conn->ping_timeout * HZ)) /
>

> Does this handle jiffies rollover ok? If jiffies rolled over and it is 1 but
> last_ping was ULONG_MAX, then ping_delay is going to be very large.

Fixed.

>
>
>
> +static noop_op_type_e
> +str_to_noop_op(char *str)
> +{
> +       noop_op_type_e op;
> +
> +       if (!strcmp("dump", str))
> +               op = NOOP_OP_DUMP;
> +       else if (!strcmp("clear", str))
> +               op = NOOP_OP_CLEAR;
> +       else
> +               op = NOOP_OP_NONE;
> +
> +       return op;
> +}
>
> Should we make these names clear that it resets/dumps the noop data. If we
> added a scsi cmd timer option, then we probably want to be able to
> dump/reset one or the other. So rename dump to dump_noop and clear to
> clear_noop.

Fixed. We also need to replace str_to_noop_op with
str_to_debug_info_op (and change the enum accordingly).

>
>
>
> case MODE_NODE:
>        if ((rc = verify_mode_params(argc, argv, "RsPIdmlSonvupTUL",
>
> This line needs a 'i'. It looks like you added the node compat support but
> you need to enable it with the 'i'. I added it when I tested your patch and
> it worked fine, so just added it when you send the patch.
>
>

OK

> Also what about debugfs? Did you think you would want the flexibility? Is
> that too many interfaces (sysfs, netlink, bsg and debugfs)?

I'm not very familiar with debugfs. What do you want to do with it?

Erez

Mike Christie

unread,
Jan 29, 2010, 1:59:02 PM1/29/10
to open-...@googlegroups.com, Erez Zilber
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'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.

Erez Zilber

unread,
Jan 31, 2010, 9:44:54 AM1/31/10
to open-...@googlegroups.com
On Fri, Jan 29, 2010 at 8:59 PM, Mike Christie <mich...@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;
}

Reply all
Reply to author
Forward
0 new messages