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>
Using "= {} at definition is clearer and save lines.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I410279bd3640e03aa9b6eeb488be4404d110b0bd
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477412
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>
Re-order operations in abort_queued_datain_task_test() to align it
with comment, and fix a type error.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Idde4726bfe370ad742b2de67e0786aebb7237d45
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477409
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>
We changed read I/O submission to stop splitting when detecting LUN
hotplug very recently. We can do the same refinement for read I/O
abortion.
In _iscsi_conn_abort_queued_datain_task(), set all remaining length
to the new task and complete it immediately. We keep the code to
process the case that queued_datain_task completed but is still in
queue, but we can change its if condition to assert.
Simplify the corresponding unit tests accordingly, and set
task->scsi.transfer_len in abort_queued_datain_tasks_test() to
exercise the changed paths.
In iscsi_pdu_payload_po_scsi_read(), if task->scsi.transfer_len is not
larger than SPDK_BDEV_LARGE_BUF_MAX_SIZE, no minimum calculation is
necessary and we can substitute task->scsi.transfer_len to
task->scsi.length simply. This change is too small to be an independent
patch and is done together.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Iea93e51b103eae141a007a0abdaf13cbe6d5287f
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476984
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.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>
By the recent refactoring, SCSI task is configured when getting
DIF context from SCSI layer. Passing not CDB and offset separately
but SCSI task to SCSI layer is more concise and do in this patch.
In iscsi_send_datain(), we have to update task->scsi.offset for the
case that data is split into a sequence, but the update is no harm
because task has completed what it must to do.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I153352dfa7aa7325db4452f03d863df11b3e0cfa
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472510
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: Paul Luse <paul.e.luse@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>
Add unit tests for PDU header handling except for SNACK Request PDU
because the caes that Error Recovery Level is not matured yet.
The purpose is mainly for increasing coverage, and most were reviewed
separately, and so merge multiple PDU types into a single patch.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ice8c522bee0fc8c0dadb0c6326be9f65d323d52e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471955
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>
This clarifies that g_write_pdu_list is global.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I4292a2fb455f3a47d79ae6ebb5dc6046f0f7dd52
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471951
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@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>
Extract PDU header handling from PDU payload handling for all PDU
types, and then group them into a new function iscsi_pdu_hdr_handle().
Then the original iscsi_execute() is renamed to iscsi_pdu_payload_handle().
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1fb1937cfaf502797f2c4edb3aeeb97d4697c7d4
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471015
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
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>
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>
conn->target is initialized to NULL in iscsi_op_login_phase_none
and then configured in iscsi_op_login_session_normal. Hence
subsequent references to target node can be done by conn->target and
related local variables can be removed.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I68b94cb8e136522ef1e0ed74c0035ec2b666bb9e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/463700
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>
Compilation Warning on fedora30.
iscsi_ut.c:184:31: warning: The left operand of '==' is a garbage value
CU_ASSERT(rsph->status_class == ISCSI_CLASS_INITIATOR_ERROR);
~~~~~~~~~~~~~~~~~~ ^
This is related to issue #822.
Change-Id: I221aa6601bae4a2d55bd557fcd1d001fbc616149
Signed-off-by: Hailiang Wang <hailiangx.e.wang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/459723
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This bug was found by code inspection. When PDU read restarts
after data segment, iov_offset must be reduced by data length.
However iov_offset had been reduced by buffer length by mistake.
Lack of UT code was the main reason of this bug. Hence add UT
code to test the fix of the bug together in this patch.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I74f2f6ae8dca2e78a64bfdef8080c8031dfabb87
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458530
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
In a test case of op_login_session_normal_test(), it is expected that
the tag of the session has any value different from the connection but
the tag of the session was not initialized.
Fixes#826
Reported-by: lorneli <lorneli@163.com>
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I62f7a4bd301c5a74ad6ba10b1b448189f750b324
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458339
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Lorne Li <lorneli@163.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
This patch adds the UT regarding below function
iscsi_op_login_session_normal()
It covers the target related, connection related
and session related operations.
Change-Id: I570df82df850a0b359b2d57b4e1694d8cb16de37
Signed-off-by: GangCao <gang.cao@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/457925
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>
SPDK_SUCCESS is a remnant and all other SPDK libraries have used
just 0. So this patch removes SPDK_SUCCESS from iSCSI library and
uses 0 instead.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ie33fd26238411e994ea9ea0c898da1abb502cad6
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/456928
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>
It's 3 functions that are only called from this compilation
unit.
Change-Id: I033ced4c19ee2a33b9537c347532ea5706d02d6a
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/454493
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 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>
Add the flag destructed to struct spdk_iscsi_tgt_node and
iscsi_op_login_check_target() refers it and returns
ISCSI_LOGIN_TARGET_REMOVED if it is true.
When destructing iSCSI target node, it will be nice if iSCSI library
can stop further connections are created but, connections are not
associated with any target node until processing login. Hence stop
creating sessions instead.
Additionally, when destructing iSCSI target node, if the flag destructed
is already set, return immediately, and the flag destructed does not
affect discovery session.
Change-Id: Ic73bdd93f2ca7d5ca1d2f897d5046cbc51650d5f
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450881
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
LUN ID is not converted from integer to structure and integer is
set to R2T PDUs.
Popular iSCSI initiators don't check this value and work correctly.
This patch uses the public helper function of the SCSI library
to fix the issue.
Additionally, private helper function to convert structure to
integer for LUN ID is replaced to the public helper function
of the SCSI library.
Change-Id: I9218c5ef7a8bfec43326c6584db7c6929fdd11a8
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/449963
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 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>
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>
This patch is for ABORT TASK SET and LUN RESET.
iSCSI layer doesn't have pending queue and hence if the target task
is read task and is queued in queued_datain_tasks, it must be
aborted before submitting ABORT TASK to SCSI layer.
Aborting the target task may not complete by an iteration because
submitted read tasks are limited. Hence use poller to complete abortion
by repetition.
Change-Id: Idabf4931d751ee698e9809eafd5c151b979f048b
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/436077
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>
By up to the previous patch in the patch series, unexpected behavior
due to write tasks in task management commands have been fixed.
But unexpected behavior due to read tasks in task management commands
have been still observed.
Remaining patches in the patch series will fix the unexpected behavior
due to read tasks in task management commands.
This patch is for ABORT TASK.
ABORT TASK is not supported in SCSI layer yet. But the initiator
doesn't care about the failure is due to not-supported or failure.
It must be avoided that the task management command returns SCSI Good
but some tasks are not aborted and return SCSI Good later. On the other
hand, it is acceptable that the task management command returns
failure but some tasks are partially aborted.
Hence this patch adds operation without checking the support status
in SCSI layer.
iSCSI layer doesn't have pending queue and hence if the target task
is read task and is queued in queued_datain_tasks, it must be
aborted before submitting ABORT TASK to SCSI layer.
Aborting the target task may not complete by an iteration because
submitted read tasks are limited. Hence use poller to complete abortion
by repetition.
Change-Id: I030a8b2f19c2f7c7d2f7b0b2c633579534db631b
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/436076
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>
As described in RFC3720, task management request will act on all
the commands from the same session having a CmdSN lower than the
task management CmdSN.
Current implementation clears all R2T tasks without checking CmdSN.
This patch changes the implementation to clear only R2T tasks
whose CmdSN is smaller than the task management request.
Additionally this patch adds to UT code to verify the change works
as expected.
Change-Id: I0b368cb13741bc05259924d27793438e9250b951
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/434761
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: Ben Walker <benjamin.walker@intel.com>
Unit tests shouldn't link in DPDK env libraries - they
should all use the test_env.c
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ib08ae33224ed9a12fa1ff255c4769a005fc10369
Reviewed-on: https://review.gerrithub.io/434259
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>
One was an API issue, another was a dereference of a null pointer that
resulted from a possible call to a stubbed function in the unittest
common code, and the last was an erroneous use-after-free due to
scan-build apparently not understanding TAILQ_REMOVE.
Change-Id: I9111817e9e990ff6e388351c08f34c2e945df62d
Signed-off-by: Seth Howell <seth.howell@intel.com>
Reviewed-on: https://review.gerrithub.io/423959
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 secure iSCSI targets, dynamic reconfiguration of CHAP secrets is
must to have.
Currently CHAP secrets are loaded for every CHAP authentication
operation. The current implementation will not work correctly
when CHAP secrets in the file are changed dynamically.
If SPDK loads CHAP secrets from the file only at boot and they can be
configured by RPCs, user can change CHAP secrets safely during run time.
Even if there are any users who expect dynamic reconfiguration of
CHAP secrets based on the current implementation, if we provide
this better alternative based on RPCs, they will be able to continue to
satisfy their requirement.
This patch change the current implementation so that SPDK loads
CHAP secrets from the file once at boot and uses them in memory
hereafter.
Besides, use fixed size buffers to hold CHAP secrets.
Previously dynamically allocated buffers by strdup() had been used,
but it required many nomem checks and should be avoided.
Other iSCSI targets/initiators have used fixed size buffers and SPDK
follows others.
Set the size of buffers for both user names and secrets to 256 (the
last byte is for NULL termination). 256 is sufficiently large
compared with others.
CHANGELOG will be updated in the separate patch because new RPCs will
be added instead.
Change-Id: I499e792817c2ed01c3d970bbd3d34a6b1fccf65b
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/421463
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
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>
This patch series will refactor/improve CHAP authentication and consolidate
it into iscsi_subsystem.c.
To get better reviews, this patch just moves spdk_iscsi_chap_get_authinfo()
from iscsi.c to iscsi_subsystem.c.
Change-Id: I953f5c851bfe67dc02f6f82966132b4216e79228
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/422764
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>
When hot removal of a LUN is started, callback is called for each
iSCSI connection which accesses the LUN. Callback checks all transfer
tasks complete and then close the LUN.
If the connection clears all transfer tasks before getting all responses
to them from the initiator, the initiator continues to retry data write.
Hence the connection have to wait until all transfer tasks complete.
Change-Id: Iad9063673cfedbd78758890d55a4254512e4fca4
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/417199
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
When multiple LUNs are attached and spdk_clear_all_transfer_tasks() is called with
a LUN, all tasks whose target is the LUN are removed and there will be room to start
queued tasks for other LUNs. However currently no migration is done.
Add UT code to verify the fix too.
Change-Id: I082d370ab86a46e5b4a74a16293a572fae663add
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/418765
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Seth Howell <seth.howell5141@gmail.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Currently R2Ts per connection are managed by an array
outstanding_r2t_tasks and two tailqs active_r2t_tasks and
queued_r2t_tasks.
These are complex but there is no UT code for them.
Added UTs are not perfect but it may be helpful to keep the
quality and understand the implementation.
Change-Id: I2a0d19d8888e7d3beb5918a1b82b657f4775e01a
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/418758
Tested-by: 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>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Residual count must be ignored and sense data must be set instead
when Check Condition occurs.
Add test code to keep current implementation.
Change-Id: I8919a28481c81606e8803269d5ff8a4e3fcbf8c2
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/418176
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Current SPDK implements SCSI Request Sense command so that a SCSI
Data-In PDU and a SCSI Response PDU are returned and underflow are
set to both of them.
On the other hand, some implementation return only a SCSI Data-In
PDU and some implementation return only a SCSI Response PDU.
Adding test code will be helpful for every implementation.
Change-Id: Icccc2d6e334146f558f8d75215bab1362b2b68f0
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/418175
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
This bug was caused by the following two changes:
iscsi: Remove duplication of the variable for write completion to bdev
https://review.gerrithub.io/#/c/393582/
iscsi: restore data_transferred accumulation for read
https://review.gerrithub.io/#/c/393713/
For write, bytes_completed is always equal to data_transferred. However,
for read, bytes_completed is based on expected data transfer length and
data_transferred is actual read size. Hence bytes_completed cannot be
used to calculate residual counts.
One of the reason why this bug cannot be found was that there was not any
test case when task->scsi.data_transferred is 0 and task->scsi.length is
not 0. Hence UT code is also added.
Same bug will occur for read sense data. Hence UT code for read sense data
is added in the next patch.
Change-Id: Ib1a283b769e5af0c2d05acb69f90948c5d658087
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/417960
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Static analyzers don't see CU_ASSERT_FATAL as truly fatal,
and will complain that code later in the function may try
to dereference a NULL pointer. So we added SPDK_CU_ASSERT_FATAL
which should be used instead.
This still trips people up sometimes though - the static analyzer
complains and then the developer will add other checks that
pointers are not NULL.
So instead, forbid use of CU_ASSERT_FATAL through check_format.sh
and explain why. While here, fix up all of the existing CU_ASSERT_FATAL
usages to either CU_ASSERT or SPDK_CU_ASSERT_FATAL.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I7974c8c85ddb89ed1b7d882db3a2eb0882ea0217
Reviewed-on: https://review.gerrithub.io/418111
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Seth Howell <seth.howell5141@gmail.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Put it in the iscsi.c
Change-Id: Ifb2843fb7c78f9ca948eac60704547b8b0635bf0
Signed-off-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-on: https://review.gerrithub.io/402484
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This function was originally named spdk_iscsi_write_pdu()
in lib/iscsi/iscsi.c. Since this is an operation on a
connection, add "conn" to the name and move it to conn.c.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Iaad022907d43788108d5b2660306abcf5e94040d
Reviewed-on: https://review.gerrithub.io/395522
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Currently cpumask cannot be specified for each portal when it is
created by JSON-RPC and portal group creation is not unified
between iSCSI.conf and JSON-RPC.
This patch does the following:
- cpumask string is decoded in spdk_iscsi_portal_create() which
is common between iSCSI.conf and JSON-RPC.
- parsing configline of portal is difficult to understand and
hence it is refactored.
- UT code is added.
JSON-RPC will be added by the next patch.
Change-Id: I13b9989263fae5facff260de32a55ec99f5d5a06
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/392447
Tested-by: 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>
