This is not used anywhere now.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: If65321abcd3601af91725c2117cdce10dd0ffc63
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474176
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Four functions are defined in param.c and so moving their declaration
from iscsi.h to param.h is a little easier to read.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Id16eab56d20d7ec99759e69525e791b091a93783
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472673
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Read task completion has been factored out into
process_read_task_completion(). Factoring out non-read task completion
into process_non_read_task_completion() makes the code a little
clearer and makes us possible to add unit tests.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I4da3cd05fc3668d0db4436301e4bcb1b554de7cd
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472905
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
iSCSI target frees iSCSI tasks when exiting connection or removing
LUN. The difference is only that the passed LUN is NULL or not.
To make the code clearer, this patch factors out freeing iSCSI
tasks from iscsi_conn_free_tasks() and _iscsi_conn_remove_lun()
into _iscsi_conn_free_tasks().
The refactoring has subtle cases and so add UT code together.
The next patch will fix the issue that secondary tasks are left
even after primary tasks are freed when exiting connection or
removing LUN, and this patch clarifies the next patch.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I18aaed6fe18a1c561ac88a0e5dc1296f9941d0e8
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473154
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
We have separated PDU header handler and payload handler, and have
PDU header handlers for each PDU type now.
By using this refinement, we can remove an aggregated helper function
spdk_iscsi_get_dif_ctx() and embed spdk_scsi_lun_get_dif_ctx() into
each PDU header handler.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ib4d9939b625858466224647c545cb67a04babf86
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471699
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
During testing, we observed both conn->data_in_cnt went negative or was
left positive unexpectedly. Hence add assert to detect both cases.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I102d4eb7c8beb0e56b6a46fd0f85b3eb1c447da5
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474437
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
We had not decremented conn->data_in_cnt when the primary is removed
from conn->queued_datain_tasks after submitting it.
If we simply add decrement into iscsi_conn_free_tasks() and
_iscsi_conn_remove_lun(), it conflicts with iscsi_transfer_in().
By recent refinements, primary is freed in either spdk_iscsi_conn_free_pdu()
or iscsi_conn_free_tasks()/_iscsi_conn_remove_lun().
Hence let's make decrement of conn->data_in_cnt for primary follow
the management of primary.
In iscsi_conn_free_tasks()/_iscsi_conn_remove_lun(), if
primary->current_datain_offset, conn->data_in_cnt is incremented, and
hence decrement it.
In spdk_iscsi_conn_free_pdu(), if primary and all subtasks are
completed, decrement conn->data_in_cnt.
This patch will fix the issue that conn->data_in_cnt ls still
positive even after all tasks are freed when removing LUN dynamically.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I70cb431ab968387749ff7a5c77cd109904687797
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474436
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
We had checked LUN again but we had not checked primary task
in iscsi_pdu_payload_op_data(). This had caused unexpected behavior
during LUN hotplug. Hence we check if primary task exists again
in iscsi_pdu_payload_op_data(), and abort the subtask immediately
if not.
This change fixes one of the failures we observed.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I5315badf0b90902e77dd5270dd0eda1437a771da
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474440
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Due to recent refinement of primary/secondary task management,
remove_acked_pdu() cannot use spdk_iscsi_conn_free_pdu(). As done
in iscsi_conn_free_tasks(), we can replace spdk_iscsi_conn_free_pdu()
by spdk_iscsi_task_put() and spdk_put_pdu() and we do that in this
patch.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I9f83569becfc6e9440fb859709f04b6123674f25
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474438
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Use whatever size the socket layer thinks is best. Before,
we limited the total amount of memory to just 32MB total. Now,
let the socket layer decide. It will likely use up to 2MB per
socket, which results in much better performance.
Change-Id: I9ef7680773b8c78a743fe74d8abb518258e19a0d
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470512
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
For large split write I/O, spdk_iscsi_task_cpl() does the same
operations for the error case as the normal path. Hence remove
duplicated operation in spdk_iscsi_conn_free_pdu(). This fixes
the issue that the reference count of the primary write task
goes negative.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I169d8932821f2a1c8e1f153347cd3175f1291bf1
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473818
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
For large split read I/O, the primary task have to be freed by the
last subtask. However, if LUN is removed in the middle of the split
read I/O sequence, the primary task is freed by the not last
subtask. This had caused critical system failure by LUN hotplug.
This patch fixes that.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I78acaf054360254dffbdc282c2d0d8bb5868e5d4
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473783
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
In spdk_iscsi_conn_handle_queued_datain_tasks(), we had overwritten
task->scsi.lun. However, if the primary task is already submitted,
it cannot process IO completion correctly because task->scsi.lun is
NULL. This patch fixes the issue.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia63f4c2e37b43477eaccbfd6dfea28fa357bde12
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473627
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Data segment length of the PDU is already cached in pdu->data_segment_len.
Hence additional caching to the local variable data_len is not necessary.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1e596999640229b1b0fa85cbdb342b1636af5076
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471879
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Based on the following description in Chapter 4.3 "iSCSI Session Types in
iSCSI specification
b) Discovery session - a session only opened for target discovery.
The target MUST ONLY accept Text Requests with the SendTargets
key and a Logout Request with reason "close the session". All
other requests MUST be rejected.
update the comment slightly, add macro constants for iSCSI logout
reason, and change the ordering of checks to be session type and then
logout reason.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ifc2ecc5b6dde546700662d3cda59d8cc465fd83a
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472672
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
conn->sess->session_type must be accessed when conn->sess is not NULL.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I3d41443352b65ee5ef4cc1f0d152b9e3221975c9
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471877
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
_iscsi_conn_free() is simple enough to embed it into the caller
and remove it.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I22bcbbdab15eca647914715754c04b8ec14ad9b2
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472901
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
After I review the function iscsi_conn_flush_pdus_internal,
I think that it may cause recursive function call issue. One of
the recursive calls in iscsi_conn_flush_pdus_internal
is:
spdk_iscsi_conn_free_pdu
spdk_iscsi_conn_handle_queued_datain_tasks
...
spdk_iscsi_task_cpl(&task->scsi);
...
process_read_task_completion
spdk_iscsi_task_response
iscsi_transfer_in
iscsi_send_datain
spdk_iscsi_conn_write_pdu
iscsi_conn_flush_pdus
iscsi_conn_flush_pdus_internal
So we have to create another list to solve this recursive issue
in the while loop. And we face the the similar issue in
NVMe/TCP before. With this patch, we can fix issues caused by
recursive calls.
Fixes #issue 1023
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I7150b962bfb30e74f53ba1a2a826fb78c73d8ea6
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472999
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Previously iSCSI task was created after allocating data buffer
and reading all data, and hence creating iSCSI task and processing
iSCSI task were not separated.
However, the recent refactoring separate PDU header handling and
PDU payload handling, and then inserted allocating data buffer and
reading data segment in the middle.
If any critical error occurs during allocating data buffer or
reading data segment, PDU payload handling is not done, and hence
created iSCSI task is left in PDU receive process.
If any critical error occurs, the current connection starts exiting
and there is no way to continue PDU receive process.
The task left in PDU receive process is never freed, and hence
LUN hotplug or exiting connection never complete.
This patch do the following:
- Consolidate freeing pre-allocated PDU to spdk_iscsi_conn_destruct()
because this is the only path to exit connection.
- Abort SCSI task of the task left in PDU receive process if found
when freeing pre-allocated PDU. If the task is not SCSI or Data Out,
remove it simply.
Fix issues #1018.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I8a2464c446c43bf4cfb5afbc0cd78b5bdef7d080
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472896
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
This reverts commit Iad6ecdc37493fa9f2d7ccab262a2c75dac2fcd48.
Both estimated cause and code change were wrong and didn't fix
the issue.
The next patch will fix the issue.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I00c8bb515ee39522c0e744dccfb839af15e946c4
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472895
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
_iscsi_conn_remove_lun() which is the callback to LUN hot-removal
returns immediately without closing the LUN if the connecion is
already in exiting, then expects that the LUN will be closed by
after the connection moves to the exited state.
LUN hot removal process doesn't check any R2T task if it is not
pending in SCSI layer but connection close process checks any R2T
task even if it is not pending in SCSI layer.
LUN hot removal will not complete until all LUN accesses are closed.
iscsi_conn_close_lun() checks if the LUN is already closed or not,
and so it will be no harm even if _iscsi_conn_remove_lun() calls
iscsi_conn_close_lun(). If the connection is in exited state,
all LUNs are already closed.
This patch changes _iscsi_conn_remove_lun() to return immediately
if the connection is in exited state.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Iad6ecdc37493fa9f2d7ccab262a2c75dac2fcd48
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472507
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Recent patches refactored iSCSI target to separate PDU header
and payload handling. However for SCSI Data-Out PDU, the division
of roles done by refactoring was wrong. Before refactoring, LUN
hotplug was checked after sending R2T, but after refactoring LUN
hotplug is checked before sending R2T. This change stopped PDU
exchange between iSCSI initiator and target and caused timeout of
LUN removal.
This patch restores the original ordering of checking LUN hotplug
and sending R2T by changing the division of roles.
SCSI Write Command PDU handling don't have any issue related with
this.
Fixes#1004
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I7b2866d8394b522fb5420d2936de2fbddc7d1daa
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472308
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
To use zero copy bdev I/O APIs for write I/O, we have to allocate
iSCSI task before allocating data buffer because allocating data
buffer will be changed to spdk_bdev_zcopy_start() call and the
allocated iSCSI task will be passed to the call.
One critical change is that we have to read all data for the
current PDU even if handling the PDU is rejected. For that purpose,
use is_rejected flag and do not call iscsi_pdu_payload_handle()
is is_rejected is true. If iscsi_pdu_hdr_handle() returns negative,
close the connection, so return the state machine immediately.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ice77b7266af8ac392d9094478523e6e6fe6d131a
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470413
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Insert _pdu_payload_ into iscsi_op_login(), iscsi_op_text(),
iscsi_op_scsi_read(), iscsi_op_scsi_write(), iscsi_op_scsi(),
iscsi_op_nopout(), and iscsi_op_data() to clarify that they
handle PDU payload.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Id28ea90948118321c7889084149083bcc6bbd27c
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471562
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Extract PDU header handling from PDU payload handling for all PDU
types, and then group them into a new function iscsi_pdu_hdr_handle().
Then the original iscsi_execute() is renamed to iscsi_pdu_payload_handle().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1fb1937cfaf502797f2c4edb3aeeb97d4697c7d4
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471015
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
To use zero copy bdev I/O APIs, we have to allocate iSCSI task
before allocating data buffer.
Hence we separate PDU header and payload handling for SCSI Data-Out,
and include iSCSI task allocation into PDU header handling.
Factor out PDU header handling in iscsi_op_data() into iscsi_pdu_hdr_op_data().
spdk_nvmf_tcp_h2c_data_hdr_handle() and spdk_nvmf_tcp_h2c_data_payload_handle()
in lib/nvmf/tcp.c are used as a reference implementation.
Use pdu->task to pass the task allocated by iscsi_pdu_hdr_op_data() into
iscsi_op_data().
In iscsi_op_data(), if it sees pdu->is_rejected is true or pdu->task
is NULL, do nothing. Besides, check LUN hot plug again to separate
PDU header handling and PDU payload handling.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I5074e945254081960744577e4ed8e0170793e5e0
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470291
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Rename iscsi_op_snack() to iscsi_pdu_hdr_op_snack() because SNACK
Request PDU has only header and doesn't have PDU payload handling.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I4d236c7963588729c32566e2be80db899edd2828
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471012
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Rename iscsi_op_logout() to iscsi_pdu_hdr_op_logout() because
Logout Request PDU has only header and doesn't have PDU payload
handling.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Id788e5da56ee51b311cff2ec448c5cf0492ace13
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471009
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Rename iscsi_op_task() to iscsi_pdu_hdr_op_task() because Task
Management Request PDU has only header and doesn't have PDU
payload handling.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I66f842aecd2c1031a6bb0be4921e0f16930117aa
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471010
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
As same as SCSI Command Request and SCSI Data-Out, separate PDU
header handling and payload handling into different functions.
In iscsi_op_nopout(), if it sees pdu->is_rejected is true, do nothing.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: If50cd64bacf18c014e2aa232fd84357b877d9821
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471011
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Separate PDU header and payload handling for Login Request PDU.
This will clarify the header handling and payload handling.
Especially store the allocated and initialized response PDU to
the current connection and refer it later. This idea is as same as
iSCSI task and will work because login processing is synchronous
and only once per connection.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I7e0eb413ee5f759724a685b83a742515a3546780
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471472
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
cid had been got in iscsi_op_login_rsp_init() but it was not
intuitive. Get cid in iscsi_op_login() and pass it simply to
iscsi_op_login_phase_none().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ief7b865784aa72f7872b6e4a98809dbab49807de
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471471
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Use rsp_pdu->data_buf_len in iscsi_op_login_rsp_handle() and its
calling functions.
This makes the code a little clearer.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I89aa7155e97c1b770b333f7f12e84e13811af953
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471470
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Use rsp_pdu->data_buf_len in iscsi_op_login_phase_none() and its
calling functions.
This makes the code a little clearer.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Id9c1f01bdfa95e8a2e9f6512777341334cb621e0
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471469
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
A local variable alloc_len had been used to hold the data buffer
length of the response PDU. But it's more intuitive to use
rsp_pdu->data_buf_len instead.
The subsequent two patches will replace related functions accordingly.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I48f96a542d6c177ba1d8409ff0230fa61c11e154
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471468
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Including getting incoming login parameters into iscsi_op_login_rsp_init()
was not so intuitive.
Initializing login response done in iscsi_op_login_rsp_init() can be
done before reading data segment but getting incoming login parameters
can be done only after reading data segment.
As a preparation to separate PDU header and payload handling for
Login Request PDU, extract getting incoming login parameters from
iscsi_op_login_rsp_init() into an new function
iscsi_op_login_store_incoming_params().
Besides, refine a few pointer and return code handling to make the
code a little clearer.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I6b677bf6e93b830e6e6fead24c8b78d9d9bc1df4
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471467
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Previously, pdu->data_segment_len had been set after reading all
data segment to the allocate data buffer pdu->data_bur together with
pdu->data and pdu->data_from_mempool.
But we don't have any ordering constraint to do that and the value of
pdu->data_segment_len has been fixed when reading all PDU BHS.
Hence move setting pdu->data_segment_len up to immediately after
reading PDU Basic Header Segment to pdu->bhs in iscsi_read_pdu().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: If6f690623687ee974c5a1d73814e8675bb7077c8
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471852
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Fix static analyzer error by ensuring 4 * total_ahs_len is
not larger than ISCSI_AHS_LEN.
When ./configure -enable-werror --enable-ubsan --enable-asan,
iscsi target reports heap-buffer-overflow error
when I feeding unexpeted pdu opcode.
Change-Id: Ib8053e1a19b5e49385642106eb79c458d35ab3c6
Signed-off-by: Hailiang Wang <hailiangx.e.wang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471489
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Liang Yan <liang.z.yan@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Refine the public helper function spdk_scsi_dev_has_pending_tasks to
be able to check tasks only from the specific initiator.
Then use the function in iSCSI target to fix the issue.
Besides add UT code to test the updated spdk_scsi_dev_has_pending_tasks().
Automated multi hosts test is much better but some UT code will be of any
help to mitigate the risk of degradation.
Fixes#985
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I50afb940de7174360c8a30479450850002a3e525
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471337
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
SCSI command uses iSCSI task and data buffer, and will use zero copy
bdev I/O APIs.
To use zero copy bdev I/O APIs, we have to allocate iSCSI task
before allocating data buffer.
Hence we separate PDU header and payload handling for SCSI command,
include iSCSI task allocation into PDU header handling first.
Use pdu->task to pass the task allocated by the former to the latter.
In iscsi_pdu_hdr_op_scsi(), we can not expect pdu->data_segment_len
is set. So getting data segment length from PDU BHS instead.
In the updated iscsi_op_scsi(), if it sees pdu->task is NULL or
pdu->is_rejected is true, do nothing. Besides, check LUN hot plug
again to separate PDU header handling and PDU payload handling.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ic09451ab022b9714381f03c63bd3008c39fedb8e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470290
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
We want to move iSCSI task allocation before data buffer allocation
to use zero copy APIs in iSCSI target. As a preparation, we have to
separate PDU header and payload handling. This patch makes it a little
easier.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ib7f5ec71933fd6022fd2248aff0a0bd89b7ad830
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470289
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Separate PDU header and payload handling for Text Request PDU.
This will clarify the header handling and payload handling.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I162af0b4bf281852bb13af5f3261dacb38dee5e0
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471451
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Double pointer is clever but reference to pointer is easier to understand.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I5168e08ca67942c22ba2cbdc10925e8a9fd6da6c
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471007
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
The subsequent patches will separate PDU header handling and PDU
payload handling. But for the PDUs which have data segment,
if PDU header is correct, we have to read data segment even if any other
error is found and the PDU is rejected.
To do that, add a flag is_rejected to pass if the PDU is rejected or
not from PDU header handling to PDU payload handling.
If PDU payload handling sees the PDU is already relected, do nothing.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ie3fe518fc0f452da9965a2fe3642568c86866480
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471005
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Subsequent patches will separate PDU handler into PDU header handler
and PDU payload handler.
Some PDU types will have both PDU header handler and PDU payload handler,
and command is executed in PDU payload handler.
Some PDU types will have only PDU header handler, and command is
executed in PDU header handler.
CmdSN must be updated appropriately depending on the case.
As a preparation, factor out updating CmdSN into a helper function
iscsi_update_cmdsn().
Besides, storing the passed CmdSN into PDU is duplicated and remove
the duplication in this patch.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ib5f271a1624a39e82d6271dfebcc181e3850e260
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471333
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
To make the code clearer,
- make the spdk_iscsi_read_pdu() private in iscsi.c and named iscsi_read_pdu(), and
- make the iscsi_conn_handle_incoming_pdus() public and named
spdk_iscsi_handle_incoming_pdus().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I967681b8e9b86681a906b18719e91e1d387450d7
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/469969
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Add PDU receive state to each connection, and each connection
transits among states during receiving incoming PDUs.
Four states, AWAIT_PDU_READY, AWAIT_PDU_HDR, AWAIT_PDU_PAYLOAD,
and ERROR are same as SPDK NVMe-TCP target.
Move clearing conn->pdu_in_progress just before putting pdu
when PDU processing completes normally to match with error case.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Id24b6d6662896b5685125a46ee20cbf216668838
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/469966
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
If PDU header digest error is detected, it indicates that the length
field of the header may have been corrupted. Hence it's may not be
possible to identify the location of the beginning of a later PDU.
So move checking header digest before handling payload and then
close the connection if header digest error is detected.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I56f6bc082dc0b244e71ad996b4da08e0203f8cdd
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471014
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Move spdk_iscsi_execute() from iscsi_conn_handle_incoming_pdus()
to spdk_iscsi_read_pdu() and then strip the prefix spdk_ from
spdk_iscsi_execute() and make it private.
This is to introduce state machine into receive incoming PDU processing.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I5d5b3e55ece0994532e924d3c75d898cb373875c
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470287
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
IO trace ISCSI_TASK_EXECUTED was taken after PDU was freed.
spdk_trace_record() doesn't dereference the passed pointers and it's fine,
but get the trace before PDU is freed to make the coder clearer.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I53a19dbd85e9599c4fa9d7458b5344cff9c76ea2
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471203
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
