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>
This direct call of LUN reset has caused unexpected behavior
during heavy stress test.
Apply correct LUN reset handling for this case too.
Even if there are multiple LUNs, LUN resets are serialized in
the SCSI layer by previous patches and parallel calls of LUN resets
will not cause any issue.
Change-Id: I920e2864ef0fb41c6bd2b6153ef3caff6876e893
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/436115
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>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@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>
By subsequent patches for iSCSI, spdk_scsi_dev_queue_mgmt_task()
will not be called directly from the function that knows TMF code,
and currently setting TMF code to SCSI task is done in
spdk_scsi_dev_queue_mgmt_task().
Hence after subsequent patches for iSCSI, to hand off TMF code to
SCSI task, any dynamic context will be required.
To avoid the dynamic context, extract setting TMF code from
spdk_scsi_dev_queue_mgmt_task() and put appropriate place for
each call of spdk_scsi_dev_queue_mgmt_task().
Additionally, in spdk_abort_transfer_task_in_task_mgmt_resp(),
ref_task_tag is got from PDU but getting it from SCSI task is
much easier. Hence get ref_task_tag from SCSI task in the callback.
Change-Id: I7add9290598d2df7cfcf1506ec75d74c70c0f236
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/436643
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>
Since a PDU can be sentout in many times, so this
function is called multiple times, so move the
caculation function in spdk_iscsi_conn_write_pdu
Change-Id: Ib4da4a74c17709d98e4b01c2e76428021afea947
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.gerrithub.io/429931
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: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
By running random, 64KB, 70% write, and 30% read workload and periodical
TCP disconnection from the initiator, segmentation fault have occurred.
By investigation, when the last write task completed and dequeued
the primary write task in spdk_iscsi_task_cpl(), the primary write task
was already dequeued in spdk_clear_all_transfer_task().
This patch dequeues all primary write tasks after all outstanding
tasks complete.
After starting to exit the connection, the connection will not receive
any command. Hence waiting for completion of all pending tasks first will
avoid double dequeue safely.
Change-Id: I8e2b6b756be3a6e096675acc81cdb93fb6a7e350
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/430397
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
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>
Purpose: We need to get the port info in other applications
(e.g., NVMe-oF TCP/IP transport)
Change-Id: I3a4636e764e44425436bb064cb0062c6f3e44035
Signed-off-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-on: https://review.gerrithub.io/428313
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.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>
These tests don't currently require the env mockup,
but they will in a future patch.
Change-Id: Ibf311f198a291f825a83e7b89708cd2442f35f7a
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/423183
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
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 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>
This is already required to build the shared library and iSCSI
components; simplify these Makefiles by adding -lcrypto to the central
SYS_LIBS in mk/spdk.common.mk.
Change-Id: I8fdaffbccc1294e24e32559387b0ed99d1deb0ce
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/415351
Tested-by: 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>
Support JSON config/info dump for portal group. These are added to
portal_grp.c. Text config dump is moved to portal_grp.c.
Add accesses to g_spdk_iscsi.pg_head are consolidated into
portal_grp.c to extract pg_head from g_spdk_iscsi.
Change-Id: I8c551b067c68a785eb0c7d83d695bde5f7219c2b
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/407850
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Support JSON config/info dump for initiator group. These are added
to init_grp.c. Text config dump is moved to init_grp.c too.
Add accesses to g_spdk_iscsi.ig_head are consolidated into
init_grp.c to extract ig_head from g_spdk_iscsi.
Change-Id: I88131d248ed13e42e4da56aed8ae0edc01a65715
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/407849
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Support JSON config/info dump for target node. These are added
to tgt_node.c Text config dump is moved to tgt_node.c too.
All accesses to g_spdk_iscsi.target_head are consolidated into
tgt_node.c to extract target_head from g_spdk_iscsi.
Change-Id: Ibdf57c6ec9457d762c5640483b5810c9a74604ef
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/407848
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
strncpy is going to be added to the list of banned functions because
it does not guarantee strings are null terminated.
Change-Id: I73adb5d3eea5c95ea705c67de4fc967e926eb6ce
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/407025
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>
ImmediateData in negotation parameters can be controlled by the config
file and currently "Yes" or "No" are valid values of it in the config
file. "Yes" or "No" means almost bool. Additionally, ImmediateData is
defined as boolean value in the iSCSI specification as follows.
However ImmediateData is an uint32_t variable and 0 or 1 are used in
SPDK now.
In the upcoming JSON config file, boolean value should be set to
ImmediateData.
Hence change all variables and constants related with ImmediateData
to boolean in this patch.
InitialR2T, DataPDUInOrder, and DataSequenceInOrder are not controllable
by the config file but these are defined as boolean value in the
iSCSI specification.
Hence change them to bool too.
Besides, AllowDuplicateIsid is used almost as boolean value. Hence
change it to bool too.
InitialR2T=<boolean-value>
ImmediateData=<boolean-value>
DataPDUInOrder=<boolean-value>
DataSequenceInOrder=<boolean-value>
Change-Id: I9f98b219687b3600194563b1d6b85551f4586144
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/403237
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Portal, portal group, initiator group, and target node don't use fixed size
array anymore. To use the same naming rule in iSCSI subsystem and in SPDK
is helpful to understand and maintain. Current naming rule is not bad but
to use unified and compact name is prioritized at this time.
This patch changes function names as follows:
- spdk_iscsi_init_tgt_nodes() -> spdk_iscsi_parse_tgt_nodes()
- spdk_cf_add_iscsi_tgt_node() -> spdk_iscsi_parse_tgt_node()
- spdk_iscsi_portal_grp_array_create() -> spdk_iscsi_parse_portal_grps()
- spdk_iscsi_portal_grp_create_from_configfile() -> spdk_iscsi_parse_portal_grp()
- spdk_iscsi_portal_create_from_configline() -> spdk_iscsi_parse_portal()
- spdk_iscsi_portal_grp_array_destroy() -> spdk_iscsi_portal_grps_destroy()
- spdk_iscsi_init_grp_array_create() -> spdk_iscsi_parse_init_grps()
- spdk_iscsi_init_grp_create_from_configfile() -> spdk_iscsi_parse_init_grp()
- spdk_iscsi_init_grp_array_destroy() -> spdk_iscsi_init_grps_destroy()
For example "parse" is already used to explain the operation in portal_grp.c.
Hence using "parse" is familiar and not strange in iSCSI.
Change-Id: Ieff952069d39bd96b7afa1586bfff927968accc2
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/403149
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>
Change-Id: Ie7a940efde885dbf0bb7c085fd17c3991dfa3341
Signed-off-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-on: https://review.gerrithub.io/403165
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@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 patch is used to implement the sock fd event group
polling mechanism if there are incoming data from network
(read event in SPDK iSCSI target side) , thus we can awake
many connections one time, and it can reduce the system call
overhead.
Change-Id: I76c26a89ef9365d7e1ccec616985e7435253896b
Signed-off-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-on: https://review.gerrithub.io/399796
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>
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ic2bae1c750d2c9b35bded8ed96c3b84832690ce2
Reviewed-on: https://review.gerrithub.io/401479
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
During converting type of CHAP params of target from int to bool
for JSON, changed names of them to be consistent with others.
In this patch, adjust variable name of struct to of JSON.
Change-Id: I1ccbfa11d57479dc55680835eb80e111bd24d9a3
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/400928
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>
This is a TODO from long ago. Complete this while adjusting dump()
and construct() format of target.
Besides names of variables and parameters about CHAP are not unified
between JSON-RPC and SPDK internal. JSON-RPC's wording looks better
and adjust SPDK internal to JSON-RPC.
Change-Id: I89bcd1ce13a11f7d63a62d51ef094dd302186d37
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/400201
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>
In early days of SPDK, there was an experimental userspace
TCP stack called libuns. The SPDK iSCSI target supported
using either the default POSIX/kernel TCP stack or this
libuns TCP stack.
libuns is no longer in use, but work has started on
supporting the FD.io VPP TCP userspace stack. Abstractions
are being put in place to allow *both* the POSIX and VPP
stacks simultaneously.
So remove the concept of a "default" net_framework that is
defined with weak symbols that can be overridden by another
object file. While here, also remove the "clear_socket_association"
concept which was specific to libuns.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I8a0385ca5ae113e34a637f835d8d69f8d510433e
Reviewed-on: https://review.gerrithub.io/400328
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Adding new initiator to an existing initiator group to allow login
will be usual. This patch support the following JSON-RPC commands:
- add_initiators_to_initiator_group
- delete_initiators_from_initiator_group
Both initiator's name and netmask are optional but already added
name or netmask cannot be added.
Test code is added too.
Change-Id: Ic101210b9d00c2b36e37ece23fcba8cfe8e44eb8
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/398361
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>
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I833ca74093f55ae296d2db4a2dd7a1e482c945dc
Reviewed-on: https://review.gerrithub.io/399730
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
This provides an abstraction layer around TCP
sockets. Previously we just used fd integers, but
we don't want to be tied to integers for alternative
userspace TCP stacks.
Future patches will do more work to enable multiple
implementations of this abstraction. For now, just
get the abstraction in place for POSIX sockets and
make all of the iSCSI changes associated with it.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I9a825e9e02eb6927c8702d205665c626f57b3771
Reviewed-on: https://review.gerrithub.io/398861
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ziye Yang <optimistyzy@gmail.com>
The socket-related code was already broken out into
lib/net/sock.c, so break out the header portions
from include/spdk/net.h into its own sock.h.
This prepares for some upcoming changes in how
TCP sockets are abstracted, to enable alternative
userspace TCP stack implementations to be used with
SPDK.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I40b162e72ea80c235b49f10b17c2085fcfb385d4
Reviewed-on: https://review.gerrithub.io/398851
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Checking uniqueness of initiator group is done without mutex and
before register/unregister. This is not thread-safe.
This patch is a preparation to dynamic addition of initiator
information to existing initiator groups.
Change-Id: I44f48c857210522eee70d14bc3735ec73b0c5c6f
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/397032
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>
Orderings of portal group create operation are diferrent between
config file and JSON-RPC. Unification is necessary for correct
concurrency control and the global accept poller like NVMf-tgt.
Hence unify the ordering of operations in this patch.
Common ordering of portal group create operation between configuration
file and JSON-RPC after this patch is the following:
- create a portal group
- create portals
- add the portals to the portal group
- open the portals of the portal group
- add the portal group to the global portal group list
After this patch, the gap between listening socket and accepting socket
will be increased a little when portals groups are creted by config file.
However this will cause no issue because of the TCP backlog and resend
mechanism.
Besides, necessary concurrency control is added and minor refactoring
is done.
About portal group delete operation, orderings of application shutdown
and JSON-RPC are already unified.
Change-Id: I1db7ef4400388127134d7734c68e138a4573b734
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/396848
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>
Checking uniqueness of portal group is done without mutex and
before register/unregister. This is not thread-safe.
Hence this patch is added to ensure PG uniqueness.
A little related refactoring is also done.
Change-Id: Iaa3b5e380f2be5cfdaa2d69f9f2763c98954b0c3
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/396847
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>
Network portal must be unique globally but mutex is not added yet.
This patch is added to ensure it.
Change-Id: I3cdd85fd524b0da767d3cd83022e0637f3a32bc9
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/396846
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Fixes github issue #218.
This patch introduces spdk_cpuset object to store and manipulate
the set of individual CPUs. The main objective of this object is
to replace cpumask declared as uint64_t and extend the limitation
of supported CPUs (lcores) above 64 CPUs.
spdk_cpuset is always allocated dynamically and accessed by opaque
pointer, what makes it easier to extend in the future without
breaking API/ABI.
This patch also extends parsing function allowing to set cpumask
using a list of cpus e.g. "[0-4,10,12]" sets mask of 0,1,2,3,4,10,12
as well as hexadecimal string with and without "0x" prefix.
Change-Id: I475c3ba7fab629021a22e03176e57e400dd24a49
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Reviewed-on: https://review.gerrithub.io/390794
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.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>
When a target is created by iSCSI.conf, only valid CHAP params
are passed to spdk_iscsi_tgt_node_construct().
When a target is created by JSON-RPC, help information encourages
users to specify valid CHAP params but
spdk_iscsi_tgt_node_construct() does not check CHAP params and
users can create targets whose CHAP params are invalid.
Change-Id: I7e9057a982f21f04782481cda74208a139c1fdad
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/394481
Tested-by: 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>
Removing an LUN from an existing iSCSI target is possible by
removing the corresponding BDEV. However adding an LUN to an
existing iSCSI target is not possible yet.
Add a new function spdk_iscsi_tgt_node_add_lun() and related
functions first toward supporting this function.
JSON-RPC for this operation will be submitted an another patch.
Informing the newly added LUN to the initiator is not included
in this patch. Hence this operation is possible only for any
inactive target.
Change-Id: I3a28f4d75a17126e49c9d12ce64c3ad68f231840
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/385180
Tested-by: 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>
There is no need to keep a lun name anymore - we always
use the bdev name as the lun name so it is not providing any
additional value. This also keeps us from associating
the same bdev with different LUNs on different iSCSI target
nodes or vhost-scsi controllers.
Side effect of this change is:
1) Use "bdev_name" across the APIs to make it more clear
what these names refer to.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I3d42fde22087352ce1d5dc80178bd8c5cac8cb7c
Reviewed-on: https://review.gerrithub.io/390843
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Splitting an iSCSI task into primary and sub tasks is complex
operation and a degradation was caused for it.
Hence adding test codes is required but there is no UT code for
it yet.
primary->bytes_completed is for read completion from bdev and
it is tested by this patch.
Additional test codes will follow:
- primary->bytes_completed is tested when read tasks do not
complete in order.
- primary->task.data_transferred is for write completion to
initiator through network.
- primary->task.data_transferred is tested by another patch
because primary->bytes_completed is used in iscsi/conn.c but
primary->task.data_transferred is used in iscsi/iscsi.c.
Change-Id: I94b47048111a3d3b249b84d5c54941b0a89ccd40
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/394143
Tested-by: 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>
This patch remove need for additional buffer when translating error code
to string.
Change-Id: Iaa60088b5c450581d3cdddbb425119b17d55a44b
Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/386114
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Only Makefiles for libraries that directly depend on DPDK (rather than
the SPDK env abstraction) should add $(ENV_CFLAGS).
Change-Id: Ifdf44d3ef8c42bbf7f20edd524b330d00658235b
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/392818
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>
Netmasks with 0 bits of prefix do not make sense. For example,
192.168.1.2/0 would allow hosts from any address, but this is
indicated with a special "ANY" value rather than a normal netmask.
Netmask prefixes of the full address size (e.g. 32 for IPv4 and 128 for
IPv6) are still allowed, since this represents a valid configuration
that matches a single specific address.
This also allows the IPv4 netmask math to be done entirely in uint32_t
instead of promoting to unsigned long long to avoid undefined behavior
when bits == 0 (shift count would have been 32 in that case).
Change-Id: I021b718e6a46f628c96a358edae816de81cd8929
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/392969
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
A few previous changes replaced ALL by ANY for the initiator group
because ANY was normal for this case. ANY was tested enough for
the initiator name of the initiator group but not tested for
the netmask of the initiator group.
Hence the previous changes caused degradation.
This is the bug fix and UT code is added together.
Change-Id: Idf7642dd4c111a4788aca31a0105b3497631aecd
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/392923
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
It looks that initiator groups which were initialized after
being inserted to linked lists caused failures.
Change-Id: I43f4a6324e77221f0e9657667c0185b15e9c229c
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/392100
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Replace the single call to spdk_scsi_dev_print() with a reimplementation
using public SCSI API functions, and also using the SPDK logging
framework instead of printf().
Change-Id: Ifa455f9e6a4a07a35d5dec311a61e9a8afaa0227
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/391320
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Then also add a check in check_format.sh and fail if
a new instance is found again.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ia35c343e1b7bb44b3b5f4f8484adb9e0d5702d67
Reviewed-on: https://review.gerrithub.io/390916
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Require braces around all conditional statements, e.g.:
if (cond)
statement();
becomes:
if (cond) {
statement();
}
This is the style used through most of the SPDK code, but several
exceptions crept in over time. Add the astyle option to make sure we
are consistent.
Change-Id: I5a71980147fe8dfb471ff42e8bc06db2124a1a7f
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/390914
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@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>
Similar to 759e5934eb (test/iscsi: initialize pgs before adding to pg
map), make sure the spdk_scsi_ig structures are zero-initialized before
passing them to spdk_iscsi_pg_map_add_ig_map().
Fixes Valgrind "Conditional jump or move depends on uninitialised
value(s)" errors in two tests.
Change-Id: I38c095aa7c6a89cb9332618b63fe12a02bcf3c2a
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/391000
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
This fixes some issues found with Valgrind (incidentally *not*
found with ASAN).
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I49bbd4b338299568eaa4a118aa82fc411037cea7
Reviewed-on: https://review.gerrithub.io/390909
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Currently even if PG mapping is removed from iSCSI target,
corresponding SCSI port is not removed. Hence dynamic
reconfiguration of PG to existing iSCSI target is not supported.
This patch supports dynamic SCSI port removal when the
corresponding PG mapping is removed from the existing target.
Some refactoring for SCSI port addition is also done
Change-Id: Ib06bc10743eb088d023f425742e39bb24f1b0df1
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/389056
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Disambiguate the log components from the trace functionality
(include/spdk/trace.h).
The internal spdk_trace_flag structure and related functions will be
renamed in a later commit - this is just a find and replace on
SPDK_TRACE_* and SPDK_LOG_REGISTER_TRACE_FLAG().
Change-Id: I617bd5a9fbe35ffb44ae6020b292658c094a0ad6
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/376421
Tested-by: 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>
This patch is a preparation for dynamic reconfiguration of PG-IG maps.
Current PG-IG map is implemented by a fixed-size array. Linked list
will make much easier to support dynamic reconfiguration of PG-IG maps.
Current:
target - (1..n) ---> PG_map (1..1) -> PG
|
--> IG_map (1..1) -> IG
This proposal:
target - (1..n) -> PG_map --- (1..1) -> PG
|
-- (1..m) -> IG_map - (1..1) -> IG
Change-Id: I92f668b81cdd8003aff222926f8f1ed96b61e56d
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/385532
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
The results of access control procedure for login in the
spdk_iscsi_tgt_node_access() is defined in the following table:
+------------------------------+
|iscsi name |netmask |result |
+------------------------------+
+------------------------------+
|denied |- |denied |
+------------------------------+
|allowed |allowed |allowed |
+------------------------------+
|allowed |denied |next IG |
+------------------------------+
|not found |- |next IG |
+------------------------------+
However current implementation have redundant repetition in the
spdk_iscsi_tgt_node_access() and the above definition is not
visible. Hence refactor spdk_iscsi_tgt_node_access().
Besides refactor spdk_iscsi_tgt_node_allow_iscsi_name() because
it has redundant repetition too.
Add UT code for these changes.
Change-Id: If06d87c1246c85439ee0482149bd887a7b53b169
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/379935
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>
Naming rule of access control of iSCSI target does not reveal its
intention. Hence change it before refactoring to avoid unnecessary
repeated procedure.
Change-Id: I4064ec0a5a2b52244b6de3958ee2ab41342d1a57
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/381248
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
In the accessibility control of iSCSI target, "ALL" is used to allow
ANY IP address-port pair or iSCSI name of initiators. However iSCSI
targets cannot know ALL initiators beforehand.
Hence "ANY" will be better than "ALL" and will avoid misunderstanding.
Comments and iscsi_tgt test code are also changed and UT code is added.
Change-Id: Id004d819df6e9ee89f6c1db2e4b4c149be062733
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/385168
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>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
To add initiators to an existing target, manage name and netmask
of initiator group by linked list. It is not easy for array to
detect duplication and change configuration.
Change-Id: Id9e77740e24ac309a33d8de1be37663681160d94
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/379932
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
spdk_iscsi_init_grp_create_from_configfile() returns 0 even if
creating an initiator group (IG) is failed due to empty netmask or
initiator name. This will be unexpected behavior for end users.
Hence change the return code from 0 to -ENVAL for this failure.
Besides do the following.
Change other error return codes in the function by using errno too.
Current IG don't have any UT code in repository. UT code to confirm
this fix is also submitted.
Change-Id: Ic918feb5c6a3e772b325664d881b2673b553a217
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/384408
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
The following functions returns 1 and 0 for succcess and error,
respectively:
- spdk_iscsi_tgt_node_allow_ipv6()
- spdk_iscsi_tgt_node_allow_ipv4()
- spdk_iscsi_tgt_node_allow_netmask()
- spdk_iscsi_tgt_node_access()
Using bool for this purpose will avoid our misunderstanding.
Change-Id: I927876e0503c0eee5364e829a4713f9a345996f6
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/383664
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
spdk_iscsi_tgt_node_access() (in lib/iscsi/tgt_node.c) regards
empty netmask of IG as ALL (allow all initiator's IP address).
However any user cannot create IG whose netmask is empty by both
JSON-RPC and config file. Instead user can create IG whose
netmask is ALL.
The code to regard empty netmask of IG as ALL never run in production.
Hence delete the code and add UT to confirm the fix.
Change-Id: Ib7206d0986db9093cfb6b36191be26293ff6c67a
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/382920
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
FirstBurstLength, MaxBurstLength and MaxRecvDataSegmentLength
cannot be configured, so there is no need to keep global data
members for these parameters - just use the default #defines
instead.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I4b47e00a5594da8ec0b87192be4a23c4a2145bde
Reviewed-on: https://review.gerrithub.io/385490
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
the function of spdk_iscsi_find_init_grp() is duplicated with
of spdk_iscsi_init_grp_find_by_tag(). The later one should be kept.
Hence delete former one.
Change-Id: I8d397648bd400550445de49c8a7731f0932f5324
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/381244
Reviewed-by: John Kariuki <John.K.Kariuki@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
A few foundational change to support safe removal of portal.
- global login poller -> login poller per portal
- Caching portal data for active connection
Change-Id: I62f4d90c9ac11a433ad47421b2b0c69bfc3c70b7
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/379930
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>
Group the code fragments of open/close portal procedure
and create spdk_iscsi_portal_open/close() function.
Change-Id: Id6e291f3f6e5a926f3ea0d01741a200fa64db81b
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/379934
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Factor out the iSCSI and GPT CRC32 functions into generic library
functions.
Change-Id: I1f1a5f3968a983b663a51bd984500492eeb12605
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/370765
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>
These variable names are changed to retain conformity with astyle 3.0.1. Astyle 3.0.1 recognizes 'final' as a keyword and incorrectly formats the iscsi_spec.h file breaking the build. Names were changed to avoid this case and maintain consistency within the struct.
Change-Id: I39c29d768c0ddfd459a9e9212c29cf564dad1963
Signed-off-by: Seth Howell <seth.howell@intel.com>
Reviewed-on: https://review.gerrithub.io/369905
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Previously, we had a pdu->ahs pointer that was always NULL (never set
anywhere), and we would try to read data into this NULL pointer if the
initiator ever sent a PDU with a non-zero TotalAHSLength.
Rename the existing ahs_data array in the PDU to just "ahs" to minimize
the necessary changes. We never actually dereference the ahs structure,
so its type is not important. (We can cast it later if we add support
for anything that requires an AHS.)
Change-Id: I10d19a6e0d99f326794cbe6469eacedadc634c67
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/369315
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>
This variable was misleading and unnecessary. Replaced all it's
occurences with SPDK_SCSI_DEV_MAX_LUN.
Change-Id: Iaaf9ed5efb60f9a89585121d2fbec7b43c3723e9
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/368115
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Added optional callback inside scsi lun hotremove, so that higher-level
abstraction can be notified about hotremove.
Change-Id: I5f1bd8160e3d770a484068dd73928bc2b64c876f
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/367309
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ibe282aa67de22e062de9d88c3b157c766be9dfae
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/366910
Tested-by: 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>