[PATCH 0/5] iscsi/iser-target fixes+cleanups for v3.11-rc1 (v2)

[Nicholas A. Bellinger]

From: Nicholas Bellinger <n...@linux-iscsi.org>

Hi folks,

This is an updated iscsi/iser-target bugfix + cleanup series headed
for v3.11-rc1 code.

The first patch includes the changes necessary to post ISCSI_OP_REJECT
payloads using iser-target, and second patch avoids iscsi_cmd->reject_comp
usage that was causing a free-after-use bug in iser-target as well, along
with a conversion of legacy iscsit_add_reject* parameter usage throughout
iscsi-target code.

The third patch interalizes all iscsi_sequence_cmd() level rejects, as to
avoid per-transport uses of iscsit_reject_cmd(), and the forth patch fixes
a seperate ISCSI_OP_SCSI_TMFUNC handling bug reported recently that was
triggering an OOPs during iscsit_close_connection() cleanup.

The fifth patch allows RDMA_CM_EVENT_DISCONNECTED events to invoke
rdma_disconnect() if rx/tx thread have not already gone into connection
shutdown. This addresess a bug where the iser client initiates CM reset
before ISCSI_OP_LOGOUT occurs, and adds isert_conn->conn_mutex for
protecting isert_conn->state checks between the two process contexts.

I'll be CC'ing this series to 3.10.y, so please review.

--nab

Nicholas Bellinger (5):
iser-target: Fix isert_put_reject payload buffer post
iscsi-target: Fix iscsit_add_reject* usage for iser
iscsi-target: Fix iscsit_sequence_cmd reject handling for iser
iscsi-target: Fix ISCSI_OP_SCSI_TMFUNC handling for iser
iser-target: Fix session reset bug with RDMA_CM_EVENT_DISCONNECTED

drivers/infiniband/ulp/isert/ib_isert.c | 114 +++++++++---
drivers/infiniband/ulp/isert/ib_isert.h | 1 +
drivers/target/iscsi/iscsi_target.c | 300 +++++++++++++-----------------
drivers/target/iscsi/iscsi_target.h | 2 +-
drivers/target/iscsi/iscsi_target_core.h | 8 +-
drivers/target/iscsi/iscsi_target_erl0.c | 7 +-
drivers/target/iscsi/iscsi_target_erl1.c | 26 ++--
drivers/target/iscsi/iscsi_target_util.c | 26 ++-
drivers/target/iscsi/iscsi_target_util.h | 3 +-
include/target/iscsi/iscsi_transport.h | 5 +-
10 files changed, 264 insertions(+), 228 deletions(-)

--
1.7.2.5

[Nicholas A. Bellinger]

From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch adds the missing isert_put_reject() logic to post
a outgoing payload buffer to hold the 48 bytes of original PDU
header request payload for the rejected cmd.

It also fixes ISTATE_SEND_REJECT handling in isert_response_completion()
-> isert_do_control_comp() code, and drops incorrect iscsi_cmd_t->reject_comp
usage.

Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Mike Christie <mich...@cs.wisc.edu>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/infiniband/ulp/isert/ib_isert.c | 23 +++++++++++++++++++----
1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index c48c948..832a263 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -1339,8 +1339,8 @@ isert_do_control_comp(struct work_struct *work)
atomic_dec(&isert_conn->post_send_buf_count);

