From 10868c491d1d16fe2879fdba30a95d855fa936a6 Mon Sep 17 00:00:00 2001 From: Xie Yongji Date: Tue, 2 Jun 2020 20:15:50 +0800 Subject: [PATCH 1/4] libiscsi: Avoid discontinuities in cmdsn ordering in some cases We should plug the cmdsn gap in order to continue to use the session when the pdus is cancelled before sending out. Signed-off-by: Xie Yongji --- lib/iscsi-command.c | 26 ++++++++++++++++++++++---- lib/nop.c | 3 ++- lib/pdu.c | 15 +++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index 87e2336..12948c4 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -271,7 +271,7 @@ iscsi_scsi_command_async(struct iscsi_context *iscsi, int lun, iscsi_pdu_set_expxferlen(pdu, task->expxferlen); /* cmdsn */ - iscsi_pdu_set_cmdsn(pdu, iscsi->cmdsn++); + iscsi_pdu_set_cmdsn(pdu, iscsi->cmdsn); /* cdb */ iscsi_pdu_set_cdb(pdu, task); @@ -285,6 +285,7 @@ iscsi_scsi_command_async(struct iscsi_context *iscsi, int lun, iscsi->drv->free_pdu(iscsi, pdu); return -1; } + iscsi->cmdsn++; /* The F flag is not set. This means we haven't sent all the unsolicited * data yet. Sent as much as we are allowed as a train of DATA-OUT PDUs. @@ -2667,6 +2668,9 @@ iscsi_scsi_cancel_task(struct iscsi_context *iscsi, struct scsi_task *task) { struct iscsi_pdu *pdu; + struct iscsi_pdu *next_pdu; + uint32_t cmdsn_gap = 0; + int ret = -1; for (pdu = iscsi->waitpdu; pdu; pdu = pdu->next) { if (pdu->itt == task->itt) { @@ -2679,18 +2683,32 @@ iscsi_scsi_cancel_task(struct iscsi_context *iscsi, return 0; } } - for (pdu = iscsi->outqueue; pdu; pdu = pdu->next) { + for (pdu = iscsi->outqueue; pdu; pdu = next_pdu) { + next_pdu = pdu->next; + + if (cmdsn_gap > 0) { + iscsi_pdu_set_cmdsn(pdu, pdu->cmdsn - cmdsn_gap); + } + if (pdu->itt == task->itt) { ISCSI_LIST_REMOVE(&iscsi->outqueue, pdu); if (pdu->callback) { pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL, pdu->private_data); } + if (!(pdu->outdata.data[0] & ISCSI_PDU_IMMEDIATE) && + (pdu->outdata.data[0] & 0x3f) != ISCSI_PDU_DATA_OUT) { + iscsi->cmdsn--; + cmdsn_gap++; + } iscsi->drv->free_pdu(iscsi, pdu); - return 0; + ret = 0; + if (!cmdsn_gap) { + break; + } } } - return -1; + return ret; } void diff --git a/lib/nop.c b/lib/nop.c index 2137530..2c1391a 100644 --- a/lib/nop.c +++ b/lib/nop.c @@ -64,7 +64,7 @@ iscsi_nop_out_async(struct iscsi_context *iscsi, iscsi_command_cb cb, iscsi_pdu_set_lun(pdu, 0); /* cmdsn */ - iscsi_pdu_set_cmdsn(pdu, iscsi->cmdsn++); + iscsi_pdu_set_cmdsn(pdu, iscsi->cmdsn); pdu->callback = cb; pdu->private_data = private_data; @@ -83,6 +83,7 @@ iscsi_nop_out_async(struct iscsi_context *iscsi, iscsi_command_cb cb, return -1; } + iscsi->cmdsn++; iscsi->nops_in_flight++; ISCSI_LOG(iscsi, (iscsi->nops_in_flight > 1) ? 1 : 6, "NOP Out Send (nops_in_flight: %d, pdu->cmdsn %08x, pdu->itt %08x, pdu->ttt %08x, iscsi->maxcmdsn %08x, iscsi->expcmdsn %08x)", diff --git a/lib/pdu.c b/lib/pdu.c index 8c8fc99..73d2f83 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -719,10 +719,14 @@ iscsi_timeout_scan(struct iscsi_context *iscsi) struct iscsi_pdu *pdu; struct iscsi_pdu *next_pdu; time_t t = time(NULL); + uint32_t cmdsn_gap = 0; for (pdu = iscsi->outqueue; pdu; pdu = next_pdu) { next_pdu = pdu->next; + if (cmdsn_gap > 0) { + iscsi_pdu_set_cmdsn(pdu, pdu->cmdsn - cmdsn_gap); + } if (pdu->scsi_timeout == 0) { /* no timeout for this pdu */ continue; @@ -731,6 +735,11 @@ iscsi_timeout_scan(struct iscsi_context *iscsi) /* not expired yet */ continue; } + if (!(pdu->outdata.data[0] & ISCSI_PDU_IMMEDIATE) && + (pdu->outdata.data[0] & 0x3f) != ISCSI_PDU_DATA_OUT) { + iscsi->cmdsn--; + cmdsn_gap++; + } ISCSI_LIST_REMOVE(&iscsi->outqueue, pdu); iscsi_set_error(iscsi, "command timed out"); iscsi_dump_pdu_header(iscsi, pdu->outdata.data); @@ -783,6 +792,12 @@ iscsi_cancel_pdus(struct iscsi_context *iscsi) NULL, pdu->private_data); } iscsi->drv->free_pdu(iscsi, pdu); + if (!(pdu->outdata.data[0] & ISCSI_PDU_IMMEDIATE) && + (pdu->outdata.data[0] & 0x3f) != ISCSI_PDU_DATA_OUT) { + iscsi->cmdsn--; + } + + } while ((pdu = iscsi->waitpdu)) { ISCSI_LIST_REMOVE(&iscsi->waitpdu, pdu); From e9c1f102588447255c75f0d57bbdc8341fc4ec96 Mon Sep 17 00:00:00 2001 From: Xie Yongji Date: Wed, 3 Jun 2020 13:49:44 +0800 Subject: [PATCH 2/4] pdu: Remove the checking for iscsi->is_loggedin in iscsi_cancel_pdus() I don't see any problems that calling the callback during connect/login in iscsi_cancel_pdus(). So let's remove this check. Otherwise, we have no way to be aware of a cancellation during login and cause something like iscsi_login_sync() hangs. Signed-off-by: Xie Yongji --- lib/pdu.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/pdu.c b/lib/pdu.c index 73d2f83..0f52504 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -784,10 +784,7 @@ iscsi_cancel_pdus(struct iscsi_context *iscsi) while ((pdu = iscsi->outqueue)) { ISCSI_LIST_REMOVE(&iscsi->outqueue, pdu); - if (iscsi->is_loggedin && pdu->callback) { - /* If an error happened during connect/login, - we don't want to call any of the callbacks. - */ + if (pdu->callback) { pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL, pdu->private_data); } @@ -801,10 +798,7 @@ iscsi_cancel_pdus(struct iscsi_context *iscsi) } while ((pdu = iscsi->waitpdu)) { ISCSI_LIST_REMOVE(&iscsi->waitpdu, pdu); - if (iscsi->is_loggedin && pdu->callback) { - /* If an error happened during connect/login, - we don't want to call any of the callbacks. - */ + if (pdu->callback) { pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL, pdu->private_data); } From aad136e5b9f0a36e4ab73f6dcd2dd5b0fe697837 Mon Sep 17 00:00:00 2001 From: wanghonghao Date: Tue, 23 Jun 2020 19:48:26 +0800 Subject: [PATCH 3/4] libiscsi: Make the cancellation aware of the pdus in old iscsi context We should check the pdus in old iscsi context when cancelling tasks. Signed-off-by: wanghonghao --- lib/iscsi-command.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index 12948c4..ef6337e 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -2708,6 +2708,11 @@ iscsi_scsi_cancel_task(struct iscsi_context *iscsi, } } } + + if (iscsi->old_iscsi) { + return iscsi_scsi_cancel_task(iscsi->old_iscsi, task); + } + return ret; } @@ -2715,4 +2720,8 @@ void iscsi_scsi_cancel_all_tasks(struct iscsi_context *iscsi) { iscsi_cancel_pdus(iscsi); + + if (iscsi->old_iscsi) { + iscsi_cancel_pdus(iscsi->old_iscsi); + } } From ee47dc73388ac30962e2f5ee1c7ac2b259c0e036 Mon Sep 17 00:00:00 2001 From: Xie Yongji Date: Tue, 23 Jun 2020 15:37:12 +0800 Subject: [PATCH 4/4] socket: Make the pdu timeout handling aware of old iscsi context We should check the pdus in old iscsi context when scanning timeout tasks during reconnecting. Signed-off-by: Xie Yongji --- lib/socket.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/socket.c b/lib/socket.c index f7c1b15..dc197ff 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -913,7 +913,7 @@ iscsi_tcp_service(struct iscsi_context *iscsi, int revents) return iscsi_reconnect(iscsi); } else { if (iscsi->old_iscsi) { - return 0; + goto check_timeout; } } } @@ -999,8 +999,14 @@ iscsi_tcp_service(struct iscsi_context *iscsi, int revents) return iscsi_service_reconnect_if_loggedin(iscsi); } } + +check_timeout: iscsi_timeout_scan(iscsi); + if (iscsi->old_iscsi) { + iscsi_timeout_scan(iscsi->old_iscsi); + } + return 0; }