This patch fixes the bug in
commit d421876f98.
We had passed not xconn but conn caressly by mistake to
_iscsi_conn_drop().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ifc2313923bc0e94b967e4ba2eca4255ddcc4611b
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1988
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Updating conn->state had not been guarded by any mutex. When SPDK
iSCSI target has multiple SPDK threads, iscsi_drop_conns() may update
conn->state by a thread different from the thread of conn->pg.
This patch ensures conn->state is updated by the thread of conn->pg
by sending a message to the thread.
This fix is not perfect but connection reschedule is done only once
per connection when moving to the full feature phase. So this fix
will be simple and enough for now.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I500474a27659438473b0eea598d35c90624a1d10
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1930
Community-CI: Mellanox Build Bot
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: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
As other small change, function iscsi_conn_pdu_generic_complete()
had been declared in conn.h but defined in iscsi.c. Move the
definition of it to iscsi.c.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1bd796288036f78a7cba8a1c0af93bd6bc19e9cf
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1890
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Seth Howell <seth.howell@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
The following patch will remove the "spdk_" prefix from iSCSI
internal APIs. The iSCSI global data g_spdk_iscsi is also local in
SPDK iSCSI library. Hence rename it by g_iscsi for consistency.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: If35e9d58b1388fd725a505ee9be870e414c37ba5
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1831
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
We will be create fine name for each poller but it will need large
effort. Replacing spdk_poller_register by the macro SPDK_POLLER_REGISTER
will provide better name than function address with minimum effort.
Following patches may improve function name for clarification.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: If862a274c5879065c3f7cb04dcb5ca7844523e68
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1781
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: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Maciej Szwed <maciej.szwed@intel.com>
Community-CI: Broadcom CI
This patch does drop in replacement of SN32_LT and SN32_GT by
spdk_sn32_lt and spdk_sn32_gt, respectively.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I5df1e461d2b25a2f20222e823fb1ec68ced049ad
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1347
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
In some cases, conn->sock will be lost, while conn pointer will still be.
A runtime error will apperars: null pointer of type 'struct spdk_sock'.
This only happens when ./configure with asan.
So I add a sssert to display this error explicitly when it occurs.
Change-Id: I7d012ac1a29a7fb0bce4815e0622582f23222a25
Signed-off-by: WANGHAILIANG <hailiangx.e.wang@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1102
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: Ben Walker <benjamin.walker@intel.com>
The next patch will create a SPDK thread for each poll group at
startup. Hence iSCSI connection will be created or destroyed on
these threads. On the other hand, logout request to the active
connection will be still invoked by the default thread.
This unmatch will hit assertion such that poller is unregistered
different thread that registered it.
Fix this unmatch by sending message to the thread explicitly to
request logout and wait for the logout process to start.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Iff361e78b9ba077230e38d9586f7187968e33314
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/490
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>
The next patch will create a SPDK thread for each poll group at
startup. Hence iSCSI configuration management will be done by
different thread from these poll group threads.
Hence send message explicitly to add connection to poll group even
at startup. We can do this for the current master branch.
Remove some code related with SPDK thread framework from unit tests
for iSCSI portal group because thread management is moved to
connection.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I40cacdb2066f65866f7ef83cf3b3e4e8b8cd322e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/489
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>
To avoid partial write issue of this PDU.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: Id9b22da844c75ae53c6881850d192b40ac4098ac
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/481948
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>
Since only after DATAIN pdu sending out, we can have
free slot to handle queued data in tasks.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I49a52597e8660453ea90c5960d020eb53f81265d
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/482048
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
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: Jim Harris <james.r.harris@intel.com>
This is prepared for the further call back usage.
Change-Id: Iccf304c87e67debfb4e7c330acc9cc233cc3ec48
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/481917
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
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: Jim Harris <james.r.harris@intel.com>
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>