cmd->i_state = ISTATE_SENT_STATUS;
- complete(&cmd->reject_comp);
isert_completion_put(&isert_cmd->tx_desc, isert_cmd, ib_dev);
+ break;
case ISTATE_SEND_LOGOUTRSP:
pr_debug("Calling iscsit_logout_post_handler >>>>>>>>>>>>>>\n");
/*
@@ -1366,7 +1366,8 @@ isert_response_completion(struct iser_tx_desc *tx_desc,
struct iscsi_cmd *cmd = &isert_cmd->iscsi_cmd;

if (cmd->i_state == ISTATE_SEND_TASKMGTRSP ||
- cmd->i_state == ISTATE_SEND_LOGOUTRSP) {
+ cmd->i_state == ISTATE_SEND_LOGOUTRSP ||
+ cmd->i_state == ISTATE_SEND_REJECT) {
isert_unmap_tx_desc(tx_desc, ib_dev);

INIT_WORK(&isert_cmd->comp_work, isert_do_control_comp);
@@ -1658,11 +1659,25 @@ isert_put_reject(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
struct isert_cmd, iscsi_cmd);
struct isert_conn *isert_conn = (struct isert_conn *)conn->context;
struct ib_send_wr *send_wr = &isert_cmd->tx_desc.send_wr;
+ struct ib_device *ib_dev = isert_conn->conn_cm_id->device;
+ struct ib_sge *tx_dsg = &isert_cmd->tx_desc.tx_sg[1];
+ struct iscsi_reject *hdr =
+ (struct iscsi_reject *)&isert_cmd->tx_desc.iscsi_header;

isert_create_send_desc(isert_conn, isert_cmd, &isert_cmd->tx_desc);
- iscsit_build_reject(cmd, conn, (struct iscsi_reject *)
- &isert_cmd->tx_desc.iscsi_header);
+ iscsit_build_reject(cmd, conn, hdr);
isert_init_tx_hdrs(isert_conn, &isert_cmd->tx_desc);
+
+ hton24(hdr->dlength, ISCSI_HDR_LEN);
+ isert_cmd->sense_buf_dma = ib_dma_map_single(ib_dev,
+ (void *)cmd->buf_ptr, ISCSI_HDR_LEN,
+ DMA_TO_DEVICE);
+ isert_cmd->sense_buf_len = ISCSI_HDR_LEN;
+ tx_dsg->addr = isert_cmd->sense_buf_dma;
+ tx_dsg->length = ISCSI_HDR_LEN;
+ tx_dsg->lkey = isert_conn->conn_mr->lkey;
+ isert_cmd->tx_desc.num_sge = 2;
+
isert_init_send_wr(isert_cmd, send_wr);

pr_debug("Posting Reject IB_WR_SEND >>>>>>>>>>>>>>>>>>>>>>\n");
--
1.7.2.5

[Nicholas A. Bellinger]

From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch changes iscsit_add_reject() + iscsit_add_reject_from_cmd()
usage to not sleep on iscsi_cmd->reject_comp to address a free-after-use
usage bug in v3.10 with iser-target code.

It saves ->reject_reason for use within iscsit_build_reject() so the
correct value for both transport cases. It also drops the legacy
fail_conn parameter usage throughput iscsi-target code and adds
two iscsit_add_reject_cmd() and iscsit_reject_cmd helper functions,
along with various small cleanups.

Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Mike Christie <mich...@cs.wisc.edu>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---

drivers/infiniband/ulp/isert/ib_isert.c | 5 -
drivers/target/iscsi/iscsi_target.c | 257 ++++++++++++------------------

drivers/target/iscsi/iscsi_target.h | 2 +-
drivers/target/iscsi/iscsi_target_core.h | 8 +-
drivers/target/iscsi/iscsi_target_erl0.c | 7 +-

drivers/target/iscsi/iscsi_target_erl1.c | 20 +--
drivers/target/iscsi/iscsi_target_util.c | 1 -
include/target/iscsi/iscsi_transport.h | 2 -
8 files changed, 122 insertions(+), 180 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 832a263..13a4464 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -939,11 +939,6 @@ sequence_cmd:
if (!rc && dump_payload == false && unsol_data)
iscsit_set_unsoliticed_dataout(cmd);

- if (rc == CMDSN_ERROR_CANNOT_RECOVER)
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
-
return 0;
}

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 19a31f9..b5e7b7e 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -628,25 +628,18 @@ static void __exit iscsi_target_cleanup_module(void)
}

static int iscsit_add_reject(
+ struct iscsi_conn *conn,
u8 reason,
- int fail_conn,
- unsigned char *buf,
- struct iscsi_conn *conn)
+ unsigned char *buf)
{
struct iscsi_cmd *cmd;
- struct iscsi_reject *hdr;
- int ret;

cmd = iscsit_allocate_cmd(conn, GFP_KERNEL);
if (!cmd)
return -1;

cmd->iscsi_opcode = ISCSI_OP_REJECT;
- if (fail_conn)
- cmd->cmd_flags |= ICF_REJECT_FAIL_CONN;
-
- hdr = (struct iscsi_reject *) cmd->pdu;
- hdr->reason = reason;
+ cmd->reject_reason = reason;

cmd->buf_ptr = kmemdup(buf, ISCSI_HDR_LEN, GFP_KERNEL);
if (!cmd->buf_ptr) {
@@ -662,23 +655,16 @@ static int iscsit_add_reject(
cmd->i_state = ISTATE_SEND_REJECT;
iscsit_add_cmd_to_response_queue(cmd, conn, cmd->i_state);

- ret = wait_for_completion_interruptible(&cmd->reject_comp);
- if (ret != 0)
- return -1;
-
- return (!fail_conn) ? 0 : -1;
+ return -1;
}

-int iscsit_add_reject_from_cmd(
+static int iscsit_add_reject_from_cmd(
+ struct iscsi_cmd *cmd,
u8 reason,
- int fail_conn,
- int add_to_conn,
- unsigned char *buf,
- struct iscsi_cmd *cmd)
+ bool add_to_conn,
+ unsigned char *buf)
{
struct iscsi_conn *conn;
- struct iscsi_reject *hdr;
- int ret;

if (!cmd->conn) {
pr_err("cmd->conn is NULL for ITT: 0x%08x\n",
@@ -688,11 +674,7 @@ int iscsit_add_reject_from_cmd(
conn = cmd->conn;

cmd->iscsi_opcode = ISCSI_OP_REJECT;
- if (fail_conn)
- cmd->cmd_flags |= ICF_REJECT_FAIL_CONN;
-
- hdr = (struct iscsi_reject *) cmd->pdu;
- hdr->reason = reason;
+ cmd->reject_reason = reason;

cmd->buf_ptr = kmemdup(buf, ISCSI_HDR_LEN, GFP_KERNEL);
if (!cmd->buf_ptr) {
@@ -709,8 +691,7 @@ int iscsit_add_reject_from_cmd(

cmd->i_state = ISTATE_SEND_REJECT;
iscsit_add_cmd_to_response_queue(cmd, conn, cmd->i_state);
-
- ret = wait_for_completion_interruptible(&cmd->reject_comp);
+#if 0
/*
* Perform the kref_put now if se_cmd has already been setup by
* scsit_setup_scsi_cmd()
@@ -719,12 +700,20 @@ int iscsit_add_reject_from_cmd(
pr_debug("iscsi reject: calling target_put_sess_cmd >>>>>>\n");
target_put_sess_cmd(conn->sess->se_sess, &cmd->se_cmd);
}
- if (ret != 0)
- return -1;
+#endif
+ return -1;
+}

- return (!fail_conn) ? 0 : -1;
+static int iscsit_add_reject_cmd(struct iscsi_cmd *cmd, u8 reason,
+ unsigned char *buf)
+{
+ return iscsit_add_reject_from_cmd(cmd, reason, true, buf);
+}
+
+int iscsit_reject_cmd(struct iscsi_cmd *cmd, u8 reason, unsigned char *buf)
+{
+ return iscsit_add_reject_from_cmd(cmd, reason, false, buf);
}
-EXPORT_SYMBOL(iscsit_add_reject_from_cmd);

/*
* Map some portion of the allocated scatterlist to an iovec, suitable for
@@ -844,8 +833,8 @@ int iscsit_setup_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
!(hdr->flags & ISCSI_FLAG_CMD_FINAL)) {
pr_err("ISCSI_FLAG_CMD_WRITE & ISCSI_FLAG_CMD_FINAL"
" not set. Bad iSCSI Initiator.\n");
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_INVALID,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_INVALID, buf);
}

if (((hdr->flags & ISCSI_FLAG_CMD_READ) ||
@@ -865,8 +854,8 @@ int iscsit_setup_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
pr_err("ISCSI_FLAG_CMD_READ or ISCSI_FLAG_CMD_WRITE"
" set when Expected Data Transfer Length is 0 for"
" CDB: 0x%02x. Bad iSCSI Initiator.\n", hdr->cdb[0]);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_INVALID,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_INVALID, buf);
}
done:

@@ -875,62 +864,62 @@ done:
pr_err("ISCSI_FLAG_CMD_READ and/or ISCSI_FLAG_CMD_WRITE"
" MUST be set if Expected Data Transfer Length is not 0."
" Bad iSCSI Initiator\n");
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_INVALID,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_INVALID, buf);
}

if ((hdr->flags & ISCSI_FLAG_CMD_READ) &&
(hdr->flags & ISCSI_FLAG_CMD_WRITE)) {
pr_err("Bidirectional operations not supported!\n");
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_INVALID,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_INVALID, buf);
}

if (hdr->opcode & ISCSI_OP_IMMEDIATE) {
pr_err("Illegally set Immediate Bit in iSCSI Initiator"
" Scsi Command PDU.\n");
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_INVALID,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_INVALID, buf);
}

if (payload_length && !conn->sess->sess_ops->ImmediateData) {
pr_err("ImmediateData=No but DataSegmentLength=%u,"
" protocol error.\n", payload_length);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_PROTOCOL_ERROR, buf);
}

- if ((be32_to_cpu(hdr->data_length )== payload_length) &&
+ if ((be32_to_cpu(hdr->data_length) == payload_length) &&
(!(hdr->flags & ISCSI_FLAG_CMD_FINAL))) {
pr_err("Expected Data Transfer Length and Length of"
" Immediate Data are the same, but ISCSI_FLAG_CMD_FINAL"
" bit is not set protocol error\n");
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_PROTOCOL_ERROR, buf);
}

if (payload_length > be32_to_cpu(hdr->data_length)) {
pr_err("DataSegmentLength: %u is greater than"
" EDTL: %u, protocol error.\n", payload_length,
hdr->data_length);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_PROTOCOL_ERROR, buf);
}

if (payload_length > conn->conn_ops->MaxXmitDataSegmentLength) {
pr_err("DataSegmentLength: %u is greater than"
" MaxXmitDataSegmentLength: %u, protocol error.\n",
payload_length, conn->conn_ops->MaxXmitDataSegmentLength);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_PROTOCOL_ERROR, buf);
}

if (payload_length > conn->sess->sess_ops->FirstBurstLength) {
pr_err("DataSegmentLength: %u is greater than"
" FirstBurstLength: %u, protocol error.\n",
payload_length, conn->sess->sess_ops->FirstBurstLength);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_INVALID,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_INVALID, buf);
}

data_direction = (hdr->flags & ISCSI_FLAG_CMD_WRITE) ? DMA_TO_DEVICE :
@@ -985,9 +974,8 @@ done:

dr = iscsit_allocate_datain_req();
if (!dr)
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);

iscsit_attach_datain_req(cmd, dr);
}
@@ -1015,18 +1003,16 @@ done:
cmd->sense_reason = target_setup_cmd_from_cdb(&cmd->se_cmd, hdr->cdb);
if (cmd->sense_reason) {
if (cmd->sense_reason == TCM_OUT_OF_RESOURCES) {
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
}

goto attach_cmd;
}

if (iscsit_build_pdu_and_seq_lists(cmd, payload_length) < 0) {
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
}

attach_cmd:
@@ -1075,10 +1061,6 @@ int iscsit_process_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,

target_put_sess_cmd(conn->sess->se_sess, &cmd->se_cmd);
return 0;
- } else if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER) {
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
}
}

@@ -1149,11 +1131,6 @@ after_immediate_data:
} else if (cmd->unsolicited_data)
iscsit_set_unsoliticed_dataout(cmd);

- if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
-
} else if (immed_ret == IMMEDIATE_DATA_ERL1_CRC_FAILURE) {
/*
* Immediate Data failed DataCRC and ERL>=1,
@@ -1190,9 +1167,8 @@ iscsit_handle_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
* traditional iSCSI block I/O.
*/
if (iscsit_allocate_iovecs(cmd) < 0) {
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, 0, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
}
immed_data = cmd->immediate_data;

