conn->data_out_cnt does not control anything now but adding assert
for conn->data_out_cnt will be helpful at least to ensure that the
current SPDK works correctly.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I31ee90769ce0555e64bd41c283e8b437326efebf
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476409
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>
iscsi_conn_free_task() is used only when exiting connection now.
Hence we can remove the parameter lun and simplify the function
and its unit tests.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I6e7bf09672edca1f70c042ac58f098114d71ec78
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476115
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>
We ensure current pending tasks are aborted and no new task or PDU
is pending after LUN is unplugged now. Then we have to ensure all
task completions and aborts are sent to initiator.
However, we had removed pending tasks without any notification to
initiator. For this implementation, we are not surprised if initiator
waits for in-flight commands for a long time.
Due to recent refinements of LUN hotplug, we can wait safely that
all existing tasks complete or abort.
In _iscsi_conn_hotremove_lun(), after aborting R2Ts, start a poller
to wait until all PDUs for the LUN are flushed. Then close the LUN.
We can safely free deferred PDUs for the LUN because they are
already flushed.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I869b79a7c93d2e8a4a1577cc20d3b466548dfaaa
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476033
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 want to use poller for LUN hotplug in iSCSI library. According to
the naming used in SCSI library, rename iscsi_conn_remove_lun by
iscsi_conn_hotremove_lun. The next patch will iscsi_conn_remove_lun.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I08bb8c92db23ac3adcde4f39c0e812f3d97430d3
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476114
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>
Previously we had allocated temporary context, struct
_iscsi_conn_remove_ctx to process LUN hotplug. However, we
had to consider out of memory during connection is active,
and we could not use any poller to process LUN hotplug because
LUN hotplug may conflict with connection exit, and this
possible conflict made us very difficult to use poller for LUN
hotplug. Introducing struct spdk_iscsi_lun will resolve all
these issues.
Allocate struct spdk_iscsi_lun per LUN and store connection,
LUN, and LUN's descriptor into the struct. Then use the struct
for LUN hotplug and free the struct after LUN is closed when
LUN is removed or connection exits.
struct spdk_iscsi_lun is similar with struct spdk_nvmf_ns in
NVMf library.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ice26330f3948070c96d2fb53b94941be3b467079
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476113
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>
This is a preparation to the subsequent patches.
Subsequent patches will introduce struct spdk_iscsi_lun
to refine LUN hotplug process and fix critical issues of it.
This change will make us easy to introduce struct spdk_iscsi_lun.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I75db59d88bb09ee2ea94e8c02e0e87003352850c
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476112
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>
In _iscsi_conn_free_tasks(), we had parsed conn->write_pdu_list
and then parsed conn->queued_datain_tasks. However when we parsed
conn->write_pdu_list, if there was any task in conn->queued_datain_tasks,
some PDUs were inserted conn->write_pdu_list. Hence after parsing
conn->write_pdu_list, new PDUs were in conn->write_pdu_list as orphan.
Then orphaned PDUs were freed later but LUN was already freed and
critical failure occurred.
This patch swaps the order of conn->queued_datain_tasks and
conn->write_pdu_list, and add comment to explain the change.
Additionally, this patch adds unit test which fails if it runs
without this fix.
Fixes issue #1030.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Icb0ffbbbac70792a62939dc55a69df05d2ab9128
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475453
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>
When a iSCSI write is large and split, if LUN is removed between
creating and submitting the last subtask, spdk_clear_all_transfer_task()
completes the primary task and then process_non_read_task_completion()
tries to complete the primary task.
This is the double free case, and the later have to be skipped.
We add a flag is_r2t_active to struct spdk_iscsi_task and use it to
check the duplication. We may be able to use primary's initiator task tag
(ITT) instead but we can not rely on ITT because it is set by the initiator.
We clear is_r2t_active even when primary is removed from
conn->queued_r2t_tasks but it will be no harm.
Fixes the issue #1064.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia6511bd7adaa8fcb9a07bc40d498e8ee0b7a7ccf
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475044
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>
In process_non_read_task_completion(), when the current I/O is
not split, we have to call only spdk_iscsi_task_response().
The next patch will fix the github issue by changing the path
executed when the current I/O is split.
Hence to make the fix easier, this patch separates split case and
non-split case.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ic1603609f760c4bdd41272ba6146e260f668b059
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475043
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>
The type of pdu deferred to be free only have
two types, R2T or DATA_IN. And the two types of pdus
are all assoicated a task, so updateing both the code and unit test case.
Also for all pdu free, we should use spdk_iscsi_conn_free function since
for normal pdu free, we all use this function.
PS: I also tested the calsoft local, it does not trigger the assert.
Fixes#1074.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I0524965baf5349a100210ef717aedaa5f8ff105e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475657
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Purpose: Simply the code, doing data_cnt_in every where
will make the code diffcult to maintain. If we put the
management in iscsi task get and free related function, then
the code will be easy to be read and easy to maintain.
Change-Id: Ib9af067326630657877a94afc2eb0db28f5d5fd1
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474914
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>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Purpose: Do not let the primary task do the I/O if it is
splitted into subtasks. This will make the code simplier,
when all the sub I/Os are finished, we can free the
primary task.
Update the corresponding unit test also.
As a result of this change, when read I/O is split into subtasks,
the primary task uses only some of its data. Hence separate
iscsi_pdu_payload_op_scsi_read() into split and non-split
case explicitly.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I9bbe4b8dd92a2996f35ad810b33676e34670c77e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473532
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Avoid the running converting timeout from sec to TSC, thus
make the behaviour same as last_nopin, i.e., initialize
when constructing the connection.
Change-Id: Ibc120fed24d2208cab9ae8a876856e9d05363075
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475711
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@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>
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>
_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>
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>
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>
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>
We can unify several error logs of spdk_iscsi_execute() failure
into a single error log.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1dd6a7312e5c615c478771beb89fd44fdb1710c1
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470286
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>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Move logout check from iscsi_conn_handle_incoming_pdus() to
spdk_iscsi_read_pdu() to introduce state machine into receive
incoming PDUs processing.
Besides, remove a debug log because similar debug log is already
collected in spdk_iscsi_conn_read_data().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I11ee800eb1fd60796669d5390bd3cd1031066ca7
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470285
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>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
In iscsi_conn_flush_pdus(), call iscsi_conn_flush_pdus_internal()
only when the connection state is RUNNING or INVALID.
Besides, we can remove the iscsi_conn_flush_pdus() call from
iscsi_conn_sock_cb() because the connection state is already EXITING.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I9e1029ffa555e5e43aade53f61e942e98343006d
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/469801
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>
Request logout to the initiator asynchronously when exiting the
running connection from the target.
Based on this patch, the next patch will make flushing PDUs possible
only when the connection is RUNNING (or INVALID) and remove the
tight loop in iscsi_conn_flush_pdus().
Set timeout of logout request as 30 seconds. 30 seconds is as same as
NOP timeout and will be reasonable for now.
Add and use logout_request_timer to check the timeout.
When the connection gets internal a logout request, move the connection
to EXITING state if the connection is in INVALID state, request logout
to initiator and start 30 seconds timer if the connection is in RUNNING
state and logout is not requested yet, or do nothing otherwise.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I43192be9fd7112ad444152c0dd88f99a14aa8d30
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470705
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>
Change logout_timeout() not to call spdk_iscsi_conn_construct() but
to move conn->state to EXITING. Then spdk_iscsi_conn_destruct() will
be called by the poller of iSCSI poll group, iscsi_poll_group_poll()
soon.
This will improve the maitainability slightly.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I78046ae5ad7992ad6c7509884f964d2eff6ced76
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470704
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>
The connection state is mainly for flushing PDUs. ISCSI_CONN_STATE_LOGGED_OUT
was as same as ISCSI_CONN_STATE_RUNNING for this point.
Replace ISCSI_CONN_STATE_LOGGED_OUT state by a is_logged_out flag
and use the latter to reject any incoming PDU after sending logout
response.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I3426a3ecbd241db402b1a2f7b3914a18e3591591
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470703
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>
Previously When the connection is in login processing, flushing PDUs
had been done by tight loop. Login processing will not fill out
send buffers but we can use poller to flush PDUs as same as when the
connection is running iscsi_conn_flush_pdus().
Hence change the check condition to include INVALID state to use
poller in iscsi_conn_flush_pdus().
This change is necessary because the subsequent patches will change
iscsi_conn_flush_pdus() not to flush PDUs when the connection is not
invalid or running.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I543a912bf1aa957df7033184ce5989ac92367e49
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470702
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>
We can remove the declaration of iscsi_conn_stop() by moving
the related functions before the caller to iscsi_conn_stop(),
_iscsi_conn_check_shutdown() and _iscsi_conn_destruct().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia86f6685c9e90692d8dae744495bd6f8fc310990
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470556
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>
This is another preparation to support per portal group CHAP authentication
for discovery session.
Add CHAP params into struct spdk_iscsi_portal_grp, and initialize them
by global parameters at spdk_iscsi_portal_grp_create().
Copy CHAP params from portal group to connection at spdk_iscsi_conn_construct().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1ecb812266ac3d090f8e6db21d1d6a090f1811d9
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/469368
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 is a preparation to support per portal group CHAP authentication
for discovery session.
Previously require_chap, disable_chap, and mutual_chap had been set
and used in iscsi_negotiate_param(), and chap_group had been set
and used in iscsi_get_authinfo().
If a connection is in a discovery session, the connection can get
all CHAP params at its creation, spdk_iscsi_conn_construct().
If a connection is in a normal session, the connection can get all
CHAP params in iscsi_op_login_negotiate_chap_param().
Each connection is in either discovery session or normal session.
So the following change is possible and is done in this patch.
spdk_iscsi_conn_construct() sets all CHAP params of the connection
by global parameters. Then iscsi_op_login_negotiate_chap_param()
overwrites them by the corresponding target's parameters.
iscsi_negotiate_chap_param() and iscsi_get_authinfo() just refer
the CHAP params.
Besides, iscsi_get_authinfo() changed to call just
spdk_iscsi_chap_get_authinfo() inside, and so inline
spdk_iscsi_chap_get_authinfo() into iscsi_auth_params() and then
remove iscsi_get_authinfo().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I8028673cc6923e1b8bc20af55e0c3cc933972fc0
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/469218
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>
Purpose: Prepare for the further optimization work
to use one bigger buffer to read more data for
reducing system calls.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: Ie92603b09308bd3149263269fdec355b67251b37
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/468538
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
The socket now automatically sets the recvbuf size
to 2MB, so this isn't necessary.
Change-Id: Id2196f4038f6835118047233f18c0395fa3f2670
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/466992
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
One recent commit destroyed the critical bug fix for LUN hot plug.
Hence this patch restores the critical bug fix. Simple revert is
not possible because connections are assigned to poll groups
instead of cores now. But we can revert easily because earlier
version of the recent patch did that.
Fixes#925
The github issue was caused by commit 8cf1945432.
The bug fix restored by this patch is commit 1f6a78620d.
The reference we can follow to create this patch is earlier version
of commit fb641c4b54.
Reported-by: yidong0635 <dongx.yi@intel.com>
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Change-Id: Ie1ba14a59ce48149a8474cbffc56aa08adc1fc4d
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/466108
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This will reduce the potential malloc failure.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ibcd631bf318ef6ece4ac337219652323ca5fd8f1
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/464136
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 poll groups are bind to SPDK thread through IO channel now.
This patch changes get_iscsi_connections RPC from synchronous to
asynchronous by using spdk_for_each_channel.
Core ID was removed from the output of the RPC but thread name
is added to the output of the RPC in this patch.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I20fdb7f11434acf838a89435ba5052b19869181c
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/463569
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>
Factor out writing connection information to JSON context into
a helper function spdk_iscsi_conn_info_json.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I3e92bcb4f21cc7a36af182f850c944b8c5dd559f
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/463568
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: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
This patch binds poll groups to SPDK thread through IO channel and
assigns connections to poll groups instead of cores.
iSCSI subsystem registers iSCSI global object as an IO device, and create
poll groups as context of IO channels of the IO device.
Each portal get and hold portal group on which the corresponding acceptor is
running.
When a connection is constructed, iSCSI subsystem assigns a poll group
to the connection by getting it from the corresponding portal.
When a connection enters full-feature phase, iSCSI subsystem schedules
the connection to a poll group by round-robin.
Then, each connection can know its running SPDK thread directly and can
use SPDK message passing infrastructure instead of SPDK event framework.
By this change, iSCSI connections are binded to SPDK thread, and not
binded to processor core anymore.
Some other changes in this patch are
- core ID is removed from the output of get_iscsi_connections RPC. The
upcoming patches will change the RPC to use spdk_for_each_channel and can
access SPDK thread safely, and add SPDK thread ID instead.
- utilize UT multithread framework added by the last patch to test
iSCSI poll groups by UT.
Change-Id: Iec73c778aa413bcabdb63141cc41d4160911ea0e
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/463359
Reviewed-by: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
