This patch eliminates the flushing logic and simplies
the writev logic. And this patch can also improve the performance.
We support async write for PDUs other than login response, logout response,
and text response in this patch. We will support async write also for them
later in this patch series.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I243f598f297d594da0bb18466bc47dab918ed3ee
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/481686
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Replace usage of fprintf(err, *MSG*) by SPDK_ERRLOG(*MSG*)
since in SPDK we always prefer later
Change-Id: I3e2b5c12caa572b32236f5ede01b754b2e1e2a53
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/478940
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
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: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Followin the last patch, move iscsi_conn_abort_queued_datain_task()
and iscsi_conn_abort_queued_datain_tasks() from iscsi.c to conn.c.
Refine unit tests to check more accurately. Adding more test cases
will be done later.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I30530e6871a78a58d9fb472f62168862298884a0
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477417
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>
Operations to queue iSCSI tasks are in iscsi.c and conn.c and cross
references due to this separation makes us difficult to create unit
tests.
This and subsequent patches will try to disentangle cross references
by moving some functions from iscsi.c to conn.c.
This patch moves spdk_iscsi_conn_handle_queued_datain_tasks() from
iscsi.c to conn.c. For unit tests, we don't add anything new in
this patch and just create necessary simple stubs. After code
movement, new unit tests will be added.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: If5b8501a1ef7ea53682a3437c7eb2375aa52ee3b
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477416
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Move spdk_iscsi_conn_read/readv_data() down closer to the functions
which calls spdk_sock_writev().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ie7ef649e8681efac48adcfb2da1f745660f71782
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477411
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Move iscsi_get_pdu_length() down closer to the caller.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Iacbe43d39f5b23c47a65631ae8004fcfb489e2cc
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477410
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
spdk_iscsi_send_nopin() is called only in a single place of conn.c.
So change it to private in conn.c. Additionally, iscsi_conn_send_nopin()
may be a little more meaningful and rename to it.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1fe70e0468e1dd43468492b8b3d359c094130ed7
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477190
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
spdk_iscsi_is_deferred_free_pdu() is called only in a single
place of conn.c. So change it to private in conn.c. Additionally,
iscsi_is_free_pdu()_deferred() may be a little more meaningful
and rename to it.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ic1e3d7ff435c454f40e81f9a4f90fe76589ec7b2
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477189
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
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>
This patch is in the patch series to migrate iSCSI connection management
from core based to SPDK thread based.
The callback to hot removal of LUN, iscsi_conn_remove_lun, is called
on the same core when the corresponding LUN is opened. Additionally,
all operations in iscsi_conn_remove_lun are completed synchronously.
Hence inline _iscsi_conn_remove_lun into iscsi_conn_remove_lun.
Add assert to check the function is called on the specified core.
This change is helpful to achieve the goal of the patch series
because spdk_event can have two parameters but spdk_msg can have
only a single variable, and hence we cannot convert simply and have
to introduce a context allocated dynamically otherwise.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Iaebf18265dfe839f7361b09539527a1806aed1c4
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/463551
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Connections will soon be assigned to poll groups, which will be
dynamically moved between CPU cores based on load. It no longer makes
sense to restrict certain portal groups to specific cpu cores in this
model.
Change-Id: Iee983d75febc9797aa60021c5bc0680335e895cd
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/463358
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Since spdk_iscsi_conn_read_data() can print error log, so we
don't need to print again in the caller, existing code will
print error log for LOGOUT and DISCOVERY cases.
Fix issue #845.
Change-Id: I547d3d667b6412ab6a59c9b401d0f28c5026307d
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/460110
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
We shoud not always return SPDK_ISCSI_CONNECTION_FATAL
in spdk_iscsi_read_pdu function.
Reason: In iscsi_conn_handle_incoming_pdus, the loop
directly return only rc==SPDK_ISCSI_CONNECTION_FATAL.
But it masks all the necessary information. So we would like
to keep some information of the return value for spdk_iscsi_read_pdu,
and we can use error log to track those information.
Then we can return SPDK_ISCSI_CONNECTION_FATAL as the error
return value for iscsi_conn_handle_incoming_pdus function.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I4c40fcb27052b55cb92e06273701a881def18e12
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/457078
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
If the qualified core wasn't found, then we should compare
spdk_env_get_core_count() with i, instead of lcore.
Signed-off-by: Li Feng <fengli@smartx.com>
Change-Id: Ie92f56712b7f0e51636008fe12fff5584b6be8ab
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/456415
Reviewed-by: wuzhouhui <wuzhouhui14@mails.ucas.ac.cn>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Added a utility function for the cleanup related operations.
Change-Id: I4e49dd9c2da899a5bda289bc36778b636af9c5df
Signed-off-by: GangCao <gang.cao@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/456599
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 throttle the number of data_in operations per
connection. Currently after a read is completed,
we try to send more data_in operations since one
has just been completed.
But we are trying to send more too early. The data_in_cnt
doesn't actually get decremented until after the PDU is
written on the socket. So this results in a case
where data_in_cnt == 64, and all 64 read operations
complete before any of those 64 are actually transmitted
onto the TCP socket. There are no more read operations
waiting, so we won't try to handle the data_in list
again, and if none of these 64 resulted in a SCSI
command completing, then the initiator may not send us
any more read I/O which would have also kicked the data_in
list.
So the solution is to kick the data_in list after the
PDU has been written - not after a read I/O is completed
back from the SCSI layer.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ia01cf96e8eb6e08ddcaaeff449386e78de7c5bc5
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/455454
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: GangCao <gang.cao@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
The datain handling code path will set the LUN to
NULL if it finds a task's LUN has been hotremoved.
This could happen before the iscsi hotplug routine
actually gets a chance to run. If this happens,
one of these tasks doesn't actually get freed, and
then will be freed after the lun is closed -
causing a segfault in the bdev layer since it may
have a bdev_io associated with it.
Found by running the iscsi_tgt/fio test after
applying the next patch in this series.
There's more work needed in this hot remove clean up
path - currently we are just freeing a lot of PDUs
rather than completing them with error status when
a LUN is hot removed. But let's tackle that
separately.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I8d27f0c7a79ae91cb6504e5ff6ffc8e346c9e54c
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/455460
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Make this explicit instead of implicit.
Change-Id: Iab1b856648e1f11722e1f76602f1eee4bc4ed0b5
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/454381
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
This function was only called from a single place, so just
move the code there.
Change-Id: I78c05ef41ca0d5684385e80cb75d699453c90792
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/453018
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This went stale and the code is currently in flux.
Change-Id: Ib0b2171f253db0f54d41573c07668eb3b2bd83b4
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452781
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Previously, this searched the list of cores for the one
with the fewest connections and scheduled there. iSCSI
is being migrated to a new threading model where the
threads themselves will float, so simplify this part
of the code while that's implemented.
Change-Id: I05a1ff51a45df9c176c0e57d7f1ae9e97928adcd
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452780
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
iSCSI is currently being adapted to SPDK's new threading model,
and this parameter doesn't make sense anymore. Remove it for
now until something functionally equivalent is added later.
Change-Id: Ia0e2f5aa81b72d99467c5a900619fbeeb42b2069
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452779
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
We don't need to have operations to remove just the socket.
Change-Id: I00e0c7fb81af98d1c072ba126ce22b41954e313b
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452728
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
For migrating a connection, don't stop it entirely.
Change-Id: I0e09698e03127c04257485a0a2af262ed1a7761a
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/453017
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: Jim Harris <james.r.harris@intel.com>
This restriction helps reduce the amount of padding when
printing out the event trace, allowing it to fit in a
small number of columns.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ifa31e5a6967c7b9bc7028069effb71533f80596f
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452736
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This was not used by any of the trace register descriptions.
Let's remove it rather keeping it around if we don't need it.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Idda809e2911db5be555ff6aa13695484a14bf665
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452734
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
This function is in charge of scheduling new connections
onto cores. It used to be used to dynamically migrate
connections, but we no longer do that.
Change-Id: I2fa04a7dfb43bf9fae7902376ba5ba6d18744469
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452727
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This can use threads instead of events.
Change-Id: I8a55ad512038dd75218a9a6af17d135eba6949f7
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452726
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
The next patch will add the function ponter typedef
spdk_scsi_dev_destruct_cb for SCSI device destruction.
Hence add lun to the names of descriptors and callback for SCSI
LUN for clarification.
This patch doesn't change any behavior.
Change-Id: I73f2bce9129f7a6f16770ab6ed18428b16589108
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450883
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
In iscsi_conns_cleanup(), g_conns_array was unmapped but was not
invalidated by setting MAP_FAILED.
So, find_iscsi_connection_by_id() caused segmentation fault if
it is called after iscsi_conns_cleanup().
Change-Id: Ib91c9240c62c2aaa32713dd4aa382d31e5ea2eed
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450901
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>
These change will be used in the next patch.
Change-Id: Ifdb4ccf20049b46e850122a4021cbbe7441e1270
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450736
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>
Subsequent patches will exit only connections belonging to the
specific target node. So add target to get_active_conns() as
an argument.
Change-Id: If0d9cad46614310e0fe17e69e75c1185146730f4
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450735
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>
When target is specified, the function starts exiting any
connection that belongs to the target. When target is not
specified, the function starts exiting all connections.
Change-Id: I0dfb56ceac5ee36d10547a9ab9a0f768ca8e02ec
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450734
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>
Factor out the operation to start exiting connections into a
function. This patch doesn't change any behavior.
Change-Id: I8f48961bcc95c480636e0e0d8fbb8ef029818d9e
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450733
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>
My iSCSI target reported segment fault after memory allocation
failure, and add the addtional check here can fix the issue.
Change-Id: Iee3e497d7028fbface6d110c78e73965ef0f178b
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/449717
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: yidong0635 <dongx.yi@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
A few private functions had still the prefix spdk_ in their names.
This patch completes the effort to remove the prefix spdk_ from
private functions in iSCSI library.
Change-Id: Ic1a1e85220edb72296ca4881bdf7b550c4f6b6b8
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/449716
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 library had used the prefix spdk_ for most functions regardless
of private or public. Using the prefix spdk_ only for public functions
will be helpful to distinguish private and public functions, and
will be helpful to investigate issues or do further improvement.
Besides in iscsi.c static variable spdk_arc4random_initialized had
the prefix spdk_, and change it to g_arc4random_initialized according
to the SPDK's good practice.
iSCSI library still have some issues but is more stable than before
and now will be the good time to adjust the naming rule to other
libraries.
This patch doesn't change any behavior.
Change-Id: Ia0b8585a7ce6662cabc0e6f57b7ccb8a40342297
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/449396
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>
spdk_iscsi_conn_read_data was changed to use sock_readv
instead of sock_recv as a refactoring effort.
However this change caused about 3% performance degradation
for write I/O.
Hence this patch changes spdk_iscsi_conn_read_data to use
sock_recv and changes spdk_iscsi_conn_readv_data to call
spdk_iscsi_conn_read_data if iovcnt is 1.
Change-Id: I5fcad03ff23dee4e9954eee879225d3c493be52e
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448021
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Generate and insert DIF for write I/O by stream fashion in
spdk_iscsi_read_pdu().
Verify DIF for read I/O in spdk_iscsi_conn_write_pdu().
Verification can be done in spdk_iscsi_build_iovs(), but how many
writes to socket is not known beforehand, and same range may be
verified multiple times. So verify DIF before starting to write to
socket.
Change-Id: I860637758164b7d518c38fe92356562cbbe3d8f8
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446406
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: Ziye Yang <ziye.yang@intel.com>
This patch leaves metadata space in data buffer for read or write I/O
to generate and verify DIF.
For write I/O,
- in spdk_iscsi_read_pdu(), leave metadata space for every block
by calling spdk_dif_set_md_interleave_iovs() and then
spdk_iscsi_conn_readv_data(), and remember this in PDU.
- in spdk_iscsi_op_scsi() and spdk_iscsi_op_data(), set not data
length but buffer length to length of an iovec. The reason is
that the actual data length sent to SCSI layer will be larger
than the data length set in PDU. SCSI task holds data length
as an independent member and this change doesn't loose any
information.
For read I/O, in spdk_iscsi_build_iovs(), leave metadata space for
every block by calling spdk_dif_set_md_interleave_iovs().
The reason why DIF context is saved in PDU is that DIF verification
will be done in spdk_iscsi_conn_write_pdu() and DIF context will be
used there first.
Change-Id: Iac5739211f26a91b00a4ecf2f306bbecec3d78dd
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446382
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 patch adds spdk_iscsi_conn_readv_data() to read PDU data
from network socket by using spdk_sock_readv().
Additionally, this patch changes the existing spdk_iscsi_conn_read_data()
to call spdk_iscsi_conn_readv_data() by creating a single struct
iovec.
Change-Id: Ied487bb71bd4261ad53c9f3744ae272e65f98d7a
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446377
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>
Upcoming patches will support DIF insert and strip feature in
iSCSI target and the feature will be implemented by utilizing
iovecs.
Even when we support the DIF feature, we want to keep current
batched PDU flush, and current requirement that there must be
enough free iovecs to map all segments of a PDU is too strict.
This patch alleviates the requirement by passing remaining
number of iovecs to spdk_iscsi_build_iovs().
Change-Id: I6206322839c363e0ff5abe84bfd524bdc09e23ca
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446176
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
To know the mapped length by iovecs, pdu length was got first and
then the size of partial written was reduced separately.
This patch unifies these two operations into spdk_iscsi_build_iovs().
Change-Id: Ic6f5eecc902b8e209ef00c010915f476ca16c002
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446175
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>
Building iovecs had been done in spdk_iscsi_build_iovs() and
fast-forwarding iovecs for the partially written first PDU had
been done in-line separately.
This patch unifies these two operations into spdk_iscsi_build_iovs().
Fast-forwarding iovecs is necessary only for the first PDU, but the
operation is applied to all PDUs after this patch.
Extra overhead will be negligible because usually at most two
iovecs are consumed, one is for the base header segment and another
is for the data segment.
Change-Id: I8feafac6d280439eb7cf73136107adbac820ae09
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446174
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
iovec_cnt and iovec_array are very descriptive and good but iovcnt
and iovs are often seen in SPDK and will be enough.
Subsequent patches will add some changes on iovec operations and
simple and familiar names will be helpful to work and review them.
This patch doesn't change any behavior.
Change-Id: I89ff74809a0ddbb358e3fc8fdc353a47338cc3c5
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446173
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: wuzhouhui <wuzhouhui@kingsoft.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>
Reviewed-by: Piotr Pelpliński <piotr.pelplinski@intel.com>
During destructing a connection, if timeout of logout timer for the
connection occurs, spdk_iscsi_conn_destruct() will be called again
to the connection.
Segmentation fault by multiple calls of spdk_iscsi_conn_destruct()
have been observed when exiting one of the sesssions during IO tests
for multiple sessions.
Fix#574
Change-Id: I69873905486953bfb0cdb43b000b8e38ff9346ac
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/440466
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
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>
For split SCSI read command, if there is any failure in the sequence of
it, the first error must be propagated to all subtasks of it.
For split SCSI write command, if there is any failure in the sequence
of it, the first error must be propagated to the primary subtask.
Before this patch,
for read task:
- any failure is propagated to already completed subtasks, but
is not propagated to any subtasks not completed yet, and
- if any failure occurs in non-primary subtasks, it is not propagated
to the primary subtask.
for write task:
- if the primary subtask completes after any failure of non-primary
subtasks, the failure will be overwritten by the success of the
primary task.
This patch fixes these issues.
Change-Id: I2d878798cbb40a8c5bd6a6fe5efb32b8de4a8ecd
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/436673
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Current implementation is a little complicated and a little
refactoring will improve maintainability.
Change-Id: I23bdbe6a0e14739631c4b4f211bedefc9742f410
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/436447
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