@@ -1282,8 +1258,8 @@ iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,

if (!payload_length) {
pr_err("DataOUT payload is ZERO, protocol error.\n");
- return iscsit_add_reject(ISCSI_REASON_PROTOCOL_ERROR, 1,
- buf, conn);
+ return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
+ buf);
}

/* iSCSI write */
@@ -1300,8 +1276,8 @@ iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,
pr_err("DataSegmentLength: %u is greater than"
" MaxXmitDataSegmentLength: %u\n", payload_length,
conn->conn_ops->MaxXmitDataSegmentLength);
- return iscsit_add_reject(ISCSI_REASON_PROTOCOL_ERROR, 1,
- buf, conn);
+ return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
+ buf);
}

cmd = iscsit_find_cmd_from_itt_or_dump(conn, hdr->itt,
@@ -1324,8 +1300,7 @@ iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,
if (cmd->data_direction != DMA_TO_DEVICE) {
pr_err("Command ITT: 0x%08x received DataOUT for a"
" NON-WRITE command.\n", cmd->init_task_tag);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, buf, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR, buf);
}
se_cmd = &cmd->se_cmd;
iscsit_mod_dataout_timer(cmd);
@@ -1334,8 +1309,7 @@ iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,
pr_err("DataOut Offset: %u, Length %u greater than"
" iSCSI Command EDTL %u, protocol error.\n",
hdr->offset, payload_length, cmd->se_cmd.data_length);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_INVALID,
- 1, 0, buf, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_BOOKMARK_INVALID, buf);
}

if (cmd->unsolicited_data) {
@@ -1543,8 +1517,8 @@ int iscsit_setup_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
if (hdr->itt == RESERVED_ITT && !(hdr->opcode & ISCSI_OP_IMMEDIATE)) {
pr_err("NOPOUT ITT is reserved, but Immediate Bit is"
" not set, protocol error.\n");
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR,
+ (unsigned char *)hdr);
}

if (payload_length > conn->conn_ops->MaxXmitDataSegmentLength) {
@@ -1552,8 +1526,8 @@ int iscsit_setup_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
" greater than MaxXmitDataSegmentLength: %u, protocol"
" error.\n", payload_length,
conn->conn_ops->MaxXmitDataSegmentLength);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR,
+ (unsigned char *)hdr);
}

pr_debug("Got NOPOUT Ping %s ITT: 0x%08x, TTT: 0x%08x,"
@@ -1612,9 +1586,7 @@ int iscsit_process_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
return 0;

if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
+ return -1;

return 0;
}
@@ -1780,8 +1752,8 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
pr_err("Task Management Request TASK_REASSIGN not"
" issued as immediate command, bad iSCSI Initiator"
"implementation\n");
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_PROTOCOL_ERROR, buf);
}
if ((function != ISCSI_TM_FUNC_ABORT_TASK) &&
be32_to_cpu(hdr->refcmdsn) != ISCSI_RESERVED_TAG)
@@ -1793,9 +1765,9 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
if (!cmd->tmr_req) {
pr_err("Unable to allocate memory for"
" Task Management command!\n");
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_NO_RESOURCES,
+ buf);
}

/*
@@ -1837,17 +1809,15 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
default:
pr_err("Unknown iSCSI TMR Function:"
" 0x%02x\n", function);
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
}

ret = core_tmr_alloc_req(&cmd->se_cmd, cmd->tmr_req,
tcm_function, GFP_KERNEL);
if (ret < 0)
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, 1, buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);

cmd->tmr_req->se_tmr_req = cmd->se_cmd.se_tmr_req;
}
@@ -1906,9 +1876,8 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
break;

if (iscsit_check_task_reassign_expdatasn(tmr_req, conn) < 0)
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_INVALID, 1, 1,
- buf, cmd);
+ return iscsit_add_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_INVALID, buf);
break;
default:
pr_err("Unknown TMR function: 0x%02x, protocol"
@@ -1932,9 +1901,7 @@ attach:
else if (cmdsn_ret == CMDSN_LOWER_THAN_EXP)
return 0;
else if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, buf, cmd);
+ return -1;
}
iscsit_ack_from_expstatsn(conn, be32_to_cpu(hdr->exp_statsn));

@@ -1970,8 +1937,8 @@ iscsit_setup_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
pr_err("Unable to accept text parameter length: %u"
"greater than MaxXmitDataSegmentLength %u.\n",
payload_length, conn->conn_ops->MaxXmitDataSegmentLength);
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR,
+ (unsigned char *)hdr);
}

pr_debug("Got Text Request: ITT: 0x%08x, CmdSN: 0x%08x,"
@@ -2033,17 +2000,16 @@ iscsit_process_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
if (!(hdr->opcode & ISCSI_OP_IMMEDIATE)) {
cmdsn_ret = iscsit_sequence_cmd(conn, cmd, hdr->cmdsn);
if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
+ return -1;
+
return 0;
}

return iscsit_execute_cmd(cmd, 0);

reject:
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 0, 0, (unsigned char *)hdr, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR,
+ (unsigned char *)hdr);
}
EXPORT_SYMBOL(iscsit_process_text_cmd);

@@ -2139,8 +2105,7 @@ iscsit_handle_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
reject:
kfree(cmd->text_in_ptr);
cmd->text_in_ptr = NULL;
- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 0, 0, buf, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR, buf);
}
EXPORT_SYMBOL(iscsit_handle_text_cmd);

@@ -2322,13 +2287,10 @@ iscsit_handle_logout_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
return ret;
} else {
cmdsn_ret = iscsit_sequence_cmd(conn, cmd, hdr->cmdsn);
- if (cmdsn_ret == CMDSN_LOWER_THAN_EXP) {
+ if (cmdsn_ret == CMDSN_LOWER_THAN_EXP)
logout_remove = 0;
- } else if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER) {
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, buf, cmd);
- }
+ else if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
+ return -1;
}

return logout_remove;
@@ -2352,8 +2314,8 @@ static int iscsit_handle_snack(
if (!conn->sess->sess_ops->ErrorRecoveryLevel) {
pr_err("Initiator sent SNACK request while in"
" ErrorRecoveryLevel=0.\n");
- return iscsit_add_reject(ISCSI_REASON_PROTOCOL_ERROR, 1,
- buf, conn);
+ return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
+ buf);
}
/*
* SNACK_DATA and SNACK_R2T are both 0, so check which function to
@@ -2377,13 +2339,13 @@ static int iscsit_handle_snack(
case ISCSI_FLAG_SNACK_TYPE_RDATA:
/* FIXME: Support R-Data SNACK */
pr_err("R-Data SNACK Not Supported.\n");
- return iscsit_add_reject(ISCSI_REASON_PROTOCOL_ERROR, 1,
- buf, conn);
+ return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
+ buf);
default:
pr_err("Unknown SNACK type 0x%02x, protocol"
" error.\n", hdr->flags & 0x0f);
- return iscsit_add_reject(ISCSI_REASON_PROTOCOL_ERROR, 1,
- buf, conn);
+ return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
+ buf);
}

