[PATCH] Disable NOP-IN and adjust state for closed conn

[PATCH] Disable NOP-IN and adjust state for closed conn

[Anton Kovalenko]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

I'm attaching the patch fixing one more problem noticed with non-zero nop_interval:

There is a couple of problems when a connection is closed by
iscsi_tcp_nop_work_handler, and there are uncompleted scsi commands for
that connection:

1) usr/iscsi/iscsid.c:iscsi_scsi_cmd_done assumes that a closed connection
has .state == STATE_CLOSE. The assumption fails if the connection is
closed not as a reaction to STATE_CLOSE, but by a raw conn_close call
outside of rx/tx handlers (like it happens for unanswered NOP-INs).
This commit takes care to set .state = STATE_CLOSE in ep_close callback,
so incomplete scsi commands will be freed on completion.

2) the connection doesn't go away from iscsi_tcp_conn_list until its
refcount reaches zero (see iscsi_tcp_release). It makes
iscsi_tcp_nop_work_handler call conn_close for the same connection
repeatedly, as long as it has incomplete scsi commands (hence non-zero
refcount). This commit zeroes out nop_interval for closed connections,
so next iscsi_tcp_nop_work_handler calls won't try to close the
connection any more.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Disable-NOP-IN-and-adjust-state-for-closed-conn.patch --]
[-- Type: text/x-patch; name="0001-Disable-NOP-IN-and-adjust-state-for-closed-conn.patch", Size: 1682 bytes --]

From ac1b732b612fe2c188dcc2441e5dd247d3452836 Mon Sep 17 00:00:00 2001
From: Anton Kovalenko <anton.kovalenko@acronis.com>
Date: Tue, 30 Aug 2016 19:02:51 +0300
Subject: [PATCH] Disable NOP-IN and adjust state for closed conn

There is a couple of problems when a connection is closed by
iscsi_tcp_nop_work_handler, and there are uncompleted scsi commands for
that connection:

1) usr/iscsi/iscsid.c:iscsi_scsi_cmd_done assumes that a closed connection
has .state == STATE_CLOSE. The assumption fails if the connection is
closed not as a reaction to STATE_CLOSE, but by a raw conn_close call
outside of rx/tx handlers (like it happens for unanswered NOP-INs).
This commit takes care to set .state = STATE_CLOSE in ep_close callback,
so incomplete scsi commands will be freed on completion.

2) the connection doesn't go away from iscsi_tcp_conn_list until its
refcount reaches zero (see iscsi_tcp_release). It makes
iscsi_tcp_nop_work_handler call conn_close for the same connection
repeatedly, as long as it has incomplete scsi commands (hence non-zero
refcount). This commit zeroes out nop_interval for closed connections,
so next iscsi_tcp_nop_work_handler calls won't try to close the
connection any more.
---
 usr/iscsi/iscsi_tcp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/usr/iscsi/iscsi_tcp.c b/usr/iscsi/iscsi_tcp.c
index 4cc2cf4..536f22e 100644
--- a/usr/iscsi/iscsi_tcp.c
+++ b/usr/iscsi/iscsi_tcp.c
@@ -508,6 +508,8 @@ static size_t iscsi_tcp_close(struct iscsi_connection *conn)
 	struct iscsi_tcp_connection *tcp_conn = TCP_CONN(conn);
 
 	tgt_event_del(tcp_conn->fd);
+	conn->state = STATE_CLOSE;
+	tcp_conn->nop_interval = 0;
 	return 0;
 }
 
-- 
2.9.3

Re: [PATCH] Disable NOP-IN and adjust state for closed conn

[FUJITA Tomonori]

On Tue, 30 Aug 2016 17:09:21 +0000
Anton Kovalenko <Anton.Kovalenko@acronis.com> wrote:

> From ac1b732b612fe2c188dcc2441e5dd247d3452836 Mon Sep 17 00:00:00 2001
> From: Anton Kovalenko <anton.kovalenko@acronis.com>
> Date: Tue, 30 Aug 2016 19:02:51 +0300
> Subject: [PATCH] Disable NOP-IN and adjust state for closed conn
> 
> There is a couple of problems when a connection is closed by
> iscsi_tcp_nop_work_handler, and there are uncompleted scsi commands for
> that connection:
> 
> 1) usr/iscsi/iscsid.c:iscsi_scsi_cmd_done assumes that a closed connection
> has .state == STATE_CLOSE. The assumption fails if the connection is
> closed not as a reaction to STATE_CLOSE, but by a raw conn_close call
> outside of rx/tx handlers (like it happens for unanswered NOP-INs).
> This commit takes care to set .state = STATE_CLOSE in ep_close callback,
> so incomplete scsi commands will be freed on completion.
> 
> 2) the connection doesn't go away from iscsi_tcp_conn_list until its
> refcount reaches zero (see iscsi_tcp_release). It makes
> iscsi_tcp_nop_work_handler call conn_close for the same connection
> repeatedly, as long as it has incomplete scsi commands (hence non-zero
> refcount). This commit zeroes out nop_interval for closed connections,
> so next iscsi_tcp_nop_work_handler calls won't try to close the
> connection any more.
> ---
>  usr/iscsi/iscsi_tcp.c | 2 ++
>  1 file changed, 2 insertions(+)

Looks good. Thanks!