return 0;
@@ -2455,14 +2417,14 @@ static int iscsit_handle_immediate_data(
pr_err("Unable to recover from"
" Immediate Data digest failure while"
" in ERL=0.\n");
- iscsit_add_reject_from_cmd(
+ iscsit_reject_cmd(cmd,
ISCSI_REASON_DATA_DIGEST_ERROR,
- 1, 0, (unsigned char *)hdr, cmd);
+ (unsigned char *)hdr);
return IMMEDIATE_DATA_CANNOT_RECOVER;
} else {
- iscsit_add_reject_from_cmd(
+ iscsit_reject_cmd(cmd,
ISCSI_REASON_DATA_DIGEST_ERROR,
- 0, 0, (unsigned char *)hdr, cmd);
+ (unsigned char *)hdr);
return IMMEDIATE_DATA_ERL1_CRC_FAILURE;
}
} else {
@@ -3595,6 +3557,7 @@ iscsit_build_reject(struct iscsi_cmd *cmd, struct iscsi_conn *conn,
struct iscsi_reject *hdr)
{
hdr->opcode = ISCSI_OP_REJECT;
+ hdr->reason = cmd->reject_reason;
hdr->flags |= ISCSI_FLAG_CMD_FINAL;
hton24(hdr->dlength, ISCSI_HDR_LEN);
hdr->ffffffff = cpu_to_be32(0xffffffff);
@@ -3868,18 +3831,11 @@ check_rsp_state:
case ISTATE_SEND_STATUS_RECOVERY:
case ISTATE_SEND_TEXTRSP:
case ISTATE_SEND_TASKMGTRSP:
+ case ISTATE_SEND_REJECT:
spin_lock_bh(&cmd->istate_lock);
cmd->i_state = ISTATE_SENT_STATUS;
spin_unlock_bh(&cmd->istate_lock);
break;
- case ISTATE_SEND_REJECT:
- if (cmd->cmd_flags & ICF_REJECT_FAIL_CONN) {
- cmd->cmd_flags &= ~ICF_REJECT_FAIL_CONN;
- complete(&cmd->reject_comp);
- goto err;
- }
- complete(&cmd->reject_comp);
- break;
default:
pr_err("Unknown Opcode: 0x%02x ITT:"
" 0x%08x, i_state: %d on CID: %hu\n",
@@ -3984,8 +3940,7 @@ static int iscsi_target_rx_opcode(struct iscsi_conn *conn, unsigned char *buf)
case ISCSI_OP_SCSI_CMD:
cmd = iscsit_allocate_cmd(conn, GFP_KERNEL);
if (!cmd)
- return iscsit_add_reject(ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, buf, conn);
+ goto reject;

ret = iscsit_handle_scsi_cmd(conn, cmd, buf);
break;
@@ -3997,32 +3952,28 @@ static int iscsi_target_rx_opcode(struct iscsi_conn *conn, unsigned char *buf)
if (hdr->ttt == cpu_to_be32(0xFFFFFFFF)) {
cmd = iscsit_allocate_cmd(conn, GFP_KERNEL);
if (!cmd)
- return iscsit_add_reject(ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, buf, conn);
+ goto reject;
}
ret = iscsit_handle_nop_out(conn, cmd, buf);
break;
case ISCSI_OP_SCSI_TMFUNC:
cmd = iscsit_allocate_cmd(conn, GFP_KERNEL);
if (!cmd)
- return iscsit_add_reject(ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, buf, conn);
+ goto reject;

ret = iscsit_handle_task_mgt_cmd(conn, cmd, buf);
break;
case ISCSI_OP_TEXT:
cmd = iscsit_allocate_cmd(conn, GFP_KERNEL);
if (!cmd)
- return iscsit_add_reject(ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, buf, conn);
+ goto reject;

ret = iscsit_handle_text_cmd(conn, cmd, buf);
break;
case ISCSI_OP_LOGOUT:
cmd = iscsit_allocate_cmd(conn, GFP_KERNEL);
if (!cmd)
- return iscsit_add_reject(ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, buf, conn);
+ goto reject;

ret = iscsit_handle_logout_cmd(conn, cmd, buf);
if (ret > 0)
@@ -4054,6 +4005,8 @@ static int iscsi_target_rx_opcode(struct iscsi_conn *conn, unsigned char *buf)
}

return ret;
+reject:
+ return iscsit_add_reject(conn, ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
}

int iscsi_target_rx_thread(void *arg)
@@ -4148,8 +4101,8 @@ restart:
(!(opcode & ISCSI_OP_LOGOUT)))) {
pr_err("Received illegal iSCSI Opcode: 0x%02x"
" while in Discovery Session, rejecting.\n", opcode);
- iscsit_add_reject(ISCSI_REASON_PROTOCOL_ERROR, 1,
- buffer, conn);
+ iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
+ buffer);
goto transport_err;
}

diff --git a/drivers/target/iscsi/iscsi_target.h b/drivers/target/iscsi/iscsi_target.h
index a0050b2..2c437cb 100644
--- a/drivers/target/iscsi/iscsi_target.h
+++ b/drivers/target/iscsi/iscsi_target.h
@@ -15,7 +15,7 @@ extern struct iscsi_np *iscsit_add_np(struct __kernel_sockaddr_storage *,
extern int iscsit_reset_np_thread(struct iscsi_np *, struct iscsi_tpg_np *,
struct iscsi_portal_group *);
extern int iscsit_del_np(struct iscsi_np *);
-extern int iscsit_add_reject_from_cmd(u8, int, int, unsigned char *, struct iscsi_cmd *);
+extern int iscsit_reject_cmd(struct iscsi_cmd *cmd, u8, unsigned char *);
extern void iscsit_set_unsoliticed_dataout(struct iscsi_cmd *);
extern int iscsit_logout_closesession(struct iscsi_cmd *, struct iscsi_conn *);
extern int iscsit_logout_closeconnection(struct iscsi_cmd *, struct iscsi_conn *);
diff --git a/drivers/target/iscsi/iscsi_target_core.h b/drivers/target/iscsi/iscsi_target_core.h
index caa0ad9..4f77a78 100644
--- a/drivers/target/iscsi/iscsi_target_core.h
+++ b/drivers/target/iscsi/iscsi_target_core.h
@@ -132,9 +132,8 @@ enum cmd_flags_table {
ICF_CONTIG_MEMORY = 0x00000020,
ICF_ATTACHED_TO_RQUEUE = 0x00000040,
ICF_OOO_CMDSN = 0x00000080,
- ICF_REJECT_FAIL_CONN = 0x00000100,
- IFC_SENDTARGETS_ALL = 0x00000200,
- IFC_SENDTARGETS_SINGLE = 0x00000400,
+ IFC_SENDTARGETS_ALL = 0x00000100,
+ IFC_SENDTARGETS_SINGLE = 0x00000200,
};

/* struct iscsi_cmd->i_state */
@@ -368,6 +367,8 @@ struct iscsi_cmd {
u8 maxcmdsn_inc;
/* Immediate Unsolicited Dataout */
u8 unsolicited_data;
+ /* Reject reason code */
+ u8 reject_reason;
/* CID contained in logout PDU when opcode == ISCSI_INIT_LOGOUT_CMND */
u16 logout_cid;
/* Command flags */
@@ -450,7 +451,6 @@ struct iscsi_cmd {
struct list_head datain_list;
/* R2T List */
struct list_head cmd_r2t_list;
- struct completion reject_comp;
/* Timer for DataOUT */
struct timer_list dataout_timer;
/* Iovecs for SCSI data payload RX/TX w/ kernel level sockets */
diff --git a/drivers/target/iscsi/iscsi_target_erl0.c b/drivers/target/iscsi/iscsi_target_erl0.c
index 8e6298c..8f074e0 100644
--- a/drivers/target/iscsi/iscsi_target_erl0.c
+++ b/drivers/target/iscsi/iscsi_target_erl0.c
@@ -746,13 +746,12 @@ int iscsit_check_post_dataout(
if (!conn->sess->sess_ops->ErrorRecoveryLevel) {
pr_err("Unable to recover from DataOUT CRC"
" failure while ERL=0, closing session.\n");
- iscsit_add_reject_from_cmd(ISCSI_REASON_DATA_DIGEST_ERROR,
- 1, 0, buf, cmd);
+ iscsit_reject_cmd(cmd, ISCSI_REASON_DATA_DIGEST_ERROR,
+ buf);
return DATAOUT_CANNOT_RECOVER;
}

- iscsit_add_reject_from_cmd(ISCSI_REASON_DATA_DIGEST_ERROR,
- 0, 0, buf, cmd);
+ iscsit_reject_cmd(cmd, ISCSI_REASON_DATA_DIGEST_ERROR, buf);
return iscsit_dataout_post_crc_failed(cmd, buf);
}
}
diff --git a/drivers/target/iscsi/iscsi_target_erl1.c b/drivers/target/iscsi/iscsi_target_erl1.c
index 40d9dbc..d00f132 100644
--- a/drivers/target/iscsi/iscsi_target_erl1.c
+++ b/drivers/target/iscsi/iscsi_target_erl1.c
@@ -162,9 +162,8 @@ static int iscsit_handle_r2t_snack(
" protocol error.\n", cmd->init_task_tag, begrun,
(begrun + runlength), cmd->acked_data_sn);

- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, buf, cmd);
+ return iscsit_reject_cmd(cmd,
+ ISCSI_REASON_PROTOCOL_ERROR, buf);
}

if (runlength) {
@@ -173,8 +172,8 @@ static int iscsit_handle_r2t_snack(
" with BegRun: 0x%08x, RunLength: 0x%08x, exceeds"
" current R2TSN: 0x%08x, protocol error.\n",
cmd->init_task_tag, begrun, runlength, cmd->r2t_sn);
- return iscsit_add_reject_from_cmd(
- ISCSI_REASON_BOOKMARK_INVALID, 1, 0, buf, cmd);
+ return iscsit_reject_cmd(cmd,
+ ISCSI_REASON_BOOKMARK_INVALID, buf);
}
last_r2tsn = (begrun + runlength);
} else
@@ -433,8 +432,7 @@ static int iscsit_handle_recovery_datain(
" protocol error.\n", cmd->init_task_tag, begrun,
(begrun + runlength), cmd->acked_data_sn);

- return iscsit_add_reject_from_cmd(ISCSI_REASON_PROTOCOL_ERROR,
- 1, 0, buf, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR, buf);
}

/*
@@ -445,14 +443,14 @@ static int iscsit_handle_recovery_datain(
pr_err("Initiator requesting BegRun: 0x%08x, RunLength"
": 0x%08x greater than maximum DataSN: 0x%08x.\n",
begrun, runlength, (cmd->data_sn - 1));
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_INVALID,
- 1, 0, buf, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_BOOKMARK_INVALID,
+ buf);
}

dr = iscsit_allocate_datain_req();
if (!dr)
- return iscsit_add_reject_from_cmd(ISCSI_REASON_BOOKMARK_NO_RESOURCES,
- 1, 0, buf, cmd);
+ return iscsit_reject_cmd(cmd, ISCSI_REASON_BOOKMARK_NO_RESOURCES,
+ buf);

dr->data_sn = dr->begrun = begrun;
dr->runlength = runlength;
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index fe712d6..96ce6f2 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -178,7 +178,6 @@ struct iscsi_cmd *iscsit_allocate_cmd(struct iscsi_conn *conn, gfp_t gfp_mask)
INIT_LIST_HEAD(&cmd->i_conn_node);
INIT_LIST_HEAD(&cmd->datain_list);
INIT_LIST_HEAD(&cmd->cmd_r2t_list);
- init_completion(&cmd->reject_comp);
spin_lock_init(&cmd->datain_lock);
spin_lock_init(&cmd->dataout_timeout_lock);
spin_lock_init(&cmd->istate_lock);
diff --git a/include/target/iscsi/iscsi_transport.h b/include/target/iscsi/iscsi_transport.h
index ce991ba..19a43ad 100644
--- a/include/target/iscsi/iscsi_transport.h
+++ b/include/target/iscsi/iscsi_transport.h
@@ -34,8 +34,6 @@ extern void iscsit_put_transport(struct iscsit_transport *);
/*
* From iscsi_target.c
*/
-extern int iscsit_add_reject_from_cmd(u8, int, int, unsigned char *,
- struct iscsi_cmd *);
extern int iscsit_setup_scsi_cmd(struct iscsi_conn *, struct iscsi_cmd *,
unsigned char *);
extern void iscsit_set_unsoliticed_dataout(struct iscsi_cmd *);
--
1.7.2.5

[Nicholas A. Bellinger]

From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch moves ISCSI_OP_REJECT failures into iscsit_sequence_cmd()
in order to avoid external iscsit_reject_cmd() reject usage for all
PDU types.

It also updates PDU specific handlers for traditional iscsi-target
code to not reset the session after posting a ISCSI_OP_REJECT during
setup.

Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Mike Christie <mich...@cs.wisc.edu>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---

drivers/infiniband/ulp/isert/ib_isert.c | 2 +-
drivers/target/iscsi/iscsi_target.c | 31 ++++++++++++++++++-----------
drivers/target/iscsi/iscsi_target_erl1.c | 6 ++--
drivers/target/iscsi/iscsi_target_util.c | 25 ++++++++++++++++++-----
drivers/target/iscsi/iscsi_target_util.h | 3 +-
include/target/iscsi/iscsi_transport.h | 3 +-
6 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 13a4464..b0d8a28 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -934,7 +934,7 @@ isert_handle_scsi_cmd(struct isert_conn *isert_conn,
}

sequence_cmd:
- rc = iscsit_sequence_cmd(conn, cmd, hdr->cmdsn);
+ rc = iscsit_sequence_cmd(conn, cmd, buf, hdr->cmdsn);

if (!rc && dump_payload == false && unsol_data)
iscsit_set_unsoliticed_dataout(cmd);

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index b5e7b7e..3cd0e66 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1054,8 +1054,11 @@ int iscsit_process_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
* be acknowledged. (See below)
*/
if (!cmd->immediate_data) {
- cmdsn_ret = iscsit_sequence_cmd(conn, cmd, hdr->cmdsn);

- if (cmdsn_ret == CMDSN_LOWER_THAN_EXP) {

+ cmdsn_ret = iscsit_sequence_cmd(conn, cmd,
+ (unsigned char *)hdr, hdr->cmdsn);
+ if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER) {
+ return -1;
+ } else if (cmdsn_ret == CMDSN_LOWER_THAN_EXP) {
if (!cmd->sense_reason)
return 0;

@@ -1122,7 +1125,10 @@ after_immediate_data:
* DataCRC, check against ExpCmdSN/MaxCmdSN if
* Immediate Bit is not set.
*/
- cmdsn_ret = iscsit_sequence_cmd(cmd->conn, cmd, hdr->cmdsn);
+ cmdsn_ret = iscsit_sequence_cmd(cmd->conn, cmd,
+ (unsigned char *)hdr, hdr->cmdsn);
+ if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
+ return -1;

if (cmd->sense_reason) {
if (iscsit_dump_data_payload(cmd->conn,
@@ -1161,7 +1167,7 @@ iscsit_handle_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,

rc = iscsit_setup_scsi_cmd(conn, cmd, buf);
if (rc < 0)
- return rc;
+ return 0;
/*
* Allocation iovecs needed for struct socket operations for

* traditional iSCSI block I/O.

@@ -1496,7 +1502,7 @@ static int iscsit_handle_data_out(struct iscsi_conn *conn, unsigned char *buf)

rc = iscsit_check_dataout_hdr(conn, buf, &cmd);
if (rc < 0)
- return rc;
+ return 0;
else if (!cmd)
return 0;

@@ -1581,10 +1587,10 @@ int iscsit_process_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
return 0;
}

- cmdsn_ret = iscsit_sequence_cmd(conn, cmd, hdr->cmdsn);
+ cmdsn_ret = iscsit_sequence_cmd(conn, cmd,
+ (unsigned char *)hdr, hdr->cmdsn);
if (cmdsn_ret == CMDSN_LOWER_THAN_EXP)
return 0;

-
if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)

return -1;

@@ -1625,7 +1631,7 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,

ret = iscsit_setup_nop_out(conn, cmd, hdr);
if (ret < 0)
- return ret;
+ return 0;
/*
* Handle NOP-OUT payload for traditional iSCSI sockets
*/
@@ -1895,7 +1901,7 @@ attach:
spin_unlock_bh(&conn->cmd_lock);

if (!(hdr->opcode & ISCSI_OP_IMMEDIATE)) {

- int cmdsn_ret = iscsit_sequence_cmd(conn, cmd, hdr->cmdsn);
+ int cmdsn_ret = iscsit_sequence_cmd(conn, cmd, buf, hdr->cmdsn);
if (cmdsn_ret == CMDSN_HIGHER_THAN_EXP)
out_of_order_cmdsn = 1;

else if (cmdsn_ret == CMDSN_LOWER_THAN_EXP)

@@ -1998,7 +2004,8 @@ iscsit_process_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
iscsit_ack_from_expstatsn(conn, be32_to_cpu(hdr->exp_statsn));

if (!(hdr->opcode & ISCSI_OP_IMMEDIATE)) {

- cmdsn_ret = iscsit_sequence_cmd(conn, cmd, hdr->cmdsn);
+ cmdsn_ret = iscsit_sequence_cmd(conn, cmd,
+ (unsigned char *)hdr, hdr->cmdsn);
if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
return -1;

@@ -2024,7 +2031,7 @@ iscsit_handle_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,

rc = iscsit_setup_text_cmd(conn, cmd, hdr);
if (rc < 0)
- return rc;
+ return 0;

rx_size = payload_length;
if (payload_length) {
@@ -2286,7 +2293,7 @@ iscsit_handle_logout_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
if (ret < 0)
return ret;
} else {
- cmdsn_ret = iscsit_sequence_cmd(conn, cmd, hdr->cmdsn);
+ cmdsn_ret = iscsit_sequence_cmd(conn, cmd, buf, hdr->cmdsn);
if (cmdsn_ret == CMDSN_LOWER_THAN_EXP)
logout_remove = 0;

else if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)

diff --git a/drivers/target/iscsi/iscsi_target_erl1.c b/drivers/target/iscsi/iscsi_target_erl1.c
index d00f132..586c268 100644
--- a/drivers/target/iscsi/iscsi_target_erl1.c
+++ b/drivers/target/iscsi/iscsi_target_erl1.c
@@ -1088,7 +1088,7 @@ int iscsit_handle_ooo_cmdsn(

ooo_cmdsn = iscsit_allocate_ooo_cmdsn();
if (!ooo_cmdsn)
- return CMDSN_ERROR_CANNOT_RECOVER;
+ return -ENOMEM;

ooo_cmdsn->cmd = cmd;
ooo_cmdsn->batch_count = (batch) ?
@@ -1099,10 +1099,10 @@ int iscsit_handle_ooo_cmdsn(

if (iscsit_attach_ooo_cmdsn(sess, ooo_cmdsn) < 0) {
kmem_cache_free(lio_ooo_cache, ooo_cmdsn);
- return CMDSN_ERROR_CANNOT_RECOVER;
+ return -ENOMEM;
}

- return CMDSN_HIGHER_THAN_EXP;
+ return 0;
}

static int iscsit_set_dataout_timeout_values(
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index 96ce6f2..c0be718 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -283,13 +283,12 @@ static inline int iscsit_check_received_cmdsn(struct iscsi_session *sess, u32 cm
* Commands may be received out of order if MC/S is in use.
* Ensure they are executed in CmdSN order.
*/
-int iscsit_sequence_cmd(
- struct iscsi_conn *conn,
- struct iscsi_cmd *cmd,
- __be32 cmdsn)
+int iscsit_sequence_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
+ unsigned char *buf, __be32 cmdsn)
{
- int ret;
- int cmdsn_ret;
+ int ret, cmdsn_ret;
+ bool reject = false;
+ u8 reason = ISCSI_REASON_BOOKMARK_NO_RESOURCES;

mutex_lock(&conn->sess->cmdsn_mutex);

@@ -299,9 +298,18 @@ int iscsit_sequence_cmd(
ret = iscsit_execute_cmd(cmd, 0);
if ((ret >= 0) && !list_empty(&conn->sess->sess_ooo_cmdsn_list))
iscsit_execute_ooo_cmdsns(conn->sess);
+ else if (ret < 0)
+ reject = true;
+
break;
case CMDSN_HIGHER_THAN_EXP:
ret = iscsit_handle_ooo_cmdsn(conn->sess, cmd, be32_to_cpu(cmdsn));
+ if (ret < 0) {
+ reject = true;
+ ret = CMDSN_ERROR_CANNOT_RECOVER;
+ break;
+ }
+ ret = CMDSN_HIGHER_THAN_EXP;
break;
case CMDSN_LOWER_THAN_EXP:
cmd->i_state = ISTATE_REMOVE;
@@ -309,11 +317,16 @@ int iscsit_sequence_cmd(
ret = cmdsn_ret;
break;
default:
+ reason = ISCSI_REASON_PROTOCOL_ERROR;
+ reject = true;
ret = cmdsn_ret;
break;
}
mutex_unlock(&conn->sess->cmdsn_mutex);

+ if (reject)
+ iscsit_reject_cmd(cmd, reason, buf);
+
return ret;
}
EXPORT_SYMBOL(iscsit_sequence_cmd);
diff --git a/drivers/target/iscsi/iscsi_target_util.h b/drivers/target/iscsi/iscsi_target_util.h
index a442265..e4fc34a 100644
--- a/drivers/target/iscsi/iscsi_target_util.h
+++ b/drivers/target/iscsi/iscsi_target_util.h
@@ -13,7 +13,8 @@ extern struct iscsi_cmd *iscsit_allocate_cmd(struct iscsi_conn *, gfp_t);
extern struct iscsi_seq *iscsit_get_seq_holder_for_datain(struct iscsi_cmd *, u32);
extern struct iscsi_seq *iscsit_get_seq_holder_for_r2t(struct iscsi_cmd *);
extern struct iscsi_r2t *iscsit_get_holder_for_r2tsn(struct iscsi_cmd *, u32);
-int iscsit_sequence_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, __be32 cmdsn);
+extern int iscsit_sequence_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
+ unsigned char * ,__be32 cmdsn);
extern int iscsit_check_unsolicited_dataout(struct iscsi_cmd *, unsigned char *);
extern struct iscsi_cmd *iscsit_find_cmd_from_itt(struct iscsi_conn *, itt_t);
extern struct iscsi_cmd *iscsit_find_cmd_from_itt_or_dump(struct iscsi_conn *,
diff --git a/include/target/iscsi/iscsi_transport.h b/include/target/iscsi/iscsi_transport.h
index 19a43ad..ce4070d 100644
--- a/include/target/iscsi/iscsi_transport.h
+++ b/include/target/iscsi/iscsi_transport.h
@@ -86,4 +86,5 @@ extern int iscsit_tmr_post_handler(struct iscsi_cmd *, struct iscsi_conn *);
* From iscsi_target_util.c
*/
extern struct iscsi_cmd *iscsit_allocate_cmd(struct iscsi_conn *, gfp_t);
-extern int iscsit_sequence_cmd(struct iscsi_conn *, struct iscsi_cmd *, __be32);
+extern int iscsit_sequence_cmd(struct iscsi_conn *, struct iscsi_cmd *,
+ unsigned char *, __be32);
--
1.7.2.5

[Nicholas A. Bellinger]

From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch adds target_get_sess_cmd reference counting for
iscsit_handle_task_mgt_cmd(), and adds a target_put_sess_cmd()
for the failure case.

It also fixes a bug where ISCSI_OP_SCSI_TMFUNC type commands
where leaking iscsi_cmd->i_conn_node and eventually triggering
an OOPs during struct isert_conn shutdown.

Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---

drivers/infiniband/ulp/isert/ib_isert.c | 17 +++++++++--------
drivers/target/iscsi/iscsi_target.c | 12 ++++++++++--
2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index b0d8a28..b17708b 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -1200,14 +1200,12 @@ isert_put_cmd(struct isert_cmd *isert_cmd)
{

struct iscsi_cmd *cmd = &isert_cmd->iscsi_cmd;

struct isert_conn *isert_conn = isert_cmd->conn;
- struct iscsi_conn *conn;
+ struct iscsi_conn *conn = isert_conn->conn;

pr_debug("Entering isert_put_cmd: %p\n", isert_cmd);

switch (cmd->iscsi_opcode) {
case ISCSI_OP_SCSI_CMD:
- conn = isert_conn->conn;
-
spin_lock_bh(&conn->cmd_lock);
if (!list_empty(&cmd->i_conn_node))
list_del(&cmd->i_conn_node);
@@ -1217,16 +1215,19 @@ isert_put_cmd(struct isert_cmd *isert_cmd)
iscsit_stop_dataout_timer(cmd);

isert_unmap_cmd(isert_cmd, isert_conn);
- /*
- * Fall-through
- */
+ transport_generic_free_cmd(&cmd->se_cmd, 0);
+ break;
case ISCSI_OP_SCSI_TMFUNC:
+ spin_lock_bh(&conn->cmd_lock);
+ if (!list_empty(&cmd->i_conn_node))
+ list_del(&cmd->i_conn_node);
+ spin_unlock_bh(&conn->cmd_lock);
+
transport_generic_free_cmd(&cmd->se_cmd, 0);
break;
case ISCSI_OP_REJECT:
case ISCSI_OP_NOOP_OUT:
- conn = isert_conn->conn;
-
+ case ISTATE_SEND_TEXTRSP:
spin_lock_bh(&conn->cmd_lock);
if (!list_empty(&cmd->i_conn_node))
list_del(&cmd->i_conn_node);
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 3cd0e66..7eb1995 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1733,8 +1733,8 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
struct se_tmr_req *se_tmr;
struct iscsi_tmr_req *tmr_req;
struct iscsi_tm *hdr;
- int out_of_order_cmdsn = 0;
- int ret;
+ int out_of_order_cmdsn = 0, ret;
+ bool sess_ref = false;
u8 function;

hdr = (struct iscsi_tm *) buf;
@@ -1790,6 +1790,9 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
conn->sess->se_sess, 0, DMA_NONE,
MSG_SIMPLE_TAG, cmd->sense_buffer + 2);

+ target_get_sess_cmd(conn->sess->se_sess, &cmd->se_cmd, true);
+ sess_ref = true;
+
switch (function) {
case ISCSI_TM_FUNC_ABORT_TASK:
tcm_function = TMR_ABORT_TASK;
@@ -1927,6 +1930,11 @@ attach:
* For connection recovery, this is also the default action for
* TMR TASK_REASSIGN.
*/
+ if (sess_ref) {
+ pr_debug("Handle TMR, using sess_ref=true check\n");
+ target_put_sess_cmd(conn->sess->se_sess, &cmd->se_cmd);
+ }
+
iscsit_add_cmd_to_response_queue(cmd, conn, cmd->i_state);
return 0;
}
--
1.7.2.5

[Nicholas A. Bellinger]

From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch addresses a bug where RDMA_CM_EVENT_DISCONNECTED may occur
before the connection shutdown has been completed by rx/tx threads,
that causes isert_free_conn() to wait indefinately on ->conn_wait.

This patch allows isert_disconnect_work code to invoke rdma_disconnect
when isert_disconnect_work() process context is started by client
session reset before isert_free_conn() code has been reached.

It also adds isert_conn->conn_mutex protection for ->state within
isert_disconnect_work(), isert_cq_comp_err() and isert_free_conn()
code, along with isert_check_state() for wait_event usage.

Cc: Or Gerlitz <oger...@mellanox.com>

Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---

drivers/infiniband/ulp/isert/ib_isert.c | 67 ++++++++++++++++++++++++++-----
drivers/infiniband/ulp/isert/ib_isert.h | 1 +
2 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index b17708b..0d1eb4c 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -388,6 +388,7 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
init_waitqueue_head(&isert_conn->conn_wait_comp_err);
kref_init(&isert_conn->conn_kref);
kref_get(&isert_conn->conn_kref);
+ mutex_init(&isert_conn->conn_mutex);

cma_id->context = isert_conn;
isert_conn->conn_cm_id = cma_id;
@@ -540,15 +541,29 @@ isert_disconnect_work(struct work_struct *work)
struct isert_conn, conn_logout_work);

pr_debug("isert_disconnect_work(): >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
-
+ mutex_lock(&isert_conn->conn_mutex);
isert_conn->state = ISER_CONN_DOWN;

if (isert_conn->post_recv_buf_count == 0 &&
atomic_read(&isert_conn->post_send_buf_count) == 0) {
pr_debug("Calling wake_up(&isert_conn->conn_wait);\n");
- wake_up(&isert_conn->conn_wait);
+ mutex_unlock(&isert_conn->conn_mutex);
+ goto wake_up;
+ }
+ if (!isert_conn->conn_cm_id) {
+ mutex_unlock(&isert_conn->conn_mutex);
+ isert_put_conn(isert_conn);
+ return;
+ }
+ if (!isert_conn->logout_posted) {
+ pr_debug("Calling rdma_disconnect for !logout_posted from"
+ " isert_disconnect_work\n");
+ rdma_disconnect(isert_conn->conn_cm_id);
}
+ mutex_unlock(&isert_conn->conn_mutex);

+wake_up:
+ wake_up(&isert_conn->conn_wait);
isert_put_conn(isert_conn);
}

@@ -1437,7 +1452,11 @@ isert_cq_comp_err(struct iser_tx_desc *tx_desc, struct isert_conn *isert_conn)
pr_debug("isert_cq_comp_err >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
pr_debug("Calling wake_up from isert_cq_comp_err\n");

- isert_conn->state = ISER_CONN_TERMINATING;
+ mutex_lock(&isert_conn->conn_mutex);
+ if (isert_conn->state != ISER_CONN_DOWN)
+ isert_conn->state = ISER_CONN_TERMINATING;
+ mutex_unlock(&isert_conn->conn_mutex);
+
wake_up(&isert_conn->conn_wait_comp_err);
}
}
@@ -2207,6 +2226,17 @@ isert_free_np(struct iscsi_np *np)
kfree(isert_np);
}

+static int isert_check_state(struct isert_conn *isert_conn, int state)
+{
+ int ret;
+
+ mutex_lock(&isert_conn->conn_mutex);
+ ret = (isert_conn->state == state);
+ mutex_unlock(&isert_conn->conn_mutex);
+
+ return ret;
+}
+
static void isert_free_conn(struct iscsi_conn *conn)
{
struct isert_conn *isert_conn = conn->context;
@@ -2216,26 +2246,43 @@ static void isert_free_conn(struct iscsi_conn *conn)
* Decrement post_send_buf_count for special case when called
* from isert_do_control_comp() -> iscsit_logout_post_handler()
*/
+ mutex_lock(&isert_conn->conn_mutex);
if (isert_conn->logout_posted)
atomic_dec(&isert_conn->post_send_buf_count);

- if (isert_conn->conn_cm_id)
+ if (isert_conn->conn_cm_id && isert_conn->state != ISER_CONN_DOWN) {
+ pr_debug("Calling rdma_disconnect from isert_free_conn\n");
rdma_disconnect(isert_conn->conn_cm_id);
+ }
/*
* Only wait for conn_wait_comp_err if the isert_conn made it
* into full feature phase..
*/
- if (isert_conn->state > ISER_CONN_INIT) {
+ if (isert_conn->state == ISER_CONN_UP) {
pr_debug("isert_free_conn: Before wait_event comp_err %d\n",
isert_conn->state);
+ mutex_unlock(&isert_conn->conn_mutex);
+
wait_event(isert_conn->conn_wait_comp_err,
- isert_conn->state == ISER_CONN_TERMINATING);
- pr_debug("isert_free_conn: After wait_event #1 >>>>>>>>>>>>\n");
+ (isert_check_state(isert_conn, ISER_CONN_TERMINATING)));
+
+ wait_event(isert_conn->conn_wait,
+ (isert_check_state(isert_conn, ISER_CONN_DOWN)));
+
+ isert_put_conn(isert_conn);
+ return;
+ }
+ if (isert_conn->state == ISER_CONN_INIT) {
+ mutex_unlock(&isert_conn->conn_mutex);
+ isert_put_conn(isert_conn);
+ return;
}
+ pr_debug("isert_free_conn: wait_event conn_wait %d\n",
+ isert_conn->state);
+ mutex_unlock(&isert_conn->conn_mutex);

- pr_debug("isert_free_conn: wait_event conn_wait %d\n", isert_conn->state);
- wait_event(isert_conn->conn_wait, isert_conn->state == ISER_CONN_DOWN);
- pr_debug("isert_free_conn: After wait_event #2 >>>>>>>>>>>>>>>>>>>>\n");
+ wait_event(isert_conn->conn_wait,
+ (isert_check_state(isert_conn, ISER_CONN_DOWN)));

isert_put_conn(isert_conn);
}
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index b104f4c..5795c82 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -102,6 +102,7 @@ struct isert_conn {
struct ib_qp *conn_qp;
struct isert_device *conn_device;
struct work_struct conn_logout_work;
+ struct mutex conn_mutex;
wait_queue_head_t conn_wait;
wait_queue_head_t conn_wait_comp_err;
struct kref conn_kref;
--
1.7.2.5